redis: re-issue SUBSCRIBE after an auto-reconnect

Closed
RO
2mo ago
+509-0

What

Bun.RedisClient with autoReconnect silently goes deaf after the first dropped connection. The client reconnects and sends HELLO 3, but it never re-issues SUBSCRIBE for the channels its listeners are registered on. The app sees connected === true, the callbacks stay registered, and no message ever arrives again. onclose is not fired on a reconnecting drop either, so the app is never told it needs to resubscribe.

node-redis, ioredis and go-redis all re-subscribe on reconnect.

Repro

Against a scripted RESP3 server that logs every command per connection:

const sub = new Bun.RedisClient(url, { autoReconnect: true, maxRetries: 10 });
await sub.connect();
await sub.subscribe("news", m => got.push(m));

const pub = new Bun.RedisClient(url, { autoReconnect: false });
await pub.connect();
console.log("receivers before the drop:", await pub.publish("news", "m1"));

serverSideSocketForSub.end();             // drop the subscriber's connection
await reconnected;

console.log("commands on the new connection:", commandsSeenOnReconnect);
console.log("receivers after:", await pub.publish("news", "m2"), "| connected =", sub.connected);
receivers before the drop: 1
commands on the new connection: [["HELLO 3"]]     <- no SUBSCRIBE
receivers after: 0 | connected = true             (the listener only ever saw "m1")

Cause

on_valkey_connect runs when the HELLO handshake completes. It caches the hello value, fires onconnect, resolves the connection promise, and then drains the offline queue. Nothing looks at the subscription callback map, even though the map is still populated and is_subscriber stays true across a reconnect. The server, meanwhile, has a brand-new connection with no subscriptions on it.

Fix

Replay SUBSCRIBE for every channel in the callback map as soon as the handshake completes, before anything queued goes out.

Two details make it safe:

  • The command goes straight into the write buffer (like HELLO/SELECT do), so it is the first thing on the wire on the new connection, ahead of whatever the offline queue still holds.
  • It carries no promise. A promise here has nobody awaiting it, so a rejection (error reply, or the connection dropping again mid-replay) would surface as an unhandled rejection. resubscribe_pending counts off the confirmations it produces instead, so they do not consume a queued command's promise pair.

That counter is load-bearing today: removing it makes SUBSCRIBE news sports consume the queued PING's reply slot, and await client.ping() resolves with 2 instead of "PONG". It also covers the error case, where the server refuses the replay (an ACL change during the outage) and replies with a single -ERR and no confirmations at all: that error must not eat a reply slot either, or the command behind it is dropped unsettled and its await hangs. (If #32858 lands, its per-pair remaining_replies accounting subsumes the counter and it can be dropped.)

Pattern subscriptions are untouched: psubscribe is a raw pass-through that registers no listener, so the client has no pattern set to replay.

Also adds JSC__JSMap__keys (a JSMapIterator over IterationKind::Keys into a JSArray) since the callback map had no key-iteration binding.

Verification

test/js/valkey/reliability/subscriber-reconnect.test.ts drives a scripted RESP3 server, so it runs without Docker. It asserts the exact command list the reconnected connection receives, that the original listener keeps getting messages, and the two negative contracts (unsubscribed channels are not replayed; a client that never subscribed sends no SUBSCRIBE).

$ bun bd test test/js/valkey/reliability/subscriber-reconnect.test.ts
(pass) replays SUBSCRIBE for every channel with a listener
(pass) does not replay channels the client unsubscribed from
(pass) a client that never subscribed sends no SUBSCRIBE on reconnect
(pass) does not replay a SUBSCRIBE the offline queue is already holding
(pass) an error reply to the replay settles the in-flight command instead of eating its reply slot
(pass) replays SUBSCRIBE after SELECT when the URL names a database

4 of the 6 fail on the unfixed build. The other two pin negative contracts: a client that never subscribed sends no SUBSCRIBE, and a lazy-connect subscribe() (no explicit connect()) is not replayed on top of the copy the offline queue already holds.

Without the fix (USE_SYSTEM_BUN=1)
error: expect(received).toEqual(expected)

  [
    "HELLO 3",
-   "SUBSCRIBE news sports",
    "PING",
  ]

The existing docker-free valkey tests (resp-nesting-depth, valkey-gc, valkey-incremental-scan, valkey-tls-verify) still pass: 21 pass, 0 fail.

Deliberately out of scope

Two adjacent bugs in the same subsystem are left alone, since fixing them here would mean rewriting code that two other open PRs already rewrite:

  • The subscriber Error arm drops pair_maybe without settling it, so a user's own await client.subscribe(ch) hangs on an error reply. Predates this PR; #32858 rewrites that arm.
  • subscribe() registers the listener before the server confirms, so a drop in that window leaves a stale map entry (and a duplicate callback if the user retries). Called out in the TODO at js_valkey_functions.rs:1678; #33290 fixes the root.

If #32858 lands first, its per-pair remaining_replies accounting subsumes resubscribe_pending and the counter can be deleted.