PR #34829 - oven-sh/bun

Hardening pass over the Redis/Valkey client (src/runtime/valkey_jsc/ + src/valkey/), focused on connection lifecycle correctness. Net -1251 LOC across 20 files.

Connection lifecycle / state machine

Handshake state machine: replaced the is_authenticated / is_selecting_db_internal booleans on ConnectionFlags with a Handshake { AwaitingHello, SelectingDb, Ready } enum. connection_ready() now requires Handshake::Ready, and a previously reachable unreachable!() in the response dispatcher is gone.
Reconnect failure path: reconnect() previously called the outer JSValkeyClient wrapper that only fires the onclose callback; it never set flags.failed, never rejected queued promises, never closed the socket. Now routes through ValkeyClient::fail_with_js_value so a failed reconnect is a real client failure. The outer wrapper is renamed call_onclose_handler so the two are no longer confusable.
onclose on connect error: a synchronous error from connect() during auto-reconnect now fires onclose instead of being dropped.
on_close socket release: the event-loop socket ref is now released even when fail() throws mid-teardown.
on_close reconnect predicate: simplified and made the reconnect decision readable.
Connect errno surfaced: on_connect_error now reports the actual errno (e.g. ECONNREFUSED) instead of a generic "Connection closed".
TLS context error surfaced: a failed TLS context build now returns the real error instead of leaking a JsResult out of connect().
Auto-flusher stays unregistered when on_auto_flush made no progress, and the dead post-enqueue re-register is removed.

Promise settlement correctness

reject_all_pending_commands drains fully: previously used ? mid-drain, so the first Err(Terminated) returned early and dropped the already-mem::replace'd queues, leaving remaining promises pending forever. Now runs both loops to completion and returns the first error afterwards.
Queue iteration visits both ring halves: has_any_pending_commands / queue scans used readable_slice(0) alone, which only yields the first contiguous segment of a wrapped ring buffer. Added LinearFifo::iter() (with a unit test) and moved the scan onto it.
Subscribe ack only consumes its own promise: a subscribe-ack push no longer pops an unrelated in-flight promise; it only settles a promise whose meta carries SUBSCRIPTION_REQUEST.
Meta single source of truth: PromisePair / Entry each carried a duplicate Meta that could diverge from Promise.meta. The duplicates are deleted; divergence is now a compile error.
Unsubscribe rollback ordering: handler-map mutation is now deferred until send() succeeds, and the subscribe rollback only removes the handler that was just added.

Protocol / parser

RESPValue::read_value returns Option so Err is always a real protocol error and "need more bytes" is None. Extracted read_aggregate_header / read_n_values / read_n_entries to collapse six near-identical loops.
RESP3 *-1 → Null, _\r\n validated, and BigNumber consistently decodes as string.
Exhaustive SubscriptionPushMessage match: dropped the is_reply_kind string-compare hack; pmessage pattern is skipped before dispatch so the payload shape matches message.
ERR_REDIS_SERVER_ERROR: new error code for server -ERR / ! blob-error replies (previously mislabelled ERR_REDIS_INVALID_RESPONSE). Documented in docs/runtime/redis.mdx.
Pruned dead RedisError variants including the JSC-layer JSError / JSTerminated that don't belong in the protocol crate.

User-facing behaviour changes (for review)

psubscribe() return value: now resolves to the handler count (a number), matching subscribe()'s documented Promise<number> shape. Previously it resolved to the raw push frame {type:"psubscribe", data:[...]}. psubscribe has no .d.ts entry and is undocumented, so nothing published breaks, but it is a change.
RESP blob errors reject: a server ! blob-error now rejects the command promise (as simple -ERR replies already do) instead of resolving with an Error instance.
PUNSUBSCRIBE typo: the auto-pipeline disallow set had UNPSUBSCRIBE, so punsubscribe() was always auto-pipelined. Now matches correctly, and the check is case-insensitive so client.send("multi", ...) is caught too.
Varargs undefined/null now throw: previously del/mget/unlink/touch/zrem/lpush/rpush etc. silently skipped undefined/null in trailing arguments, and set/sadd/srem/hmget silently truncated at the first one (e.g. client.del('a', undefined, 'b') sent DEL a b). These now throw ERR_INVALID_ARG_TYPE synchronously. The published .d.ts never accepted undefined in these positions, so type-correct callers are unaffected.
Arity/type error wording: argument errors go through throw_invalid_argument_type / ERR_MISSING_ARGS with consistent codes and labels (e.g. the first arg is now labelled key, not additional arguments).

Resource ownership

_secure wrapped in the existing boringssl::c::OwnedSslCtx RAII type; manual SSL_CTX_free dropped.
Address / TLS derive Clone; the unreachable!() reconstruction in disconnect() is gone.
hset_impl uses JSValue::to_slice for RAII string release.
Fixed a WTFStringImpl leak and swallowed throw in the invoke_callbacks debug-log path.
SubscriptionCtx saved-flags bools folded into Option<SavedFlags>.

