Sync: a failed diff can no longer wedge the whole session - #50
Merged
Conversation
session.flush() only advances `shadow` AFTER a successful diff, so an uncaught throw from state.diff() poisons every later flush: the same delta is re-diffed, throws again, and nothing syncs for the rest of the session — on any slide, not just the element that broke. No error surfaces; peers just stop receiving. That amplifier is what turned the ~200KB-text stack overflow (fixed in 7e73247, splice-without-spread in applyTxtToState) into TOTAL, silent collab failure rather than one broken element. The trigger is gone; the amplifier stayed armed for the next differ bug. This disarms it. RECOVERY, best-effort: - Advance the shadow past the poison delta. Re-diffing it would just re-throw, and a wedged differ is strictly worse than one lost delta. - Ship a state snapshot. diff() mints ops and advances its registers as it goes, so a mid-diff throw leaves changes stamped but never broadcast — they now live only in our doc VALUES. That is exactly the offline-fork case, and a snapshot is how those travel (see hello()). forkPending re-sends it on the next reconnect too. Deliberately NOT a SyncNotice: those codes name conditions a user can act on (too large, room full). A differ crash is a bug — console is the right surface, and a user-facing string would mean new copy in all 7 catalogs for a should-never-happen path. VERIFIED in the browser against the real session (dev server, session temporarily exposed then reverted): - the reported 300KB repro through session.flush(): no throw, 6ms - forced differ failure: flush() does not propagate, `snap` broadcast, forkPending set - the repair snapshot CARRIES the edit the failed diff never emitted - after recovery: 0 ops from re-diffing the poison delta (shadow advanced), 1 op from a subsequent healthy edit — not wedged Convergence rig ALL PASS (45,362 checks; SEEDS=300 → 46,402). tsc clean.
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.
Context
A background report flagged
RangeError: Maximum call stack size exceededincrdt.tson a single large text edit (~300KB), with the note: "Consider also whethersession.flush()should be defensive so one bad diff can't wedge the differ."The crdt.ts half was already fixed by 7e73247 (splice-without-spread in
applyTxtToState) — the report was made against a pre-fix checkout. Verified empirically rather than taken on trust, same repro against both versions:7e73247^mainThe
flush()half was not addressed, and is the more interesting defect. That's what this PR fixes.The bug
flush()advancesshadowonly after a successful diff, so an uncaught throw fromstate.diff()poisons every later flush: the same delta is re-diffed, throws again, and nothing syncs for the rest of the session — on any slide, not just the element that broke. Nothing surfaces; peers just stop receiving.That amplifier is what turned the 200KB stack overflow into total silent collab failure rather than one broken element. The trigger is gone; the amplifier stayed armed for the next differ bug.
Recovery
diff()mints ops and advances registers as it goes, so a mid-diff throw leaves changes stamped but never broadcast — they live only in our doc values. That is exactly the offline-fork case, and a snapshot is how those travel (hello()does the same).forkPendingre-sends on reconnect.Deliberately not a
SyncNotice: those codes name conditions a user can act on (too large, room full). A differ crash is a bug — console is the right surface, and a user-facing string would mean new copy in all 7 catalogs for a should-never-happen path.Verification
In the browser against the real session (dev server; session temporarily exposed, then reverted):
session.flush()— no throw, 6msflush()does not propagate,snapbroadcast,forkPendingsetConvergence rig
ALL PASS(45,362 checks;SEEDS=300→ 46,402).tsc -bclean.One file, 48 insertions.
crdt.tsuntouched.