feat(ui): add error boundaries so one throw stops taking the whole page - #90
Merged
Merged
Conversation
The app had no error boundary anywhere - no error.tsx, no global-error.tsx, no ErrorBoundary in src/. Any throw during a client render escaped to Next's default global error screen and replaced the entire page. That was not theoretical: the formatDateTime RangeError fixed in #89 blanked the whole contact page from a single bad Contact_Date. That fix stopped one cause; this contains the class. Three boundaries, because placement is the whole design -------------------------------------------------- src/app/(web)/error.tsx anything below the (web) layout, rendered INSIDE the shell so Header, user menu and sign-out survive src/app/error.tsx /signin, /session-error, /auth-error - bare, since those routes have no shell src/app/global-error.tsx a throw in the root layout itself; replaces it error.tsx never wraps the layout of its OWN segment, so one boundary will not do. src/app/error.tsx alone would replace the (web) shell on any page error and take the user's sign-out with it - the exact trap /session-error exists to avoid. And neither error.tsx catches a root-layout throw, which is what global-error is for. Next 16: the prop is `retry`, not `reset` -------------------------------------------------- Earlier versions passed `reset`. Next 16 renamed it to `retry`, which re-fetches and re-renders the segment; `reset` survives but only clears error state without re-fetching. A boundary wired to the stale name renders fine and its button silently does nothing, so each boundary has a test asserting `retry` is called. Logging: identifiers only, never the message -------------------------------------------------- These boundaries sit above components that render pastoral notes, names and emails. Unlike a controlled catch block around an HTTP call, a render error's message is not guaranteed to be content-free, so none of them log it - only `{ boundary, name, digest }` under the structured event `ui.render.error`, with `digest` as the join key to the un-redacted server log. Each boundary has a test that fails if a message ever reaches the log, and the shell boundary also asserts the message never reaches the page. This follows the F5 logging policy (.claude/references/auth.md § Logging policy). global-error constraints, all load-bearing -------------------------------------------------- It imports nothing from the app (a test enforces this by reading the source) - whatever failed may be that very code, and per Next's docs global-error does not receive the app's global styles anyway. Styling is therefore inline, which is safe ONLY because the CSP is `style-src 'self' 'unsafe-inline'` with no nonce (see .claude/references/security-headers.md); a nonce-based style-src would silently drop all of it. The tab title uses React's <title> element because a client component cannot export metadata. Verification -------------------------------------------------- `npm run build` succeeds, which is what actually proves Next accepts and wires these file conventions - a unit test of the component cannot. Full suite, all coverage thresholds, `npx tsc --noEmit` and `npx eslint .` are clean. Statements 99.73% -> 99.74% (1159/1162) Branches 97.18% -> 97.21% (593/610) Functions 99.29% -> 99.31% (291/293) Lines 99.91% -> 99.91% (1126/1127) 996 -> 1015 tests. All three new files are at 100%. Two testing mechanics are documented in the testing reference because they cost time here and present as component bugs: React 19 HOISTS <html>/<body>/<title> out of the render container, so the structural contract has to be asserted with renderToStaticMarkup rather than querying the DOM; and these components cannot be called as plain functions the way layout.test.tsx does, because they use useEffect. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
The app had no error boundary anywhere — no
error.tsx, noglobal-error.tsx, noErrorBoundaryinsrc/. Any throw during a client render escaped to Next's default global error screen and replaced the entire page.That wasn't theoretical: the
formatDateTimeRangeErrorfixed in #89 blanked the whole contact page from a single badContact_Date. That fix removed one cause; this contains the class.Three boundaries, because placement is the whole design
src/app/(web)/error.tsx(web)layoutsrc/app/error.tsx/signin,/session-error,/auth-errorsrc/app/global-error.tsxlayout.tsxitselferror.tsxnever wraps the layout of its own segment, so one boundary will not do:src/app/error.tsxalone would replace the(web)shell on any page error and take the user's sign-out with it — the exact trap/session-errorexists to avoid.error.tsxcatches a root-layout throw. That is whatglobal-error.tsxis for.Next 16 renamed the prop:
retry, notresetEarlier versions passed
reset. Next 16 passesretry, which re-fetches and re-renders the segment;resetsurvives but only clears error state without re-fetching. A boundary wired to the stale name renders fine and its button silently does nothing — so each boundary has a test assertingretryis called.Logging: identifiers only, never the message
These boundaries sit above components that render pastoral notes, names and emails. Unlike a controlled catch block around an HTTP call, a render error's message is not guaranteed content-free. So none of them log it — only
{ boundary, name, digest }under the structured eventui.render.error, withdigestas the join key to the un-redacted server log.Each boundary has a test that fails if a message ever reaches the log, and the shell boundary also asserts the message never reaches the page. Follows the F5 logging policy (
.claude/references/auth.md§ Logging policy).global-errorconstraints, all load-bearingglobal-errordoesn't receive the app's global styles anyway.style-src 'self' 'unsafe-inline'with no nonce (.claude/references/security-headers.md). A nonce-basedstyle-srcwould silently drop every one of those styles.<title>, because a client component cannot exportmetadata.Verification
npm run buildsucceeds — which is what actually proves Next accepts and wires these file conventions; a unit test of the component cannot. Full suite, all coverage thresholds,npx tsc --noEmitandnpx eslint .are clean.996 → 1015 tests. All three new files are at 100%.
Two testing mechanics worth knowing
Both are documented in the testing reference, because each presents as a component bug rather than a failed assertion:
<html>,<body>and<title>out of the render container. Afterrender(<GlobalError />),container.querySelector("html")isnulland the first child is the inner<div>— even though the component returns them. The structural contract is asserted withrenderToStaticMarkupinstead.layout.test.tsxdoes, because they useuseEffectand hooks need a real render.🤖 Generated with Claude Code