Skip to content

Commit f6d37f6

Browse files
ParkreinerParkreiner
authored
fix(site): ensure Error Boundary catches render errors correctly (#15963)
## Changes made - Replaced previous `ErrorBoundary` functionality with `GlobalErrorBoundary` component - Wired up `GlobalErrorBoundary` to React Router rather than the top of the app
1 parent d788223 commit f6d37f6

13 files changed

+334
-318
lines changed

‎site/src/App.tsx

+5-6
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import {
1010
import { HelmetProvider } from "react-helmet-async";
1111
import { QueryClient, QueryClientProvider } from "react-query";
1212
import { RouterProvider } from "react-router-dom";
13-
import { ErrorBoundary } from "./components/ErrorBoundary/ErrorBoundary";
1413
import { GlobalSnackbar } from "./components/GlobalSnackbar/GlobalSnackbar";
1514
import { ThemeProvider } from "./contexts/ThemeProvider";
1615
import { AuthProvider } from "./contexts/auth/AuthProvider";
@@ -81,11 +80,11 @@ export const AppProviders: FC<AppProvidersProps> = ({
8180
export const App: FC = () => {
8281
return (
8382
<StrictMode>
84-
<ErrorBoundary>
85-
<AppProviders>
86-
<RouterProvider router={router} />
87-
</AppProviders>
88-
</ErrorBoundary>
83+
<AppProviders>
84+
{/* If you're wondering where the global error boundary is,
85+
it's connected to the router */}
86+
<RouterProvider router={router} />
87+
</AppProviders>
8988
</StrictMode>
9089
);
9190
};

‎site/src/components/Button/Button.tsx

+12-2
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,19 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
5555
const Comp = asChild ? Slot : "button";
5656
return (
5757
<Comp
58-
className={cn(buttonVariants({ variant, size, className }))}
59-
ref={ref}
6058
{...props}
59+
ref={ref}
60+
className={cn(buttonVariants({ variant, size }), className)}
61+
// Adding default button type to make sure that buttons don't
62+
// accidentally trigger form actions when clicked. But because
63+
// this Button component is so polymorphic (it's also used to
64+
// make <a> elements look like buttons), we can only safely
65+
// default to adding the prop when we know that we're rendering
66+
// a real HTML button instead of an arbitrary Slot. Adding the
67+
// type attribute to any non-buttons will produce invalid HTML
68+
type={
69+
props.type === undefined && Comp === "button" ? "button" : props.type
70+
}
6171
/>
6272
);
6373
},

‎site/src/components/CustomLogo/CustomLogo.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,6 @@ export const CustomLogo: FC<{ css?: Interpolation<Theme> }> = (props) => {
2828
className="application-logo"
2929
/>
3030
) : (
31-
<CoderIcon {...props} css={[{ fontSize: 64, fill: "white" }, props.css]} />
31+
<CoderIcon {...props} className="w-12 h-12" />
3232
);
3333
};

‎site/src/components/ErrorBoundary/ErrorBoundary.tsx

-39
This file was deleted.

‎site/src/components/ErrorBoundary/GlobalErrorBoundary.stories.tsx

+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
import type { Meta, StoryObj } from "@storybook/react";
2+
import { expect, userEvent } from "@storybook/test";
3+
import { within } from "@testing-library/react";
4+
import type { ErrorResponse } from "react-router-dom";
5+
import { GlobalErrorBoundaryInner } from "./GlobalErrorBoundary";
6+
7+
/**
8+
* React Router ErrorResponses have a "hidden" internal field that RR uses to
9+
* detect whether something is a loader error. The property doesn't exist in
10+
* the type information, but it does exist at runtime, and we need it to mock
11+
* out the story correctly
12+
*/
13+
type FullErrorResponse = Readonly<
14+
ErrorResponse & {
15+
internal: true;
16+
}
17+
>;
18+
19+
const meta = {
20+
title: "components/GlobalErrorBoundary",
21+
component: GlobalErrorBoundaryInner,
22+
} satisfies Meta<typeof GlobalErrorBoundaryInner>;
23+
24+
export default meta;
25+
type Story = StoryObj<typeof meta>;
26+
27+
export const VanillaJavascriptError: Story = {
28+
args: {
29+
error: new Error("Something blew up :("),
30+
},
31+
play: async ({ canvasElement, args }) => {
32+
const error = args.error as Error;
33+
const canvas = within(canvasElement);
34+
const showErrorButton = canvas.getByRole("button", {
35+
name: /Show error/i,
36+
});
37+
await userEvent.click(showErrorButton);
38+
39+
// Verify that error message content is now on screen; defer to
40+
// accessible name queries as much as possible
41+
canvas.getByRole("heading", { name: /Error/i });
42+
43+
const p = canvas.getByTestId("description");
44+
expect(p).toHaveTextContent(error.message);
45+
46+
const codeBlock = canvas.getByTestId("code");
47+
expect(codeBlock).toHaveTextContent(error.name);
48+
expect(codeBlock).toHaveTextContent(error.message);
49+
},
50+
};
51+
52+
export const ReactRouterErrorResponse: Story = {
53+
args: {
54+
error: {
55+
internal: true,
56+
status: 500,
57+
statusText: "Aww, beans!",
58+
data: { message: "beans" },
59+
} satisfies FullErrorResponse,
60+
},
61+
play: async ({ canvasElement, args }) => {
62+
const error = args.error as FullErrorResponse;
63+
const canvas = within(canvasElement);
64+
const showErrorButton = canvas.getByRole("button", {
65+
name: /Show error/i,
66+
});
67+
await userEvent.click(showErrorButton);
68+
69+
// Verify that error message content is now on screen; defer to
70+
// accessible name queries as much as possible
71+
const header = canvas.getByRole("heading", { name: /Aww, beans!/i });
72+
expect(header).toHaveTextContent(String(error.status));
73+
74+
const codeBlock = canvas.getByTestId("code");
75+
const content = codeBlock.innerText;
76+
const parsed = JSON.parse(content);
77+
expect(parsed).toEqual(error.data);
78+
},
79+
};
80+
81+
export const UnparsableError: Story = {
82+
args: {
83+
error: class WellThisIsDefinitelyWrong {},
84+
},
85+
play: async ({ canvasElement }) => {
86+
const canvas = within(canvasElement);
87+
const showErrorButton = canvas.queryByRole("button", {
88+
name: /Show error/i,
89+
});
90+
expect(showErrorButton).toBe(null);
91+
},
92+
};

‎site/src/components/ErrorBoundary/GlobalErrorBoundary.tsx

+183
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
/**
2+
* @file A global error boundary designed to work with React Router.
3+
*
4+
* This is not documented well, but because of React Router works, it will
5+
* automatically intercept any render errors produced in routes, and will
6+
* "swallow" them, preventing the errors from bubbling up to any error
7+
* boundaries above the router. The global error boundary must be explicitly
8+
* bound to a route to work as expected.
9+
*/
10+
import type { Interpolation } from "@emotion/react";
11+
import Link from "@mui/material/Link";
12+
import { Button } from "components/Button/Button";
13+
import { CoderIcon } from "components/Icons/CoderIcon";
14+
import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata";
15+
import { type FC, useState } from "react";
16+
import { Helmet } from "react-helmet-async";
17+
import {
18+
type ErrorResponse,
19+
isRouteErrorResponse,
20+
useLocation,
21+
useRouteError,
22+
} from "react-router-dom";
23+
24+
const errorPageTitle = "Something went wrong";
25+
26+
// Mocking React Router's error-handling logic is a pain; the next best thing is
27+
// to split it off from the rest of the code, and pass the value via props
28+
export const GlobalErrorBoundary: FC = () => {
29+
const error = useRouteError();
30+
return <GlobalErrorBoundaryInner error={error} />;
31+
};
32+
33+
type GlobalErrorBoundaryInnerProps = Readonly<{ error: unknown }>;
34+
export const GlobalErrorBoundaryInner: FC<GlobalErrorBoundaryInnerProps> = ({
35+
error,
36+
}) => {
37+
const [showErrorMessage, setShowErrorMessage] = useState(false);
38+
const { metadata } = useEmbeddedMetadata();
39+
const location = useLocation();
40+
41+
const coderVersion = metadata["build-info"].value?.version;
42+
const isRenderableError =
43+
error instanceof Error || isRouteErrorResponse(error);
44+
45+
return (
46+
<div className="bg-surface-primary text-center w-full h-full flex justify-center items-center">
47+
<Helmet>
48+
<title>{errorPageTitle}</title>
49+
</Helmet>
50+
51+
<main className="flex gap-6 w-full max-w-prose p-4 flex-col flex-nowrap">
52+
<div className="flex gap-2 flex-col items-center">
53+
<CoderIcon className="w-11 h-11" />
54+
55+
<div className="text-content-primary flex flex-col gap-1">
56+
<h1 className="text-2xl font-normal m-0">{errorPageTitle}</h1>
57+
<p className="leading-6 m-0">
58+
Please try reloading the page. If reloading does not work, you can
59+
ask for help in the{" "}
60+
<Link
61+
href="https://discord.gg/coder"
62+
target="_blank"
63+
rel="noreferer"
64+
>
65+
Coder Discord community
66+
<span className="sr-only"> (link opens in a new tab)</span>
67+
</Link>{" "}
68+
or{" "}
69+
<Link
70+
target="_blank"
71+
rel="noreferer"
72+
href={publicGithubIssueLink(
73+
coderVersion,
74+
location.pathname,
75+
error,
76+
)}
77+
>
78+
open an issue on GitHub
79+
<span className="sr-only"> (link opens in a new tab)</span>
80+
</Link>
81+
.
82+
</p>
83+
</div>
84+
</div>
85+
86+
<div className="flex flex-row flex-nowrap justify-center gap-4">
87+
<Button asChild className="min-w-32 font-medium">
88+
<Link href={location.pathname}>Reload page</Link>
89+
</Button>
90+
91+
{isRenderableError && (
92+
<Button
93+
variant="outline"
94+
className="min-w-32"
95+
onClick={() => setShowErrorMessage(!showErrorMessage)}
96+
>
97+
{showErrorMessage ? "Hide error" : "Show error"}
98+
</Button>
99+
)}
100+
</div>
101+
102+
{isRenderableError && showErrorMessage && <ErrorStack error={error} />}
103+
</main>
104+
</div>
105+
);
106+
};
107+
108+
type ErrorStackProps = Readonly<{ error: Error | ErrorResponse }>;
109+
const ErrorStack: FC<ErrorStackProps> = ({ error }) => {
110+
return (
111+
<aside className="p-4 text-left rounded-md border-[1px] border-content-tertiary border-solid">
112+
{isRouteErrorResponse(error) ? (
113+
<>
114+
<h2 className="text-base font-bold text-content-primary m-0">
115+
HTTP {error.status} - {error.statusText}
116+
</h2>
117+
<pre className="m-0 py-2 px-0 overflow-x-auto text-xs">
118+
<code data-testid="code">{serializeDataAsJson(error.data)}</code>
119+
</pre>
120+
</>
121+
) : (
122+
<>
123+
<h2 className="text-base font-bold text-content-primary m-0">
124+
{error.name}
125+
</h2>
126+
<p data-testid="description" className="pb-4 leading-5 m-0">
127+
{error.message}
128+
</p>
129+
{error.stack && (
130+
<pre className="m-0 py-2 px-0 overflow-x-auto text-xs">
131+
<code data-testid="code">{error.stack}</code>
132+
</pre>
133+
)}
134+
</>
135+
)}
136+
</aside>
137+
);
138+
};
139+
140+
function serializeDataAsJson(data: unknown): string | null {
141+
try {
142+
return JSON.stringify(data, null, 2);
143+
} catch {
144+
return null;
145+
}
146+
}
147+
148+
function publicGithubIssueLink(
149+
coderVersion: string | undefined,
150+
pathName: string,
151+
error: unknown,
152+
): string {
153+
const baseLink = "https://github.com/coder/coder/issues/new";
154+
155+
// Anytime you see \`\`\`txt, that's wrapping the text in a GitHub codeblock
156+
let printableError: string;
157+
if (error instanceof Error) {
158+
printableError = [
159+
`${error.name}: ${error.message}`,
160+
error.stack ? `\`\`\`txt\n${error.stack}\n\`\`\`` : "No stack",
161+
].join("\n");
162+
} else if (isRouteErrorResponse(error)) {
163+
const serialized = serializeDataAsJson(error.data);
164+
printableError = [
165+
`HTTP ${error.status} - ${error.statusText}`,
166+
serialized ? `\`\`\`txt\n${serialized}\n\`\`\`` : "(No data)",
167+
].join("\n");
168+
} else {
169+
printableError = "No error message available";
170+
}
171+
172+
const messageBody = `\
173+
**Version**
174+
${coderVersion ?? "-- Set version --"}
175+
176+
**Path**
177+
\`${pathName}\`
178+
179+
**Error**
180+
${printableError}`;
181+
182+
return `${baseLink}?body=${encodeURIComponent(messageBody)}`;
183+
}

‎site/src/components/ErrorBoundary/RuntimeErrorState.stories.tsx

-24
This file was deleted.

0 commit comments

Comments
 (0)