coder · Parkreiner · Jan 3, 2025 · Dec 26, 2024 · Dec 26, 2024 · Dec 26, 2024
diff --git a/site/src/App.tsx b/site/src/App.tsx
@@ -10,7 +10,6 @@ import {
 import { HelmetProvider } from "react-helmet-async";
 import { QueryClient, QueryClientProvider } from "react-query";
 import { RouterProvider } from "react-router-dom";
-import { ErrorBoundary } from "./components/ErrorBoundary/ErrorBoundary";
 import { GlobalSnackbar } from "./components/GlobalSnackbar/GlobalSnackbar";
 import { ThemeProvider } from "./contexts/ThemeProvider";
 import { AuthProvider } from "./contexts/auth/AuthProvider";
@@ -81,11 +80,11 @@ export const AppProviders: FC<AppProvidersProps> = ({
 export const App: FC = () => {
 	return (
 		<StrictMode>
-			<ErrorBoundary>
-				<AppProviders>
-					<RouterProvider router={router} />
-				</AppProviders>
-			</ErrorBoundary>
+			<AppProviders>
+				{/* If you're wondering where the global error boundary is,
+				    it's connected to the router */}
+				<RouterProvider router={router} />
+			</AppProviders>
 		</StrictMode>
 	);
 };
diff --git a/site/src/components/Button/Button.tsx b/site/src/components/Button/Button.tsx
@@ -55,9 +55,19 @@ export const Button = forwardRef<HTMLButtonElement, ButtonProps>(
 		const Comp = asChild ? Slot : "button";
 		return (
 			<Comp
-				className={cn(buttonVariants({ variant, size, className }))}
-				ref={ref}
 				{...props}
+				ref={ref}
+				className={cn(buttonVariants({ variant, size }), className)}
+				// Adding default button type to make sure that buttons don't
+				// accidentally trigger form actions when clicked. But because
+				// this Button component is so polymorphic (it's also used to
+				// make <a> elements look like buttons), we can only safely
+				// default to adding the prop when we know that we're rendering
+				// a real HTML button instead of an arbitrary Slot. Adding the
+				// type attribute to any non-buttons will produce invalid HTML
+				type={
+					props.type === undefined && Comp === "button" ? "button" : props.type
+				}
 			/>
 		);
 	},

diff --git a/site/src/components/CustomLogo/CustomLogo.tsx b/site/src/components/CustomLogo/CustomLogo.tsx
@@ -28,6 +28,6 @@ export const CustomLogo: FC<{ css?: Interpolation<Theme> }> = (props) => {
 			className="application-logo"
 		/>
 	) : (
-		<CoderIcon {...props} css={[{ fontSize: 64, fill: "white" }, props.css]} />
+		<CoderIcon {...props} className="w-12 h-12" />
 	);
 };
diff --git a/site/src/components/ErrorBoundary/ErrorBoundary.tsx b/site/src/components/ErrorBoundary/ErrorBoundary.tsx
diff --git a/site/src/components/ErrorBoundary/GlobalErrorBoundary.stories.tsx b/site/src/components/ErrorBoundary/GlobalErrorBoundary.stories.tsx
@@ -0,0 +1,92 @@
+import type { Meta, StoryObj } from "@storybook/react";
+import { expect, userEvent } from "@storybook/test";
+import { within } from "@testing-library/react";
+import type { ErrorResponse } from "react-router-dom";
+import { GlobalErrorBoundaryInner } from "./GlobalErrorBoundary";
+
+/**
+ * React Router ErrorResponses have a "hidden" internal field that RR uses to
+ * detect whether something is a loader error. The property doesn't exist in
+ * the type information, but it does exist at runtime, and we need it to mock
+ * out the story correctly
+ */
+type FullErrorResponse = Readonly<
+	ErrorResponse & {
+		internal: true;
+	}
+>;
+
+const meta = {
+	title: "components/GlobalErrorBoundary",
+	component: GlobalErrorBoundaryInner,
+} satisfies Meta<typeof GlobalErrorBoundaryInner>;
+
+export default meta;
+type Story = StoryObj<typeof meta>;
+
+export const VanillaJavascriptError: Story = {
+	args: {
+		error: new Error("Something blew up :("),
+	},
+	play: async ({ canvasElement, args }) => {
+		const error = args.error as Error;
+		const canvas = within(canvasElement);
+		const showErrorButton = canvas.getByRole("button", {
+			name: /Show error/i,
+		});
+		await userEvent.click(showErrorButton);
+
+		// Verify that error message content is now on screen; defer to
+		// accessible name queries as much as possible
+		canvas.getByRole("heading", { name: /Error/i });
+
+		const p = canvas.getByTestId("description");
+		expect(p).toHaveTextContent(error.message);
+
+		const codeBlock = canvas.getByTestId("code");
+		expect(codeBlock).toHaveTextContent(error.name);
+		expect(codeBlock).toHaveTextContent(error.message);
+	},
+};
+
+export const ReactRouterErrorResponse: Story = {
+	args: {
+		error: {
+			internal: true,
+			status: 500,
+			statusText: "Aww, beans!",
+			data: { message: "beans" },
+		} satisfies FullErrorResponse,
+	},
+	play: async ({ canvasElement, args }) => {
+		const error = args.error as FullErrorResponse;
+		const canvas = within(canvasElement);
+		const showErrorButton = canvas.getByRole("button", {
+			name: /Show error/i,
+		});
+		await userEvent.click(showErrorButton);
+
+		// Verify that error message content is now on screen; defer to
+		// accessible name queries as much as possible
+		const header = canvas.getByRole("heading", { name: /Aww, beans!/i });
+		expect(header).toHaveTextContent(String(error.status));
+
+		const codeBlock = canvas.getByTestId("code");
+		const content = codeBlock.innerText;
+		const parsed = JSON.parse(content);
+		expect(parsed).toEqual(error.data);
+	},
+};
+
+export const UnparsableError: Story = {
+	args: {
+		error: class WellThisIsDefinitelyWrong {},
+	},
+	play: async ({ canvasElement }) => {
+		const canvas = within(canvasElement);
+		const showErrorButton = canvas.queryByRole("button", {
+			name: /Show error/i,
+		});
+		expect(showErrorButton).toBe(null);
+	},
+};
diff --git a/site/src/components/ErrorBoundary/GlobalErrorBoundary.tsx b/site/src/components/ErrorBoundary/GlobalErrorBoundary.tsx
@@ -0,0 +1,183 @@
+/**
+ * @file A global error boundary designed to work with React Router.
+ *
+ * This is not documented well, but because of React Router works, it will
+ * automatically intercept any render errors produced in routes, and will
+ * "swallow" them, preventing the errors from bubbling up to any error
+ * boundaries above the router. The global error boundary must be explicitly
+ * bound to a route to work as expected.
+ */
+import type { Interpolation } from "@emotion/react";
+import Link from "@mui/material/Link";
+import { Button } from "components/Button/Button";
+import { CoderIcon } from "components/Icons/CoderIcon";
+import { useEmbeddedMetadata } from "hooks/useEmbeddedMetadata";
+import { type FC, useState } from "react";
+import { Helmet } from "react-helmet-async";
+import {
+	type ErrorResponse,
+	isRouteErrorResponse,
+	useLocation,
+	useRouteError,
+} from "react-router-dom";
+
+const errorPageTitle = "Something went wrong";
+
+// Mocking React Router's error-handling logic is a pain; the next best thing is
+// to split it off from the rest of the code, and pass the value via props
+export const GlobalErrorBoundary: FC = () => {
+	const error = useRouteError();
+	return <GlobalErrorBoundaryInner error={error} />;
+};
+
+type GlobalErrorBoundaryInnerProps = Readonly<{ error: unknown }>;
+export const GlobalErrorBoundaryInner: FC<GlobalErrorBoundaryInnerProps> = ({
+	error,
+}) => {
+	const [showErrorMessage, setShowErrorMessage] = useState(false);
+	const { metadata } = useEmbeddedMetadata();
+	const location = useLocation();
+
+	const coderVersion = metadata["build-info"].value?.version;
+	const isRenderableError =
+		error instanceof Error || isRouteErrorResponse(error);
+
+	return (
+		<div className="bg-surface-primary text-center w-full h-full flex justify-center items-center">
+			<Helmet>
+				<title>{errorPageTitle}</title>
+			</Helmet>
+
+			<main className="flex gap-6 w-full max-w-prose p-4 flex-col flex-nowrap">
+				<div className="flex gap-2 flex-col items-center">
+					<CoderIcon className="w-11 h-11" />
+
+					<div className="text-content-primary flex flex-col gap-1">
+						<h1 className="text-2xl font-normal m-0">{errorPageTitle}</h1>
+						<p className="leading-6 m-0">
+							Please try reloading the page. If reloading does not work, you can
+							ask for help in the{" "}
+							<Link
+								href="https://discord.gg/coder"
+								target="_blank"
+								rel="noreferer"
+							>
+								Coder Discord community
+								<span className="sr-only"> (link opens in a new tab)</span>
+							</Link>{" "}
+							or{" "}
+							<Link
+								target="_blank"
+								rel="noreferer"
+								href={publicGithubIssueLink(
+									coderVersion,
+									location.pathname,
+									error,
+								)}
+							>
+								open an issue on GitHub
+								<span className="sr-only"> (link opens in a new tab)</span>
+							</Link>
+							.
+						</p>
+					</div>
+				</div>
+
+				<div className="flex flex-row flex-nowrap justify-center gap-4">
+					<Button asChild className="min-w-32 font-medium">
+						<Link href={location.pathname}>Reload page</Link>
+					</Button>
+
+					{isRenderableError && (
+						<Button
+							variant="outline"
+							className="min-w-32"
+							onClick={() => setShowErrorMessage(!showErrorMessage)}
+						>
+							{showErrorMessage ? "Hide error" : "Show error"}
+						</Button>
+					)}
+				</div>
+
+				{isRenderableError && showErrorMessage && <ErrorStack error={error} />}
+			</main>
+		</div>
+	);
+};
+
+type ErrorStackProps = Readonly<{ error: Error | ErrorResponse }>;
+const ErrorStack: FC<ErrorStackProps> = ({ error }) => {
+	return (
+		<aside className="p-4 text-left rounded-md border-[1px] border-content-tertiary border-solid">
+			{isRouteErrorResponse(error) ? (
+				<>
+					<h2 className="text-base font-bold text-content-primary m-0">
+						HTTP {error.status} - {error.statusText}
+					</h2>
+					<pre className="m-0 py-2 px-0 overflow-x-auto text-xs">
+						<code data-testid="code">{serializeDataAsJson(error.data)}</code>
+					</pre>
+				</>
+			) : (
+				<>
+					<h2 className="text-base font-bold text-content-primary m-0">
+						{error.name}
+					</h2>
+					<p data-testid="description" className="pb-4 leading-5 m-0">
+						{error.message}
+					</p>
+					{error.stack && (
+						<pre className="m-0 py-2 px-0 overflow-x-auto text-xs">
+							<code data-testid="code">{error.stack}</code>
+						</pre>
+					)}
+				</>
+			)}
+		</aside>
+	);
+};
+
+function serializeDataAsJson(data: unknown): string | null {
+	try {
+		return JSON.stringify(data, null, 2);
+	} catch {
+		return null;
+	}
+}
+
+function publicGithubIssueLink(
+	coderVersion: string | undefined,
+	pathName: string,
+	error: unknown,
+): string {
+	const baseLink = "https://github.com/coder/coder/issues/new";
+
+	// Anytime you see \`\`\`txt, that's wrapping the text in a GitHub codeblock
+	let printableError: string;
+	if (error instanceof Error) {
+		printableError = [
+			`${error.name}: ${error.message}`,
+			error.stack ? `\`\`\`txt\n${error.stack}\n\`\`\`` : "No stack",
+		].join("\n");
+	} else if (isRouteErrorResponse(error)) {
+		const serialized = serializeDataAsJson(error.data);
+		printableError = [
+			`HTTP ${error.status} - ${error.statusText}`,
+			serialized ? `\`\`\`txt\n${serialized}\n\`\`\`` : "(No data)",
+		].join("\n");
+	} else {
+		printableError = "No error message available";
+	}
+
+	const messageBody = `\
+**Version**
+${coderVersion ?? "-- Set version --"}
+
+**Path**
+\`${pathName}\`
+
+**Error**
+${printableError}`;
+
+	return `${baseLink}?body=${encodeURIComponent(messageBody)}`;
+}
diff --git a/site/src/components/ErrorBoundary/RuntimeErrorState.stories.tsx b/site/src/components/ErrorBoundary/RuntimeErrorState.stories.tsx