fix(ui): catch render throws with an error boundary around routed content
A purchase in a currency with no configured exchange rate makes src/domain/fx.ts throw MissingRateError (correct behavior — never silently convert at a wrong rate). Nothing caught it: 5 of 6 routes call monthView/yearView/summaryView and threw during render, and with no error boundary anywhere in src/, the throw unmounted the entire React tree including the nav bar. Since the catch-all route redirects to the current month on first load, one un-rated foreign-currency purchase white-screened the app immediately on landing, with the only way out (Settings, where the rate is added) unreachable because the nav was gone too. Adds RouteErrorBoundary (a class component, as required for getDerivedStateFromError/componentDidCatch) that renders the thrown message plus a link to Settings and a "Try again" reset, and RouteErrorBoundaryReset, a wrapper that keys the boundary on the current route so it clears on navigation. Wraps only <Routes> in App.tsx, not <nav>, so navigation stays usable when a route throws. Also guards the happy-dom per-file GlobalRegistrator.register() call in registerSw.test.ts and the new ErrorBoundary.test.tsx with isRegistered, since Bun can run both files in one process. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 5
parent
6eb77c8733
commit
da0d1ac35c
+12
-9
@@ -8,6 +8,7 @@ import { PostYearRoute } from "./routes/PostYearRoute.tsx";
|
||||
import { SummaryRoute } from "./routes/SummaryRoute.tsx";
|
||||
import { SettingsRoute } from "./routes/SettingsRoute.tsx";
|
||||
import { UpdatePrompt } from "./components/UpdatePrompt.tsx";
|
||||
import { RouteErrorBoundaryReset } from "./components/ErrorBoundary.tsx";
|
||||
|
||||
export function AppRoutes() {
|
||||
const year = currentMonth.slice(0, 4);
|
||||
@@ -28,15 +29,17 @@ export function AppRoutes() {
|
||||
</NavLink>
|
||||
</nav>
|
||||
<main className="p-6">
|
||||
<Routes>
|
||||
<Route path="/month/:monthId" element={<MonthRoute />} />
|
||||
<Route path="/year/:year" element={<YearRoute />} />
|
||||
<Route path="/post/:postId/month/:monthId" element={<PostMonthRoute />} />
|
||||
<Route path="/post/:postId/year/:year" element={<PostYearRoute />} />
|
||||
<Route path="/summary" element={<SummaryRoute />} />
|
||||
<Route path="/settings" element={<SettingsRoute />} />
|
||||
<Route path="*" element={<Navigate to={`/month/${currentMonth}`} replace />} />
|
||||
</Routes>
|
||||
<RouteErrorBoundaryReset>
|
||||
<Routes>
|
||||
<Route path="/month/:monthId" element={<MonthRoute />} />
|
||||
<Route path="/year/:year" element={<YearRoute />} />
|
||||
<Route path="/post/:postId/month/:monthId" element={<PostMonthRoute />} />
|
||||
<Route path="/post/:postId/year/:year" element={<PostYearRoute />} />
|
||||
<Route path="/summary" element={<SummaryRoute />} />
|
||||
<Route path="/settings" element={<SettingsRoute />} />
|
||||
<Route path="*" element={<Navigate to={`/month/${currentMonth}`} replace />} />
|
||||
</Routes>
|
||||
</RouteErrorBoundaryReset>
|
||||
</main>
|
||||
<UpdatePrompt />
|
||||
</div>
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
import { GlobalRegistrator } from "@happy-dom/global-registrator";
|
||||
// Bun can run multiple test files in one process, and registerSw.test.ts
|
||||
// registers happy-dom globally too — guard so both files can run together.
|
||||
if (!GlobalRegistrator.isRegistered) {
|
||||
GlobalRegistrator.register();
|
||||
}
|
||||
|
||||
// Silences React's "environment not configured to support act(...)" warning;
|
||||
// this file drives React purely through explicit `act()` calls below.
|
||||
(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true;
|
||||
|
||||
import { test, expect, afterEach } from "bun:test";
|
||||
import { act } from "react";
|
||||
import { createRoot, type Root } from "react-dom/client";
|
||||
import { Link, MemoryRouter, useLocation } from "react-router";
|
||||
import { RouteErrorBoundary, RouteErrorBoundaryReset } from "./ErrorBoundary.tsx";
|
||||
|
||||
let container: HTMLDivElement | null = null;
|
||||
let root: Root | null = null;
|
||||
|
||||
function mount(children: React.ReactNode) {
|
||||
container = document.createElement("div");
|
||||
document.body.appendChild(container);
|
||||
root = createRoot(container);
|
||||
act(() => {
|
||||
root!.render(children);
|
||||
});
|
||||
return container;
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
if (root) {
|
||||
act(() => {
|
||||
root!.unmount();
|
||||
});
|
||||
root = null;
|
||||
}
|
||||
if (container) {
|
||||
container.remove();
|
||||
container = null;
|
||||
}
|
||||
});
|
||||
|
||||
function Thrower(): never {
|
||||
throw new Error("No exchange rate configured for EUR");
|
||||
}
|
||||
|
||||
function Fine() {
|
||||
return <div>all good</div>;
|
||||
}
|
||||
|
||||
test("a throwing child renders the fallback including the thrown message", () => {
|
||||
const el = mount(
|
||||
<MemoryRouter>
|
||||
<RouteErrorBoundary>
|
||||
<Thrower />
|
||||
</RouteErrorBoundary>
|
||||
</MemoryRouter>,
|
||||
);
|
||||
|
||||
expect(el.textContent).toContain("This view could not be displayed");
|
||||
expect(el.textContent).toContain("No exchange rate configured for EUR");
|
||||
// The way back to Settings must still be present.
|
||||
const link = el.querySelector("a[href*='/settings']");
|
||||
expect(link).not.toBeNull();
|
||||
// "Try again" control is present to retry without a full reload.
|
||||
expect(el.textContent).toContain("Try again");
|
||||
});
|
||||
|
||||
test("a non-throwing child renders normally", () => {
|
||||
const el = mount(
|
||||
<MemoryRouter>
|
||||
<RouteErrorBoundary>
|
||||
<Fine />
|
||||
</RouteErrorBoundary>
|
||||
</MemoryRouter>,
|
||||
);
|
||||
|
||||
expect(el.textContent).toContain("all good");
|
||||
expect(el.textContent).not.toContain("could not be displayed");
|
||||
});
|
||||
|
||||
function View() {
|
||||
const location = useLocation();
|
||||
return (
|
||||
<RouteErrorBoundaryReset>
|
||||
{location.pathname === "/a" ? <Thrower /> : <Fine />}
|
||||
</RouteErrorBoundaryReset>
|
||||
);
|
||||
}
|
||||
|
||||
test("RouteErrorBoundaryReset resets the error state when the route changes", () => {
|
||||
const el = mount(
|
||||
<MemoryRouter initialEntries={["/a"]}>
|
||||
<Link to="/b">go to settings</Link>
|
||||
<View />
|
||||
</MemoryRouter>,
|
||||
);
|
||||
expect(el.textContent).toContain("No exchange rate configured for EUR");
|
||||
|
||||
// A real navigation (e.g. following the boundary's Settings link) changes
|
||||
// the route; the boundary is keyed on location.pathname, so it should
|
||||
// remount with a clean error state instead of staying stuck.
|
||||
const link = el.querySelector("a");
|
||||
expect(link).not.toBeNull();
|
||||
act(() => {
|
||||
link!.dispatchEvent(new MouseEvent("click", { bubbles: true, cancelable: true }));
|
||||
});
|
||||
|
||||
expect(el.textContent).toContain("all good");
|
||||
expect(el.textContent).not.toContain("could not be displayed");
|
||||
});
|
||||
@@ -0,0 +1,82 @@
|
||||
import { Component } from "react";
|
||||
import type { ErrorInfo, ReactNode } from "react";
|
||||
import { Link, useLocation } from "react-router";
|
||||
import { Button } from "@/components/ui/button";
|
||||
|
||||
interface RouteErrorBoundaryProps {
|
||||
children: ReactNode;
|
||||
}
|
||||
|
||||
interface RouteErrorBoundaryState {
|
||||
error: Error | null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Catches render-time throws from routed views — most notably
|
||||
* `MissingRateError` from `src/domain/fx.ts`, which is thrown deliberately
|
||||
* whenever a purchase's currency has no configured exchange rate. Silently
|
||||
* converting at a fallback rate would show the user wrong numbers, so the
|
||||
* domain throws instead; this boundary is what turns that throw into a
|
||||
* recoverable screen instead of an unmounted app.
|
||||
*
|
||||
* Must be positioned in `App.tsx` so it wraps only the routed content, not
|
||||
* the `<nav>` — the whole point is that navigation (in particular the link
|
||||
* to Settings, where exchange rates are configured) stays usable.
|
||||
*/
|
||||
export class RouteErrorBoundary extends Component<
|
||||
RouteErrorBoundaryProps,
|
||||
RouteErrorBoundaryState
|
||||
> {
|
||||
override state: RouteErrorBoundaryState = { error: null };
|
||||
|
||||
static getDerivedStateFromError(error: Error): RouteErrorBoundaryState {
|
||||
return { error };
|
||||
}
|
||||
|
||||
override componentDidCatch(error: Error, info: ErrorInfo) {
|
||||
// eslint-disable-next-line no-console
|
||||
console.error("Route failed to render:", error, info.componentStack);
|
||||
}
|
||||
|
||||
private reset = () => {
|
||||
this.setState({ error: null });
|
||||
};
|
||||
|
||||
override render() {
|
||||
const { error } = this.state;
|
||||
if (!error) return this.props.children;
|
||||
|
||||
return (
|
||||
<div className="max-w-2xl space-y-4">
|
||||
<h1 className="text-2xl font-semibold">This view could not be displayed</h1>
|
||||
<p className="text-overspend font-money text-sm">{error.message}</p>
|
||||
<p className="text-sm text-muted-foreground">
|
||||
If this mentions a missing exchange rate, add it under{" "}
|
||||
<Link
|
||||
to="/settings"
|
||||
className="text-[var(--budget-accent)] hover:underline"
|
||||
onClick={this.reset}
|
||||
>
|
||||
Settings
|
||||
</Link>{" "}
|
||||
— that is where currencies and rates are configured.
|
||||
</p>
|
||||
<Button variant="outline" onClick={this.reset}>
|
||||
Try again
|
||||
</Button>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Function-component wrapper so the boundary can be keyed on the current
|
||||
* route. Without this, navigating away from the failing route (even to
|
||||
* Settings) would leave the boundary's error state stuck, since class
|
||||
* components don't re-run render logic on prop/location changes by
|
||||
* themselves — remounting via `key` is the simplest correct reset.
|
||||
*/
|
||||
export function RouteErrorBoundaryReset({ children }: { children: ReactNode }) {
|
||||
const location = useLocation();
|
||||
return <RouteErrorBoundary key={location.pathname}>{children}</RouteErrorBoundary>;
|
||||
}
|
||||
@@ -1,5 +1,9 @@
|
||||
import { GlobalRegistrator } from "@happy-dom/global-registrator";
|
||||
GlobalRegistrator.register();
|
||||
// Bun can run multiple test files in one process, and ErrorBoundary.test.tsx
|
||||
// registers happy-dom globally too — guard so both files can run together.
|
||||
if (!GlobalRegistrator.isRegistered) {
|
||||
GlobalRegistrator.register();
|
||||
}
|
||||
|
||||
import { test, expect } from "bun:test";
|
||||
import { registerServiceWorker } from "./registerSw.ts";
|
||||
|
||||
Reference in New Issue
Block a user