Commit Graph
3 Commits
Author SHA1 Message Date
bermudalambandClaude Opus 5 557703f86d docs: mark the error-boundary design implemented (#62)
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>
2026-08-20 18:32:27 -05:00
bermudalambandClaude Opus 5 683139b8d8 docs: correct the error-boundary design's fallback actions (#62)
Found while working the design into a plan: the obvious implementation of the fallback's escape actions is wrong, and the spec was recommending it.

A React error boundary does not reset when the route changes. The original spec justified placing the root boundary inside BrowserRouter on the grounds that the fallback needed router context to offer a way back — but a fallback offering a Link would change the URL and go on rendering the fallback, which reads as the app being permanently broken rather than recovering. Every escape action is therefore a hard navigation: reload, or setting window.location.href. The placement is unchanged, but it is now justified by what the boundary guards rather than by reasoning that does not hold.

Three consequences recorded while there. The three fallbacks get distinct titles rather than one shared string, so a customer learns which part failed and the tests get an unambiguous locator for which boundary caught. The modal throw trigger mounts as an unconditional sibling inside its boundary, so /?boom=modal exercises it with the storefront behind rather than depending on /account resolving a session first. And a fourth end-to-end test asserts the report actually reaches /api/client-errors by observing the request, rather than trusting the reporter was called.

Also recorded: new files use antd/es deep imports, this project's documented convention — not antd/lib, which #65 notes loads a second React context and breaks ConfigProvider.

Refs #62
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-20 16:25:48 -05:00
bermudalambandClaude Opus 5 e8374e391a docs: design for React error boundaries (#62)
SonarQube Analysis / sonarqube (pull_request) Failing after 13m37s
Tests / lint (pull_request) Successful in 2m27s
Tests / backend-unit (pull_request) Successful in 40s
Tests / frontend-e2e (pull_request) Failing after 9m4s
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>
2026-08-20 16:17:29 -05:00