Prevented reconnect

[yaroslavputria]

Hi All,

This line (if this._connectLock === true) prevents manually invoked reconnect, if the close method was called before the connection has ever been established.

My case: user decides when he wants to stop reconnecting retries, but later he should be able to proceed with reconnecting retries.

Could you please adjust the reconnect method (I hope it's not been used internally and it will not affect other workflows) to be able to call "force" reconnect manually?

Thanks,
Yaroslav

[nadine-nguyen]

I've also needed this use case too.
What I've done as a temporary solution is add these two lines after this line in the connect() method:

...
this._debug('max retries reached', this._retryCount, '>=', maxRetries);
+   this._shouldReconnect = false;
+   this._connectLock = false;
return
...

And also add this line in the close() method:

...
this._closeCalled = true;
+ this._connectLock = false;
this._shouldReconnect = false;
...

[ppena-LiveData]

The fix is just to also set this._connectLock = false; in the if (this._closeCalled) case of _connect(), i.e. after line 378 in reconnecting-websocket.ts, since the return on line 379 is preventing the this._connectLock = false on line 386 from running.

[votsa]

There is another bug/issue. After this._retryCount reaches maxRetries, this._connectLock remains true and there is no way to set it to false so reconnect() method does not have any effect.

[ppena-LiveData]

Good catch! Yes, the early exit from _connect() in the if (this._retryCount >= maxRetries) block also needs to reset this._connectLock = false, just like the early exit in if (this._closeCalled).

[alecgibson]

I think @ppena-LiveData 's solution is the correct one.

If you reset _connectLock in the close() method, you can wind up opening several WebSockets if you rapidly call close() and reconnect() in quick succession.

This is a test case that passes if you keep the _connectLock reset in the _wait() promise resolution, but fails if you move it to close():

test('rapidly toggling connection', done => {
    const wss = new WebSocketServer({port: PORT});
    const ws = new ReconnectingWebSocket(URL, undefined, {
        minReconnectionDelay: 100,
        maxReconnectionDelay: 200,
    });

    ws.close();
    ws.reconnect();
    ws.close();
    ws.reconnect();

    let connections = 0;
    wss.on('connection', () => {
        connections++;
    });

    ws.addEventListener('open', () => {
        wss.close(() => {
            setTimeout(() => {
                expect(connections).toBe(1);
                done();
            }, 1000);
        });
    });
});

[alecgibson]

Also, since I've written them if anyone wants tests for the other cases discussed here:

test('reconnect after max retries', done => {
    const ws = new ReconnectingWebSocket(URL, undefined, {
        maxRetries: 1,
        maxReconnectionDelay: 200,
    });

    let closed = false;
    ws.addEventListener('error', () => {
        if (ws.retryCount === 1 && !closed) {
            const wss = new WebSocketServer({port: PORT});
            ws.reconnect();
            ws.addEventListener('open', () => {
                wss.close(() => {
                    closed = true;
                    setTimeout(() => {
                        done();
                    }, 1000);
                });
            });
        }
    });
});

test('reconnect after closing during connection', done => {
    const wss = new WebSocketServer({port: PORT});
    const ws = new ReconnectingWebSocket(URL, undefined, {
        minReconnectionDelay: 100,
        maxReconnectionDelay: 200,
    });

    ws.close();
    setTimeout(() => {
        ws.reconnect();

        ws.addEventListener('open', () => {
            wss.close(() => {
                setTimeout(() => {
                    done();
                }, 1000);
            });
        });
    }, 1000);
});

[nadine-nguyen]

@alecgibson Great catch! My mistake, use @ppena-LiveData's solutions 🙂