From 1bcdc2281655d14c9acb5cc35768111cbb51356f Mon Sep 17 00:00:00 2001 From: Alistair Smith Date: Tue, 18 Aug 2026 13:05:05 -0700 Subject: [PATCH 1/2] valkey: a duplicate starts with no close history --- src/runtime/valkey_jsc/js_valkey.rs | 5 --- src/runtime/valkey_jsc/valkey.rs | 4 +- .../reliability/connection-failures.test.ts | 45 +++++++++++++++++-- 3 files changed, 43 insertions(+), 11 deletions(-) diff --git a/src/runtime/valkey_jsc/js_valkey.rs b/src/runtime/valkey_jsc/js_valkey.rs index 7185779a0b59..0b2c77145f41 100644 --- a/src/runtime/valkey_jsc/js_valkey.rs +++ b/src/runtime/valkey_jsc/js_valkey.rs @@ -866,9 +866,6 @@ impl JSValkeyClient { tls, database: client.database, flags: valkey::ConnectionFlags { - // If the user manually closed the connection, then duplicating a closed client - // means the new client remains finalized. - is_manually_closed: client.flags.is_manually_closed, enable_offline_queue: if sub_ctx.is_subscriber { sub_ctx.original_enable_offline_queue } else { @@ -881,8 +878,6 @@ impl JSValkeyClient { } else { client.flags.enable_auto_pipelining }, - // Duplicating a finalized client means it stays finalized. - finalized: client.flags.finalized, ..Default::default() }, max_retries: client.max_retries, diff --git a/src/runtime/valkey_jsc/valkey.rs b/src/runtime/valkey_jsc/valkey.rs index f6e08d03ae71..4366a34cf5cd 100644 --- a/src/runtime/valkey_jsc/valkey.rs +++ b/src/runtime/valkey_jsc/valkey.rs @@ -595,9 +595,7 @@ impl ValkeyClient { // A failure the client detected itself (idle timeout, protocol or // handshake error) has always been a deliberate close; `on_close` reads - // `failed` and skips the retry policy. It is not `is_manually_closed`: - // that flag is copied into `duplicate()`, and a duplicate of a failed - // client should still reconnect. + // `failed` and skips the retry policy. let closed = self.close(uws::CloseCode::Failure); // unconditionally, whatever `val` is val.and(closed) } diff --git a/test/js/valkey/reliability/connection-failures.test.ts b/test/js/valkey/reliability/connection-failures.test.ts index ee5e4028fd85..239c0dbdf6c8 100644 --- a/test/js/valkey/reliability/connection-failures.test.ts +++ b/test/js/valkey/reliability/connection-failures.test.ts @@ -896,9 +896,48 @@ describe("Valkey: Recovering After fail()", () => { await expect(client.ping()).rejects.toMatchObject({ code: "ERR_REDIS_INVALID_RESPONSE_TYPE" }); expect(client.connected).toBe(false); duplicate = await client.duplicate(); - // A duplicate copies the original's manual-close state; a failure is not - // one, so the drop of connection 2 goes through the retry policy: no - // onclose, a second onconnect, and the queued PING answered by connection 3. + // A duplicate carries no close history from its source, so the drop of + // connection 2 goes through the retry policy: no onclose, a second + // onconnect, and the queued PING answered by connection 3. + const reconnected = Promise.withResolvers(); + let connects = 0; + duplicate.onconnect = () => { + if (++connects === 2) reconnected.resolve(); + }; + duplicate.onclose = err => reconnected.reject(err); + expect(await duplicate.ping()).toBe("PONG"); + await reconnected.promise; + expect(await duplicate.ping()).toBe("PONG"); + expect(fake.connections).toBe(3); + } finally { + duplicate?.close(); + client.close(); + fake.server.close(); + } + }); + + test("a duplicate of a closed client auto-reconnects", async () => { + // Connection 2 (the duplicate's first) is dropped by the server right + // after it answers PING. + const fake = helloServer({ + PING: (connection, socket) => { + if (connection === 2) { + socket.end("+PONG\r\n"); + return null; + } + return "+PONG\r\n"; + }, + }); + const port = await fake.listen(); + const client = new RedisClient(`redis://127.0.0.1:${port}`, { autoReconnect: true }); + let duplicate: RedisClient | undefined; + try { + await client.connect(); + client.close(); + duplicate = await client.duplicate(); + // The duplicate has no close history of its own, so the drop of + // connection 2 goes through the retry policy: no onclose, a second + // onconnect, and the next PING answered by connection 3. const reconnected = Promise.withResolvers(); let connects = 0; duplicate.onconnect = () => { From dec40a72b95410e8631543a08c674c698739e771 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 18 Aug 2026 23:31:49 +0000 Subject: [PATCH 2/2] test(valkey): the first PING of the failed-client duplicate is answered by connection 2, not queued --- test/js/valkey/reliability/connection-failures.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/js/valkey/reliability/connection-failures.test.ts b/test/js/valkey/reliability/connection-failures.test.ts index 239c0dbdf6c8..1edcee317bea 100644 --- a/test/js/valkey/reliability/connection-failures.test.ts +++ b/test/js/valkey/reliability/connection-failures.test.ts @@ -898,7 +898,7 @@ describe("Valkey: Recovering After fail()", () => { duplicate = await client.duplicate(); // A duplicate carries no close history from its source, so the drop of // connection 2 goes through the retry policy: no onclose, a second - // onconnect, and the queued PING answered by connection 3. + // onconnect, and the next PING answered by connection 3. const reconnected = Promise.withResolvers(); let connects = 0; duplicate.onconnect = () => {