Repository navigation
[lexical][lexical-y] Bug Fix: Discard an update that throws when onError rethrows its error - #9509
Merged
Merged
Conversation
zurfyx
requested review from
acywatson,
etrepum,
ivailop7 and
potatowagon
as code owners
October 29, 2026 01:11
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
lexical
@lexical/a11y
@lexical/clipboard
@lexical/code
@lexical/code-core
@lexical/code-prism
@lexical/code-shiki
@lexical/compiler
@lexical/devtools-core
@lexical/dragon
@lexical/eslint-plugin
@lexical/extension
@lexical/fast-check
@lexical/file
@lexical/hashtag
@lexical/headless
@lexical/history
@lexical/html
@lexical/internal
@lexical/link
@lexical/list
@lexical/mark
@lexical/markdown
@lexical/mdast
@lexical/offset
@lexical/overflow
@lexical/plain-text
@lexical/react
@lexical/rich-text
@lexical/selection
@lexical/table
@lexical/tailwind
@lexical/text
@lexical/utils
@lexical/y
@lexical/yjs
commit: |
This was referenced Oct 29, 2026
vjeux
approved these changes
Oct 29, 2026
…s its error ## Description When an update's callback, a nested update, a node transform or a selection change listener throws, `$beginUpdate` reports the error to `onError` and then rolls the update back: it replaces the pending state with the committed one and commits it, so the next update starts from the committed state. The rollback ran after `onError`, so when `onError` rethrows, which `buildEditorFromExtensions` does by default and many apps do, it never ran. The failed update's pending state stayed in place with its changes, dirty nodes and update tags, and with no commit scheduled: - The next update reused that state and committed the failed update's changes and tags. In a collaborative editor, the change of an update that threw (and whose error the app caught) reached every client. - An update that changes nothing reused it too, did not commit, and left its tags on it. `@lexical/y` reading a remote change that shows nothing new on that client is such an update: the client's next local edit committed with the read's `collaboration` tag, the binding took it for a remote change and never wrote it, and the client kept showing an edit no one else had. - With batched (non-discrete) updates, nothing was scheduled to commit the leftover state, so a later local edit stayed pending until the binding committed it inside a remote update's observer, at offsets that no longer matched the document: text nobody edited was lost on every client. With this change, a failed update is discarded the same way whether `onError` rethrows or only reports the error, and nothing of it stays: the editor goes back to its last committed state before `onError` runs, then that state is committed to restore the DOM, and the error `onError` rethrows reaches the caller after that. In detail: - `onError` runs after the discard, so a read in `onError` sees the committed state. Before, a reporting `onError` that read the editor committed the failed update (its listeners saw it, `@lexical/y` wrote it), and the rollback then reverted the editor without telling them. - The updates the failed update queued (an `editor.update()` in its callback that had not run yet) are dropped with it. Those queued before it, by a listener of an earlier commit, still run. - An update nested in the running update whose pending state it shares (a selection change notified by a commit forced inside an update, for one) leaves its error to that update: it is reported once, and the update is discarded as a whole. Before, `onError` was called twice with a rethrowing `onError`, and with a reporting one the outer update went on writing to a state that had been discarded. - The editor goes back to the last committed state (`editor._editorState`), not to the state captured when the failed update began. The two differ only when a commit happened during the failed update (`setEditorState` inside it, or a read that commits): that commit's listeners have run, and it now stays instead of being reverted behind them. - When restoring the editor throws too (an update listener), the caller still gets the error `onError` rethrew, and the second error is reported to `onError`. Updates queued meanwhile run in a microtask instead of waiting for an unrelated update. Errors thrown by update listeners of a successful commit are unchanged: the commit has happened, and the error propagates. Not changed either: `onUpdate` callbacks of a failed update still run with the next commit, as they did with a reporting `onError`. ## Changeset `.changeset/lexical-discard-failed-update.md`: `[fix]` for `lexical`, with a `Verify:` paragraph for the behavior change. ## Test plan New: `packages/lexical/src/__tests__/unit/LexicalUpdateErrors.test.ts` (29 tests). With an `onError` that rethrows (the app catches the error) and with one that only reports it: a callback that changes text and throws (discrete and batched), a callback that throws before changing anything followed by an update that changes nothing (the binding's read), a node transform, a nested update, an update batched after another, a selection change listener, and a commit made during the failed update; each checks that the next update commits only its own change, with only its own tag. Then the error handling: a read in `onError`, updates the failed update queued and updates queued before it, an error thrown while the editor is restored, an update `onError` queues then, and a nested update sharing the failed update's pending state. A last test checks that an update listener's error does not undo its commit. `./node_modules/.bin/vitest --run --project unit packages/lexical/src/__tests__/unit/LexicalUpdateErrors.test.ts` ### Before On `main` (92d6af7), 19 of 29 fail: every case with a rethrowing `onError`, and with a reporting one the reverted commit, the read in `onError`, the queued update, the update queued while restoring, and the nested update: ``` × a callback that changes text, then throws (discrete: true) 41ms × a callback that changes text, then throws (discrete: false) 6ms × a callback that throws before any change, then an update that changes nothing 5ms × a transform that throws 5ms × a nested update that throws discards the update it is part of 5ms × an update batched after another one discards both 5ms × a selection change listener that throws 6ms × what the failed update committed before it threw stays 7ms ✓ a callback that changes text, then throws (discrete: true) 3ms ✓ a callback that changes text, then throws (discrete: false) 2ms ✓ a callback that throws before any change, then an update that changes nothing 1ms ✓ a transform that throws 1ms ✓ a nested update that throws discards the update it is part of 1ms ✓ an update batched after another one discards both 2ms ✓ a selection change listener that throws 1ms × what the failed update committed before it threw stays 3ms × a read in onError sees the committed state and commits nothing of the failed update 3ms × the updates it queued are dropped with it 4ms × the updates queued before it still run 2ms × an error thrown while the editor is restored is reported too 2ms × an update onError queues runs, also when restoring the editor throws 3ms × a nested update sharing its pending state: reported once, discarded as a whole 4ms × a read in onError sees the committed state and commits nothing of the failed update 2ms × the updates it queued are dropped with it 2ms ✓ the updates queued before it still run 1ms ✓ an error thrown while the editor is restored is reported too 1ms × an update onError queues runs, also when restoring the editor throws 2ms × a nested update sharing its pending state: reported once, discarded as a whole 4ms ✓ an update listener that throws (1) ✓ does not undo the commit, whatever onError does 4ms FAIL |unit| packages/lexical/src/__tests__/unit/LexicalUpdateErrors.test.ts > an update that throws, onError rethrows > a callback that changes text, then throws (discrete: false) AssertionError: expected [ { …(4) } ] to deeply equal [ { dirtyText: [ 'two!' ], …(3) } ] - Expected + Received [ { "dirtyText": [ + "CHANGED", "two!", ], - "next": "one + "next": "CHANGED two!", "prev": "one two", "tags": [ + "failed", "next", ], }, ] FAIL |unit| packages/lexical/src/__tests__/unit/LexicalUpdateErrors.test.ts > error handling of an update that throws, onError reports > a read in onError sees the committed state and commits nothing of the failed update AssertionError: expected [ 'FAILED\n\ntwo' ] to deeply equal [ 'one\n\ntwo' ] FAIL |unit| packages/lexical/src/__tests__/unit/LexicalUpdateErrors.test.ts > error handling of an update that throws, onError reports > the updates it queued are dropped with it AssertionError: expected 'one\n\nTWO' to be 'one\n\ntwo' // Object.is equality FAIL |unit| packages/lexical/src/__tests__/unit/LexicalUpdateErrors.test.ts > error handling of an update that throws, onError rethrows > a nested update sharing its pending state: reported once, discarded as a whole AssertionError: expected [ 'app bug', 'app bug' ] to deeply equal [ 'app bug' ] Test Files 1 failed (1) Tests 19 failed | 10 passed (29) ``` ### After ``` ✓ |unit| packages/lexical/src/__tests__/unit/LexicalUpdateErrors.test.ts (29 tests) 110ms Test Files 1 passed (1) Tests 29 passed (29) ``` The `lexical` unit suite (`./node_modules/.bin/vitest --run --project unit packages/lexical/`): 102 files, 2382 tests passed. `pnpm run ci-check` passes. Not run: browser-mode and e2e tests.
…mit before reading a remote change
## Description
Before it reads a remote change, the binding commits the local updates still pending (`commitPending`), so that the read commits alone: a read committed together with a local update gives it the read's update tags (`collaboration`, ...), and the binding never writes a commit with those tags. It committed them with `editor.update(() => {}, {discrete: true})`, which commits a pending state only when the state holds a change. A pending state that holds none stayed in place. Lexical left one behind after an update that threw when `onError` rethrew the error (fixed in the previous commit). The read then reused that state, and when the read changed nothing on that client (a collaborator's undo of a format on text another collaborator had deleted), it did not commit either and its tags stayed on the state. The client's next local edit committed with them and was never written: the client kept showing an edit no one else had, with no warning.
`commitPending` now commits such a state as it is (`editor.read()` commits whatever is pending), so the read always starts from a committed state and a read that changes nothing leaves no tags behind. With the previous commit Lexical no longer leaves such a state; this keeps the binding's invariant whatever the editor has pending.
The binding still decides from update tags which commits it writes: an update tagged `COLLABORATION_TAG` is not written, which is documented (`editYState` refuses the tag for that reason), so an app's update tagged that way must stay unwritten. The fix keeps the binding's own tags from reaching another update instead.
`UpdateErrors.test.ts` also covers what the previous commit fixed in collaboration: the client whose update threw keeps writing its edits, its failed change reaches no one, and batched updates and a reader's throwing transform converge, each with an `onError` that rethrows and one that reports.
One known limit is left as an expected failure, unrelated to updates that throw: Lexical stops notifying a commit's update listeners when one of them throws, so a binding registered after the listener that threw never writes that commit.
## Changeset
`.changeset/lexical-y-commit-pending-state.md`: `[fix]` for `@lexical/y`.
## Test plan
New: `packages/lexical-y/src/__tests__/unit/UpdateErrors.test.ts` (11 tests). With an `onError` that rethrows (the app catches the error) and one that only reports it: a collaborator's undo of a format on text another deleted, then the next edit; an update that changed text before throwing; batched updates with concurrent deletes; a reader's transform that throws on text a collaborator typed. Then a seeded invariant over random edits, undos, offline peers and updates that throw (either `onError`, discrete or batched, before or after a change): every editor shows what a fresh reader of the document shows, and each peer's next edit reaches every peer. Then the binding on its own: an editor that has a pending state with nothing to commit when a remote change arrives, as a core that kept a failed update's state left it. Last, the known limit as `test.fails`.
`./node_modules/.bin/vitest --run --project unit packages/lexical-y/src/__tests__/unit/UpdateErrors.test.ts`
### Before
On `main` (92d6af7):
```
× then a collaborator undoes a format on text another deleted, then the next edit (S7-30) 178ms
× that changed text: nobody gets the change, and the next edit is written 43ms
× in updates batched as browsers commit them, then concurrent deletes (S7-34) 38ms
× in a transform of a reader, on text a collaborator typed 35ms
✓ an update that throws, onError reports (4)
✓ then a collaborator undoes a format on text another deleted, then the next edit (S7-30) 54ms
✓ that changed text: nobody gets the change, and the next edit is written 24ms
✓ in updates batched as browsers commit them, then concurrent deletes (S7-34) 25ms
✓ in a transform of a reader, on text a collaborator typed 25ms
× editors show the document and every later edit is written 256ms
× a pending state with nothing to commit is committed before a remote read, which then does not tag the next edit 28ms
✓ a commit whose update listeners stopped before the binding's is not written (known limit) 21ms
FAIL |unit| packages/lexical-y/src/__tests__/unit/UpdateErrors.test.ts > an update that throws, onError rethrows > then a collaborator undoes a format on text another deleted, then the next edit (S7-30)
AssertionError: expected 'paragraph("t1z t2z t6z t7z t8z t9z")…' to be 'paragraph("t1z t2z t6z t9z") / para…' // Object.is equality
Expected: "paragraph("t1z t2z t6z t9z") / paragraph("tail line")"
Received: "paragraph("t1z t2z t6z t7z t8z t9z") / paragraph("tail line")"
FAIL |unit| packages/lexical-y/src/__tests__/unit/UpdateErrors.test.ts > an update that throws, onError rethrows > that changed text: nobody gets the change, and the next edit is written
Expected: "paragraph("t1z t2z t3z t4z t5z t6z t7z t8z t9z") / paragraph(">tail line!")"
Received: "paragraph("CHANGED") / paragraph(">tail line!")"
FAIL |unit| packages/lexical-y/src/__tests__/unit/UpdateErrors.test.ts > an update that throws, onError rethrows > in updates batched as browsers commit them, then concurrent deletes (S7-34)
Expected: "paragraph(" t2z t3z t4z t5z t9z") / paragraph("tail line")"
Received: "paragraph(" " "t2z"{2} " t3z t4z t5z t8z t9z") / paragraph("tail line")"
Test Files 1 failed (1)
Tests 6 failed | 4 passed | 1 expected fail (11)
```
With the previous commit only, the binding's own test fails:
```
× a pending state with nothing to commit is committed before a remote read, which then does not tag the next edit 32ms
FAIL |unit| packages/lexical-y/src/__tests__/unit/UpdateErrors.test.ts > the binding writes every committed change > a pending state with nothing to commit is committed before a remote read, which then does not tag the next edit
AssertionError: expected 'paragraph("t1z t2z t6z t7z t8z t9z")…' to be 'paragraph("t1z t2z t6z t9z") / para…' // Object.is equality
Expected: "paragraph("t1z t2z t6z t9z") / paragraph("tail line")"
Received: "paragraph("t1z t2z t6z t7z t8z t9z") / paragraph("tail line")"
Tests 1 failed | 9 passed | 1 expected fail (11)
```
### After
```
✓ ... an update that throws, onError rethrows > then a collaborator undoes a format on text another deleted, then the next edit (S7-30) 141ms
✓ ... an update that throws, onError rethrows > that changed text: nobody gets the change, and the next edit is written 25ms
✓ ... an update that throws, onError rethrows > in updates batched as browsers commit them, then concurrent deletes (S7-34) 31ms
✓ ... an update that throws, onError rethrows > in a transform of a reader, on text a collaborator typed 31ms
✓ ... an update that throws, onError reports > then a collaborator undoes a format on text another deleted, then the next edit (S7-30) 44ms
✓ ... an update that throws, onError reports > that changed text: nobody gets the change, and the next edit is written 28ms
✓ ... an update that throws, onError reports > in updates batched as browsers commit them, then concurrent deletes (S7-34) 37ms
✓ ... an update that throws, onError reports > in a transform of a reader, on text a collaborator typed 35ms
✓ ... the invariant, over random edits with updates that throw > editors show the document and every later edit is written 1880ms
✓ ... the binding writes every committed change > a pending state with nothing to commit is committed before a remote read, which then does not tag the next edit 27ms
✓ ... the binding writes every committed change > a commit whose update listeners stopped before the binding's is not written (known limit) 25ms
Test Files 1 passed (1)
Tests 10 passed | 1 expected fail (11)
```
The invariant also passes with 400 seeds (`LEXICAL_Y_SEEDS=400`). The `@lexical/y` unit suite (`./node_modules/.bin/vitest --run --project unit packages/lexical-y/`): 137 files passed, 4684 tests passed, 13 expected failures. `pnpm run ci-check` passes. Not run: browser-mode and e2e tests.
zurfyx
force-pushed
the
zurfyx/update-error-rollback
branch
from
October 29, 2026 17:18
53ab2b2 to
e86a226
Compare
This branch was successfully deployed
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.
Note
Stack, part 1 of 10. Based on main. Review only this PR's own commits.
The
@lexical/yfixes touch the same writer, cursor, reader and binding internals, so they are one linear stack: each PR is based on the one below it, each conflict is resolved once, in the PR whose change causes it, and every CI run tests the real combination.Merge order: bottom first, #9509 → #9523 → #9522 → #9505 → #9508 → #9502 → #9504 → #9527 → #9507 → #9510. With squash merges, rebase the next PR onto
mainbefore merging it (git rebase --onto main <old base branch>).The stack
zurfyx/update-error-rollbackmainzurfyx/write-before-unread-changezurfyx/y-keep-chars-at-format-boundaryzurfyx/y-undo-member-closerzurfyx/y-initial-state-bindingzurfyx/y-reorder-intentzurfyx/y-preserve-unknown-attrszurfyx/y-move-keeps-unknown-fieldszurfyx/y-composition-holdbackzurfyx/y-incremental-read-marksFollow-ups for after the stack (not in it):
Unread(changes Yjs applied that the binding has not read) and [lexical-playground][lexical-website][lexical-y] Bug Fix: Hold back remote changes to the line being composed until the composition ends #9507'sHidden(changes held back while a line is composed) answer one question, and call sites combine them by hand. One owner module (isGone,isShown,attrOf/attrsOfand the write primitives), an item-level held overlay instead of thewas/nowmaps, and a lint or grep test that allows raw.deleted/getAttr(s)only in the Reader and in named provenance scans. [lexical-website][lexical-y] Bug Fix: Stop a format, comment or link from spreading to the end of the document after an undo or delete races an edit inside it #9505's run-opener scans (liveItemBefore,runOpener,restatedEnd) still read raw deletedness and move there too.$groupReference,$priorGroupItem, the nearest neighbour's marker before the write,priorGroupMarker), with separate predecessor stores for markers and nodes. One stored-predecessor lookup would replace it.Description
An update that throws was left pending when
onErrorrethrew the error, and the next update committed it. This PR discards the failed update whetheronErrorrethrows or only reports the error (lexical, first commit), and makes@lexical/ycommit any pending state before it reads a remote change, so its read can never leave update tags on another update (second commit).lexical: discard an update that throws.$beginUpdatereported the error toonErrorand only then rolled the update back, so anonErrorthat rethrows (the default ofbuildEditorFromExtensions, and what many apps do) skipped the rollback. The failed update's pending state stayed in place with its changes, dirty nodes and tags, and with no commit scheduled:@lexical/yreading a remote change that shows nothing new is such an update, so the client's next local edit committed with the read'scollaborationtag, was taken for a remote change and never written, and the client kept showing an edit no one else had (no warning, statusactive);A failed update is now discarded the same way whether
onErrorrethrows or only reports the error: the editor goes back to its last committed state beforeonErrorruns, that state is committed to restore the DOM, and the erroronErrorrethrows reaches the caller after that. So a read inonErrorsees the committed state (a reportingonErrorthat read the editor used to commit the failed update, and its listeners and@lexical/ysaw it), the updates the failed update queued are dropped with it (those queued before it still run), and an update nested in the failed one that shares its pending state reports its error once. The editor goes back to the last committed state rather than the state captured when the failed update began, so a commit made during the failed update (setEditorStateinside it, or a read that commits) stays instead of being reverted after its listeners ran. When restoring the editor throws too, the caller still gets the erroronErrorrethrew, and the second one is reported toonError. Errors thrown by update listeners of a successful commit are unchanged, and so are theonUpdatecallbacks of a failed update, which still run with the next commit (a separate policy question, left for a follow-up).@lexical/y: commit a pending state that holds nothing to commit before a remote read.commitPendingcommitted pending local updates witheditor.update(() => {}, {discrete: true}), which leaves a pending state that holds no change in place; the read then reused it and, when it changed nothing, left its tags on it for the next local edit. It now commits such a state as it is (editor.read()), so the binding's invariant (a read commits alone) holds whatever the editor has pending. The binding still decides from update tags what it writes: an update taggedCOLLABORATION_TAGis not written, which is documented (editYStaterefuses the tag for that reason).Known limit, left as
test.fails: Lexical stops notifying a commit's update listeners when one of them throws, so a binding registered after the listener that threw never writes that commit. It is unrelated to updates that throw; a full rewrite of the document from the editor is not safe there (text the editor read since would be attributed to the local edit), so it needs its own design.Restack (October 29)
Unchanged: still based on
main(92d6af70), head53ab2b28. It is the bottom of the stack, so nothing here was rebased.Changeset
.changeset/lexical-discard-failed-update.md:[fix]forlexical, with aVerify:paragraph for the behavior change..changeset/lexical-y-commit-pending-state.md:[fix]for@lexical/y.Test plan
packages/lexical/src/__tests__/unit/LexicalUpdateErrors.test.ts(29 tests): with anonErrorthat rethrows and one that reports, a callback that throws (discrete and batched, before or after a change, followed by an update that changes nothing), a transform, a nested update, an update batched after another, a selection change listener, a commit made during the failed update; each checks that the next update commits only its own change and tag. Then the error handling: a read inonError, queued updates, an error thrown while the editor is restored, and a nested update sharing the failed update's pending state. An update listener's error does not undo its commit.packages/lexical-y/src/__tests__/unit/UpdateErrors.test.ts(11 tests): the collaboration shapes (a collaborator's undo of a format on deleted text, then the next edit; a failed update that changed text; batched updates with concurrent deletes; a reader's throwing transform) in bothonErrormodes, a seeded invariant over random edits, undos, offline peers and updates that throw (every editor shows what a fresh reader shows, and each peer's next edit reaches everyone), the binding on its own with a pending state left behind, and the known limit../node_modules/.bin/vitest --run --project unit packages/lexical/src/__tests__/unit/LexicalUpdateErrors.test.ts packages/lexical-y/src/__tests__/unit/UpdateErrors.test.tsBefore
On
main(92d6af7):LexicalUpdateErrors19 failed of 29 (every case with a rethrowingonError; with a reporting one, the reverted commit, the read inonError, the queued updates and the nested update), for example:UpdateErrors6 failed of 11, for example:With the first commit only, the binding's own test fails the same way (the next edit after a read that changed nothing is not written).
After
lexicalunit suite: 102 files, 2382 tests passed.@lexical/yunit suite: 137 files passed, 4684 tests passed, 13 expected failures.pnpm run ci-checkpasses.mainfor the same seeds (24, 17 and 35 per 1,500); with discrete updates, agent edits and undo, 0 of 1,500 (1 onmain).