fix(app): show a recovery screen instead of a blank window when a render throws - #3402
Open
pranshu26 wants to merge 1 commit into
Open
fix(app): show a recovery screen instead of a blank window when a render throws#3402pranshu26 wants to merge 1 commit into
pranshu26 wants to merge 1 commit into
Conversation
…der throws
React unmounts the whole tree when a render throws, and the renderer had no
boundary above the routes — only LexicalErrorBoundary in the composer and the
tool-part boundary in components/chat/message-list.tsx. That second one already
documents this failure mode in its own comment ("without this boundary a single
bad part unmounts the entire app (white screen). Seen in production on v0.15.3").
The same shape reaches the whole app from route-level render work. different-ai#3372 is a
current example: a local MCP server configured with a string `command` throws
`config.command?.some is not a function` inside a `useMemo` in settings-route,
and the user gets a blank window with nothing to report.
This adds a last-resort boundary around the tree that shows the error, its
stack, a Reload button, and Copy details. It renders plain elements and does not
call `t()` on purpose: locale init runs before the tree mounts and has itself
been a startup failure (different-ai#2767), so the screen that reports a crash must not
depend on it.
This does not fix any individual crash — it stops them presenting as a blank
window.
Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
Contributor
|
Someone is attempting to deploy a commit to the Different AI Team on Vercel. A member of the Team first needs to authorize it. |
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.
Summary
Why
React unmounts the entire tree when a render throws. The renderer has only two boundaries today —
LexicalErrorBoundary(composer/editor.tsx:1249) and the tool-part boundary incomponents/chat/message-list.tsx:148— and nothing above the routes. The comment on that second one already describes this failure mode:The same shape reaches the whole app from route-level render work. #3372 is a live example: a local MCP server configured with a string
commandthrowsconfig.command?.some is not a functioninside auseMemoinsettings-route.tsx:1797, and the user gets a blank window with nothing to report.composer.tsx:1605throws the same way and takes out the main chat screen.This does not fix any individual crash — #3373 is the right fix for #3372 specifically. It changes what a crash looks like: an error card with a copyable stack instead of a white screen, which also makes the next one of these reportable.
Issue
command(config.command?.some is not a function) #3372, [Bug]: The app is usuable #3137 — blank/unusable screens where the underlying throw is invisible to the user. Not claiming to close either; the boundary changes presentation, not the root cause.Scope
apps/app/src/react-app/shell/app-error-boundary.tsx(new) — the boundary and its fallback UI.apps/app/src/index.react.tsx— wrap the tree, insideStrictModeso provider failures are caught too.apps/app/tests/app-error-boundary.test.tsx(new).Out of scope
command(config.command?.some is not a function) #3372 has fix: normalize local MCP command string+args (fixes #3372) #3373 open).window.addEventListener("error"/"unhandledrejection")capture in production.debug-logger.ts:243,255registers these butisEnabled()returnsfalseunderimport.meta.env.PRODunless a localStorage flag is set, so production has no global capture. Separate concern, happy to take it in a follow-up.Testing
Ran
bun test --isolate tests/app-error-boundary.test.tsxbun test --isolate tests/(full app suite)pnpm --filter @openwork/app typecheckResult
One note on how the tests are written:
react-dom/serverrethrows instead of running error boundaries, sorenderToStaticMarkup(<Boundary><Throws/></Boundary>)cannot exercise the catch path — I tried that first and it fails. The tests instead drive the state transition directly (getDerivedStateFromError→render()), which covers the same code the boundary runs. The actual catching is verified in the running app below.Manual verification
pnpm --filter @openwork/app devconfig.command?.some is not a functioninside the router, simulating Settings page renders blank/white when a local MCP server uses stringcommand(config.command?.some is not a function) #3372.Before: blank window, nothing rendered, no indication of what happened.
After: the recovery screen renders — heading "OpenWork hit an unexpected error", the full stack in a scrollable block (
at Boom (http://localhost:5199/src/index.react.tsx:28:9)etc.), and working Reload / Copy details buttons. Correct in dark theme.The throwing component was removed before committing; the diff contains no test scaffolding.
CI status
OpenWork Testsandi18n Auditsit inaction_requiredpending maintainer approval, and the Vercel checks fail with "Authorization required to deploy". Both are independent of this diff.Evidence
Risk
Low. The boundary is inert until something throws; the passthrough case is covered by a test asserting the children render byte-identically. Worst case is a cosmetic issue on a screen only reachable when the app has already broken.
Two deliberate choices worth flagging for review:
@/componentsprimitives, and does not callt().initLocale()runs before the tree mounts and has itself been a startup failure (Windows Electron 启动时序问题:LevelDB 在 initLocale() 执行时尚未完全加载,导致回退到英文 #2767), so the screen that reports a crash shouldn't depend on i18n or on component code that may be implicated in the crash. This means the strings are English-only — tell me if you'd rather take the i18n dependency and I'll switch it.StrictModebut outsideQueryClientProvider, so provider-level failures are caught too.Rollback
Revert the commit — remove the wrapper in
index.react.tsxand delete the two new files. Nothing else imports the boundary.