You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I'm having some issues with the reconnect strategy in this library. It seems to work as per the documentation on the initial connection to Redis, however the behaviour changes if the connection to Redis is lost at a later stage.
The documentation states that there are three different types that can be returned:
A number can be returned to indicate that the client should retry after the specified delay
An error can be returned to terminate the connection
false can be returned to prevent reconnection and hold the client in some kind of broken state for testing
Behaviour on Initial Connection
The RedisSocket.connect() function is called
This calls the private function RedisSocket.#connect() which deals with retries
There is a line in packages/client/lib/client/socket.ts:218 where the output of the reconnect strategy function is evaluated - if it's a number then a retry is scheduled; if it's not a number then it throws whatever it is.
Assuming the value is not false (which I assume is a noop in javascript) it bubbles up to the initial call to RedisSocket.connect() and the promise called by the library user rejects with the error
This is the behaviour that is documented.
Behaviour when connection is lost
The socket "error" event fires, which calls into RedisSocket.#onSocketError()
RedisSocket.#onSocketError() re-emits the error event and also emits "reconnecting" before calling this.#connect() on line 269
Crucially, that line functionally looks like this: this.#connect().catch(() => {}));
Any error in connecting is completely ignored. This means that is has the same effect as if the reconnect strategy function returned false which (quoting the documentation here) "should only be used for testing purposes". This silently leaves the client in a broken state.
What I expect to happen
At the very least expect the "error" event to be emitted with the error returned by the reconnect strategy. Even better would be for a "terminated" or similar signal to be emitted to tell the library that the retry strategy failed and allow it to do it's own recovery.
Node.js Version
18.20.6
Redis Server Version
7.3.241
Node Redis Version
5.0.1
Platform
Linux
Logs
The text was updated successfully, but these errors were encountered:
Description
I'm having some issues with the reconnect strategy in this library. It seems to work as per the documentation on the initial connection to Redis, however the behaviour changes if the connection to Redis is lost at a later stage.
The documentation states that there are three different types that can be returned:
number
can be returned to indicate that the client should retry after the specified delayfalse
can be returned to prevent reconnection and hold the client in some kind of broken state for testingBehaviour on Initial Connection
RedisSocket.connect()
function is calledRedisSocket.#connect()
which deals with retriesfalse
(which I assume is a noop in javascript) it bubbles up to the initial call toRedisSocket.connect()
and the promise called by the library user rejects with the errorThis is the behaviour that is documented.
Behaviour when connection is lost
"error"
event fires, which calls intoRedisSocket.#onSocketError()
RedisSocket.#onSocketError()
re-emits the error event and also emits"reconnecting"
before callingthis.#connect()
on line 269this.#connect().catch(() => {}));
Any error in connecting is completely ignored. This means that is has the same effect as if the reconnect strategy function returned
false
which (quoting the documentation here) "should only be used for testing purposes". This silently leaves the client in a broken state.What I expect to happen
At the very least expect the
"error"
event to be emitted with the error returned by the reconnect strategy. Even better would be for a"terminated"
or similar signal to be emitted to tell the library that the retry strategy failed and allow it to do it's own recovery.Node.js Version
18.20.6
Redis Server Version
7.3.241
Node Redis Version
5.0.1
Platform
Linux
Logs
The text was updated successfully, but these errors were encountered: