docs: implementation plan for React error boundaries (#62)
Four tasks, each ending in an independently testable deliverable: the backend endpoint with its own rate limiter, the boundary and fallback components, the three mount points with end-to-end coverage, and the production-gate verification. Refs #62 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,787 @@
|
|||||||
|
# React Error Boundaries Implementation Plan
|
||||||
|
|
||||||
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||||
|
|
||||||
|
**Goal:** Stop a single render error from unmounting the whole storefront into a blank white page, and make sure that when it happens it leaves a trace in the server log.
|
||||||
|
|
||||||
|
**Architecture:** One `ErrorBoundary` class component mounted at three points — the root, the item grid, and the modal block — each rendering the same parameterized `ErrorFallback` in a different container. `componentDidCatch` fires a fire-and-forget POST to a new `/api/client-errors` endpoint that logs and returns 204. A development-only `DevThrow` component, gated behind `import.meta.env.DEV` so it cannot reach production, lets the end-to-end suite fire each boundary deliberately.
|
||||||
|
|
||||||
|
**Tech Stack:** React 18.3 + TypeScript, antd 5.20, react-router-dom 6.26, Vite 5, Express + `express-rate-limit`, Jest + supertest (backend), Playwright (end-to-end).
|
||||||
|
|
||||||
|
**Spec:** `docs/superpowers/specs/2026-08-20-error-boundary-design.md`
|
||||||
|
|
||||||
|
## Global Constraints
|
||||||
|
|
||||||
|
- **Branch:** `feature/62-error-boundary`. Every commit subject ends `(#62)`. Do not push — the repository owner pushes.
|
||||||
|
- **Node:** the dev machine defaults to Node 18, which cannot run this project's tooling. Prefix every command with `export PATH="/c/Users/tlamb/AppData/Local/nvm/v24.13.1:$PATH"`.
|
||||||
|
- **Integration tests need Postgres.** Port 55432 is Hyper-V-reserved on this machine. Start the container on another port and pass `TEST_PGPORT` rather than editing `docker-compose.test.yml`.
|
||||||
|
- **antd imports in new files:** deep imports from `antd/es/*`. Never `antd/lib/*` — #65 records that it loads a second React context and breaks `ConfigProvider`.
|
||||||
|
- **Contexts are exactly** `'page' | 'catalogue' | 'modal'`, spelled identically in the frontend type and the server's allowlist.
|
||||||
|
- **Truncation limits:** message 500, stack 4000, componentStack 4000, path 200.
|
||||||
|
- **Rate limit:** 30 requests per 15 minutes, keyed on the caller only.
|
||||||
|
- **Fallback titles**, used verbatim as test locators: page `Something went wrong`, catalogue `The item list didn't load`, modal `Couldn't open that`.
|
||||||
|
- **Every escape action is a hard navigation** (`window.location.reload()` / `window.location.href = '/'`). A boundary does not reset on client-side navigation, so a `<Link>` would leave the fallback on screen.
|
||||||
|
- **Arrays of JSX elements need `key` props.** #81 fixed exactly this bug; the `Result` `extra` array is an array of elements.
|
||||||
|
- **Lint must stay at 0 errors.** `sonarjs/prefer-read-only-props` is on as a warning — declare component props as `Readonly<{…}>` from the start.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 1: Backend endpoint — `POST /api/client-errors`
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `backend/src/rateLimit.ts` (append a second limiter)
|
||||||
|
- Create: `backend/src/routes/clientErrors.ts`
|
||||||
|
- Modify: `backend/src/app.ts` (import near the other route imports, mount beside the other `app.use('/api/...')` lines)
|
||||||
|
- Test: `backend/tests/integration/clientErrors.integration.test.ts`
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Consumes: nothing from earlier tasks.
|
||||||
|
- Produces: `POST /api/client-errors` accepting `{ context: 'page'|'catalogue'|'modal', message?: string, stack?: string, componentStack?: string, path?: string }` → `204` on success, `400` on an unrecognised `context`. Task 2's `reportClientError` posts this exact shape.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Start a test database and write the failing test**
|
||||||
|
|
||||||
|
Start Postgres first (once per session):
|
||||||
|
|
||||||
|
```bash
|
||||||
|
MSYS_NO_PATHCONV=1 docker run -d --name rd-eb-db \
|
||||||
|
-e POSTGRES_USER=redefined_test -e POSTGRES_PASSWORD=redefined_test -e POSTGRES_DB=redefined_test \
|
||||||
|
-p 55435:5432 --tmpfs /var/lib/postgresql/data postgres:16
|
||||||
|
```
|
||||||
|
|
||||||
|
Create `backend/tests/integration/clientErrors.integration.test.ts`:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
import request from 'supertest';
|
||||||
|
import app from '../../src/app';
|
||||||
|
import { pool } from '../../src/db';
|
||||||
|
import { closeDb } from './setup/testDb';
|
||||||
|
|
||||||
|
// No resetDb: this endpoint never touches the database. The pool still has to
|
||||||
|
// be closed or Jest reports an open handle.
|
||||||
|
afterAll(async () => {
|
||||||
|
await pool.end();
|
||||||
|
await closeDb();
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('POST /api/client-errors', () => {
|
||||||
|
let errorSpy: jest.SpyInstance;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
errorSpy = jest.spyOn(console, 'error').mockImplementation(() => undefined);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
errorSpy.mockRestore();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('accepts a well-formed report and logs it', async () => {
|
||||||
|
const res = await request(app).post('/api/client-errors').send({
|
||||||
|
context: 'catalogue',
|
||||||
|
message: 'Cannot read properties of undefined',
|
||||||
|
stack: 'Error: Cannot read properties of undefined\n at ItemCard',
|
||||||
|
componentStack: '\n at Catalogue\n at App',
|
||||||
|
path: '/?category=3'
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(res.status).toBe(204);
|
||||||
|
expect(errorSpy).toHaveBeenCalledTimes(1);
|
||||||
|
const logged = errorSpy.mock.calls[0][0] as string;
|
||||||
|
expect(logged).toContain('[client-error]');
|
||||||
|
expect(logged).toContain('context=catalogue');
|
||||||
|
expect(logged).toContain('Cannot read properties of undefined');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('refuses a context it does not recognise rather than logging under a guess', async () => {
|
||||||
|
const res = await request(app)
|
||||||
|
.post('/api/client-errors')
|
||||||
|
.send({ context: 'checkout', message: 'boom' });
|
||||||
|
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(res.body.error).toBe('invalid context');
|
||||||
|
expect(errorSpy).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('refuses a report with no context at all', async () => {
|
||||||
|
const res = await request(app).post('/api/client-errors').send({ message: 'boom' });
|
||||||
|
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(errorSpy).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
// Truncated rather than refused: an over-long report is still the only record
|
||||||
|
// of the failure, and dropping it would lose the thing worth having.
|
||||||
|
it('truncates an oversized message instead of refusing it', async () => {
|
||||||
|
const res = await request(app).post('/api/client-errors').send({
|
||||||
|
context: 'page',
|
||||||
|
message: 'x'.repeat(5000)
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(res.status).toBe(204);
|
||||||
|
const logged = errorSpy.mock.calls[0][0] as string;
|
||||||
|
expect(logged).toContain('[truncated]');
|
||||||
|
expect(logged.length).toBeLessThan(2000);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run the test and confirm it fails for the right reason**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
export PATH="/c/Users/tlamb/AppData/Local/nvm/v24.13.1:$PATH"
|
||||||
|
cd backend && TEST_PGPORT=55435 npx jest -c jest.integration.config.js --runInBand clientErrors
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: all four fail with `404` responses, because nothing is mounted at that path yet. A failure for any other reason means the harness is wrong — fix that before writing the route.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Add the limiter**
|
||||||
|
|
||||||
|
Append to `backend/src/rateLimit.ts`:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Client error reports carry no email, so this one is keyed on the caller
|
||||||
|
// alone — deliberately not reusing passwordResetRequestLimiter, whose comment
|
||||||
|
// above explains why its key is wrong for an endpoint without an email.
|
||||||
|
//
|
||||||
|
// `trust proxy` is set in app.ts, so `req.ip` is the real client address from
|
||||||
|
// X-Forwarded-For rather than Nginx Proxy Manager's, making this a per-customer
|
||||||
|
// allowance rather than one shared by everybody behind the proxy.
|
||||||
|
//
|
||||||
|
// Generous, because hitting the limit is harmless: the reporter ignores the
|
||||||
|
// response either way. It exists so a render loop cannot fill the log.
|
||||||
|
const CLIENT_ERROR_WINDOW_MS = 15 * 60 * 1000;
|
||||||
|
const CLIENT_ERROR_MAX_REQUESTS = 30;
|
||||||
|
|
||||||
|
export const clientErrorLimiter = rateLimit({
|
||||||
|
windowMs: CLIENT_ERROR_WINDOW_MS,
|
||||||
|
limit: CLIENT_ERROR_MAX_REQUESTS,
|
||||||
|
standardHeaders: 'draft-7',
|
||||||
|
legacyHeaders: false,
|
||||||
|
message: { error: 'too many reports' }
|
||||||
|
});
|
||||||
|
```
|
||||||
|
|
||||||
|
Note it uses the default key generator rather than a custom one. That is deliberate: the custom `keyByCallerAndEmail` is what triggers the `ERR_ERL_KEY_GEN_IPV6` warning already visible in the backend log, and the default handles IPv6 correctly.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Write the route**
|
||||||
|
|
||||||
|
Create `backend/src/routes/clientErrors.ts`:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
import { Router, Request, Response } from 'express';
|
||||||
|
import { clientErrorLimiter } from '../rateLimit';
|
||||||
|
|
||||||
|
const router = Router();
|
||||||
|
|
||||||
|
// The three error boundaries in the frontend. An unrecognised context means the
|
||||||
|
// client and the server disagree about something, which is worth surfacing
|
||||||
|
// rather than logging under a guessed label — the same reasoning as
|
||||||
|
// parseItemFilters refusing a malformed filter instead of coercing it.
|
||||||
|
const CONTEXTS: readonly string[] = ['page', 'catalogue', 'modal'];
|
||||||
|
|
||||||
|
const MAX_MESSAGE = 500;
|
||||||
|
const MAX_STACK = 4000;
|
||||||
|
const MAX_COMPONENT_STACK = 4000;
|
||||||
|
const MAX_PATH = 200;
|
||||||
|
|
||||||
|
// Anything that is not a string becomes empty rather than 'undefined' or
|
||||||
|
// '[object Object]', so a malformed field cannot dress itself up as content.
|
||||||
|
function clip(value: unknown, max: number): string {
|
||||||
|
if (typeof value !== 'string') {
|
||||||
|
return '';
|
||||||
|
}
|
||||||
|
return value.length > max ? `${value.slice(0, max)}… [truncated]` : value;
|
||||||
|
}
|
||||||
|
|
||||||
|
// No asyncRoute: this handler is synchronous, so there is no promise for the
|
||||||
|
// error middleware to miss.
|
||||||
|
router.post('/', clientErrorLimiter, (req: Request, res: Response) => {
|
||||||
|
const context: unknown = req.body?.context;
|
||||||
|
if (typeof context !== 'string' || !CONTEXTS.includes(context)) {
|
||||||
|
return res.status(400).json({ error: 'invalid context' });
|
||||||
|
}
|
||||||
|
|
||||||
|
console.error(
|
||||||
|
`[client-error] context=${context} path=${clip(req.body?.path, MAX_PATH)}\n` +
|
||||||
|
` message: ${clip(req.body?.message, MAX_MESSAGE)}\n` +
|
||||||
|
` stack: ${clip(req.body?.stack, MAX_STACK)}\n` +
|
||||||
|
` componentStack: ${clip(req.body?.componentStack, MAX_COMPONENT_STACK)}`
|
||||||
|
);
|
||||||
|
|
||||||
|
res.status(204).end();
|
||||||
|
});
|
||||||
|
|
||||||
|
export default router;
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 5: Mount it**
|
||||||
|
|
||||||
|
In `backend/src/app.ts`, add the import beside the other route imports:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
import clientErrorsRouter from './routes/clientErrors';
|
||||||
|
```
|
||||||
|
|
||||||
|
and the mount beside the other `app.use('/api/...')` lines:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
app.use('/api/client-errors', clientErrorsRouter);
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 6: Run the tests and confirm they pass**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && TEST_PGPORT=55435 npx jest -c jest.integration.config.js --runInBand clientErrors
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: 4 passed.
|
||||||
|
|
||||||
|
- [ ] **Step 7: Run the whole backend suite plus build and lint**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd backend && npm run build && npm run lint && npm run test:unit
|
||||||
|
cd backend && TEST_PGPORT=55435 npm run test:integration
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: build clean, lint 0 errors, 78 unit pass, and **138 integration pass** — 134 existing plus the 4 new. Confirm the total actually rose by 4 rather than trusting the number here. `routesAreWrapped.test.ts` must still pass; it scans for unwrapped async handlers, and the new handler is synchronous.
|
||||||
|
|
||||||
|
- [ ] **Step 8: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/src/rateLimit.ts backend/src/routes/clientErrors.ts backend/src/app.ts backend/tests/integration/clientErrors.integration.test.ts
|
||||||
|
git commit -m "feat(backend): log client-side render errors to the server (#62)"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 2: The boundary, the fallback, and the reporter
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Create: `frontend/src/errorReporting.ts`
|
||||||
|
- Create: `frontend/src/components/ErrorBoundary.tsx`
|
||||||
|
- Create: `frontend/src/components/ErrorFallback.tsx`
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Consumes: `POST /api/client-errors` from Task 1.
|
||||||
|
- Produces:
|
||||||
|
- `type ErrorContext = 'page' | 'catalogue' | 'modal'`
|
||||||
|
- `reportClientError(report: ClientErrorReport): void`
|
||||||
|
- `<ErrorBoundary context={ErrorContext} fallback={(error: Error) => React.ReactNode}>` — default export of `components/ErrorBoundary`
|
||||||
|
- `<ErrorFallback error={Error} title={string} actions={React.ReactNode} fullPage?={boolean} />` — default export of `components/ErrorFallback`
|
||||||
|
|
||||||
|
Task 3 mounts all three of these.
|
||||||
|
|
||||||
|
Nothing here is independently testable — the frontend has no unit suite, which is #72's business. The gate for this task is that both files compile and lint clean; the behaviour is proved in Task 3.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the reporter**
|
||||||
|
|
||||||
|
Create `frontend/src/errorReporting.ts`:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
// Where a caught render error came from. Kept in step with the allowlist in
|
||||||
|
// backend/src/routes/clientErrors.ts, which refuses anything else rather than
|
||||||
|
// logging under a guess — change one and you must change the other.
|
||||||
|
export type ErrorContext = 'page' | 'catalogue' | 'modal';
|
||||||
|
|
||||||
|
export interface ClientErrorReport {
|
||||||
|
context: ErrorContext;
|
||||||
|
message: string;
|
||||||
|
stack?: string;
|
||||||
|
componentStack?: string;
|
||||||
|
path: string;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Fire and forget, and deliberately swallowing — the one place in this change
|
||||||
|
// where swallowing is correct. This runs inside componentDidCatch, so a
|
||||||
|
// reporter that rejected would throw from the very thing that exists to stop
|
||||||
|
// throws, and there would be nothing left to catch it.
|
||||||
|
export function reportClientError(report: ClientErrorReport): void {
|
||||||
|
void fetch('/api/client-errors', {
|
||||||
|
method: 'POST',
|
||||||
|
headers: { 'Content-Type': 'application/json' },
|
||||||
|
body: JSON.stringify(report)
|
||||||
|
}).catch(() => undefined);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Write the boundary**
|
||||||
|
|
||||||
|
Create `frontend/src/components/ErrorBoundary.tsx`:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
import React from 'react';
|
||||||
|
import { ErrorContext, reportClientError } from '../errorReporting';
|
||||||
|
|
||||||
|
type ErrorBoundaryProps = Readonly<{
|
||||||
|
context: ErrorContext;
|
||||||
|
fallback: (error: Error) => React.ReactNode;
|
||||||
|
children: React.ReactNode;
|
||||||
|
}>;
|
||||||
|
|
||||||
|
interface ErrorBoundaryState {
|
||||||
|
error: Error | null;
|
||||||
|
}
|
||||||
|
|
||||||
|
// The only class component in the codebase. getDerivedStateFromError and
|
||||||
|
// componentDidCatch have no hook equivalent, so a boundary cannot be written as
|
||||||
|
// a function component.
|
||||||
|
//
|
||||||
|
// It knows nothing about antd and nothing about how reporting reaches the
|
||||||
|
// server. The fallback is the caller's business, which is what lets one
|
||||||
|
// boundary serve a full page, an inline region and a modal.
|
||||||
|
export default class ErrorBoundary extends React.Component<ErrorBoundaryProps, ErrorBoundaryState> {
|
||||||
|
state: ErrorBoundaryState = { error: null };
|
||||||
|
|
||||||
|
static getDerivedStateFromError(error: Error): ErrorBoundaryState {
|
||||||
|
return { error };
|
||||||
|
}
|
||||||
|
|
||||||
|
componentDidCatch(error: Error, info: React.ErrorInfo): void {
|
||||||
|
reportClientError({
|
||||||
|
context: this.props.context,
|
||||||
|
message: error.message,
|
||||||
|
stack: error.stack,
|
||||||
|
componentStack: info.componentStack ?? undefined,
|
||||||
|
path: `${window.location.pathname}${window.location.search}`
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
render(): React.ReactNode {
|
||||||
|
if (this.state.error) {
|
||||||
|
return this.props.fallback(this.state.error);
|
||||||
|
}
|
||||||
|
return this.props.children;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 3: Write the fallback**
|
||||||
|
|
||||||
|
Create `frontend/src/components/ErrorFallback.tsx`:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
import React from 'react';
|
||||||
|
import Result from 'antd/es/result';
|
||||||
|
import Typography from 'antd/es/typography';
|
||||||
|
|
||||||
|
const { Paragraph, Text } = Typography;
|
||||||
|
|
||||||
|
type ErrorFallbackProps = Readonly<{
|
||||||
|
error: Error;
|
||||||
|
title: string;
|
||||||
|
actions: React.ReactNode;
|
||||||
|
fullPage?: boolean;
|
||||||
|
}>;
|
||||||
|
|
||||||
|
// The single place that decides whether a customer is shown a stack trace.
|
||||||
|
// Gated on DEV so a developer sees the throw immediately while a production
|
||||||
|
// bundle cannot render it at all — one decision in one file rather than the
|
||||||
|
// same judgement repeated at three mount points, where they would drift.
|
||||||
|
export default function ErrorFallback({ error, title, actions, fullPage = false }: ErrorFallbackProps) {
|
||||||
|
return (
|
||||||
|
<Result
|
||||||
|
status="error"
|
||||||
|
title={title}
|
||||||
|
subTitle="This has been reported. Nothing you did caused it."
|
||||||
|
style={{ paddingBlock: fullPage ? 64 : 24 }}
|
||||||
|
extra={actions}
|
||||||
|
>
|
||||||
|
{import.meta.env.DEV ? (
|
||||||
|
<Paragraph>
|
||||||
|
<Text code>{error.message}</Text>
|
||||||
|
</Paragraph>
|
||||||
|
) : null}
|
||||||
|
</Result>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Build and lint**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
export PATH="/c/Users/tlamb/AppData/Local/nvm/v24.13.1:$PATH"
|
||||||
|
cd frontend && npm run build && npm run lint
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: build clean. Lint 0 errors and **31 warnings** — the count on `main`. If it rose, the new files introduced a warning; fix it rather than accepting it. The likeliest is `sonarjs/prefer-read-only-props`, which the `Readonly<{…}>` props above are written to avoid.
|
||||||
|
|
||||||
|
- [ ] **Step 5: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add frontend/src/errorReporting.ts frontend/src/components/ErrorBoundary.tsx frontend/src/components/ErrorFallback.tsx
|
||||||
|
git commit -m "feat(frontend): add an error boundary, its fallback, and error reporting (#62)"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 3: Mount the three boundaries and prove each one catches
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Create: `frontend/src/components/DevThrow.tsx`
|
||||||
|
- Modify: `frontend/src/main.tsx` (the `Root` function's `BrowserRouter` block, and the end of `AppRoutes`' returned fragment)
|
||||||
|
- Modify: `frontend/src/App.tsx` (around the `<Catalogue …/>` element)
|
||||||
|
- Test: `frontend/tests/e2e/error-boundary.spec.ts`
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Consumes: `ErrorBoundary`, `ErrorFallback`, `ErrorContext` from Task 2; `POST /api/client-errors` from Task 1.
|
||||||
|
- Produces: nothing later tasks build on.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the failing end-to-end spec**
|
||||||
|
|
||||||
|
Create `frontend/tests/e2e/error-boundary.spec.ts`:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
import { test, expect } from './fixtures';
|
||||||
|
|
||||||
|
// The ?boom= trigger only exists on the dev server, which is what Playwright
|
||||||
|
// runs against. A production build drops it entirely — verified separately by
|
||||||
|
// grepping dist for the marker.
|
||||||
|
test.describe('Error boundaries', () => {
|
||||||
|
test('a throw below the root shows a message rather than a blank page', async ({ page }) => {
|
||||||
|
await page.goto('/?boom=page');
|
||||||
|
|
||||||
|
await expect(page.getByRole('heading', { name: 'Something went wrong' })).toBeVisible();
|
||||||
|
await expect(page.getByRole('button', { name: 'Reload' })).toBeVisible();
|
||||||
|
await expect(page.getByRole('button', { name: 'Back to the shop' })).toBeVisible();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a throw in the item grid leaves the header and theme switch usable', async ({ page }) => {
|
||||||
|
await page.goto('/?boom=catalogue');
|
||||||
|
|
||||||
|
await expect(page.getByRole('heading', { name: "The item list didn't load" })).toBeVisible();
|
||||||
|
|
||||||
|
// The claim this boundary exists to make: a bad item no longer takes
|
||||||
|
// navigation down with it.
|
||||||
|
await expect(page.getByRole('heading', { name: 'Redefined Designs' })).toBeVisible();
|
||||||
|
await expect(page.getByRole('switch')).toBeVisible();
|
||||||
|
|
||||||
|
// And the root boundary did not also fire — only the nearest one should.
|
||||||
|
await expect(page.getByRole('heading', { name: 'Something went wrong' })).toHaveCount(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a throw in the modal block leaves the storefront behind it intact', async ({ page }) => {
|
||||||
|
await page.goto('/?boom=modal');
|
||||||
|
|
||||||
|
await expect(page.getByRole('heading', { name: "Couldn't open that" })).toBeVisible();
|
||||||
|
await expect(page.getByRole('heading', { name: 'Redefined Designs' })).toBeVisible();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('a caught error is reported to the server', async ({ page }) => {
|
||||||
|
const reports: string[] = [];
|
||||||
|
page.on('request', (request) => {
|
||||||
|
if (request.url().includes('/api/client-errors')) {
|
||||||
|
reports.push(request.postData() ?? '');
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
await page.goto('/?boom=catalogue');
|
||||||
|
await expect(page.getByRole('heading', { name: "The item list didn't load" })).toBeVisible();
|
||||||
|
|
||||||
|
// Observed on the wire rather than trusting that the reporter was called.
|
||||||
|
//
|
||||||
|
// Greater-than-zero, not exactly one: StrictMode double-invokes render in
|
||||||
|
// development, so the dev server this runs against may report twice where
|
||||||
|
// production reports once. Asserting an exact count would make the test
|
||||||
|
// fail for a reason that has nothing to do with the boundary.
|
||||||
|
await expect.poll(() => reports.length).toBeGreaterThan(0);
|
||||||
|
expect(reports[0]).toContain('"context":"catalogue"');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run it and confirm it fails**
|
||||||
|
|
||||||
|
The backend and a test database must be running. If they are not:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
export PATH="/c/Users/tlamb/AppData/Local/nvm/v24.13.1:$PATH"
|
||||||
|
cd backend && PGHOST=localhost PGPORT=55435 PGUSER=redefined_test PGPASSWORD=redefined_test \
|
||||||
|
PGDATABASE=redefined_test PORT=3000 DEMO_MODE=true UPLOADS_DIR=/tmp/redefined-uploads \
|
||||||
|
node migrate.js up && node dist/server.js &
|
||||||
|
```
|
||||||
|
|
||||||
|
Then:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd frontend && TEST_PGPORT=55435 npx playwright test tests/e2e/error-boundary.spec.ts
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: 4 failed. The `?boom=` parameter does nothing yet, so the storefront renders normally and no fallback heading is found.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Write the development-only trigger**
|
||||||
|
|
||||||
|
Create `frontend/src/components/DevThrow.tsx`:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
import { ErrorContext } from '../errorReporting';
|
||||||
|
|
||||||
|
// A deliberate throw, reachable only from the dev server. Every mount site
|
||||||
|
// guards it with `import.meta.env.DEV &&`, which Vite replaces with `false` in
|
||||||
|
// a production build so Rollup drops both the element and this module.
|
||||||
|
//
|
||||||
|
// The marker is in the thrown message so a production bundle can be grepped for
|
||||||
|
// it. A gate that is silently always-off looks identical to one that works, so
|
||||||
|
// this is checked rather than trusted — the same reasoning as #61's coverage
|
||||||
|
// instrumentation gate.
|
||||||
|
export const DEV_THROW_MARKER = '__DEV_THROW_BOUNDARY__';
|
||||||
|
|
||||||
|
export default function DevThrow({ scope }: Readonly<{ scope: ErrorContext }>) {
|
||||||
|
const requested = new URLSearchParams(window.location.search).get('boom');
|
||||||
|
if (requested === scope) {
|
||||||
|
throw new Error(`${DEV_THROW_MARKER} deliberate throw in ${scope}`);
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Mount the root and modal boundaries in `main.tsx`**
|
||||||
|
|
||||||
|
Add to the imports at the top of `frontend/src/main.tsx`:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
import Button from 'antd/es/button';
|
||||||
|
import ModalDialog from 'antd/es/modal';
|
||||||
|
import ErrorBoundary from './components/ErrorBoundary';
|
||||||
|
import ErrorFallback from './components/ErrorFallback';
|
||||||
|
import DevThrow from './components/DevThrow';
|
||||||
|
```
|
||||||
|
|
||||||
|
`ModalDialog` rather than `Modal`, because `main.tsx` is one of the nine files still importing from the `antd` barrel and a bare `Modal` risks colliding with a future barrel import there. Renaming at the import keeps this change from depending on #65 landing first.
|
||||||
|
|
||||||
|
In `Root`, replace:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
<BrowserRouter>
|
||||||
|
<AppRoutes />
|
||||||
|
</BrowserRouter>
|
||||||
|
```
|
||||||
|
|
||||||
|
with:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
<BrowserRouter>
|
||||||
|
<ErrorBoundary
|
||||||
|
context="page"
|
||||||
|
fallback={(error) => (
|
||||||
|
<ErrorFallback
|
||||||
|
error={error}
|
||||||
|
title="Something went wrong"
|
||||||
|
fullPage
|
||||||
|
actions={[
|
||||||
|
<Button key="reload" type="primary" onClick={() => window.location.reload()}>
|
||||||
|
Reload
|
||||||
|
</Button>,
|
||||||
|
<Button
|
||||||
|
key="home"
|
||||||
|
onClick={() => {
|
||||||
|
window.location.href = '/';
|
||||||
|
}}
|
||||||
|
>
|
||||||
|
Back to the shop
|
||||||
|
</Button>
|
||||||
|
]}
|
||||||
|
/>
|
||||||
|
)}
|
||||||
|
>
|
||||||
|
<AppRoutes />
|
||||||
|
</ErrorBoundary>
|
||||||
|
</BrowserRouter>
|
||||||
|
```
|
||||||
|
|
||||||
|
Then in `AppRoutes`, wrap the modal block. Replace the section that currently begins with the `{/* Rendered outside the Routes above… */}` comment and ends with the closing of the `/reset-password` branch, so that the whole run of `{modalPath === … && …}` expressions sits inside a boundary:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
{/* Rendered outside the Routes above, which are showing the backdrop. */}
|
||||||
|
<ErrorBoundary
|
||||||
|
context="modal"
|
||||||
|
fallback={(error) => (
|
||||||
|
<ModalDialog
|
||||||
|
open
|
||||||
|
title="Couldn't open that"
|
||||||
|
footer={null}
|
||||||
|
onCancel={() => {
|
||||||
|
window.location.href = '/';
|
||||||
|
}}
|
||||||
|
>
|
||||||
|
<ErrorFallback
|
||||||
|
error={error}
|
||||||
|
title="Couldn't open that"
|
||||||
|
actions={
|
||||||
|
<Button
|
||||||
|
type="primary"
|
||||||
|
onClick={() => {
|
||||||
|
window.location.href = '/';
|
||||||
|
}}
|
||||||
|
>
|
||||||
|
Close
|
||||||
|
</Button>
|
||||||
|
}
|
||||||
|
/>
|
||||||
|
</ModalDialog>
|
||||||
|
)}
|
||||||
|
>
|
||||||
|
{/* Unconditional, so /?boom=modal fires this boundary with the
|
||||||
|
storefront rendered behind it — no session needed. */}
|
||||||
|
{import.meta.env.DEV && <DevThrow scope="modal" />}
|
||||||
|
{modalPath === '/account' && <Account onClose={closeModal} />}
|
||||||
|
{modalPath === '/login' && (
|
||||||
|
<AuthRouteModal mode="login" onClose={closeModal} onNavigate={goWithinAuth} />
|
||||||
|
)}
|
||||||
|
{modalPath === '/register' && (
|
||||||
|
<AuthRouteModal mode="register" onClose={closeModal} onNavigate={goWithinAuth} />
|
||||||
|
)}
|
||||||
|
{modalPath === '/forgot-password' && (
|
||||||
|
<ForgotPassword onClose={closeModal} onBackToSignIn={() => goWithinAuth('/login')} />
|
||||||
|
)}
|
||||||
|
{modalPath === '/reset-password' && (
|
||||||
|
<ResetPassword
|
||||||
|
onClose={closeModal}
|
||||||
|
onRequestNewLink={() => goWithinAuth('/forgot-password')}
|
||||||
|
onBackToSignIn={() => goWithinAuth('/login')}
|
||||||
|
/>
|
||||||
|
)}
|
||||||
|
</ErrorBoundary>
|
||||||
|
```
|
||||||
|
|
||||||
|
The `ModalDialog` carries the title and the `ErrorFallback` repeats it. That is intentional: the antd `Modal` header is what a customer reads, and the `Result` title is what the test locates and what keeps the three fallbacks consistent.
|
||||||
|
|
||||||
|
Finally, add the page-scope trigger just inside the `<>` that `AppRoutes` returns, immediately before `<Routes location={backdrop}>`:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
{import.meta.env.DEV && <DevThrow scope="page" />}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 5: Mount the catalogue boundary in `App.tsx`**
|
||||||
|
|
||||||
|
Add to the imports at the top of `frontend/src/App.tsx`:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
import ErrorBoundary from './components/ErrorBoundary';
|
||||||
|
import ErrorFallback from './components/ErrorFallback';
|
||||||
|
import DevThrow from './components/DevThrow';
|
||||||
|
```
|
||||||
|
|
||||||
|
Replace the `<Catalogue … />` element with:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
<ErrorBoundary
|
||||||
|
context="catalogue"
|
||||||
|
fallback={(error) => (
|
||||||
|
<ErrorFallback
|
||||||
|
error={error}
|
||||||
|
title="The item list didn't load"
|
||||||
|
actions={
|
||||||
|
<Button type="primary" onClick={() => window.location.reload()}>
|
||||||
|
Reload
|
||||||
|
</Button>
|
||||||
|
}
|
||||||
|
/>
|
||||||
|
)}
|
||||||
|
>
|
||||||
|
{import.meta.env.DEV && <DevThrow scope="catalogue" />}
|
||||||
|
<Catalogue
|
||||||
|
failed={failed}
|
||||||
|
loading={loading}
|
||||||
|
items={items}
|
||||||
|
filters={filters}
|
||||||
|
needsFavoritesAuth={needsFavoritesAuth}
|
||||||
|
onRetry={handleRetry}
|
||||||
|
onSignIn={openAuthModal}
|
||||||
|
onClearFilters={clearFilters}
|
||||||
|
onChanged={reload}
|
||||||
|
/>
|
||||||
|
</ErrorBoundary>
|
||||||
|
```
|
||||||
|
|
||||||
|
`Button` is already imported in `App.tsx` from the `antd` barrel, so no new import is needed for it.
|
||||||
|
|
||||||
|
- [ ] **Step 6: Run the end-to-end spec and confirm it passes**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd frontend && TEST_PGPORT=55435 npx playwright test tests/e2e/error-boundary.spec.ts
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: 4 passed.
|
||||||
|
|
||||||
|
If the tests fail with clicks timing out or the page obscured, check whether Vite's error overlay is rendering over the app — the spec flags this as a known risk. The fix is to disable the overlay for the dev server (`server: { hmr: { overlay: false } }` in `vite.config.ts`), not to work around it in the tests.
|
||||||
|
|
||||||
|
- [ ] **Step 7: Run the full frontend suite**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd frontend && npm run build && npm run lint && TEST_PGPORT=55435 npm run test:e2e
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: build clean, lint 0 errors and 31 warnings, **87 e2e passing** (83 existing plus 4 new). If the database has already served an earlier run, recreate it first — the suite is not idempotent against a reused database, which produces failures that look like regressions in whatever you just changed.
|
||||||
|
|
||||||
|
- [ ] **Step 8: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add frontend/src/components/DevThrow.tsx frontend/src/main.tsx frontend/src/App.tsx frontend/tests/e2e/error-boundary.spec.ts
|
||||||
|
git commit -m "feat(frontend): mount error boundaries at the root, the item grid and the modals (#62)"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 4: Prove the trigger cannot reach production, then close out
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `docs/superpowers/specs/2026-08-20-error-boundary-design.md` (Status line)
|
||||||
|
|
||||||
|
**Interfaces:**
|
||||||
|
- Consumes: everything from Tasks 1–3.
|
||||||
|
- Produces: nothing.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Confirm the development gate is on in development**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd frontend && npm run dev &
|
||||||
|
curl -s "http://localhost:5173/src/components/DevThrow.tsx" | grep -c "__DEV_THROW_BOUNDARY__"
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `1` or more. The module is served by the dev server. Stop the dev server afterwards.
|
||||||
|
|
||||||
|
This direction matters as much as the other: a gate that is always off looks identical to a gate that works, and only checking the production side would not tell them apart.
|
||||||
|
|
||||||
|
- [ ] **Step 2: Confirm it is absent from a production build**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd frontend && npm run build
|
||||||
|
grep -c "__DEV_THROW_BOUNDARY__" dist/assets/*.js
|
||||||
|
grep -c "boom" dist/assets/*.js
|
||||||
|
```
|
||||||
|
|
||||||
|
Expected: `0` for the marker. The `boom` count is informational — a coincidental match in a minified bundle is possible, so the marker is the assertion and `boom` is a sanity check.
|
||||||
|
|
||||||
|
If the marker is present, `import.meta.env.DEV` is not being treated as statically false. Do not ship it; fix the gate.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Run everything, both workspaces**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
export PATH="/c/Users/tlamb/AppData/Local/nvm/v24.13.1:$PATH"
|
||||||
|
cd backend && npm run build && npm run lint && npm run test:unit
|
||||||
|
cd backend && TEST_PGPORT=55435 npm run test:integration
|
||||||
|
cd frontend && npm run build && npm run lint
|
||||||
|
cd frontend && TEST_PGPORT=55435 npm run test:e2e
|
||||||
|
```
|
||||||
|
|
||||||
|
Record the actual numbers rather than asserting success. Expected: backend build and lint clean, 78 unit, 138 integration; frontend build clean, lint 0 errors / 31 warnings, 87 e2e.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Tear down the test environment**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
docker rm -f rd-eb-db
|
||||||
|
```
|
||||||
|
|
||||||
|
and stop the backend `node dist/server.js` process.
|
||||||
|
|
||||||
|
- [ ] **Step 5: Mark the spec implemented and commit**
|
||||||
|
|
||||||
|
Change the spec's `**Status:** Approved` to `**Status:** Implemented`. If anything in the design turned out to be wrong during implementation, correct the spec in place — it is the durable record, and a spec that documents a decision nobody followed is worse than none.
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add docs/superpowers/specs/2026-08-20-error-boundary-design.md
|
||||||
|
git commit -m "docs: mark the error-boundary design implemented (#62)"
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 6: Report on the issue**
|
||||||
|
|
||||||
|
Comment on #62 with what landed, the verification numbers actually observed, and anything the implementation contradicted in the design. Do not push — the repository owner pushes.
|
||||||
Reference in New Issue
Block a user