From 19a500f1899bd2f63a716aafab295d95968fc60a Mon Sep 17 00:00:00 2001 From: maxymlyskov Date: Tue, 29 Sep 2026 21:07:13 +0300 Subject: [PATCH 1/3] fix(client): keep the selected database when a pipelined command fails _executePipeline recorded the database from multi().select() only after every command in the batch had resolved. When a later command failed, Promise.all rejected and the record was skipped, although the server had already run the SELECT. The connection stayed on the new database, the client still held the old one, and the next reconnect's handshake went back to database 0 without an error. The database is now recorded when the SELECT's own reply arrives. A SELECT that fails, such as an index out of range, is still not recorded. --- packages/client/lib/client/index.spec.ts | 42 ++++++++++++++++++++++++ packages/client/lib/client/index.ts | 12 ++++--- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/packages/client/lib/client/index.spec.ts b/packages/client/lib/client/index.spec.ts index b97e8631189..d67ca0a3996 100644 --- a/packages/client/lib/client/index.spec.ts +++ b/packages/client/lib/client/index.spec.ts @@ -789,6 +789,48 @@ describe('Client', () => { minimumDockerVersion: [6, 2] // CLIENT INFO }); + testUtils.testWithClient('should remember selected db when a pipelined command fails', async client => { + // Regression: the SELECT ran on the server, but the INCR rejected the batch + // before the client recorded the database, so the reconnect went back to 0. + // The second batch's SELECT is out of range and must not replace the recorded 1. + await assert.rejects( + client.multi() + .select(1) + .set('key', 'value') + .incr('key') + .execAsPipeline() + ); + + const { databases } = await client.configGet('databases'); + await assert.rejects( + client.multi() + .select(Number(databases)) + .ping() + .execAsPipeline() + ); + + const duplicate = await client.duplicate().connect(); + try { + await Promise.all([ + once(client, 'error'), + duplicate.clientKill({ + filter: 'ID', + id: await client.clientId() + }) + ]); + } finally { + duplicate.destroy(); + } + + assert.equal( + (await client.clientInfo()).db, + 1 + ); + }, { + ...GLOBAL.SERVERS.OPEN, + minimumDockerVersion: [6, 2] // CLIENT INFO + }); + testUtils.testWithClient('should handle error replies (#2665)', async client => { await assert.rejects( client.multi() diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index d090e143481..7deb3ad565a 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -1868,8 +1868,9 @@ export default class RedisClient< return trace(CHANNELS.TRACE_BATCH, async () => { const chainId = Symbol('Pipeline Chain'); + const selectIndex = selectedDB === undefined ? -1 : commands.findLastIndex(({ args }) => args[0] === 'SELECT'); const promise = Promise.all( - commands.map(({ args }) => { + commands.map(({ args }, i) => { const traced = trace(CHANNELS.TRACE_COMMAND, () => this._self.#queue.addCommand(args, { chainId, @@ -1886,6 +1887,11 @@ export default class RedisClient< // rejections are collected by Promise.all, but the tracePromise wrapper // is a separate branch that nobody awaits. traced.catch(noop); + if (i === selectIndex) { + traced.then(() => { + this._self.#selectedDB = selectedDB!; + }, noop); + } return traced; }) ); @@ -1893,10 +1899,6 @@ export default class RedisClient< const result = await promise; - if (selectedDB !== undefined) { - this._self.#selectedDB = selectedDB; - } - return result; }, () => ({ From bbf34729c69dc9a869a4d17dff9327f4684af81b Mon Sep 17 00:00:00 2001 From: maxymlyskov Date: Tue, 29 Sep 2026 23:53:03 +0300 Subject: [PATCH 2/3] fix(client): record the selected database from each SELECT reply The previous commit recorded the database from multi().select() when the reply to the last SELECT in the batch arrived. When that last SELECT failed, as in select(1).select(99), nothing was recorded, although the server had run select(1) and stayed on database 1, so the next reconnect still went back to 0. Each SELECT now records the database from its own argument when its reply arrives, so the last SELECT the server ran is the one kept, and a SELECT that fails is never recorded. A pipelined SELECT added with addCommand() is recorded the same way. _executePipeline keeps its selectedDB parameter, no longer read, because cluster passes slotNumber after it. --- packages/client/lib/client/index.spec.ts | 13 ++++--------- packages/client/lib/client/index.ts | 7 +++---- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/packages/client/lib/client/index.spec.ts b/packages/client/lib/client/index.spec.ts index d67ca0a3996..59a55d900b8 100644 --- a/packages/client/lib/client/index.spec.ts +++ b/packages/client/lib/client/index.spec.ts @@ -792,20 +792,15 @@ describe('Client', () => { testUtils.testWithClient('should remember selected db when a pipelined command fails', async client => { // Regression: the SELECT ran on the server, but the INCR rejected the batch // before the client recorded the database, so the reconnect went back to 0. - // The second batch's SELECT is out of range and must not replace the recorded 1. + // The last of the three SELECTs is out of range, so the server ends on db 1. + const { databases } = await client.configGet('databases'); await assert.rejects( client.multi() - .select(1) + .select(2) .set('key', 'value') .incr('key') - .execAsPipeline() - ); - - const { databases } = await client.configGet('databases'); - await assert.rejects( - client.multi() + .select(1) .select(Number(databases)) - .ping() .execAsPipeline() ); diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index 7deb3ad565a..bf373e7c623 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -1868,9 +1868,8 @@ export default class RedisClient< return trace(CHANNELS.TRACE_BATCH, async () => { const chainId = Symbol('Pipeline Chain'); - const selectIndex = selectedDB === undefined ? -1 : commands.findLastIndex(({ args }) => args[0] === 'SELECT'); const promise = Promise.all( - commands.map(({ args }, i) => { + commands.map(({ args }) => { const traced = trace(CHANNELS.TRACE_COMMAND, () => this._self.#queue.addCommand(args, { chainId, @@ -1887,9 +1886,9 @@ export default class RedisClient< // rejections are collected by Promise.all, but the tracePromise wrapper // is a separate branch that nobody awaits. traced.catch(noop); - if (i === selectIndex) { + if (args[0] === 'SELECT') { traced.then(() => { - this._self.#selectedDB = selectedDB!; + this._self.#selectedDB = Number(args[1]); }, noop); } return traced; From ffafe209781312076976187a62ec595a199d02d9 Mon Sep 17 00:00:00 2001 From: maxymlyskov Date: Wed, 30 Sep 2026 20:37:54 +0300 Subject: [PATCH 3/3] fix(client): record a pipelined SELECT sent in lowercase or as a Buffer The pipeline recorded a SELECT only when its command name was the string 'SELECT'. Redis also runs select and a Buffer name sent through multi().addCommand(), so the client kept its old database and the next reconnect went back to database 0. The name is now compared as String(args[0]).toUpperCase(), as the HIMPORT check in the same file already does. --- packages/client/lib/client/index.spec.ts | 27 ++++++++++++++++++++++++ packages/client/lib/client/index.ts | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/packages/client/lib/client/index.spec.ts b/packages/client/lib/client/index.spec.ts index 59a55d900b8..3a7b7b43341 100644 --- a/packages/client/lib/client/index.spec.ts +++ b/packages/client/lib/client/index.spec.ts @@ -826,6 +826,33 @@ describe('Client', () => { minimumDockerVersion: [6, 2] // CLIENT INFO }); + testUtils.testWithClient('should remember a db selected by a raw lowercase Buffer SELECT in a pipeline', async client => { + await client.multi() + .addCommand([Buffer.from('select'), '2']) + .execAsPipeline(); + + const duplicate = await client.duplicate().connect(); + try { + await Promise.all([ + once(client, 'error'), + duplicate.clientKill({ + filter: 'ID', + id: await client.clientId() + }) + ]); + } finally { + duplicate.destroy(); + } + + assert.equal( + (await client.clientInfo()).db, + 2 + ); + }, { + ...GLOBAL.SERVERS.OPEN, + minimumDockerVersion: [6, 2] // CLIENT INFO + }); + testUtils.testWithClient('should handle error replies (#2665)', async client => { await assert.rejects( client.multi() diff --git a/packages/client/lib/client/index.ts b/packages/client/lib/client/index.ts index bf373e7c623..15986b55057 100644 --- a/packages/client/lib/client/index.ts +++ b/packages/client/lib/client/index.ts @@ -1886,7 +1886,7 @@ export default class RedisClient< // rejections are collected by Promise.all, but the tracePromise wrapper // is a separate branch that nobody awaits. traced.catch(noop); - if (args[0] === 'SELECT') { + if (String(args[0]).toUpperCase() === 'SELECT') { traced.then(() => { this._self.#selectedDB = Number(args[1]); }, noop);