fix: address the final review of the error boundaries (#62)

Five findings from a whole-branch review, two of which mattered.

The catalogue boundary could not be recovered by the very controls it exists to keep alive. When the item grid threw, the header, filter chips and Clear filters stayed mounted — that was the point — but clicking Clear filters only changed the URL. A boundary does not reset on a client-side state change, so the fallback went on rendering over a catalogue that would by then have loaded perfectly well, and the only offered action reloaded the same failing URL. The shop read as permanently broken, which is the failure this whole change exists to prevent, reintroduced one level down. ErrorBoundary now takes an optional resetKey and clears itself when it changes; the catalogue boundary passes the filter key. The page and modal boundaries deliberately do not take one, because their escapes are hard navigations that remount the tree already — recorded on the prop so nobody completes the pattern by symmetry.

The client-error endpoint could fill the disk. It is unauthenticated, each accepted report wrote about 8.7 KB, and Docker's default json-file driver has no size cap — so the rate limiter bounded a render loop, as its comment claims, but not a few hundred cheap source addresses. Stack and component stack now truncate at 1000 rather than 4000, which is still around fifteen frames and cuts the worst case to under 3 KB, and the QA compose file caps and rotates the log. Production is a Portainer stack outside this repository and needs the same option applied there; noted in the design doc rather than left implied.

Three smaller things. A falsy thrown value defeated the boundary entirely: throw null is legal, and branching on the error object alone treated it as no error, re-rendered the children, threw again, and would have taken the root down — a blank page, the one outcome this is all here to avoid. The boundary now tracks hasError separately and synthesises a real Error for non-Error throws. The modal fallback announced its title twice to a screen reader, once as the dialog's name and once as the heading inside it, so the redundant dialog title is gone. And the design doc claimed the development-only detail shows the component stack when it only ever showed the message; corrected, with a note that the stack still reaches the server log, which is where it is useful.

Verified after all five: backend lint 0 errors, 144 integration tests, frontend lint 0 errors and 31 warnings, 87 end-to-end tests, all against a freshly created database.

Refs #62
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

frontend/src/App.tsx

@@ -256,6 +256,13 @@ export default function App() {
         {loading && !items.length && !failed ? <Spin /> : null}
         <ErrorBoundary
           context="catalogue"
+          // Lets Clear filters (and any other filter change) recover the grid
+          // without a reload: filterKey changes whenever the filters change,
+          // which resets the boundary the next time it renders. `?boom=catalogue`
+          // is not itself a known filter, so filterKey is unaffected by it and
+          // the existing end-to-end assertion that the fallback renders still
+          // holds.
+          resetKey={filterKey}
           fallback={(error) => (
             <ErrorFallback
               error={error}

frontend/src/components/ErrorBoundary.tsx

@@ -5,12 +5,43 @@ type ErrorBoundaryProps = Readonly<{
   context: ErrorContext;
   fallback: (error: Error) => React.ReactNode;
   children: React.ReactNode;
+  // Lets a boundary recover without a hard navigation, for the one case where
+  // the fallback's own surrounding controls can make the error go away on
+  // their own — the catalogue boundary's "Clear filters". Pass a value that
+  // changes whenever the underlying condition the boundary is guarding
+  // against has changed; when it does and the boundary is currently showing
+  // an error, the error clears and children render again.
+  //
+  // Do NOT add this to the page or modal boundaries. Their escapes
+  // (`window.location.reload()` / `window.location.href = '/'`) are hard
+  // navigations by design — see the design doc — which already remount the
+  // tree and clear the error. Giving them a resetKey too would just be a
+  // second, redundant reset mechanism for a boundary that does not need one.
+  resetKey?: string;
 }>;
 
 interface ErrorBoundaryState {
+  // Tracked separately from `error` because a thrown value is not guaranteed
+  // to be truthy — `throw null` and `throw ''` are both legal JavaScript, and
+  // React does not stop them. Branching on `error` alone would treat a falsy
+  // caught value as "no error", render the children again, throw again, and
+  // eventually take the whole root down with it — the exact blank page this
+  // boundary exists to prevent.
+  hasError: boolean;
   error: Error | null;
 }
 
+// Not every thrown value is an Error, but the rest of this boundary — and the
+// fallback it hands the error to — needs a real `.message` (and ideally a
+// `.stack`) to report and to render. Synthesize one rather than passing the
+// raw value through.
+function toError(value: unknown): Error {
+  if (value instanceof Error) {
+    return value;
+  }
+  return new Error(`Non-Error value thrown: ${String(value)}`);
+}
+
 // The only class component in the codebase. getDerivedStateFromError and
 // componentDidCatch have no hook equivalent, so a boundary cannot be written as
 // a function component.
@@ -19,25 +50,38 @@ interface ErrorBoundaryState {
 // 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 };
+  state: ErrorBoundaryState = { hasError: false, error: null };
 
-  static getDerivedStateFromError(error: Error): ErrorBoundaryState {
-    return { error };
+  static getDerivedStateFromError(error: unknown): ErrorBoundaryState {
+    return { hasError: true, error: toError(error) };
   }
 
-  componentDidCatch(error: Error, info: React.ErrorInfo): void {
+  componentDidUpdate(prevProps: ErrorBoundaryProps): void {
+    // Only ever fires for the catalogue boundary, which is the only caller
+    // that passes resetKey — see the comment on the prop.
+    if (
+      this.state.hasError &&
+      this.props.resetKey !== undefined &&
+      this.props.resetKey !== prevProps.resetKey
+    ) {
+      this.setState({ hasError: false, error: null });
+    }
+  }
+
+  componentDidCatch(error: unknown, info: React.ErrorInfo): void {
+    const normalized = toError(error);
     reportClientError({
       context: this.props.context,
-      message: error.message,
-      stack: error.stack,
+      message: normalized.message,
+      stack: normalized.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);
+    if (this.state.hasError) {
+      return this.props.fallback(this.state.error ?? new Error('Unknown error'));
     }
     return this.props.children;
   }

frontend/src/main.tsx

@@ -107,7 +107,9 @@ function AppRoutes() {
         fallback={(error) => (
           <ModalDialog
             open
-            title="Couldn't open that"
+            // No `title` here: ErrorFallback renders the same string as an
+            // <h3>, and antd would otherwise announce the dialog's accessible
+            // name and then the identical heading right after it.
             footer={null}
             onCancel={() => {
               window.location.href = '/';