feat: create generic error page (#113)

kantord · web-flow · commit 55cde55b90b4 · 2025-12-02T14:34:39.000+01:00
diff --git a/AGENTS.md b/AGENTS.md
@@ -352,7 +352,7 @@ describe("Component", () => {
   it("does something", async () => {
     render(<Component />);
     await waitFor(() => {
-      expect(screen.getByText("Expected")).toBeInTheDocument();
+      expect(screen.getByText("Expected")).toBeVisible();
     });
   });
 });
diff --git a/CLAUDE.md b/CLAUDE.md
@@ -202,6 +202,10 @@ pnpm generate-client:nofetch # Regenerate without fetching
 - Loading states and skeleton screens
 - Accessibility (keyboard navigation, screen readers)
 
+### Testing Best Practices
+
+- **Prefer `toBeVisible()` over `toBeInTheDocument()`** - `toBeVisible()` checks that an element is actually visible to the user (not hidden via CSS, `aria-hidden`, etc.), while `toBeInTheDocument()` only checks DOM presence. Use `toBeVisible()` for positive assertions and `.not.toBeInTheDocument()` for absence checks.
+
 ### Mocking & Testing
 
 - **MSW Auto-Mocker**
@@ -230,13 +234,13 @@ describe("ServerList", () => {
     render(<ServerList />);
 
     await waitFor(() => {
-      expect(screen.getByText("Server 1")).toBeInTheDocument();
+      expect(screen.getByText("Server 1")).toBeVisible();
     });
 
     const copyButton = screen.getByRole("button", { name: /copy url/i });
     await userEvent.click(copyButton);
 
-    expect(screen.getByText(/copied/i)).toBeInTheDocument();
+    expect(screen.getByText(/copied/i)).toBeVisible();
   });
 });
 ```
diff --git a/src/app/catalog/[repoName]/[serverName]/[version]/not-found.test.tsx b/src/app/catalog/[repoName]/[serverName]/[version]/not-found.test.tsx
@@ -8,15 +8,15 @@ describe("NotFound", () => {
 
     expect(
       screen.getByRole("heading", { name: /server not found/i }),
-    ).toBeInTheDocument();
+    ).toBeVisible();
   });
 
   it("displays a descriptive message", () => {
     render(<NotFound />);
 
     expect(
       screen.getByText(/doesn't exist or has been removed/i),
-    ).toBeInTheDocument();
+    ).toBeVisible();
   });
 
   it("has a link to browse the catalog", () => {
@@ -29,13 +29,13 @@ describe("NotFound", () => {
   it("has a back button", () => {
     render(<NotFound />);
 
-    expect(screen.getByRole("button", { name: /back/i })).toBeInTheDocument();
+    expect(screen.getByRole("button", { name: /back/i })).toBeVisible();
   });
 
   it("displays a decorative illustration", () => {
     const { container } = render(<NotFound />);
 
     const svg = container.querySelector("svg[aria-hidden='true']");
-    expect(svg).toBeInTheDocument();
+    expect(svg).toBeVisible();
   });
 });
diff --git a/src/app/catalog/[repoName]/[serverName]/[version]/not-found.tsx b/src/app/catalog/[repoName]/[serverName]/[version]/not-found.tsx
@@ -1,5 +1,5 @@
 import Link from "next/link";
-import { ErrorPage } from "@/components/error-page";
+import { ErrorPageLayout } from "@/components/error-page/error-page";
 import { NavigateBackButton } from "@/components/navigate-back-button";
 import { Button } from "@/components/ui/button";
 
@@ -13,7 +13,7 @@ export default function NotFound() {
         className="w-fit"
       />
 
-      <ErrorPage
+      <ErrorPageLayout
         title="Server Not Found"
         actions={
           <Button asChild variant="default">
@@ -23,7 +23,7 @@ export default function NotFound() {
       >
         The MCP server you're looking for doesn't exist or has been removed from
         the catalog.
-      </ErrorPage>
+      </ErrorPageLayout>
     </div>
   );
 }
diff --git a/src/app/catalog/actions.ts b/src/app/catalog/actions.ts
@@ -11,7 +11,7 @@ export async function getServers(): Promise<V0ServerJson[]> {
 
   if (resp.error) {
     console.error("[catalog] Failed to fetch servers:", resp.error);
-    return [];
+    throw resp.error;
   }
 
   if (!resp.data) {
diff --git a/src/app/catalog/components/__tests__/servers-wrapper.test.tsx b/src/app/catalog/components/__tests__/servers-wrapper.test.tsx
@@ -161,7 +161,6 @@ describe("ServersWrapper", () => {
       const awsCanvas = screen.queryByText("aws-nova-canvas");
       const googleApps = screen.queryByText("google-applications");
 
-      expect(awsCanvas).toBeInTheDocument();
       expect(awsCanvas).toBeVisible();
       expect(googleApps).not.toBeInTheDocument();
     });
diff --git a/src/app/catalog/components/__tests__/servers.test.tsx b/src/app/catalog/components/__tests__/servers.test.tsx
@@ -224,7 +224,7 @@ describe("Servers", () => {
         />,
       );
 
-      expect(container.querySelector("svg")).toBeInTheDocument();
+      expect(container.querySelector("svg")).toBeVisible();
     });
 
     it("does not show clear search button when no servers and no search", () => {
diff --git a/src/app/error.tsx b/src/app/error.tsx
@@ -0,0 +1,12 @@
+"use client";
+
+import { ErrorPage } from "@/components/error-page/error-page-client";
+
+interface ErrorProps {
+  error: Error & { digest?: string };
+  reset: () => void;
+}
+
+export default function RootErrorPage({ error, reset }: ErrorProps) {
+  return <ErrorPage error={error} reset={reset} />;
+}
diff --git a/src/app/not-found.test.tsx b/src/app/not-found.test.tsx
@@ -12,15 +12,15 @@ describe("NotFound (root)", () => {
 
     expect(
       screen.getByRole("heading", { name: /page not found/i }),
-    ).toBeInTheDocument();
+    ).toBeVisible();
   });
 
   it("displays a generic error message", async () => {
     render(await NotFound());
 
     expect(
       screen.getByText(/the page you're looking for doesn't exist/i),
-    ).toBeInTheDocument();
+    ).toBeVisible();
   });
 
   it("has a link to browse the catalog", async () => {
@@ -42,12 +42,12 @@ describe("NotFound (root)", () => {
     const { container } = render(await NotFound());
 
     const svg = container.querySelector("svg[aria-hidden='true']");
-    expect(svg).toBeInTheDocument();
+    expect(svg).toBeVisible();
   });
 
   it("displays the navbar", async () => {
     render(await NotFound());
 
-    expect(screen.getByTestId("navbar")).toBeInTheDocument();
+    expect(screen.getByTestId("navbar")).toBeVisible();
   });
 });
diff --git a/src/app/not-found.tsx b/src/app/not-found.tsx
@@ -1,5 +1,5 @@
 import Link from "next/link";
-import { ErrorPage } from "@/components/error-page";
+import { ErrorPageLayout } from "@/components/error-page/error-page";
 import { Navbar } from "@/components/navbar";
 import { Button } from "@/components/ui/button";
 
@@ -8,7 +8,7 @@ export default async function NotFound() {
     <div className="flex flex-col h-screen">
       <Navbar />
       <main className="flex flex-col flex-1 overflow-hidden px-4 py-5">
-        <ErrorPage
+        <ErrorPageLayout
           title="Page Not Found"
           actions={
             <Button asChild variant="default">
@@ -17,7 +17,7 @@ export default async function NotFound() {
           }
         >
           The page you're looking for doesn't exist or has been moved.
-        </ErrorPage>
+        </ErrorPageLayout>
       </main>
     </div>
   );
diff --git a/src/components/error-page/error-page-client.test.tsx b/src/components/error-page/error-page-client.test.tsx
@@ -0,0 +1,53 @@
+import { render, screen } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import { describe, expect, it, vi } from "vitest";
+import { ErrorPage } from "./error-page-client";
+
+describe("ErrorPage", () => {
+  it("logs errors to console on mount", () => {
+    const consoleError = vi
+      .spyOn(console, "error")
+      .mockImplementation(() => {});
+    const error = new Error("Test error");
+
+    render(<ErrorPage error={error} reset={vi.fn()} />);
+
+    expect(consoleError).toHaveBeenCalledWith(error);
+    consoleError.mockRestore();
+  });
+
+  it("displays 'Something went wrong' title", () => {
+    render(<ErrorPage error={new Error("Test")} reset={vi.fn()} />);
+
+    expect(
+      screen.getByRole("heading", { name: /something went wrong/i }),
+    ).toBeVisible();
+  });
+
+  it("displays error description", () => {
+    render(<ErrorPage error={new Error("Test")} reset={vi.fn()} />);
+
+    expect(screen.getByText(/an unexpected error occurred/i)).toBeVisible();
+  });
+
+  it("calls reset function when Try again button is clicked", async () => {
+    const user = userEvent.setup();
+    const reset = vi.fn();
+
+    render(<ErrorPage error={new Error("Test")} reset={reset} />);
+
+    await user.click(screen.getByRole("button", { name: /try again/i }));
+
+    expect(reset).toHaveBeenCalledTimes(1);
+  });
+
+  it("does not expose error message to user", () => {
+    const error = new Error("Sensitive internal error details");
+
+    render(<ErrorPage error={error} reset={vi.fn()} />);
+
+    expect(
+      screen.queryByText(/sensitive internal error/i),
+    ).not.toBeInTheDocument();
+  });
+});
diff --git a/src/components/error-page/error-page-client.tsx b/src/components/error-page/error-page-client.tsx
@@ -0,0 +1,29 @@
+"use client";
+
+import { useEffect } from "react";
+import { Button } from "@/components/ui/button";
+import { ErrorPageLayout } from "./error-page";
+
+interface ErrorPageProps {
+  error: Error & { digest?: string };
+  reset: () => void;
+}
+
+export function ErrorPage({ error, reset }: ErrorPageProps) {
+  useEffect(() => {
+    console.error(error);
+  }, [error]);
+
+  return (
+    <ErrorPageLayout
+      title="Something went wrong"
+      actions={
+        <Button onClick={reset} variant="default">
+          Try again
+        </Button>
+      }
+    >
+      An unexpected error occurred. Please try again.
+    </ErrorPageLayout>
+  );
+}
diff --git a/src/components/error-page/error-page.test.tsx b/src/components/error-page/error-page.test.tsx
@@ -1,43 +1,45 @@
 import { render, screen } from "@testing-library/react";
 import { describe, expect, it } from "vitest";
-import { ErrorPage } from "./error-page";
+import { ErrorPageLayout } from "./error-page";
 
-describe("ErrorPage", () => {
+describe("ErrorPageLayout", () => {
   it("displays the title", () => {
-    render(<ErrorPage title="Test Error">Error description</ErrorPage>);
+    render(
+      <ErrorPageLayout title="Test Error">Error description</ErrorPageLayout>,
+    );
 
-    expect(
-      screen.getByRole("heading", { name: /test error/i }),
-    ).toBeInTheDocument();
+    expect(screen.getByRole("heading", { name: /test error/i })).toBeVisible();
   });
 
   it("displays children as description", () => {
-    render(<ErrorPage title="Error">This is the error message</ErrorPage>);
+    render(
+      <ErrorPageLayout title="Error">
+        This is the error message
+      </ErrorPageLayout>,
+    );
 
-    expect(screen.getByText(/this is the error message/i)).toBeInTheDocument();
+    expect(screen.getByText(/this is the error message/i)).toBeVisible();
   });
 
   it("displays action buttons when provided", () => {
     render(
-      <ErrorPage
+      <ErrorPageLayout
         title="Error"
         actions={<button type="button">Click me</button>}
       >
         Description
-      </ErrorPage>,
+      </ErrorPageLayout>,
     );
 
-    expect(
-      screen.getByRole("button", { name: /click me/i }),
-    ).toBeInTheDocument();
+    expect(screen.getByRole("button", { name: /click me/i })).toBeVisible();
   });
 
   it("displays a decorative illustration", () => {
     const { container } = render(
-      <ErrorPage title="Error">Description</ErrorPage>,
+      <ErrorPageLayout title="Error">Description</ErrorPageLayout>,
     );
 
     const svg = container.querySelector("svg[aria-hidden='true']");
-    expect(svg).toBeInTheDocument();
+    expect(svg).toBeVisible();
   });
 });
diff --git a/src/components/error-page/error-page.tsx b/src/components/error-page/error-page.tsx
@@ -1,17 +1,21 @@
 import type { ReactNode } from "react";
-import { IllustrationNoLocation } from "@/components/illustrations/illustration-no-location";
+import { IllustrationError } from "@/components/illustrations/illustration-error";
 
-interface ErrorPageProps {
+interface ErrorPageLayoutProps {
   title: string;
   children: ReactNode;
   actions?: ReactNode;
 }
 
-export function ErrorPage({ title, children, actions }: ErrorPageProps) {
+export function ErrorPageLayout({
+  title,
+  children,
+  actions,
+}: ErrorPageLayoutProps) {
   return (
     <div className="flex items-center justify-center min-h-[80vh]">
       <div className="flex flex-col items-center text-center gap-4 max-w-md">
-        <IllustrationNoLocation className="size-40" />
+        <IllustrationError className="size-40" />
 
         <div className="space-y-2">
           <h2 className="text-2xl font-bold tracking-tight">{title}</h2>
diff --git a/src/components/error-page/index.ts b/src/components/error-page/index.ts
diff --git a/src/components/illustrations/illustration-error.tsx b/src/components/illustrations/illustration-error.tsx
diff --git a/src/components/user-menu/user-menu.test.tsx b/src/components/user-menu/user-menu.test.tsx
diff --git a/src/mocks/handlers.ts b/src/mocks/handlers.ts

Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ export async function getServers(): Promise<V0ServerJson[]> {`
`11`	`11`
`12`	`12`	`if (resp.error) {`
`13`	`13`	`console.error("[catalog] Failed to fetch servers:", resp.error);`
`14`		`- return [];`
	`14`	`+ throw resp.error;`
`15`	`15`	`}`
`16`	`16`
`17`	`17`	`if (!resp.data) {`