From 346ca063ea1f7b24cee6d53e322c78c0097fd379 Mon Sep 17 00:00:00 2001 From: Tom McKenzie Date: Wed, 26 Aug 2026 15:36:34 +1000 Subject: [PATCH] =?UTF-8?q?fix(server):=20guard=20#send=20against=20closed?= =?UTF-8?q?=20sockets=20=E2=80=94=20no=20uncaught=20throw=20on=20abrupt=20?= =?UTF-8?q?sub?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A client that subscribes then closes before the snapshot finishes streaming (dispose, navigate-away, StrictMode teardown, forced reconnect) drove `#send` to call `ws.send()` on a CLOSING/CLOSED socket, surfacing `Can't call send() after close()` as an uncaught exception. The socket's `webSocketClose` already tears down its subs, so a frame it can no longer read is a benign no-op. Skip sends on a non-OPEN socket and wrap `ws.send` in try/catch for the OPEN→closed race the check can't cover. Outbound-only; no state impact, no behavior change for OPEN sockets. The `Broadcaster` egress path is covered too — it routes through the same `#send` closure. Fixes #40 Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 7 +++++++ src/server/mixin.ts | 14 +++++++++++++- tests/ws-lifecycle.test.ts | 31 +++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ea372b..b55d30d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,13 @@ While pre-1.0, the public API may change between 0.x releases. ### Fixed +- `#send` no longer throws an uncaught `Can't call send() after close()` when a + client subscribes then closes before the snapshot finishes streaming (normal + churn: dispose, navigate-away, StrictMode teardown, forced reconnect). The + server now skips sends on a non-`OPEN` socket and treats a post-close send as + a benign no-op — the socket's `webSocketClose` already tore down its subs. + Covers the `Broadcaster` egress path too (it routes through the same `#send`). + Outbound-only; no state impact, no behavior change for OPEN sockets (issue #40). - `unsubscribe` during an in-flight `subscribe`'s connect no longer sends the sub after the socket opens — previously the server persisted a ghost subscription (ADR-0019) with no local consumer until the socket dropped. diff --git a/src/server/mixin.ts b/src/server/mixin.ts index 3c9a463..8109dd7 100644 --- a/src/server/mixin.ts +++ b/src/server/mixin.ts @@ -1093,7 +1093,19 @@ export function Syncable() { `full-row rebroadcast; column projection (issue #28) is the real fix`, ) } - ws.send(encoded) + // A socket can transition to CLOSING/CLOSED between when we decided to + // send and now — a client disposing, navigating away, or a forced + // reconnect dropping mid-snapshot (issue #40). `webSocketClose` already + // tears down this socket's subs (#dropSocketSubs), so a frame it can no + // longer read is a benign no-op, NOT an error to surface as an uncaught + // throw. Pre-check readyState; the try/catch covers the OPEN→closed race + // the check can't (state can change under us between check and send). + if (ws.readyState !== WebSocket.OPEN) return + try { + ws.send(encoded) + } catch { + // "Can't call send() after close()" — the close beat us here. See above. + } } /** The attachment bound at upgrade, surviving hibernation. */ diff --git a/tests/ws-lifecycle.test.ts b/tests/ws-lifecycle.test.ts index 8c6f6c5..cbde438 100644 --- a/tests/ws-lifecycle.test.ts +++ b/tests/ws-lifecycle.test.ts @@ -1,5 +1,6 @@ import { env, runInDurableObject, SELF } from "cloudflare:test" import { describe, expect, it } from "vitest" +import { createFrameCodec } from "../src/wire/frame-codec.ts" // WHY: the lifecycle is the load-bearing transport. These drive a real // WebSocket through workerd end to end. They pin: the upgrade contract, the @@ -65,4 +66,34 @@ describe("SyncDurableObject WebSocket lifecycle (M2)", () => { expect(await nextMessage(ws)).toBe("pong") ws.close() }) + + // A client that subscribes then vanishes before the snapshot finishes + // streaming is normal churn (dispose, navigate-away, StrictMode teardown, + // forced reconnect). The server must not throw "Can't call send() after + // close()" into the runtime as an uncaught exception (issue #40). Drive the + // exact path deterministically: close the server-side socket, then hand it + // the `sub` frame — every #send target (here, the terminal `snap-end`) is now + // post-close. On unguarded `ws.send` this rejects the message promise; the + // guard makes it a benign no-op. + it("a sub arriving on a socket closed mid-snapshot is a no-op, not an uncaught throw", async () => { + const ws = await openWs("/sync/room-abrupt-close") + const stub = env.SYNC_DO.get(env.SYNC_DO.idFromName("room-abrupt-close")) + const frame = createFrameCodec().encode({ t: "sub", subId: "s1", collection: "messages" }) + const bytes = frame instanceof Uint8Array ? frame : new TextEncoder().encode(frame) + + await runInDurableObject(stub, async (instance, state) => { + const [serverWs] = state.getWebSockets() + expect(serverWs).toBeDefined() + serverWs!.close(1000, "gone") + // Must resolve — an unguarded #send throws here and rejects the promise. + await expect( + (instance as { webSocketMessage(ws: WebSocket, m: ArrayBuffer): Promise }).webSocketMessage( + serverWs!, + bytes.buffer.slice(bytes.byteOffset, bytes.byteOffset + bytes.byteLength) as ArrayBuffer, + ), + ).resolves.toBeUndefined() + }) + + ws.close() + }) })