Prevent composer crash from Slate node-key errors (MAILSPRING-CLIENT-2X)

[bengotow]

What I observed

Sentry issue MAILSPRING-CLIENT-2X — Error: `Node.assertPath` could not find node with path or key: <n> — is the highest-impact unassigned issue in the last 14 days (50 users / 250 occurrences all-time), with culprit: ElementInterface.<computed> [as assertPath] (slate/lib/slate).

Every event's stack trace is 100% inside the vendored Slate library (in_app_frame_mix: "system-only" — zero Mailspring frames), which is why it's been hard to pin down. Two distinct call patterns show up across events, both bottoming out at slate.js:14824 (assertPath throwing because document.getPath(key)/getKeysToPathsTable() can't find the given key in the current document):

1. Commands$2.moveNodeByKey called from insertFragmentAtRange (slate.js:6809) — triggered by editor.insertFragment(...).
2. Commands$2.moveNodeByKey called from unwrapBlockAtRange (slate.js:7179, the "unwrap all children of a wrapping block" branch) — triggered by editor.unwrapBlock(...).

I grepped the Mailspring composer source for the only two call sites that invoke these Slate APIs directly, and the line numbers/behavior line up exactly:

• app/src/components/composer-editor/composer-editor.tsx — onPaste calls editor.insertFragment(value.document) when pasting HTML.
• app/src/components/composer-editor/base-block-plugins.tsx — toggleBlockTypeWithBreakout() calls editor.splitBlock() then loops calling editor.unwrapBlock({ type }) once per nesting level. This runs both when clicking the blockquote toolbar button and whenever the user presses Enter while their cursor is inside a (possibly multi-level nested) quoted reply — a very common interaction, which explains the issue's broad user impact.

Both of Slate's change functions (unwrapBlockAtRange, insertFragmentAtRange) are "compound" operations: they snapshot node keys up front, then perform several sequential moveNodeByKey/removeNodeByKey calls against the document. For certain nested-blockquote depths or multi-block paste shapes, an earlier step in the same compound operation invalidates a key a later step still relies on, and Slate's own assertPath throws instead of recovering. This is a bug inside the vendored Slate fork's core change functions, not something reachable/fixable from application/plugin code without patching Slate itself.

The fix

Since a mid-command exception in Slate is never committed (Slate only calls the parent onChange — and thus only persists a new value to the draft — once a top-level command fully completes), catching the exception at the two call sites and simply stopping is safe: the user's persisted draft content is never corrupted, and the app no longer crashes/reports to Sentry.

• Wrapped the splitBlock/unwrapBlock loop in toggleBlockTypeWithBreakout in a try/catch that logs a warning and aborts cleanly instead of throwing.
• Wrapped the editor.insertFragment(...) call in onPaste in a try/catch that logs a warning; the paste is prevented from partially applying and silently no-ops instead of crashing.

Comments at both sites reference this issue and explain why a try/catch (rather than a "real" fix) is the appropriate mitigation given the bug lives in the vendored Slate library.

Fixes MAILSPRING-CLIENT-2X

Test plan

• Manually reproduce: reply to a reply (nested blockquote, 2+ levels), place cursor inside the quoted text, press Enter to break out — should no longer be reproducible as a crash even if the underlying Slate edge case is hit (worst case: a no-op with a console warning instead of a thrown exception).
• Paste rich HTML content (e.g. copied from a webpage with nested block elements) into a reply with existing multi-block content — should not throw.
• Normal blockquote toggling/paste behavior remains unaffected in the non-buggy path.

🤖 Generated with Claude Code

https://claude.ai/code/session_01RLUjkxhpBL8yT4tTHrhyQK

---

Generated by Claude Code

[indent-staging]

Warning

Indent Zero is shutting down on August 7th. Please migrate over to Indent 2.0 to continue getting PR reviews.

PR Summary

Wraps two Slate compound-change call sites in the composer editor (splitBlock + unwrapBlock loop in toggleBlockTypeWithBreakout, and insertFragment in the paste handler) in try/catch so the "could not find node with path or key" exception from Slate's internal moveNodeByKey/assertPath no longer crashes the composer mid-mutation. Mitigates Sentry issue MAILSPRING-CLIENT-2X (~50 users) by logging via console.warn and no-op'ing the failed command instead of letting the exception escape.

• app/src/components/composer-editor/base-block-plugins.tsx: try/catch around editor.splitBlock + the editor.unwrapBlock loop in toggleBlockTypeWithBreakout; on catch, logs and returns before the trailing setBlocks(div). Outer Enter handler still calls event.preventDefault().
• app/src/components/composer-editor/composer-editor.tsx: try/catch around editor.insertFragment(value.document) in onPaste; on catch, logs and still calls event.preventDefault() + returns (paste is silently dropped).

Issues

2 potential issues found:

• Commit-message justification is inaccurate: Slate schedules flush()/onChange on a microtask from the first applyOperation (slate.js:11179), so a caught mid-command throw still hands the partially-applied value to ComposerEditor.onChange on the next tick — it does not "never reach the persisted draft." Fix is still valid in practice (the composer's getDocumentBrokenReason recovery softens most partial states), but the reasoning should be corrected so future maintainers don't rely on a false invariant when swallowing other Slate exceptions. → Autofix
• After a caught throw, Slate's withoutNormalizing leaves editor.tmp.normalize stuck at false (no try/finally in vendored slate.js:11515), so every later editor command in the same composer session skips normalization and can silently accumulate schema violations until the draft is closed. → Autofix

CI Checks

Waiting for CI checks...

Custom Rules 3 rules evaluated, 3 passed, 0 failed

This is a longer title to see what happens when they are too long to fit
B
Ben Rule

---

[indent]

Warning

This organization's free trial has ended, so Indent couldn't start this review. Manage billing to resume reviews on this repository.

[indent-staging]

Latent: tmp.normalize stuck at false after this catch fires.

Both unwrapBlockAtRange (slate.js:7161) and insertFragmentAtRange (slate.js:6721) do their moves inside editor.withoutNormalizing(fn), and that helper (slate.js:11515) is not exception-safe:

function withoutNormalizing(fn) {
  var value = this.tmp.normalize;
  this.tmp.normalize = false;
  fn(controller);              // ← throws here
  this.tmp.normalize = value;  // never runs
  normalizeDirtyPaths(this);   // never runs
}

Once fn throws, this.tmp.normalize stays false for the lifetime of the Editor. normalizeDirtyPaths (slate.js:11697) is a no-op when tmp.normalize === false, so every subsequent command in this composer session runs without schema normalization. Dirty paths accumulate and the document can drift into invalid shapes that ComposerEditor.onChange's getDocumentBrokenReason recovery won't catch (only the most severe shapes are repaired).

The same applies to the sibling catch in composer-editor.tsx around editor.insertFragment. A pragmatic mitigation in both catches: try { editor.tmp.normalize = true; } catch {} (or reset via the controller) before returning, so at least the next command re-enables normalization. Scope is bounded to the current composer session — closing/reopening the draft rebuilds the Editor and clears the flag — but a user who hits this once will keep editing in a non-normalizing editor until then.

[bengotow]

Good catch, thank you — confirmed against the actual vendored bundle and fixed in 6de5899.

One correction to the suggested mitigation: editor.tmp.normalize = true on its own wouldn't have worked here. The editor object these catch blocks receive is slate-react's <Editor> React component, not the core Slate editor — it has its own unrelated tmp (React-internal bookkeeping: resolves/mounted/updates/change/isUpdatingSelection, see slate-react.js ~5212), while the real tmp.normalize/tmp.dirty flags live on editor.controller (the core slate.Editor instance the component forwards commands to, per the "Mimic the API of the Editor controller" comment at slate-react.js:5238). So the fix reaches through editor.controller.tmp.normalize instead, and also calls .controller.normalize() to force an immediate re-normalization pass rather than just waiting for the next incidental command.

That second part turned out to matter for another reason you also flagged in the PR summary: Slate schedules its flush-to-onChange on a microtask queued by the first successful operation in a batch (slate.js:11179-11184), so a partial mutation from an interrupted command does reach ComposerEditor.onChange — my original commit message was wrong to claim otherwise. Calling .normalize() synchronously in the catch, before that pending microtask fires, cleans up the partial mutation so what actually reaches onChange is normalized rather than raw. Extracted both fixes into a shared recoverFromInterruptedSlateCommand helper used by both catch sites.

---

Generated by Claude Code

[bengotow]

Good catch, thank you — confirmed against the actual vendored bundle and fixed in 6de5899.

One correction to the suggested mitigation: editor.tmp.normalize = true on its own wouldn't have worked here. The editor object these catch blocks receive is slate-react's <Editor> React component, not the core Slate editor — it has its own unrelated tmp (React-internal bookkeeping: resolves/mounted/updates/change/isUpdatingSelection, see slate-react.js ~5212), while the real tmp.normalize/tmp.dirty flags live on editor.controller (the core slate.Editor instance the component forwards commands to, per the "Mimic the API of the Editor controller" comment at slate-react.js:5238). So the fix reaches through editor.controller.tmp.normalize instead, and also calls .controller.normalize() to force an immediate re-normalization pass rather than just waiting for the next incidental command.

That second part turned out to matter for another reason you also flagged in the PR summary: Slate schedules its flush-to-onChange on a microtask queued by the first successful operation in a batch (slate.js:11179-11184), so a partial mutation from an interrupted command does reach ComposerEditor.onChange — my original commit message was wrong to claim otherwise. Calling .normalize() synchronously in the catch, before that pending microtask fires, cleans up the partial mutation so what actually reaches onChange is normalized rather than raw. Extracted both fixes into a shared recoverFromInterruptedSlateCommand helper used by both catch sites.

---

Generated by Claude Code