Cleanup

mod.rs flattened: the 87-line alias maze (four paths to Command, a dead index.rs, an empty ValkeyContext ZST, self-referential re-export loop) is now 5 pub mod lines + one pub use. ValkeyCommand.rs is renamed command.rs.
~190 lines of dead code removed: connection_strings, Status::is_active, on_valkey_timeout, a duplicate UnwrapOrOom, reader_pos, unreachable subscriber branches, stale Zig-port comments.
JsTerminated<T> alias deleted: it was type JsTerminated<T> = JsResult<T>, which lied about narrowing and came with an identity narrow_terminated shim. Callers now see honest -> JsResult<()>.
on_valkey_* forwarders inlined; global_object takes &self.
valkey.classes.ts table-driven: 55 command prototypes had length: 0; now derived from a table.
bun_valkey Cargo.toml: dropped six unused deps (bitflags, const_format, enum-map, enumset, libc, scopeguard).
Extracted parse_valkey_url + new_disconnected to dedupe the two constructor sites (Bun.redis default + new RedisClient); the BunObject.rs call site is now a one-liner.

Verification

bun bd                           # builds clean
bun bd test test/js/valkey/      # 49 pass / 1065 skip / 0 fail

The docker-gated integration suites skip locally (no docker daemon); CI exercises them.

Possibly related issues

The lifecycle fixes here overlap with the symptoms in several open issues, but this PR does not include dedicated regression tests for them, so they are linked here rather than marked as fixed:

#27861 (response mismatch): the reject_all_pending_commands drain fix and wrapped-ring LinearFifo::iter() scan both touch in-flight promise bookkeeping across reconnects.
#18895 (connect() behaviour on failure): failed reconnects now reject queued promises and close the socket, and on_connect_error surfaces the real errno.
#22812 (onclose not invoked): onclose now fires when connect() errors during auto-reconnect.
#33103 (close while subscribed keeps process alive): on_close now releases the socket ref even when fail() throws. See also #33104.
#29042, #29145: tangentially related at best; #32085 is the targeted fix for #29145.

Related open PRs that touch nearby lifecycle code

This is a structural pass and may textually conflict with several targeted fixes already open: #33306, #32779, #33479, #32803, #32858, #33450, #34760. Happy to rebase over whichever land first.

[review] gate passed · iteration 3 · 21 files touched

fails on main (without fix)

ASAN without fix: 9 failed, 978 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/valkey/reliability/connection-failures.test.ts test/js/valkey/reliability/error-handling.test.ts test/js/valkey/reliability/resp-nesting-depth.test.ts test/js/valkey/valkey-incremental-scan.test.ts test/js/valkey/valkey.test.ts
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (a90e1e265)

test/js/valkey/valkey.test.ts:
failed to connect to the docker API at unix:///var/run/docker.sock; check if the path is correct and if the daemon is running: dial unix /var/run/docker.sock: connect: no such file or directory
Redis is not enabled, skipping tests
(skip) Valkey Redis Client (tls) > (unnamed)
(skip) Valkey Redis Client (tls) > Basic Operations > should keep process alive when connecting
(skip) Valkey Redis Client (tls) > Basic Operations > should set and get strings
(skip) Valkey Redis Client (tls) > Basic Ope
... (truncated)

release without fix: 978 skipped
bun test v1.4.0-canary.1 (9a44e8d75)

test/js/valkey/valkey.test.ts:
failed to connect to the docker API at unix:///var/run/docker.sock; check if the path is correct and if the daemon is running: dial unix /var/run/docker.sock: connect: no such file or directory
Redis is not enabled, skipping tests
(skip) Valkey Redis Client (tls) > (unnamed)
(skip) Valkey Redis Client (tls) > Basic Operations > should keep process alive when connecting
(skip) Valkey Redis Client (tls) > Basic Operations > should set and get strings
(skip) Valkey Redis Client (tls) > Basic Operations > should test key existence
(skip) Valkey Redis Client (tls) > Basic Operations > should increment and decrement counters
(skip) Valkey Redis Client (tls) > Basic Operations > should increment by specified amount with INCRBY
(skip) Valkey Redis Client (tls) > Basic Operations > should increment by float amount with INCRBYFLOAT
(skip) Valkey Redis Client (tls) > Basic Operations > should decrement by specified amount with DECRBY
(skip) Valkey Redis Client (tls) > Basic Operations > should rename a key with RENAME
(skip) Valkey Redis Client (tls) > Basic Operations > should rename a key with RENAME overwr
... (truncated)

passes on PR (with fix)

ASAN with fix: 978 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/js/valkey/reliability/connection-failures.test.ts test/js/valkey/reliability/error-handling.test.ts test/js/valkey/reliability/resp-nesting-depth.test.ts test/js/valkey/valkey-incremental-scan.test.ts test/js/valkey/valkey.test.ts
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
bun test v1.4.0 (a90e1e265)

