From 9b0cf0b784b5aab5fd67ad61389a882b8dec0280 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 02:04:59 +0000 Subject: [PATCH 1/7] redis: propagate the real failure reason to the connect() promise A -WRONGPASS or -NOAUTH reply to HELLO, a connectionTimeout expiry, and a TCP connect error all surfaced as the same ERR_REDIS_CONNECTION_CLOSED / "Connection closed" when awaited via client.connect(). The server's actual error text was discarded. The failure was already passed to fail() which rejected queued commands with the right message, but on_valkey_close() then built a fresh generic error for the connection_promise and the onclose callback. Store the first failure in close_reason and use it in on_valkey_close() so connect() rejects with ERR_REDIS_AUTHENTICATION_FAILED + the server's WRONGPASS/NOAUTH text, ERR_REDIS_CONNECTION_TIMEOUT for a connectionTimeout, and "Failed to connect" for a TCP connect error. Fixes #3064 --- src/runtime/valkey_jsc/js_valkey.rs | 24 +++-- src/runtime/valkey_jsc/valkey.rs | 10 ++ .../reliability/connection-failures.test.ts | 93 +++++++++++++++++++ 3 files changed, 121 insertions(+), 6 deletions(-) diff --git a/src/runtime/valkey_jsc/js_valkey.rs b/src/runtime/valkey_jsc/js_valkey.rs index 28fbc111edc7..b198c3639af2 100644 --- a/src/runtime/valkey_jsc/js_valkey.rs +++ b/src/runtime/valkey_jsc/js_valkey.rs @@ -810,6 +810,7 @@ impl JSValkeyClient { reply_scanner: Default::default(), retry_attempts: 0, auto_flusher: Default::default(), + close_reason: None, }), global_object, this_value: JsCell::new(JsRef::empty()), @@ -930,6 +931,7 @@ impl JSValkeyClient { reply_scanner: Default::default(), retry_attempts: 0, auto_flusher: Default::default(), + close_reason: None, }), global_object, this_value: JsCell::new(JsRef::empty()), @@ -1045,6 +1047,7 @@ impl JSValkeyClient { // Without this, every subsequent command rejects with "Connection has // failed" forever — see https://github.com/oven-sh/bun/issues/29925. self.client_mut().flags.failed = false; + self.client_mut().close_reason = None; let self_br = BackRef::new(self); let _update = scopeguard::guard(self_br, |p| p.update_poll_ref()); @@ -1374,12 +1377,14 @@ impl JSValkeyClient { }; this_jsvalue.ensure_still_alive(); - // Create an error value - let error_value = protocol_jsc::valkey_error_to_js( - &global_object, - b"Connection closed", - protocol::RedisError::ConnectionClosed, - ); + let error_value = match self.client_mut().close_reason.take() { + Some((message, err)) => protocol_jsc::valkey_error_to_js(&global_object, &*message, err), + None => protocol_jsc::valkey_error_to_js( + &global_object, + b"Connection closed", + protocol::RedisError::ConnectionClosed, + ), + }; let _exit = self.vm().enter_event_loop_scope(); @@ -1978,6 +1983,13 @@ impl SocketHandler { p.update_poll_ref(); }); + if this.client.get().close_reason.is_none() { + this.client_mut().close_reason = Some(( + Box::<[u8]>::from(&b"Failed to connect"[..]), + protocol::RedisError::ConnectionClosed, + )); + } + narrow_terminated(this.client_mut().on_close()) } diff --git a/src/runtime/valkey_jsc/valkey.rs b/src/runtime/valkey_jsc/valkey.rs index eed40edb6a8f..53e4a77ece7a 100644 --- a/src/runtime/valkey_jsc/valkey.rs +++ b/src/runtime/valkey_jsc/valkey.rs @@ -275,6 +275,11 @@ pub struct ValkeyClient { pub flags: ConnectionFlags, + /// First failure recorded via `fail()`. `on_valkey_close()` uses this so + /// the `connect()` promise rejects with the real cause (HELLO auth error + /// text, connection timeout, …) instead of a generic "Connection closed". + pub close_reason: Option<(Box<[u8]>, RedisError)>, + // Auto-pipelining pub auto_flusher: AutoFlusher, @@ -573,6 +578,10 @@ impl ValkeyClient { return Ok(()); } + if self.close_reason.is_none() { + self.close_reason = Some((Box::<[u8]>::from(message), err)); + } + if self.flags.finalized { // We can't run promises inside finalizers. if self.queue.readable_length() + self.in_flight.readable_length() > 0 { @@ -1270,6 +1279,7 @@ impl ValkeyClient { self.flags.failed = false; self.flags.is_authenticated = false; self.flags.is_selecting_db_internal = false; + self.close_reason = None; if matches!(self.socket, AnySocket::SocketTcp(_)) { // if is tcp, we need to start the connection process // if is tls, we need to wait for the handshake to complete diff --git a/test/js/valkey/reliability/connection-failures.test.ts b/test/js/valkey/reliability/connection-failures.test.ts index bcf8964ac348..14fe17309a7a 100644 --- a/test/js/valkey/reliability/connection-failures.test.ts +++ b/test/js/valkey/reliability/connection-failures.test.ts @@ -337,6 +337,99 @@ describe.skipIf(!isEnabled)("Valkey: Connection Failures", () => { }); }); +describe("Valkey: connect() error identity", () => { + // https://github.com/oven-sh/bun/issues/3064 + // Previously all connect-time failures surfaced as ERR_REDIS_CONNECTION_CLOSED / "Connection closed". + const CRLF = "\r\n"; + + async function stubServer(reply: (text: string) => string | null) { + const server = net.createServer(sock => { + sock.on("error", () => {}); + sock.on("data", d => { + const r = reply(d.toString().toUpperCase()); + if (r !== null) sock.write(r); + }); + }); + await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); + const { port } = server.address() as net.AddressInfo; + return { port, close: () => server.close() }; + } + + async function connectError(url: string, options: any) { + const client = new RedisClient(url, options); + try { + await client.connect(); + return { code: undefined, message: "" }; + } catch (e: any) { + return { code: e?.code, message: e?.message }; + } finally { + try { + client.close(); + } catch {} + } + } + + test("-WRONGPASS reply to HELLO rejects connect() with the server's error text", async () => { + const srv = await stubServer(t => + t.includes("HELLO") ? `-WRONGPASS invalid username-password pair or user is disabled.${CRLF}` : `+OK${CRLF}`, + ); + try { + expect(await connectError(`redis://:bad@127.0.0.1:${srv.port}`, { autoReconnect: false })).toEqual({ + code: "ERR_REDIS_AUTHENTICATION_FAILED", + message: "WRONGPASS invalid username-password pair or user is disabled.", + }); + } finally { + srv.close(); + } + }); + + test("-NOAUTH reply to HELLO rejects connect() with the server's error text", async () => { + const srv = await stubServer(t => + t.includes("HELLO") + ? `-NOAUTH HELLO must be called with the client already authenticated${CRLF}` + : `-NOAUTH Authentication required.${CRLF}`, + ); + try { + expect(await connectError(`redis://127.0.0.1:${srv.port}`, { autoReconnect: false })).toEqual({ + code: "ERR_REDIS_AUTHENTICATION_FAILED", + message: "NOAUTH HELLO must be called with the client already authenticated", + }); + } finally { + srv.close(); + } + }); + + test("connectionTimeout expiry rejects connect() with ERR_REDIS_CONNECTION_TIMEOUT", async () => { + const srv = await stubServer(() => null); + try { + const err = await connectError(`redis://127.0.0.1:${srv.port}`, { + autoReconnect: false, + connectionTimeout: 200, + }); + expect(err.code).toBe("ERR_REDIS_CONNECTION_TIMEOUT"); + expect(err.message).toContain("Connection timeout reached after"); + } finally { + srv.close(); + } + }); + + test("TCP connect error rejects connect() with a distinguishable message", async () => { + const listener = net.createServer(); + await new Promise(resolve => listener.listen(0, "127.0.0.1", resolve)); + const { port } = listener.address() as net.AddressInfo; + await new Promise(resolve => listener.close(() => resolve())); + + const err = await connectError(`redis://127.0.0.1:${port}`, { + autoReconnect: false, + connectionTimeout: 2000, + }); + expect(err).toEqual({ + code: "ERR_REDIS_CONNECTION_CLOSED", + message: "Failed to connect", + }); + }); +}); + describe("Valkey: Auto-Reconnect In-Flight Commands", () => { function readCommands(state: { buffer: Buffer }): string[][] { const commands: string[][] = []; From d12c80bc8f8534744bb1fc2d34e3e4bcd7b511cf Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sat, 25 Jul 2026 02:07:16 +0000 Subject: [PATCH 2/7] [autofix.ci] apply automated fixes --- src/runtime/valkey_jsc/js_valkey.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/runtime/valkey_jsc/js_valkey.rs b/src/runtime/valkey_jsc/js_valkey.rs index b198c3639af2..6254096a3c58 100644 --- a/src/runtime/valkey_jsc/js_valkey.rs +++ b/src/runtime/valkey_jsc/js_valkey.rs @@ -1378,7 +1378,9 @@ impl JSValkeyClient { this_jsvalue.ensure_still_alive(); let error_value = match self.client_mut().close_reason.take() { - Some((message, err)) => protocol_jsc::valkey_error_to_js(&global_object, &*message, err), + Some((message, err)) => { + protocol_jsc::valkey_error_to_js(&global_object, &*message, err) + } None => protocol_jsc::valkey_error_to_js( &global_object, b"Connection closed", From 7cfefe888fe9dbd1c70b6aab415055eff7b200c3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 02:09:01 +0000 Subject: [PATCH 3/7] test: drop incorrect issue reference from comment --- test/js/valkey/reliability/connection-failures.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/js/valkey/reliability/connection-failures.test.ts b/test/js/valkey/reliability/connection-failures.test.ts index 14fe17309a7a..ac7c5ea3c363 100644 --- a/test/js/valkey/reliability/connection-failures.test.ts +++ b/test/js/valkey/reliability/connection-failures.test.ts @@ -338,7 +338,6 @@ describe.skipIf(!isEnabled)("Valkey: Connection Failures", () => { }); describe("Valkey: connect() error identity", () => { - // https://github.com/oven-sh/bun/issues/3064 // Previously all connect-time failures surfaced as ERR_REDIS_CONNECTION_CLOSED / "Connection closed". const CRLF = "\r\n"; From 8610e6e6e2c0a06cec3ce6aaef287ad8ed04ed1e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 02:39:28 +0000 Subject: [PATCH 4/7] redis: clear close_reason at the start of every connect() attempt on_connect_error under autoReconnect left the stale "Failed to connect" close_reason alive into the next attempt; if that attempt then timed out before on_open(), the connect() promise reported the stale reason instead of ERR_REDIS_CONNECTION_TIMEOUT. --- src/runtime/valkey_jsc/js_valkey.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/runtime/valkey_jsc/js_valkey.rs b/src/runtime/valkey_jsc/js_valkey.rs index 6254096a3c58..4a7abe3ef50c 100644 --- a/src/runtime/valkey_jsc/js_valkey.rs +++ b/src/runtime/valkey_jsc/js_valkey.rs @@ -1511,6 +1511,7 @@ impl JSValkeyClient { fn connect(&self) -> Result<(), crate::Error> { self.client_mut().flags.needs_to_open_socket = false; + self.client_mut().close_reason = None; let _guard = self.ref_scope(); From f44ca52ea9b7de9a441b94e748a1da6fdca8ed75 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 02:44:35 +0000 Subject: [PATCH 5/7] test: assert the exact connectionTimeout message --- .../valkey/reliability/connection-failures.test.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/test/js/valkey/reliability/connection-failures.test.ts b/test/js/valkey/reliability/connection-failures.test.ts index ac7c5ea3c363..a2ac877b2204 100644 --- a/test/js/valkey/reliability/connection-failures.test.ts +++ b/test/js/valkey/reliability/connection-failures.test.ts @@ -401,12 +401,15 @@ describe("Valkey: connect() error identity", () => { test("connectionTimeout expiry rejects connect() with ERR_REDIS_CONNECTION_TIMEOUT", async () => { const srv = await stubServer(() => null); try { - const err = await connectError(`redis://127.0.0.1:${srv.port}`, { - autoReconnect: false, - connectionTimeout: 200, + expect( + await connectError(`redis://127.0.0.1:${srv.port}`, { + autoReconnect: false, + connectionTimeout: 200, + }), + ).toEqual({ + code: "ERR_REDIS_CONNECTION_TIMEOUT", + message: "Connection timeout reached after 200ms", }); - expect(err.code).toBe("ERR_REDIS_CONNECTION_TIMEOUT"); - expect(err.message).toContain("Connection timeout reached after"); } finally { srv.close(); } From 61a2166ce7e294f8a73ffe34a4888fdd90484ea3 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 02:50:04 +0000 Subject: [PATCH 6/7] test: accumulate HELLO bytes before replying in the connect() error stub The stub only answers the initial HELLO so it now buffers until the full CRLF-terminated command is received and replies exactly once, avoiding any reliance on a single-write TCP read boundary. --- .../reliability/connection-failures.test.ts | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/test/js/valkey/reliability/connection-failures.test.ts b/test/js/valkey/reliability/connection-failures.test.ts index a2ac877b2204..dbfcedf4f3f8 100644 --- a/test/js/valkey/reliability/connection-failures.test.ts +++ b/test/js/valkey/reliability/connection-failures.test.ts @@ -341,12 +341,18 @@ describe("Valkey: connect() error identity", () => { // Previously all connect-time failures surfaced as ERR_REDIS_CONNECTION_CLOSED / "Connection closed". const CRLF = "\r\n"; - async function stubServer(reply: (text: string) => string | null) { + async function stubServer(helloReply: string | null) { const server = net.createServer(sock => { + let received = ""; + let replied = false; sock.on("error", () => {}); sock.on("data", d => { - const r = reply(d.toString().toUpperCase()); - if (r !== null) sock.write(r); + if (replied || helloReply === null) return; + received += d.toString().toUpperCase(); + if (received.includes("HELLO") && received.endsWith(CRLF)) { + replied = true; + sock.write(helloReply); + } }); }); await new Promise(resolve => server.listen(0, "127.0.0.1", resolve)); @@ -369,9 +375,7 @@ describe("Valkey: connect() error identity", () => { } test("-WRONGPASS reply to HELLO rejects connect() with the server's error text", async () => { - const srv = await stubServer(t => - t.includes("HELLO") ? `-WRONGPASS invalid username-password pair or user is disabled.${CRLF}` : `+OK${CRLF}`, - ); + const srv = await stubServer(`-WRONGPASS invalid username-password pair or user is disabled.${CRLF}`); try { expect(await connectError(`redis://:bad@127.0.0.1:${srv.port}`, { autoReconnect: false })).toEqual({ code: "ERR_REDIS_AUTHENTICATION_FAILED", @@ -383,11 +387,7 @@ describe("Valkey: connect() error identity", () => { }); test("-NOAUTH reply to HELLO rejects connect() with the server's error text", async () => { - const srv = await stubServer(t => - t.includes("HELLO") - ? `-NOAUTH HELLO must be called with the client already authenticated${CRLF}` - : `-NOAUTH Authentication required.${CRLF}`, - ); + const srv = await stubServer(`-NOAUTH HELLO must be called with the client already authenticated${CRLF}`); try { expect(await connectError(`redis://127.0.0.1:${srv.port}`, { autoReconnect: false })).toEqual({ code: "ERR_REDIS_AUTHENTICATION_FAILED", @@ -399,7 +399,7 @@ describe("Valkey: connect() error identity", () => { }); test("connectionTimeout expiry rejects connect() with ERR_REDIS_CONNECTION_TIMEOUT", async () => { - const srv = await stubServer(() => null); + const srv = await stubServer(null); try { expect( await connectError(`redis://127.0.0.1:${srv.port}`, { From fb01fcce551e6966b7e47cd9265167bb63637c46 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 25 Jul 2026 02:54:04 +0000 Subject: [PATCH 7/7] ci: retrigger gate (workspace was polluted with unrelated quic files)