Fix crash tokenizing a destroyed session via signature help - #4
Open
browniefed wants to merge 1 commit into
Open
Fix crash tokenizing a destroyed session via signature help#4browniefed wants to merge 1 commit into
browniefed wants to merge 1 commit into
Conversation
Production RUM shows repeated `TypeError: Cannot read properties of null (reading 'getLine')` from the `$tokenizeRow` override installed by `addSemanticTokenSupport`. When an editor is destroyed (e.g. a React wrapper unmounting), `session.destroy()` nulls `bgTokenizer.doc`, but the SignatureTooltip can still drive tokenization of that session: `update()` skips `$activateEditor` while the tooltip is open, so `$activeEditor` stays pointed at the destroyed editor and every subsequent selection change in a live editor requests signature help against the dead session; the response callback then calls `session.getTokenAt` -> `$tokenizeRow` -> `null.getLine` and throws, on every cursor move. - guard `$tokenizeRow` when `bgTokenizer.doc` is null - make `SignatureTooltip.update` re-activate the editor that produced the event even while the tooltip is open - bail out of `provideSignatureHelp` when the active editor's session is gone or destroyed Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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.
Summary
Fixes the top JS error in
/admin/workflows/?/edit/node/?):Root cause
addSemanticTokenSupport()replacesbgTokenizer.$tokenizeRowwith a version that dereferencesbgTokenizer.docunguarded. When an editor is destroyed (e.g. a React wrapper unmounting),session.destroy()callsbgTokenizer.setDocument(null), sodocis null.SignatureTooltip.update()skips$activateEditor(editor)while the tooltip is open, so$activeEditorcan stay pointed at a destroyed editor. Every subsequent selection change in a live editor then requests signature help against the dead session; the response callback callssession.getTokenAt→ patched$tokenizeRow→null.getLine. Because the throw also prevents the tooltip from ever resetting, this repeats on every cursor move — matching the bursty hundreds-of-errors-per-session pattern in RUM.Upstream
mkslanc/ace-lintershas since removed the$tokenizeRowmonkey-patch entirely (semantic tokens are applied as markers), so this crash is specific to the fork.Changes
$tokenizeRowwhenbgTokenizer.docis null (returns an empty token row).SignatureTooltip.update()re-activates the editor that produced the event even while the tooltip is open.provideSignatureHelp()bails (and hides the tooltip) when the active editor's session is gone or destroyed.tests/unit/destroyed-session.tests.tscovering all three (each watched failing first; the first test reproduces the production TypeError verbatim).Testing
npm run test:unit: 78 passing (75 baseline + 3 new).npm run buildcompiles clean.Publishing note
mainalready carries an unpublished1.8.3version bump; publishing after merge picks up this fix. brain-app will consume it via a version bump (apnpm patchwith the same guard is going up separately to stop the bleeding immediately).🤖 Generated with Claude Code