-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Description
Description
During testing with a local Redis-Sentinel setup in Docker I've encountered a bug with the automatic connection handling of the Sentinels. All containers were connected via a host network and the initial connection of the master, its replicas and all Sentinels between each other was successful.
When I turn the master off my connection went silent. After enabling "passthroughClientErrorEvents" it threw an ECONNREFUSED error but was inoperable netherless.

Since I expected my connection is being managed by the Sentinels and automatically updated in the event of a master shutdown I started to trace back this behavior.
It fails in the "transform" method in sentinel/index.ts as Promises are awaited and those Promises aren't resolved.


Those Promises won't be resolved as the process seems to get stuck in an endless loop of trying to reconnect to the old masters address. But as the master is down and doesn't respond the process runs into this error handling in client/socket.ts

The shouldReconnect method now loads a defaultReconnectStrategy.

But this defaultReconnectStrategy only stops reconnecting if a SocketTimeoutError appears otherwise it keeps trying to reconnect endlessly.

Since this all happens in a While-Do-Loop and my failed server produces a ConnectionRefused error this means that this event isn't handled, the socket is never closed and the address of the new master is never retrieved from the Sentinels.
To fix this I tried to create a custom reconnectStrategy and inject it into the process.

But this strategy was never loaded and used so I went digging again. Then I learned that this strategy should be handed over by the createClient method in sentinel/index.ts.
But in the "transform" method this method gets called with two params only.

while my custom reconnectStrategy would've been loaded by a third param.

After adding this third param and injecting my custom reconnectStrategy a normal connection handling with establishing of a new master in case of failure was achieved.
In summary I want to point out that the defaultReconnectStrategy doesn't provide a handling strategy for a connection loss of the master due to failure and that it is not possible to inject own strategies at the moment.
Node.js Version
22.18.0
Redis Server Version
8.2.0
Node Redis Version
5.8.0
Platform
Linux