bermudalamb/redefined-designs
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects 3 Releases Wiki Activity

docs: design for React error boundaries (#62) #83

Merged
bermudalamb merged 6 commits from feature/62-error-boundary into main 2026-08-20 17:46:27 -05:00
Conversation 0 Commits 6 Files Changed 10
+22 -9
1 changed files with 22 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 683139b8d8 - Show all commits
docs/superpowers/specs/2026-08-20-error-boundary-design.md
+22 -9
View File
@@ -42,7 +42,9 @@ StrictMode
{modalPath === '/account' && <Account/>} … {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 root boundary sits inside `BrowserRouter`**, because `AppRoutes` is what it guards. Placing it outside would only additionally catch a throw from the router itself, which is not a realistic failure here.
**Every escape action is a hard navigation, not a `<Link>`.** This is worth stating because the obvious implementation is wrong: a React error boundary does not reset when the route changes. A fallback offering `<Link to="/">` would change the URL and go on rendering the fallback, which reads as the app being permanently broken. So Reload calls `window.location.reload()` and the two "leave this page" actions set `window.location.href`, both of which remount the tree and clear the error. An earlier draft of this design justified the boundary's placement by the fallback needing router context to link; that reasoning was wrong and the placement is justified above instead.
**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 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.
@@ -70,11 +72,13 @@ props: { children, context: 'page' | 'catalogue' | 'modal', fallback: (error: Er
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. 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 | | Mount point | Title | Container | Actions |
| --- | --- | --- | | --- | --- | --- | --- |
| `page` | Full-page antd `Result` | Reload, Back to the shop | | `page` | Something went wrong | Full-page antd `Result` | Reload, Back to the shop |
| `catalogue` | Inline, in place of the grid | Reload | | `catalogue` | The item list didn't load | Inline, in place of the grid | Reload |
| `modal` | antd `Modal` | Close, returning to the storefront | | `modal` | Couldn't open that | antd `Modal` | Close, returning to the storefront |
The three titles are deliberately distinct rather than one shared string. They tell a customer which part failed — the difference between "the shop is broken" and "the list didn't load but everything else works" — and they give the end-to-end tests an unambiguous locator for *which* boundary caught, which a shared title could not.
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. 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.
@@ -92,6 +96,8 @@ Posts to `/api/client-errors` and ignores the outcome — `.catch(() => undefine
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. 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.
The modal trigger is mounted as an unconditional sibling inside the modal boundary rather than inside one of the `modalPath === …` branches. That way `/?boom=modal` exercises it with the storefront rendered behind, which is the exact assertion the test needs to make, without depending on `/account` first resolving a session.
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. 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` ## Backend: `POST /api/client-errors`
@@ -121,9 +127,12 @@ A new router mounted in `app.ts`, taking `{ context, message, stack, componentSt
**End-to-end**, in a new `error-boundary.spec.ts`: **End-to-end**, in a new `error-boundary.spec.ts`:
1. `?boom=page` renders the full-page fallback rather than a blank document. 1. `/?boom=page` renders the full-page fallback rather than a blank document.
2. `?boom=catalogue` renders 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. 2. `/?boom=catalogue` renders the inline fallback while the wordmark and the theme switch remain visible — the specific claim that a bad item no longer takes down navigation.
3. `?boom=modal` on `/account` leaves the storefront rendered behind, and Close returns to it. 3. `/?boom=modal` leaves the storefront rendered behind the modal fallback.
4. A caught error actually reaches `/api/client-errors`, asserted by observing the request from the page rather than by trusting that the reporter was called.
Assertions are on the three distinct fallback titles, so a test cannot pass because *some* boundary caught when the wrong one did.
**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. **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.
@@ -135,6 +144,10 @@ A new router mounted in `app.ts`, taking `{ context, message, stack, componentSt
**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. **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.
## A note on antd import style
The new frontend files use deep imports from `antd/es/*`, which is this project's documented convention and the style every recently-added file follows. Not `antd/lib/*`: #65 records that as the mistake which loads a second React context and breaks `ConfigProvider`. The nine files still using the `antd` barrel are #65's business, not this change's.
## Out of scope ## 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. Error reporting to an external service, persisting errors, alerting, and a frontend unit-test suite — the last belongs to #72, which already owns it.