fix(gui): contain a failed lazy chunk instead of taking down the app - #224
Merged
Merged
Conversation
Suspense covers only the pending half of a dynamic import. When the chunk fails to load the promise rejects and the error propagates to the nearest boundary -- and neither lazy() site had one: the diff editor in PrPanel and a comment''s inline code context in CommentItem. DiffView carries an internal boundary around DiffPane, but that cannot see its own chunk failing to arrive. So a Monaco snippet that could not be fetched cost the user the entire application. Each lazy subtree now owns a LazyBoundary (Suspense plus a boundary) and the failure stays inside the pane that could not load. A stale chunk is singled out. The window holds a hashed module URL the app no longer has -- it was rebuilt or updated while the window stayed open -- so re-rendering re-requests the same dead URL and fails identically; only a reload recovers. Offering "retry" there is offering a button that cannot work, so isChunkLoadError detects the case and both the pane fallback and the root crash screen lead with reload and say why. Other errors keep retry. Observed via the root crash screen added earlier, which reported the failing module by name -- the diagnosis this containment work was meant to make possible. Co-Authored-By: Claude Opus 5 (1M context) <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
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.
Caught by the root crash screen added in #221, which reported the failing module by name:
That is the diagnosis the containment work was meant to make possible — previously this would have been an unrecoverable black window with nothing in the log.
The structural defect
Suspensecovers only the pending half of a dynamic import. When the chunk fails to load the promise rejects, and the error propagates to the nearest error boundary — and neitherlazy()site had one:PrPanel.tsx:214— the diff editor;CommentItem.tsx:149— a comment''s inline code context.DiffViewdoes carry an internal boundary aroundDiffPane, but that cannot see its own chunk failing to arrive. So a Monaco snippet that could not be fetched cost the user the entire application, comment thread and all.Each lazy subtree now owns a
LazyBoundary(Suspense plus a boundary) and the failure stays inside the pane that could not load.Stale chunks get their own treatment
For this class of failure the page is holding a hashed module URL the app no longer has — it was rebuilt or updated while the window stayed open. Re-rendering re-requests the same dead URL and fails identically, so "retry" is a button that cannot work.
isChunkLoadErrordetects the case, and both the pane fallback and the root crash screen lead with reload and explain why; other errors keep retry as before.On the origin of this particular crash
Worth stating plainly: this specific occurrence was almost certainly self-inflicted. The path is a local build output, and I ran
npm run buildrepeatedly while the app was open, replacing hashed chunks under a live window.That makes it unlikely to be the cause of the originally reported black screen — a packaged app reads from a read-only asar that does not change under a running window. That trigger is still unknown. The defect fixed here is real regardless of what surfaced it: one lazily-loaded pane failing should never cost the whole app, and in a packaged build the same failure is still reachable when the app updates while a window stays open.
Docs
03-gui/01-ui-interaction.md: the failure-containment section goes from three layers to four, with the lazy-subtree layer and the stale-chunk distinction written down. CHANGELOG in both languages.
lint/typecheck/test/buildpass. Not exercised visually in the running app — the fallback paths were verified by construction, not by forcing a chunk failure under the UI.🤖 Generated with Claude Code