feat(frontend): mount error boundaries at the root, the item grid and the modals (#62)
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>
This commit is contained in:
@@ -18,6 +18,9 @@ import AuthPromptModal from './customer/AuthPromptModal';
|
||||
import { useThemeMode } from './theme/ThemeContext';
|
||||
import { useCustomerAuth } from './customer/CustomerAuthContext';
|
||||
import { useCart } from './cart/CartContext';
|
||||
import ErrorBoundary from './components/ErrorBoundary';
|
||||
import ErrorFallback from './components/ErrorFallback';
|
||||
import DevThrow from './components/DevThrow';
|
||||
|
||||
const { Header, Content, Footer } = Layout;
|
||||
const { Title } = Typography;
|
||||
@@ -251,6 +254,21 @@ export default function App() {
|
||||
</div>
|
||||
|
||||
{loading && !items.length && !failed ? <Spin /> : null}
|
||||
<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}
|
||||
@@ -262,6 +280,7 @@ export default function App() {
|
||||
onClearFilters={clearFilters}
|
||||
onChanged={reload}
|
||||
/>
|
||||
</ErrorBoundary>
|
||||
</Content>
|
||||
<Footer style={{ textAlign: 'center', background: token.colorBgContainer }}>
|
||||
<Link to="/privacy">Privacy Policy</Link>
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
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;
|
||||
}
|
||||
@@ -2,7 +2,7 @@ import React from 'react';
|
||||
import Result from 'antd/es/result';
|
||||
import Typography from 'antd/es/typography';
|
||||
|
||||
const { Paragraph, Text } = Typography;
|
||||
const { Title, Paragraph, Text } = Typography;
|
||||
|
||||
type ErrorFallbackProps = Readonly<{
|
||||
error: Error;
|
||||
@@ -19,7 +19,16 @@ export default function ErrorFallback({ error, title, actions, fullPage = false
|
||||
return (
|
||||
<Result
|
||||
status="error"
|
||||
title={title}
|
||||
// antd's Result renders `title` as a plain div, with no heading
|
||||
// semantics — a screen-reader user navigating by headings would find
|
||||
// nothing on a page whose entire content is this error. Wrapped in
|
||||
// Title so the fallback has a real heading; do not simplify this back
|
||||
// to a bare string.
|
||||
title={
|
||||
<Title level={3} style={{ marginBottom: 0 }}>
|
||||
{title}
|
||||
</Title>
|
||||
}
|
||||
subTitle="This has been reported. Nothing you did caused it."
|
||||
style={{ paddingBlock: fullPage ? 64 : 24 }}
|
||||
extra={actions}
|
||||
|
||||
@@ -4,7 +4,12 @@ import { BrowserRouter, Routes, Route, useLocation, useNavigate } from 'react-ro
|
||||
import type { Location } from 'react-router-dom';
|
||||
import { ConfigProvider, theme as antdTheme } from 'antd';
|
||||
import 'antd/dist/reset.css';
|
||||
import Button from 'antd/es/button';
|
||||
import ModalDialog from 'antd/es/modal';
|
||||
import App from './App';
|
||||
import ErrorBoundary from './components/ErrorBoundary';
|
||||
import ErrorFallback from './components/ErrorFallback';
|
||||
import DevThrow from './components/DevThrow';
|
||||
import Admin from './admin/Admin';
|
||||
import AuthRouteModal from './customer/AuthRouteModal';
|
||||
import Account from './customer/Account';
|
||||
@@ -88,6 +93,7 @@ function AppRoutes() {
|
||||
|
||||
return (
|
||||
<>
|
||||
{import.meta.env.DEV && <DevThrow scope="page" />}
|
||||
<Routes location={backdrop}>
|
||||
<Route path="/" element={<App />} />
|
||||
<Route path="/admin" element={<Admin />} />
|
||||
@@ -96,6 +102,37 @@ function AppRoutes() {
|
||||
<Route path="/verify-email" element={<VerifyEmail />} />
|
||||
</Routes>
|
||||
{/* 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} />
|
||||
@@ -113,6 +150,7 @@ function AppRoutes() {
|
||||
onBackToSignIn={() => goWithinAuth('/login')}
|
||||
/>
|
||||
)}
|
||||
</ErrorBoundary>
|
||||
</>
|
||||
);
|
||||
}
|
||||
@@ -138,7 +176,31 @@ function Root() {
|
||||
}}
|
||||
>
|
||||
<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>
|
||||
</ConfigProvider>
|
||||
);
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
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"');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user