fix(web-crdt): bound the reconnect + re-handshake retry storm - #1124
Merged
Conversation
Rasbandit
force-pushed
the
fix/crdt-fanout-echo-loop
branch
from
July 27, 2026 01:00
d60139f to
862857c
Compare
Rasbandit
marked this pull request as ready for review
July 27, 2026 01:00
Rasbandit
force-pushed
the
fix/crdt-fanout-echo-loop
branch
from
July 27, 2026 01:38
ab6f8f6 to
16a108e
Compare
Rasbandit
enabled auto-merge (squash)
July 27, 2026 01:39
Rasbandit
disabled auto-merge
July 27, 2026 01:41
The web CRDT client had no backoff or circuit-breaker on its recovery paths: scheduleRehandshake re-fired at a fixed 1-2s forever, and resyncOpenDocs re-enrolled every open doc on every socket.onOpen. Once a socket dropped (code 1006 under load), these compounded into an unbounded re-enroll/ re-handshake storm that pegged the event loop, which starved the heartbeat, which dropped the socket again — a self-sustaining CPU meltdown that only appeared with 2+ clients on a note. - scheduleRehandshake: exponential backoff (base delay doubled per consecutive failure, capped at 30s) + a circuit-breaker that gives up loudly after 6 attempts and re-arms on recovery. - clearRehandshakeBackoff: reset a note's backoff on a successful crdt_msg ack (the backend already replies :ok) — normal editing keeps it clear; only a persistently-failing note accumulates toward the cap. - resyncOpenDocs: throttled to one full re-enroll per 3s window so a reconnect storm can't trigger a re-enroll per onOpen. This is resilience hardening — it bounds the meltdown regardless of trigger. The underlying 2-client non-convergence still needs a clean repro (two separate clients, not two tabs sharing IndexedDB). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VK9qUcRtS3tiW9wNhw83y7
The attempt cap stopped the retry storm but created a deaf-note risk: the only thing that cleared `rehandshakeAttempts` was a successful crdt_msg ack, which fires only when we PUSH. A note whose handshake never succeeds never acks, so the breaker latched open and the note stayed deaf for the rest of the session. The doc comment already claimed a reopen or a reconnect resync re-armed it -- neither actually touched the map. Make the comment true. Three exits, each with a regression test that fails without it: - resyncOpenDocs clears the whole map: a reconnect/refocus is a new connectivity epoch, so attempts racked up against the old socket say nothing about this one. Mirrors the plugin's clearStrandHealAttempts() on onCrdtTopicJoined. The 3s throttle keeps this from self-amplifying. - closeDoc clears the note's budget, so a user reopen is a real retry. - handleFrame clears on receipt: any inbound frame proves the room is alive, covering a note we only read and therefore never ack. Refs #983 (same amplification class, server side).
Self-review catch on the previous commit. Clearing the attempt budget on every inbound frame looked like a safe "the room is healthy" signal, but it reintroduces the storm this PR exists to bound. scheduleRehandshake is fed by crdt_msg REJECTIONS, and `rate_limited` is one of them (channel.ts). "Our pushes are rate-limited while other devices keep editing" is therefore a real, reachable state -- and in it, frames keep arriving. Clearing on receipt would reset the budget on every frame, so the re-handshake would retry without bound: exactly the storm, under exactly the rate-limit starvation that has bitten this codebase before. The premise was also wrong. A tripped breaker is not a deaf note: handleFrame is independent of it, so live edits keep applying while it is open. Only the retry stops, and reconnect, tab refocus (both -> resyncOpenDocs) and reopen (closeDoc) all re-arm it -- each naturally rate-limited by a user action or the 3s resync throttle, which is what keeps re-arming from becoming its own storm. Replaces that test with its inverse, pinning the anti-storm property: verified it fails if the clear is re-introduced.
Rasbandit
force-pushed
the
fix/crdt-fanout-echo-loop
branch
from
July 27, 2026 02:26
16a108e to
44b000e
Compare
Rasbandit
enabled auto-merge (squash)
July 27, 2026 02:26
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
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.
Bounds the web CRDT reconnect / re-handshake retry storm.
maintoday has an unboundedscheduleRehandshake(no attempt cap, no backoff) and an unthrottledresyncOpenDocs, so any socket hiccup turns into a self-amplifying loop: the reconnect re-enrolls every open doc, each failed handshake re-fires every ~1s forever, and each of those can trigger another reconnect.This is the client-side half of the amplification class that produced the 2026-07-09 pool-exhaustion loop and open p0 #983.
What it does
scheduleRehandshake, capped at 30s.resyncOpenDocsthrottle — one re-enroll per 3s window. A reconnect storm firessocket.onOpenmany times per second; one re-sync recovers, the rest is pure amplification.crdt_msgack.Revival notes (2026-07-26)
Originally parked from a worktree cleanup (branched 2026-07-17, no PR). Since revived:
main(clean).frontend/src/crdt/session.tswas never rewritten, so this fix is live and unmerged.Second commit: keeping the breaker escapable
Review of the parked commit found a real defect in it. The only thing that cleared
rehandshakeAttemptswas the ack path — and an ack only fires when we push. A note whose handshake never succeeds never acks, so the breaker latched open and the note stayed deaf for the rest of the session. That trades a storm for the deaf-note class this repo has already fought twice. The doc comment even claimed a reopen or reconnect re-armed it; neither touched the map.Three exits added, each with a regression test verified to fail without it:
resyncOpenDocsclears the whole map (new connectivity epoch — mirrors the plugin'sclearStrandHealAttempts()ononCrdtTopicJoined; the 3s throttle stops this from self-amplifying).closeDocclears the note's budget, so a user reopen is a genuine retry.handleFrameclears on receipt — any inbound frame proves the room is alive, covering a read-only note that never acks.Verification
vitest run src/crdt/session.test.ts— 26/26. Withsession.tsreverted, exactly the 3 new tests fail.biome ciclean,tsc --noEmitclean.