fix(bb-realtime): auto-reconnect + resubscribe production WebSocket on unexpected close - #497
Open
soberm wants to merge 9 commits into
Open
fix(bb-realtime): auto-reconnect + resubscribe production WebSocket on unexpected close#497soberm wants to merge 9 commits into
soberm wants to merge 9 commits into
Conversation
…n unexpected close The production Realtime middleware (aws-middleware.ts) previously did not reconnect after an unexpected WebSocket close — only the local mock did. API Gateway WebSockets have hard limits (2h max connection duration, 10-min idle timeout), so a long-running agent turn (now up to 8h on AgentCore) could silently lose its stream when the socket dropped, with no client recovery. Production now mirrors the mock's recovery model: - Auto-reconnect on unexpected close (not on normal 1000/1005 or client unsubscribe) with exponential backoff (min(1000*2^(n-1), 30_000)), capped at MAX_RECONNECT=5. - Resubscribe all stored channels with their replayed tokens on the new socket. - Re-establish the keep-alive ping timer on reconnect. - Pending establishment promises are kept intact across a transient drop so the reconnect resolves them. Adds an optional onReconnect() callback to SubscribeOptions, fired after a successful resubscribe (after the drop's onDisconnect). Purely additive: existing subscribe(handler) and onDisconnect callers are unaffected.
🦋 Changeset detectedLatest commit: dc4f75e The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…ify, stale-token surfacing, no zombie connection
Resolves code-review findings on the production reconnect path:
- onDisconnect now fires on EVERY unexpected drop (handlers are cleared only
on a terminal 1000/1005 close, not when scheduling a reconnect).
- Stale replayed token on resubscribe (2h token TTL vs up to 8h sessions) is
now surfaced to onDisconnect('error') and drains resubscribePending so
surviving channels still fire onReconnect — instead of a silent drop.
- onerror no longer rejects/clears pendingEstablished; onclose alone decides
terminal-reject vs reconnect-preserve, so an in-flight subscribe() during a
transient drop resolves on reconnect instead of rejecting.
- Hitting MAX_RECONNECT now tears the connection down (reject pending, terminal
onDisconnect, clear timers, delete pool entry) so a later subscribe()
rebuilds — no zombie entry that never reconnects.
- reconnectAttempts resets only after resubscribe is confirmed (not on every
onopen), so a flapping open-then-close socket still hits the cap; 1006
onDisconnect double-fire deduped per socket.
- A channel-less reconnect (all tokens stale/unsubscribed) tears down instead
of marching to the cap.
Adds failure-path unit tests (every-drop notify, flapping cap, stale-token
surfacing, onerror-preserves-pending, post-cap rebuild). 87/87 pass.
…eword landed tests Address second-cycle review on PR #497: - Wire onReconnect through the mock transport (mock-middleware.ts) so local dev matches production: subscribeTo takes onReconnect, stored in reconnectHandlers, destructured in hydrate's subscribe closure, and fired after the mock's onopen replays stored channels on reconnect. Closes the T4 mock/runtime fidelity gap. - Expose subscribeTo's `connection` as a live getter (reads current conn.ws) in both middlewares, so `.connection` reflects the reconnected socket instead of the stale one captured at subscribe time. - Reword reconnect.test.ts from pre-implementation "RED / does not exist yet" narration to present-tense coverage of the shipped reconnect behavior.
3 tasks
…wn (fixes hung test suite) The mock middleware's ws.onclose called scheduleReconnect unconditionally, so a client-initiated close (unsubscribe) or __resetConnectionsForTest teardown re-armed a reconnect setTimeout and recreated a pooled connection AFTER teardown. The stale timer kept the Node event loop alive, so `node --test` never exited — the bb-realtime suite hung after the mock reconnect test (CI "PR Checks" job stuck >40min; locally `npm test` timed out with "Promise resolution is still pending but the event loop has already resolved"). Fix (mock-middleware.ts): - scheduleReconnect now reads the existing pool entry instead of recreating one, and short-circuits when the connection is torn down or has zero subscriptions (mirrors the production aws-middleware guard) — a stale timer can no longer resurrect a deleted connection. - Add a per-connection `tornDown` flag; __resetConnectionsForTest sets it, clears the reconnect timer, and detaches onmessage/onerror/onclose before close(), so the async close cannot schedule a new reconnect. Fully quiescent afterward. Legitimate unexpected-drop reconnect + resubscribe + onReconnect is unchanged (token-replay and fires-onReconnect tests still pass). Suite now exits cleanly: 89/89 pass, exit 0. aws-middleware.ts already had these guards — untouched.
3 tasks
Adds an over-the-wire e2e (test-apps/comprehensive) that subscribes to a dedicated channel, forces a mid-stream socket close via sub.connection.close(), waits for onReconnect, and asserts a message published AFTER the drop is still delivered on the reconnected socket — exercising the production auto-reconnect + resubscribe path end-to-end. Cleans up the subscription in a finally block. Passes locally against the mock middleware.
…nd-trip
The reconnect e2e passed on the mock (which fires onReconnect synchronously on
frame-send) but failed on E2E Sandbox and E2E Production with 'onReconnect did
not fire within 15s' (~15.5s observed). Diagnosis: NOT a bug — the real AWS
ws-server DOES send {type:'subscribe_success', channel} on every successful
resubscribe (index.aws.ts), and the middleware's settleResubscribe/onReconnect
keys on exactly that. It's simply slower than the mock: >=1s reconnect backoff
+ $connect handshake + $default Lambda (possible cold start) + Secrets Manager
token-secret fetch + token validation + DynamoDB put + PostToConnection. That
round-trip exceeds the mock-tuned 15s deadline.
Widen the reconnect deadline to 60s and the post-reconnect delivery window to
30s so the test reflects a realistic AWS cold-start reconnect budget. The mock
still fires onReconnect in well under a second, so local remains fast.
…01 (intent-based classification)
Verified against a real AWS sandbox: the reconnect e2e was failing because the
AWS middleware classified WebSocket close codes {1000,1005} as terminal and only
reconnected on other codes. A client-side sub.connection.close() with no args
sends 1005 ("no status"), which was treated as a clean/terminal close, so the
transport never reconnected and onReconnect never fired (the 60s test wait
expired). The mock reconnects on ANY close (guarded by tornDown/no-subscriptions),
which is why it passed locally — a mock/AWS parity gap.
Fix: classify by INTENT, not close code. Track an explicit intentionalClose flag
set only on client-initiated teardown (last-channel unsubscribe,
__resetConnectionsForTest, MAX_RECONNECT give-up). onclose now reconnects on any
close unless intentionalClose or subscriptions.size===0 — so clean closes
(1000/1005), going-away (1001), and abnormal (1006) all reconnect unless the
client asked to stop. The close-code→reason mapping for onDisconnect
(1001→timeout, 1006→error, else unknown) is preserved. This also de-risks the
case where API Gateway emits a clean 1000/1005 on a forced disconnect.
Empirical: against a real deployed sandbox (996099992135/us-west-2), the reconnect
e2e now passes in ~5.7s (onReconnect fires) vs the prior 60s timeout. Unit: 94/94.
…ite exits cleanly
The comprehensive e2e suite intermittently force-exited with code 1 via the 15s
open-handle backstop ("Open handles prevented clean exit") even when every test
passed. Root cause: the realtime client middleware keeps a module-level pooled
WebSocket (+ keep-alive/reconnect timers) that test.after never closed — killing
the dev server stops the server, not the client pool — so node:test could not
exit and the backstop fired.
Fix: in test.after, before arming the backstop, best-effort dynamic-import the
realtime middleware and call __resetConnectionsForTest(). Reset BOTH the mock
(local) and aws (sandbox/production) middleware modules so it's environment-
agnostic; each is a safe no-op over an empty pool. Dynamic import keeps the
inactive module out of the hydration chain during tests. The backstop is kept as
a safety net.
Local: full suite now exits 0 (tests 390 / pass 389 / fail 0 / skipped 1), no
open-handle message.
…blew the block budget on AWS)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Issue #, if available: internal — useChat WebSocket timeout handling for long agent turns
The production Realtime middleware (
packages/bb-realtime/src/aws-middleware.ts) did not reconnect after an unexpected WebSocket close — only the local mock did. API Gateway WebSockets enforce a 2h max connection duration and a 10-min idle timeout, so a long-running agent turn (now up to 8h on AgentCore) could silently lose its stream when the socket dropped, with no client-side recovery.This is PR 1 of 2 (the transport-layer fix). A stacked PR on top wires
useChat(bb-agent) to re-sync conversation state from the DB on reconnect and reset loading on send-path failures.Changes
Brings production to parity with the mock's recovery model:
1000/1005close or a client-initiatedunsubscribe) with exponential backoffmin(1000 * 2^(n-1), 30_000), capped atMAX_RECONNECT = 5.onReconnect()onSubscribeOptions, fired once after a successful resubscribe (after the drop'sonDisconnect). Purely additive — existingsubscribe(handler)/onDisconnectcallers are unaffected.Potentially sensitive parts (all covered by tests + reviewed):
onDisconnectfires on every drop (handlers cleared only on terminal close).onDisconnect('error')and drainsresubscribePendingso surviving channels still fireonReconnect— no silent drop.onerrorno longer rejectspendingEstablished;onclosealone decides terminal-reject vs reconnect-preserve, so an in-flightsubscribe()during a transient drop resolves on reconnect.MAX_RECONNECT(or a channel-less reconnect) tears the connection down so a latersubscribe()rebuilds — no zombie pool entry.Validation
packages/bb-realtime/src/reconnect.test.ts(node:test, self-contained FakeWebSocket + mock timers) — 87/87 bb-realtime tests pass. Covers: auto-reconnect after1006with retry cap; resubscribe with replayed token;onReconnectordering; keep-alive re-arm; and the failure paths — every-drop notify, flapping-socket cap, stale-token surfacing,onerrorpreserves in-flightestablished, post-cap rebuild.npm run build✅ ·npm run lint:deps✅ ·biome lint✅ (0 errors) · API report regenerated (SubscribeOptionsgainsonReconnect?). Changeset added (@aws-blocks/bb-realtimeminor).Maintainer notes
minorper the repo's 0.x convention. Please eyeballMAX_RECONNECT=5/ backoff adequacy for an 8h session crossing the 2h boundary (~4 forced reconnects) plus transient drops.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.