Skip to content

feat(frontend): error and loading boundaries, and a render error that reaches Sentry - #81

Merged
khafifithebork merged 1 commit into
masterfrom
feat/error-boundaries
Sep 27, 2026
Merged

khafifithebork merged 1 commit into
masterfrom
feat/error-boundaries

Conversation

@khafifithebork

Copy link
Copy Markdown
Owner

Roadmap D1, implementing ADR-031. Three files, and the third of the audit's five defects closed.

What was broken

There was no error.tsx, loading.tsx or global-error.tsx anywhere in src/app. A render error in the learner surface — client components almost entirely (ADR-026 decision 2) — produced Next's built-in page: unstyled, no shell, no navigation, no skip link, no way out but the browser's controls. The worst-placed learner was the one mid-lesson, whose unsent progress beat went down with the component.

The reporting gap matters more. ADR-027 configured Sentry for the browser, but a React render error is caught by React and handed to a boundary. With no boundary, Next's handler took it — so what reached Sentry was whatever the framework happened to do, which is exactly the situation ADR-027 §1 set out to end.

Three files

File Notes
(learner)/error.tsx reset() retry, says recent progress may be unsaved, links to /my-courses
global-error.tsx Replaces the document; deliberately plain, inline styles, no design system
.../lessons/[lessonSlug]/loading.tsx The only route where it earns its place — the one dynamic route

reportBoundaryError lives in lib/observability/sentry.ts, not in the boundaries, because observability.test.ts asserts that module is the only file in frontend/ naming the vendor. That rule is right: two entry points to the same reporting path is how the privacy options come to differ between them.

The assertion this stands on

Nothing about the error is rendered. The fixture is an error whose message carries a URL, a lesson id and an email address, plus a digest. redact.ts strips addresses on the way out to Sentry and the visible page must not undo that — {error.message} is what a hurried implementation writes, and it looks helpful. Provoked: rendering the message fails two tests.

global-error.tsx made two existing guards object, and both were right

It replaces the document, so it needs its own <h1> where PageTitle is unavailable, and a plain <a> where next/link needs the router. The scenario it exists for is the root layout failing — which is where the design system or the router is the likely culprit.

Both are pinned, not exempted, so removing either also fails. The one ESLint rule disabled is no-html-link-for-pages, with the reason written out: it exists to prevent a full reload where a soft navigation would do, and here the full reload is the feature.

⚠️ One of my own guards could not fail

boundaries.test.tsx asserted global-error used no design token by checking --color-, --radius- and --shadow- — and passed when the provocation substituted var(--font-display), a token it hadn't thought to name.

A guard that enumerates what it knows about will always miss the next thing. It now asserts the property instead: no var(-- at all, and no className. Re-provoked three ways — a font token, a colour token, and a className — all failing.

Not verified in a browser

Inducing a real render error means changing code to break it. The 11 tests cover the behaviour; nobody has watched a learner hit one of these pages.

450 frontend tests across 40 files, tsc, eslint, verify:css, verify:a11y, and verify:static still reporting four prerendered public routes — confirming no boundary pulled a static route dynamic.

🤖 Generated with Claude Code

… reaches Sentry

Roadmap D1, implementing ADR-031. Three files, and the third of the audit's five
defects closed.

Before this there was no `error.tsx`, `loading.tsx` or `global-error.tsx` anywhere
in `src/app`. A render error in the learner surface — client components almost
entirely, by ADR-026 decision 2 — produced Next's built-in page: unstyled, no
shell, no navigation, no skip link, no way out but the browser's own controls. The
worst-placed learner was the one mid-lesson, whose unsent progress beat was gone
with the component.

**And the reporting gap, which matters more.** ADR-027 configured Sentry for the
browser, but a React render error is caught by *React* and handed to a boundary;
with no boundary, Next's own handler took it, so what reached Sentry was whatever
the framework happened to do. `reportBoundaryError` now makes that explicit.

It lives in `lib/observability/sentry.ts` rather than in the boundaries, because
`observability.test.ts` asserts that module is the only file in `frontend/` naming
the vendor — and that rule is right: two entry points to the same reporting path
is how the privacy options come to differ between them.

**The security-relevant assertion is that nothing about the error is rendered.**
The fixture is an error whose message carries a URL, a lesson id and an email
address, plus a digest. `redact.ts` strips addresses on the way *out* to Sentry,
and the visible page must not undo that — `{error.message}` is what a hurried
implementation writes and it looks helpful. Provoked: rendering the message fails
two tests.

**`global-error.tsx` forced two existing guards to grow a pinned entry**, and both
are correct to have objected. It replaces the document, so it needs its own `<h1>`
where `PageTitle` is unavailable, and a plain `<a>` where `next/link` needs the
router — and the scenario it exists for is the root layout failing, which is where
the design system or the router is the likely culprit. Pinned rather than
exempted, so removing either also fails. The one ESLint rule disabled is
`no-html-link-for-pages`, with the reason written out: it exists to prevent a full
reload where a soft navigation would do, and here the full reload is the feature.

**One of my own guards could not fail.** `boundaries.test.tsx` asserted
`global-error` used no design token by checking `--color-`, `--radius-` and
`--shadow-` — and **passed when the provocation substituted `var(--font-display)`**,
a token it had not thought to name. A guard enumerating what it knows about will
always miss the next one, so it now asserts the property: no `var(--` at all, and
no `className`. Re-provoked three ways, all failing.

Also: `loading.tsx` goes only on the lesson route, the one dynamic route in the
product, with an `aria-live` message and a reserved box so the page does not jump.
Everything else is prerendered and a boundary there would flash for a frame.

**Not verified in a browser.** Inducing a real render error means changing code to
break it; the 11 tests cover the behaviour and nobody has watched a learner hit
one of these pages.

450 frontend tests across 40 files, tsc, eslint, verify:css, verify:a11y, and
verify:static still reporting four prerendered public routes — confirming no
boundary pulled a static route dynamic.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@khafifithebork
khafifithebork merged commit 00a981d into master Sep 27, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant