fix(frontend): contain viewer/widget crashes with error boundaries
A render error in one viewer or gate widget unmounted the whole React root (white screen). Add a reusable ErrorBoundary (class component, no new dep) with resetKeys + an on-brand fallback, and wire it at two surfaces: - WidgetHost: isolates the gate widget (reset on descriptor id) so a malformed gate payload no longer blanks the conversation - SessionDocumentsPanel: wraps each document (reset on doc key/content) so one crashing viewer degrades only its section; siblings and the panel survive The observed crash: a schema-linking doc that parses but lacks `candidates` makes SchemaLinkingViewer throw. Verified live via Playwright against the mock backend. TDD throughout; tsc clean, 123/123 vitest (+8 new tests). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,101 @@
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import userEvent from "@testing-library/user-event";
|
||||
import { useState } from "react";
|
||||
import { ErrorBoundary } from "./ErrorBoundary";
|
||||
|
||||
/** A child that throws on demand. Toggle `boom` to control whether it crashes. */
|
||||
function Boom({ boom, label = "kaboom" }: { boom: boolean; label?: string }) {
|
||||
if (boom) throw new Error(label);
|
||||
return <div>child ok</div>;
|
||||
}
|
||||
|
||||
// React logs caught render errors to console.error; silence it so the suite output
|
||||
// stays readable while still exercising the real error path.
|
||||
let errorSpy: ReturnType<typeof vi.spyOn>;
|
||||
beforeEach(() => {
|
||||
errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
});
|
||||
afterEach(() => {
|
||||
errorSpy.mockRestore();
|
||||
});
|
||||
|
||||
test("renders children when nothing throws", () => {
|
||||
render(
|
||||
<ErrorBoundary>
|
||||
<Boom boom={false} />
|
||||
</ErrorBoundary>,
|
||||
);
|
||||
expect(screen.getByText("child ok")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test("renders the default fallback when a child throws", () => {
|
||||
render(
|
||||
<ErrorBoundary label="step">
|
||||
<Boom boom />
|
||||
</ErrorBoundary>,
|
||||
);
|
||||
expect(screen.queryByText("child ok")).not.toBeInTheDocument();
|
||||
// Default fallback names the labelled surface.
|
||||
expect(screen.getByText(/couldn't be displayed/i)).toBeInTheDocument();
|
||||
expect(screen.getByText(/step/i)).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test("uses a custom fallback render prop with the error", () => {
|
||||
render(
|
||||
<ErrorBoundary fallback={(err) => <p>custom: {err.message}</p>}>
|
||||
<Boom boom label="explode" />
|
||||
</ErrorBoundary>,
|
||||
);
|
||||
expect(screen.getByText("custom: explode")).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test("calls onError when a child throws", () => {
|
||||
const onError = vi.fn();
|
||||
render(
|
||||
<ErrorBoundary onError={onError}>
|
||||
<Boom boom label="reported" />
|
||||
</ErrorBoundary>,
|
||||
);
|
||||
expect(onError).toHaveBeenCalledTimes(1);
|
||||
expect(onError.mock.calls[0][0]).toBeInstanceOf(Error);
|
||||
expect((onError.mock.calls[0][0] as Error).message).toBe("reported");
|
||||
});
|
||||
|
||||
test("recovers when resetKeys change to non-crashing content", async () => {
|
||||
function Harness() {
|
||||
const [boom, setBoom] = useState(true);
|
||||
return (
|
||||
<>
|
||||
<button onClick={() => setBoom(false)}>fix</button>
|
||||
<ErrorBoundary resetKeys={[boom]}>
|
||||
<Boom boom={boom} />
|
||||
</ErrorBoundary>
|
||||
</>
|
||||
);
|
||||
}
|
||||
render(<Harness />);
|
||||
// Starts crashed → fallback visible.
|
||||
expect(screen.getByText(/couldn't be displayed/i)).toBeInTheDocument();
|
||||
// Flip the data (and thus resetKeys); the boundary clears and re-renders children.
|
||||
await userEvent.click(screen.getByRole("button", { name: "fix" }));
|
||||
expect(screen.getByText("child ok")).toBeInTheDocument();
|
||||
expect(screen.queryByText(/couldn't be displayed/i)).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
test("Try again resets the boundary", async () => {
|
||||
let boom = true;
|
||||
function Flaky() {
|
||||
if (boom) throw new Error("transient");
|
||||
return <div>recovered</div>;
|
||||
}
|
||||
render(
|
||||
<ErrorBoundary>
|
||||
<Flaky />
|
||||
</ErrorBoundary>,
|
||||
);
|
||||
expect(screen.getByText(/couldn't be displayed/i)).toBeInTheDocument();
|
||||
// Simulate the underlying condition clearing, then retry.
|
||||
boom = false;
|
||||
await userEvent.click(screen.getByRole("button", { name: /try again/i }));
|
||||
expect(screen.getByText("recovered")).toBeInTheDocument();
|
||||
});
|
||||
@@ -0,0 +1,101 @@
|
||||
import { Component, type ErrorInfo, type ReactNode } from "react";
|
||||
|
||||
interface Props {
|
||||
children: ReactNode;
|
||||
/**
|
||||
* When any value in this array changes, a boundary that is currently showing an
|
||||
* error clears itself. Pass the identity of the thing being rendered (a session
|
||||
* id, widget id, document key) so navigating away from bad data recovers instead
|
||||
* of staying stuck on the fallback.
|
||||
*/
|
||||
resetKeys?: unknown[];
|
||||
/** Render a custom fallback. Receives the caught error and a reset callback. */
|
||||
fallback?: (error: Error, reset: () => void) => ReactNode;
|
||||
/** Names the wrapped surface in the default fallback ("step", "\"Final SQL\""). */
|
||||
label?: string;
|
||||
/** Side effect on catch, e.g. reporting. */
|
||||
onError?: (error: Error, info: ErrorInfo) => void;
|
||||
}
|
||||
|
||||
interface State {
|
||||
error: Error | null;
|
||||
}
|
||||
|
||||
function changed(a: unknown[] = [], b: unknown[] = []): boolean {
|
||||
return a.length !== b.length || a.some((v, i) => !Object.is(v, b[i]));
|
||||
}
|
||||
|
||||
/**
|
||||
* Catches render errors in its subtree and shows a fallback instead of letting the
|
||||
* crash unmount the whole React root (a white screen). One bad viewer or widget
|
||||
* should degrade only its own region — the rail, composer, and other documents keep
|
||||
* working.
|
||||
*/
|
||||
export class ErrorBoundary extends Component<Props, State> {
|
||||
state: State = { error: null };
|
||||
|
||||
static getDerivedStateFromError(error: Error): State {
|
||||
return { error };
|
||||
}
|
||||
|
||||
componentDidCatch(error: Error, info: ErrorInfo) {
|
||||
this.props.onError?.(error, info);
|
||||
}
|
||||
|
||||
componentDidUpdate(prev: Props) {
|
||||
if (this.state.error && changed(prev.resetKeys, this.props.resetKeys)) {
|
||||
this.reset();
|
||||
}
|
||||
}
|
||||
|
||||
reset = () => this.setState({ error: null });
|
||||
|
||||
render() {
|
||||
const { error } = this.state;
|
||||
if (!error) return this.props.children;
|
||||
if (this.props.fallback) return this.props.fallback(error, this.reset);
|
||||
return <DefaultFallback error={error} label={this.props.label} onRetry={this.reset} />;
|
||||
}
|
||||
}
|
||||
|
||||
function DefaultFallback({
|
||||
error,
|
||||
label,
|
||||
onRetry,
|
||||
}: {
|
||||
error: Error;
|
||||
label?: string;
|
||||
onRetry: () => void;
|
||||
}) {
|
||||
const subject = label ? `${label} ` : "";
|
||||
return (
|
||||
<div
|
||||
role="alert"
|
||||
className="rounded-xl border border-destructive/25 bg-destructive/5 px-4 py-3 text-sm"
|
||||
>
|
||||
<p className="font-medium text-foreground">This {subject}couldn't be displayed.</p>
|
||||
<p className="mt-1 leading-relaxed text-muted-foreground">
|
||||
Something went wrong while rendering it. The rest of the session is unaffected.
|
||||
</p>
|
||||
<div className="mt-3 flex items-center gap-3">
|
||||
<button
|
||||
type="button"
|
||||
onClick={onRetry}
|
||||
className="rounded-md border border-border/70 bg-card px-2.5 py-1 text-xs font-semibold shadow-xs transition-colors hover:bg-muted"
|
||||
>
|
||||
Try again
|
||||
</button>
|
||||
{error.message && (
|
||||
<details className="min-w-0">
|
||||
<summary className="cursor-pointer text-xs text-muted-foreground hover:text-foreground">
|
||||
Details
|
||||
</summary>
|
||||
<pre className="mt-1.5 max-w-full overflow-x-auto rounded-md bg-muted px-2.5 py-1.5 font-mono text-[0.7rem] text-muted-foreground">
|
||||
{error.message}
|
||||
</pre>
|
||||
</details>
|
||||
)}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -50,3 +50,26 @@ test("hides Resume for an archived session", async () => {
|
||||
await screen.findByText("Original question");
|
||||
expect(screen.queryByRole("button", { name: /resume/i })).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
test("a malformed document renders a fallback without taking down its siblings", async () => {
|
||||
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
// Valid JSON but missing `candidates` — SchemaLinkingViewer crashes on this shape.
|
||||
// Without a per-document boundary this crash blanks the whole panel (React 18
|
||||
// unmounts the tree); the good sibling document must still render.
|
||||
// resetHandlers(...) replaces the beforeEach handler so this response is used.
|
||||
server.resetHandlers(
|
||||
http.get("http://localhost:8787/sessions/s1/documents", () =>
|
||||
HttpResponse.json([
|
||||
{ phase: "F4", key: "schema", title: "Schema linking", format: "schema-linking", content: '{"joins":[]}' },
|
||||
{ phase: "—", key: "question", title: "Original question", format: "text", content: "SIBLING-SURVIVES" },
|
||||
]),
|
||||
),
|
||||
);
|
||||
wrap(<SessionDocumentsPanel session={base} onClose={vi.fn()} onResume={vi.fn()} />);
|
||||
// The healthy sibling rendered — the panel did not white-screen.
|
||||
expect(await screen.findByText("SIBLING-SURVIVES")).toBeInTheDocument();
|
||||
// The broken document degraded to a fallback in place.
|
||||
expect(screen.getByRole("alert")).toBeInTheDocument();
|
||||
expect(screen.getByText(/couldn't be displayed/i)).toBeInTheDocument();
|
||||
errorSpy.mockRestore();
|
||||
});
|
||||
|
||||
@@ -3,6 +3,7 @@ import { X } from "lucide-react";
|
||||
import { getSessionDocuments } from "../api/sessions";
|
||||
import type { SessionDocument, SessionSummary } from "../api/types";
|
||||
import { Button } from "../components/ui/button";
|
||||
import { ErrorBoundary } from "../components/ErrorBoundary";
|
||||
import { SqlViewer } from "../viewers/SqlViewer";
|
||||
import { SchemaLinkingViewer } from "../viewers/SchemaLinkingViewer";
|
||||
import { MarkdownView } from "../viewers/MarkdownView";
|
||||
@@ -110,7 +111,12 @@ export function SessionDocumentsPanel({ session, onClose, onResume }: Props) {
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
<DocBody doc={doc} />
|
||||
{/* Isolate each document: a viewer that crashes on malformed
|
||||
content degrades to a fallback in place, leaving the rest of
|
||||
the panel intact instead of blanking the whole tree. */}
|
||||
<ErrorBoundary resetKeys={[doc.key, doc.content]} label="document">
|
||||
<DocBody doc={doc} />
|
||||
</ErrorBoundary>
|
||||
</section>
|
||||
))}
|
||||
</div>
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
import { render, screen } from "@testing-library/react";
|
||||
import { WidgetHost } from "./WidgetHost";
|
||||
import { useSessionStore } from "../store/sessionStore";
|
||||
import type { WidgetDescriptor } from "../api/types";
|
||||
|
||||
// Replace the widget registry with a renderer that always throws, simulating a
|
||||
// viewer/widget that crashes on malformed gate data (the real white-screen path).
|
||||
vi.mock("../widgets", () => ({
|
||||
resolve: () => function Exploding() {
|
||||
throw new Error("bad gate payload");
|
||||
},
|
||||
}));
|
||||
|
||||
let errorSpy: ReturnType<typeof vi.spyOn>;
|
||||
beforeEach(() => {
|
||||
errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
|
||||
useSessionStore.getState().resetSession();
|
||||
});
|
||||
afterEach(() => {
|
||||
errorSpy.mockRestore();
|
||||
useSessionStore.getState().resetSession();
|
||||
});
|
||||
|
||||
const gate: WidgetDescriptor = { id: "g1", widget: "select", title: "Choose" };
|
||||
|
||||
test("a crashing widget shows a fallback instead of unmounting the app", () => {
|
||||
useSessionStore.setState({ pendingWidget: gate });
|
||||
render(<WidgetHost sessionId="s1" />);
|
||||
// No throw escaped: the fallback rendered.
|
||||
expect(screen.getByRole("alert")).toBeInTheDocument();
|
||||
expect(screen.getByText(/couldn't be displayed/i)).toBeInTheDocument();
|
||||
// The store (and thus the surrounding app) is still alive and interactive.
|
||||
expect(useSessionStore.getState().pendingWidget).toEqual(gate);
|
||||
});
|
||||
@@ -2,6 +2,7 @@ import { useSessionStore } from "../store/sessionStore";
|
||||
import { resolve } from "../widgets";
|
||||
import { postResponse } from "../api/sessions";
|
||||
import type { UiResponse } from "../api/types";
|
||||
import { ErrorBoundary } from "../components/ErrorBoundary";
|
||||
|
||||
export function WidgetHost({ sessionId }: { sessionId: string | null }) {
|
||||
const pending = useSessionStore((s) => s.pendingWidget);
|
||||
@@ -32,5 +33,11 @@ export function WidgetHost({ sessionId }: { sessionId: string | null }) {
|
||||
});
|
||||
clearPending();
|
||||
};
|
||||
return <Renderer descriptor={pending} onRespond={onRespond} />;
|
||||
// A malformed gate payload must not white-screen the whole app: isolate the
|
||||
// widget render. resetKeys on the descriptor id so the next gate starts clean.
|
||||
return (
|
||||
<ErrorBoundary resetKeys={[pending.id]} label="step">
|
||||
<Renderer descriptor={pending} onRespond={onRespond} />
|
||||
</ErrorBoundary>
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user