bermudalambandClaude Opus 5
e8374e391a
Settles the four questions #62 left open, and records why each alternative lost. Three mount points rather than one: root, the catalogue, and the modal block. The catalogue boundary is the one that earns its keep, because the likeliest throw in this app is a component rendering API data and the item grid renders the most of it per page — containing it there keeps the header, cart badge and filters alive instead of handing the customer one dead page. The modal boundary exists because the modal-route arrangement couples two independent trees: without a boundary between them a throw in Account blanks the storefront behind it, and a throw in the storefront takes the open modal with it. Errors get reported to a new POST /api/client-errors that logs and returns 204, with no storage. A boundary that only shows a message leaves nobody knowing it happened, which is the exact failure shape this project has designed against three times already. A persisted store with an admin screen was rejected as a subsystem larger than the rest of the issue. Rate limiting needs its own limiter rather than the existing one. rateLimit.ts already documents that passwordResetRequestLimiter is keyed on caller and email, and that reusing it where there is no email collapses every caller into one shared bucket — so this endpoint gets a separate limiter keyed on req.ip, which is the real client address because trust proxy is already set. Recorded as rejected: an outermost boundary around the providers, which would sit outside ConfigProvider and need a second hand-styled fallback for a case that is remote — their render bodies are state and JSX with no data mapping. Flagged for revisiting if that stops being true. Also recorded: the rate limiter is deliberately not asserted in the integration suite, because its store is process-wide and a test that exhausts the allowance leaks into every later test keyed on the same address. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
10 KiB
React Error Boundaries — Design
Issue: #62 — No React error boundary Date: 2026-08-20 Status: Approved
Goal
Stop a single render error from unmounting the whole storefront and leaving a blank white page, and make sure that when it happens somebody can find out.
Why this one matters here
This project has been bitten repeatedly by failures that present as silence, and has designed against it each time: the storefront distinguishes "request failed" from "no items" because rendering an outage as an empty shop hid a production incident; the favorites filter answers 401 rather than an empty list; logout reports failure rather than appearing to succeed.
A missing error boundary defeats all of it one layer up. The storefront's careful "Couldn't load items" alert cannot render if the component that would render it has already thrown. A malformed API response, an unexpected null, or a bad .map anywhere in the tree produces a white page instead of any of the considered failure states.
Decisions
Four decisions were open in the issue. All four are settled.
| Question | Decision |
|---|---|
| How many boundaries, and where | Three mount points: root, catalogue, modal block |
| Whether errors get reported anywhere | Yes — a server log endpoint, no storage |
| What the fallback offers | An action appropriate to each context; error detail in development only |
| How it gets verified | A development-gated throw trigger, exercised by end-to-end tests |
Placement
StrictMode
ThemeModeProvider → CustomerAuthProvider → CartProvider → FavoritesProvider
Root → ConfigProvider
BrowserRouter
[EB "page"] full-page fallback
AppRoutes
<Routes location={backdrop}>
App
[EB "catalogue"] inline fallback
<Catalogue/>
[EB "modal"] antd Modal fallback
{modalPath === '/account' && <Account/>} …
The root boundary sits inside BrowserRouter, not outside it. The fallback offers a way back to the storefront, which needs router context. Placing it outside would mean the fallback could not link anywhere, and would only additionally catch a throw from the router itself — which is not a realistic failure here.
The catalogue boundary is the one that earns its keep. The likeliest throw in this application is a component rendering data from the API, and the item grid is where the most API data is rendered per page. Containing it there keeps the header, the cart badge, the filters and the footer alive, so the customer can still navigate rather than being handed one dead page.
The modal boundary exists because the modal-route arrangement couples two independent trees. /account, /login, /register, /forgot-password and /reset-password render as modals over the storefront as a backdrop. Without a boundary between them, a throw in Account blanks the storefront behind it and a throw in the storefront takes the open modal down with it. One boundary around the modal block separates the two in both directions.
Deliberately excluded: an outermost boundary around the providers
An additional boundary outside ThemeModeProvider would catch a throw from the providers themselves. It is not included. Their render bodies are state, useCallback, useMemo and JSX, with no data mapping and no array indexing — the throw risk is remote. It would also sit outside ConfigProvider, so its fallback could not use antd and would need a second, hand-styled fallback to maintain for a case that is unlikely to occur.
If a provider does start doing real work later, this decision should be revisited rather than assumed still valid.
Components
components/ErrorBoundary.tsx
The only class component in the codebase — getDerivedStateFromError and componentDidCatch have no hook equivalent, so this cannot be a function component.
props: { children, context: 'page' | 'catalogue' | 'modal', fallback: (error: Error) => ReactNode }
getDerivedStateFromError records the error; componentDidCatch hands it to the reporter along with the component stack. The boundary itself knows nothing about antd and nothing about how reporting works, so it stays testable and the fallback stays the caller's business.
components/ErrorFallback.tsx
One component, three containers. It renders a title, a set of actions, and — only under import.meta.env.DEV — the error message and component stack.
| Mount point | Container | Actions |
|---|---|---|
page |
Full-page antd Result |
Reload, Back to the shop |
catalogue |
Inline, in place of the grid | Reload |
modal |
antd Modal |
Close, returning to the storefront |
Keeping the development-only detail in one component means there is exactly one place where a decision about showing customers a stack trace lives, rather than three that can drift apart.
The modal fallback is a Modal rather than inline markup because the modal block renders after the routed content in the DOM. An inline fallback there would appear at the foot of the page, disconnected from the thing that failed.
errorReporting.ts
reportClientError({ context, message, stack, componentStack, path })
Posts to /api/client-errors and ignores the outcome — .catch(() => undefined). A reporter that throws inside componentDidCatch is the one failure this whole change must not introduce, so it swallows deliberately and that is the single place in this design where swallowing is correct.
components/DevThrow.tsx
Throws when the current URL carries ?boom=<scope> matching its own scope. Mounted inside each boundary as {import.meta.env.DEV && <DevThrow scope="…" />}, so Rollup eliminates both the element and its import from a production build.
This is test-only code adjacent to a production path, which is exactly what the project's test/production boundary rule exists for. The gate is therefore verified rather than trusted: the production build is grepped for a distinctive marker string and must contain zero occurrences.
Backend: POST /api/client-errors
A new router mounted in app.ts, taking { context, message, stack, componentStack, path }.
Validation. context must be one of page, catalogue, modal; anything else is a 400. This follows parseItemFilters, which refuses a malformed filter rather than coercing it — a report from an unknown context means the client and server disagree about something, and that is worth surfacing rather than logging under a guessed label.
Truncation, applied before logging rather than rejecting, because a report that is too long is still worth having:
| Field | Limit |
|---|---|
message |
500 |
stack |
4000 |
componentStack |
4000 |
path |
200 |
Rate limiting needs its own limiter, not the existing one. rateLimit.ts documents that passwordResetRequestLimiter is keyed on caller and email, and that "applying the same limiter to an endpoint without one collapses every caller into a single ip: bucket". This endpoint has no email, so it gets a separate limiter keyed on req.ip alone at 30 requests per 15 minutes. app.set('trust proxy', 1) is already in place, so req.ip is the real client address from X-Forwarded-For rather than the proxy's — per-customer, not per-deployment. Hitting the limit is harmless: the client ignores the response either way.
Logs with a [client-error] prefix and returns 204. No body, because the client ignores the response.
Unauthenticated, because render errors happen to signed-out visitors and an error report that requires a session would miss exactly the cases worth knowing about.
No storage. The container log is where this project's operational visibility already lives. A table, a retention policy and an admin screen are a subsystem, and were rejected as larger than the rest of this issue.
Verification
End-to-end, in a new error-boundary.spec.ts:
?boom=pagerenders the full-page fallback rather than a blank document.?boom=cataloguerenders the inline fallback while the header, the wordmark and the cart badge remain visible — the specific claim that a bad item no longer takes down navigation.?boom=modalon/accountleaves the storefront rendered behind, and Close returns to it.
Backend integration, for the new route: a well-formed report returns 204, an unknown context is a 400, and an oversized message is truncated rather than rejected. The rate limiter is deliberately not asserted in the integration suite — its store is in-memory and process-wide, so a test that exhausts the allowance leaks that state into every later test keyed on the same address, and the order-dependent failure it produces later would cost more than the assertion is worth. The limit is verified by reading the configuration, the way the other limiter is.
Production safety, both directions, as #61 did for its instrumentation gate: a normal npm run build produces a bundle containing zero occurrences of the DevThrow marker, and the development server does throw when the parameter is present. A gate that is silently always-off looks identical to a gate that works.
Risks to confirm during implementation
React 18 StrictMode double-invokes render in development, so a caught error may be reported twice locally. Harmless in production, where StrictMode's double render does not apply, but the end-to-end assertions must not depend on a single report.
Vite's error overlay may intercept runtime errors in the development server that Playwright drives. If it renders over the page it will block clicks and the tests will fail in a way that looks like the boundary not working. If that happens, the overlay is disabled for the test run rather than the tests worked around.
Out of scope
Error reporting to an external service, persisting errors, alerting, and a frontend unit-test suite — the last belongs to #72, which already owns it.