fix(redis): keep reconnecting when connection closes during client setup (#2099)

[ranjeetcao]

Summary

Fixes #2099 — when enableReadyCheck: false, the client can permanently stop reconnecting after the socket closes during the CLIENT SETNAME / CLIENT SETINFO handshake.

connectHandler() (lib/redis/event_handler.ts) calls readyHandler() from Promise.all(clientCommandPromises).finally() without validating that the connection is still live. If the socket closes during that window and closeHandler() reaches the maxRetriesPerRequest threshold (default 20), it flushes the in-flight handshake commands. The individual handshake promises swallow rejection via .catch(noop), so the flush settles Promise.all and runs the now-stale .finally(). By then closeHandler() has already moved status to "reconnecting" and scheduled a reconnect; the stale callback overwrites it with "ready" and resets retryAttempts. The pending connect() then rejects with "Redis is already connecting/connected" — swallowed by .catch(noop) — and no further reconnect is ever scheduled.

This regressed in #2033, which moved the handshake commands out of readyHandler() and made the enableReadyCheck: false readiness emission asynchronous. Detailed root-cause analysis is in the issue comment.

Fix

Guard the .finally() callback the same way the enableReadyCheck: true ready-check callback (line 136) already is, plus a status check:

if (
  connectionEpoch !== self.connectionEpoch ||
  self.status !== "connect"
) {
  return;
}

A stale callback becomes a no-op.

Alignment with existing handshake / reconnect behavior

The fix deliberately reuses patterns already established in this file rather than introducing new mechanisms:

• connectionEpoch guard mirrors the existing guards in the AUTH callback (event_handler.ts:29) and the _readyCheck callback (event_handler.ts:136). Both are captured the same way (const { connectionEpoch } = self; at line 26) and compared identically.
• status !== "connect" guard is needed in addition to the epoch check because connectionEpoch is only incremented inside _connect() (Redis.ts:210). Between closeHandler() setting status to "reconnecting" and the reconnect timer firing _connect(), the epoch still matches the captured one — only the status reflects that the connection is no longer the one we're setting up. The status check covers exactly that window.
• No behavior change on the live path: when the connection is still healthy, both connectionEpoch === self.connectionEpoch and self.status === "connect" hold, so readyHandler() is called exactly as before.
• No change to closeHandler(), _connect(), flushQueue(), or readyHandler(): the bug is purely that a stale callback runs; the surrounding state machine already does the right thing on its own.
• Symmetry with the enableReadyCheck: true branch: that branch is naturally protected because _readyCheck() sends INFO and a dead socket cannot answer, and additionally re-checks connectionEpoch at line 136. The fix gives the enableReadyCheck: false branch equivalent protection (epoch + status) without introducing a synthetic round-trip.
• fix: emit connect once all handshake commands have been resolved #2040 is not affected: it touched the same handshake-command flow but for a different (p)subscribe-during-connect race (info command issued after client enters subscriber mode #2037) and did not add a guard for the connection closing mid-handshake.

Regression test

test/functional/connection.ts — enableReadyCheck: false → keeps reconnecting after the connection is closed during client setup (#2099).

The test uses MockServer to deterministically reproduce the race:

• First connection: server destroys the socket while CLIENT SETNAME is in flight (flags.hang = true; socket.destroy()).
• Client config: enableReadyCheck: false, disableClientInfo: true, connectionName: "test-2099", maxRetriesPerRequest: 0, retryStrategy: () => 50.
• maxRetriesPerRequest: 0 is what makes the failure 100% deterministic — it forces closeHandler() to call flushQueue() on every close (retryAttempts % (0 + 1) === 0 is always true), which is what surfaces the stale .finally() against the dead connection. With the default of 20, the same scenario only triggers every 21st failed attempt, which is why the issue reporter saw it intermittently when starting/stopping Redis.
• Assertion: a "ready" event is only emitted on a real reconnection (second connect accepted by the mock server). Without the fix, the client wedges in "ready" on the dead first connection and the test times out.

Test plan

• New regression test passes with the fix; fails without it (verified locally).
• Full functional suite passes: npm run test:js — 713 passing.
• Cluster suite passes: npm run test:cluster — 17 passing.
• No changes outside lib/redis/event_handler.ts and test/functional/connection.ts.

---

Note

Medium Risk
Touches core connect/ready/reconnect timing in event_handler.ts; behavior change is narrowly scoped to stale callbacks, with a targeted regression test.

Overview
Fixes a reconnect wedge when enableReadyCheck: false and the socket dies during the post-connect CLIENT handshake (SETNAME / SETINFO).

In connectHandler, the Promise.all(clientCommandPromises).finally() path could still call readyHandler() after closeHandler had already moved the client to reconnecting and scheduled a retry—especially when in-flight setup commands are flushed (e.g. maxRetriesPerRequest). That stale callback marked a dead connection ready and reset retry state, so the client stopped reconnecting (#2099).

The change bails out of that .finally() unless connectionEpoch still matches and status === "connect", mirroring guards already used for AUTH and the enableReadyCheck ready-check callback. A new functional test drops the first mock connection mid-CLIENT SETNAME and asserts a second connection reaches ready.

^{Reviewed by Cursor Bugbot for commit ac232d3. Bugbot is set up for automated code reviews on this repo. Configure here.}

[jit-ci]

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

[PavelPashov]

@ranjeetcao Thanks for the PR. I’ll take a look.

[github-actions]

🎉 This PR is included in version 6.0.0-beta.1 🎉

The release is available on:

• npm package (@beta dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[github-actions]

🎉 This PR is included in version 6.0.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