test/js/valkey/valkey.test.ts:
failed to connect to the docker API at unix:///var/run/docker.sock; check if the path is correct and if the daemon is running: dial unix /var/run/docker.sock: connect: no such file or directory
Redis is not enabled, skipping tests
(skip) Valkey Redis Client (tls) > (unnamed)
(skip) Valkey Redis Client (tls) > Basic Operations > should keep process alive when connecting
(skip) Valkey Redis Client (tls) > Basic Operations > should set and get strings
(skip) Valkey Redis Client (tls) > Basic Ope
... (truncated)

release with fix: 978 skipped
$ bun scripts/build.ts --profile=release
info: syncing channel updates for nightly-2026-05-06-x86_64-unknown-linux-gnu
info: latest update on 2026-05-06 for version 1.97.0-nightly (e95e73209 2026-05-05)
info: component rust-src is up to date
info: checking for self-update (current version: 1.29.0)
[configured] bun-profile → bun (stripped) in 695ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[1/77] gen ErrorCode+*.h
[2/31] gen generated_host_exports.rs
generated_host_exports.rs: 91 exports (host=3, lazy=10, generic=78, rust=0); 244 extern-C blocks audited
[3/31] gen ZigGeneratedClasses.{cpp,h,rs}
Found 2 classes from /workspace/bun/src/jsc/resolve_message.classes.ts
  - ResolveMessage (13 fields)
  - BuildMessage (10 fields)
Found 1 classes from /workspace/bun/src/runtime/api/Archive.classes.ts
  - Archive (4 fields, 1 class fields)
Found 2 classes from /workspace/bun/src/runtime/api/BunObject.classes.ts
  - ResourceUsage (8 fields)
  - Subprocess (20 fields)
Found 1 classes from /workspace/bun/src/runtime/api/cron.classes.ts
  - CronJob (5 fields)
Found 3 classes from /workspace/bun/src/runtime/api/filesystem_router.classes.ts
  - FileSyst
... (truncated)

diff hotspot

Cargo.lock                                         |    6 -
 docs/runtime/redis.mdx                             |    1 +
 src/collections/linear_fifo.rs                     |   29 +
 src/jsc/bindings/ErrorCode.ts                      |    4 +-
 src/runtime/api/BunObject.rs                       |   17 +-
 .../valkey_jsc/{ValkeyCommand.rs => command.rs}    |  158 ++-
 src/runtime/valkey_jsc/index.rs                    |   21 -
 src/runtime/valkey_jsc/js_valkey.rs                | 1385 +++++++++---------
 src/runtime/valkey_jsc/js_valkey_functions.rs      | 1468 +++++++-------------
 src/runtime/valkey_jsc/mod.rs                      |   81 +-
 src/runtime/valkey_jsc/protocol_jsc.rs             |  125 +-
 src/runtime/valkey_jsc/valkey.classes.ts           |  728 +++-------
 src/runtime/valkey_jsc/valkey.rs                   |  909 +++++-------
 src/valkey/Cargo.toml                              |    6 -
 src/valkey/lib.rs                                  |    7 +-
 src/valkey/valkey_protocol.rs                      |  392 +++---
 .../valkey/reliability/connection-failures.test.ts |   30 +-
 test/js/valkey/reliability/error-handling.test.ts  |    2 +-
 .../valkey/reliability/resp-nesting-depth.test.ts  |   44 +-
 test/js/valkey/valkey-incremental-scan.test.ts     |    9 +-
 test/js/valkey/valkey.test.ts                      |   12 +-
 21 files changed, 2133 insertions(+), 3301 deletions(-)

gate history · 7 passed · 1 rejected · iteration 3

evidence per changed file

file                                           reads  edits  tests
Cargo.lock                                         0      0      0
docs/runtime/redis.mdx                             0      0      0
src/collections/linear_fifo.rs                     1      1      0
src/jsc/bindings/ErrorCode.ts                      1      1      0
src/runtime/api/BunObject.rs                       0      0      0
src/runtime/valkey_jsc/command.rs                  1      1      0
src/runtime/valkey_jsc/index.rs                    0      0      0
src/runtime/valkey_jsc/js_valkey.rs               14      8      0
src/runtime/valkey_jsc/js_valkey_functions.rs      0      0      0
src/runtime/valkey_jsc/mod.rs                      0      0      0
src/runtime/valkey_jsc/protocol_jsc.rs             0      0      0
src/runtime/valkey_jsc/valkey.classes.ts           0      0      0
src/runtime/valkey_jsc/valkey.rs                   7      4      0
src/valkey/Cargo.toml                              0      0      0
src/valkey/lib.rs                                  0      0      0
src/valkey/valkey_protocol.rs                      0      0      0
(+ 5 more files)