Skip to content

fix(client): do not record a database whose SELECT failed inside MULTI - #3489

Merged
nkaradzhov merged 1 commit into
redis:masterfrom
maxymlyskov:fix/failed-select-inside-multi
Oct 7, 2026
Merged

nkaradzhov merged 1 commit into
redis:masterfrom
maxymlyskov:fix/failed-select-inside-multi

Conversation

@maxymlyskov

@maxymlyskov maxymlyskov commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Description

client.multi().select(99).set('k', 'v').exec() rejects with MultiErrorReply, but the client still records 99 as its database. Redis checks the SELECT index when EXEC runs, so the connection stays on db 0 and the error sits in the SELECT slot of the EXEC reply, while _executeMulti records the database for any non-null EXEC reply. After the next disconnect the handshake sends SELECT 99, fails and retries, and the client never becomes ready again.

Repro against a local Redis 8.10.2, killing the connection with CLIENT KILL from a second client and waiting 4 s. On master:

PROBE 4s after the connection was killed: isReady = false , isOpen = true
PROBE error event x6: ERR DB index is out of range

With this change the client reconnects and is ready.

Each SELECT in the transaction is now recorded from its own slot in the EXEC reply and skipped on an ErrorReply, same as _executePipeline (#3484). That also keeps db 1 for select(1).select(99) and records a raw select added with addCommand().

Added two tests in index.spec.ts for those two cases, both fail on master. The packages/client tests pass; one sentinel lifecycle test failed once on master and passed here.


Checklist

  • Does npm test pass with this change (including linting)?
  • Is the new or changed code fully tested?
  • Is a documentation update included (if this change modifies existing APIs, or introduces new ones)?

Note

Medium Risk
Changes reconnect handshake database selection after MULTI/EXEC; incorrect tracking could strand clients, but scope is limited to transaction SELECT handling with added regression tests.

Overview
Fixes a reconnect bug where the client could persist an invalid database index after a failed SELECT inside MULTI.

_executeMulti no longer applies the multi builder’s queued selectedDB after a successful EXEC. It walks each command in the transaction and updates #selectedDB only when the command is SELECT and that command’s slot in the EXEC reply is not an ErrorReply—matching _executePipeline (#3484). Failed out-of-range SELECTs are ignored; successful earlier SELECTs in the same transaction still apply. Raw lowercase select via addCommand is recognized via case-insensitive command matching.

Two integration tests in index.spec.ts cover failed vs successful SELECT in MULTI and reconnect handshake behavior (CLIENT INFO db).

Reviewed by Cursor Bugbot for commit 809585f. Bugbot is set up for automated code reviews on this repo. Configure here.

_executeMulti recorded the database from multi().select() for any
non-null EXEC reply. Redis checks the SELECT index when EXEC runs, so
multi().select(99).exec() gets an error in the SELECT slot while the
connection stays on its database, yet the client recorded 99. The next
reconnect sent SELECT 99 in the handshake, failed, and kept retrying,
so the client never became ready again.

Each SELECT in the transaction is now recorded from its own slot in the
EXEC reply and skipped when that slot is an error, as _executePipeline
already does for a pipeline. select(1).select(99) keeps db 1, and a raw
SELECT added with addCommand() is recorded the same way.

@nkaradzhov nkaradzhov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Matches the #3484 pipeline fix, and the new tests fail on master and pass here. LGTM.

@nkaradzhov
nkaradzhov merged commit 5033f59 into redis:master Oct 7, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants