fix(api): buffer live events during SSE backlog replay for ordered delivery#361
fix(api): buffer live events during SSE backlog replay for ordered delivery#361duyet wants to merge 1 commit into
Conversation
…m-hub Inline note for reviewers ahead of the fix: `watch()` currently registers the writer before the backlog replay completes (the known ordering bug from plans/008), and separately, `writeBacklog`'s writes cannot resolve until a reader attaches to `stream.readable`, which never happens because the Response wrapping it is only returned after the backlog write loop finishes -- deadlocking any request with a non-empty backlog. Verified against the real Durable Object runtime. Functional fix follows in a subsequent commit on this branch. Co-Authored-By: Duyet Le <me@duyet.net> Co-Authored-By: duyetbot <bot@duyet.net>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Status: DRAFT — in progress, functional fix not yet committed
Opening now to hit a compressed deadline. Per instruction: not rushing the
correctness of this MED-risk Durable Object streaming change. This commit is
documentation-only (an inline
FIXMEnote); the actual fix and tests followin subsequent commits on this same branch.
Task
Executing plans/008-sse-backlog-ordering.md:
StateStreamHub.watch()registers the SSE writer in
this.writersbefore replaying theD1-backed backlog, so a concurrent
/notifybroadcast during replay caninterleave or duplicate events ahead of the ordered backlog stream. Planned
fix: buffer-then-drain — hold live broadcasts in a pending buffer while
backlog replays, flush pending events (deduped/sorted by
sequence), thenregister the writer atomically (no
awaitbetween flush and registration).Bigger finding from investigation (verified against the real DO runtime
via
@cloudflare/vitest-pool-workers, not a mock)The current
/watchDO path (the non-once=truepath) deadlocksindefinitely whenever the backlog has 1+ rows. Repro: seed one state
event, then
GET /watch?after=0through the DO — the request neverresolves (confirmed hung past 10s; an empty backlog resolves in ~295ms).
Root cause:
watch()doesawait this.writeBacklog(writer, ...)beforereturn new Response(stream.readable, ...). ATransformStream's writescannot resolve until something reads from
stream.readable(initialbackpressure per the WHATWG streams spec), but nothing can ever read it
because the
Responsewrapping that readable stream isn't returned untilwriteBacklogfinishes — a structural circular wait for any non-emptybacklog.
This means the plan's literal "Target design" (buffer → await writeBacklog
→ flush → register writer, Response still returned only at the end) would
still deadlock before a client observes anything, so it can't be
implemented or tested as written. The real fix needs to return
Response(stream.readable, ...)immediately, then do backlog-write +pending-flush + writer-registration as background work (e.g. via
this.ctx.waitUntil), keeping heartbeat/abort wiring intact for bothphases. Public SSE wire format and the
/notify+/watchroute contractstay unchanged — only the internal sequencing changes.
Verified so far
git diff --stat ce9a1fa..HEAD -- packages/api/src/state-stream-hub.ts— no drift.SELF.fetchin the real vitest-pool-workers/workerd environment. That scratch test was not committed.NOT done / NOT verified yet
Next steps (follow-up commits, same branch)
packages/api/src/state-stream-hub.ts.packages/api/test/state-stream-hub.test.ts: backlog-only ordering, live broadcast, concurrent-notify-during-replay regression, abort-during-replay cleanup.bunx biome check packages/api/src/,bunx tsc --noEmit -p packages/api/tsconfig.json,cd packages/api && bunx vitest runand report actual results.Co-Authored-By: Duyet Le me@duyet.net
Co-Authored-By: duyetbot bot@duyet.net