Feature/62 error boundary #85

Merged

bermudalamb merged 5 commits from feature/62-error-boundary into main 2026-08-20 18:32:28 -05:00

Conversation 0 Commits 5 Files Changed 11

+315 -55

bermudalamb commented 2026-08-20 18:32:11 -05:00

Owner

Copy Link

Copy Source

No description provided.

bermudalamb added 4 commits 2026-08-20 18:32:11 -05:00

fix(frontend): make the error reporter genuinely unable to throw (#62) ... b4be15ae0d

The comment claimed the reporter could not throw and the code did not deliver it. JSON.stringify(report) and the call to fetch both run synchronously, as arguments, before the promise carrying the .catch exists — so a throw from either escaped straight out of componentDidCatch, where nothing remains to catch it. The boundary that exists to stop errors would itself have been the thing that crashed.

Not merely theoretical: React does not guarantee the value handed to componentDidCatch is a real Error despite the parameter's type, because code can throw anything. An object whose message or stack is circular makes JSON.stringify throw.

The body is now wrapped in try/catch for the synchronous part, and the existing .catch still covers rejection once the request is in flight. Neither covers the other, so both are kept, and the comment now says so rather than asserting a guarantee the code did not make.

Verified: build clean, lint 0 errors and 31 warnings, unchanged.

Refs #62
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

feat(frontend): mount error boundaries at the root, the item grid and the modals (#62) ... 5b9cf91cef

Three mount points, so a render error costs the smallest part of the page it can.

The catalogue boundary is the one that earns its keep. The likeliest throw in this app is a component rendering data from the API, and the item grid renders the most of it per page — contained there, the header, cart badge, filters and footer all survive, so a customer can still navigate instead of being handed one dead page.

The modal boundary exists because the modal-route arrangement couples two independent trees. /account, /login and the rest render as modals over the storefront as a backdrop, so 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. One boundary separates them in both directions.

Every escape action is a hard navigation rather than a Link. This is worth stating because the obvious implementation is wrong: a boundary does not reset when the route changes, so a Link would change the URL and go on rendering the fallback, which reads as the app being permanently broken.

ErrorFallback changed too, outside this change's original scope and for a reason worth recording. antd's Result renders its title as a plain div with no heading semantics, so a page whose entire content is an error message offered a screen-reader user navigating by headings nothing at all to find. The title is now wrapped in Typography.Title. The tests assert a heading role and were right to; the component was what needed fixing, not the assertion.

DevThrow throws on ?boom=<scope> and is mounted only behind import.meta.env.DEV, so Rollup drops it from a production build. Checked in both directions rather than trusted: the dev server serves it, and a production bundle greps to zero occurrences of its marker. A gate that is silently always-off looks identical to one that works.

Verified: 87 end-to-end tests pass, 4 of them new — each boundary catches rather than blanking, the header survives a catalogue throw, the storefront survives a modal throw, and the report is observed reaching /api/client-errors on the wire rather than assumed. Build clean, lint 0 errors and 31 warnings.

Refs #62
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

docs: mark the error-boundary design implemented (#62) ... 911e7ff77e

Records the two things the design got wrong. antd's Result renders its title as a plain div, so the design's Result usage and its getByRole('heading') assertions contradicted each other and the tests could never have passed as written — resolved by giving the title real heading semantics rather than by loosening the assertion, because an error page with no heading leaves a screen-reader user navigating by headings nothing to find. And import.meta.env had no ambient declaration anywhere in the app, so the DEV gate did not type-check until vite-env.d.ts was added.

The Vite error overlay risk the design flagged did not materialise.

Refs #62
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

fix: address the final review of the error boundaries (#62) ... ecf989cff7

Five findings from a whole-branch review, two of which mattered.

The catalogue boundary could not be recovered by the very controls it exists to keep alive. When the item grid threw, the header, filter chips and Clear filters stayed mounted — that was the point — but clicking Clear filters only changed the URL. A boundary does not reset on a client-side state change, so the fallback went on rendering over a catalogue that would by then have loaded perfectly well, and the only offered action reloaded the same failing URL. The shop read as permanently broken, which is the failure this whole change exists to prevent, reintroduced one level down. ErrorBoundary now takes an optional resetKey and clears itself when it changes; the catalogue boundary passes the filter key. The page and modal boundaries deliberately do not take one, because their escapes are hard navigations that remount the tree already — recorded on the prop so nobody completes the pattern by symmetry.

The client-error endpoint could fill the disk. It is unauthenticated, each accepted report wrote about 8.7 KB, and Docker's default json-file driver has no size cap — so the rate limiter bounded a render loop, as its comment claims, but not a few hundred cheap source addresses. Stack and component stack now truncate at 1000 rather than 4000, which is still around fifteen frames and cuts the worst case to under 3 KB, and the QA compose file caps and rotates the log. Production is a Portainer stack outside this repository and needs the same option applied there; noted in the design doc rather than left implied.

Three smaller things. A falsy thrown value defeated the boundary entirely: throw null is legal, and branching on the error object alone treated it as no error, re-rendered the children, threw again, and would have taken the root down — a blank page, the one outcome this is all here to avoid. The boundary now tracks hasError separately and synthesises a real Error for non-Error throws. The modal fallback announced its title twice to a screen reader, once as the dialog's name and once as the heading inside it, so the redundant dialog title is gone. And the design doc claimed the development-only detail shows the component stack when it only ever showed the message; corrected, with a note that the stack still reaches the server log, which is where it is useful.

Verified after all five: backend lint 0 errors, 144 integration tests, frontend lint 0 errors and 31 warnings, 87 end-to-end tests, all against a freshly created database.

Refs #62
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

bermudalamb added 1 commit 2026-08-20 18:32:19 -05:00

Merge branch 'main' into feature/62-error-boundary faf993b25e

bermudalamb merged commit cf45b7a8eb into main 2026-08-20 18:32:28 -05:00

bermudalamb deleted branch feature/62-error-boundary 2026-08-20 18:32:28 -05:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

calamb (Chelsea Lamb) bermudalamb (Thom Lamb)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: bermudalamb/redefined-designs#85