Skip to content

Commit

Permalink
Merge pull request #522 from NYPL/SFR-2136/add-error-boundaries
Browse files Browse the repository at this point in the history
SFR-2136: Implement error boundary
  • Loading branch information
jackiequach authored Aug 29, 2024
2 parents feff036 + e01c916 commit 2cd719e
Show file tree
Hide file tree
Showing 11 changed files with 134 additions and 35 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- Fix error when a collection is empty
- SFR-2077: Validate top 5 language filters for Frontend DRB search results
- Add Playwright integration test for cookie validation
- Add Error Boundary for uncaught exceptions

## [0.18.2]

Expand Down
20 changes: 20 additions & 0 deletions mocks/handlers.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { http, HttpResponse, passthrough } from "msw";
import { oneCollectionListData } from "~/src/__tests__/fixtures/CollectionFixture";
import { workDetailWithUp } from "~/src/__tests__/fixtures/WorkDetailFixture";
import {
API_URL,
COLLECTION_LIST_PATH,
INVALID_COLLECTION_PATH,
DOWNLOAD_PATH,
FULFILL_PATH,
LIMITED_ACCESS_WORK_PATH,
Expand All @@ -15,6 +18,11 @@ const isAuthenticated = (request) => {
const workUrl = new URL(LIMITED_ACCESS_WORK_PATH, API_URL).toString();
const fulfillUrl = new URL(FULFILL_PATH, API_URL).toString();
const downloadUrl = new URL(DOWNLOAD_PATH, API_URL).toString();
const collectionListUrl = new URL(COLLECTION_LIST_PATH, API_URL).toString();
const invalidCollectionUrl = new URL(
INVALID_COLLECTION_PATH,
API_URL
).toString();

/** A collection of handlers to be used by default for all tests. */
const handlers = [
Expand Down Expand Up @@ -55,6 +63,18 @@ const handlers = [
status: 200,
});
}),

http.get(collectionListUrl, () => {
return HttpResponse.json(oneCollectionListData);
}),

http.get(invalidCollectionUrl, () => {
return HttpResponse.json({
status: 500,
title: "Something went wrong",
detail: "An unknown error occurred on the server",
});
}),
];

export default handlers;
4 changes: 1 addition & 3 deletions mocks/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
async function initMocks() {
export async function initMocks() {
if (typeof window === "undefined") {
const { server } = await import("./server");
server.listen({
Expand All @@ -12,6 +12,4 @@ async function initMocks() {
}
}

initMocks();

export {};
5 changes: 5 additions & 0 deletions mocks/mockEnv.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,8 @@ export const FULFILL_PATH = "/fulfill/12345";
export const LIMITED_ACCESS_WORK_PATH =
"/work/12345678-1234-1234-1234-1234567890ab";
export const DOWNLOAD_PATH = "/test-download-pdf";
export const HOME_PATH = "/";
export const COLLECTION_LIST_PATH = "/collection/list";
export const COLLECTION_PATH =
"/collection/978ea0e0-8ecc-4de2-bfe8-032fea641d8e?page=1";
export const INVALID_COLLECTION_PATH = "/collection/invalid-collection";
3 changes: 0 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,6 @@
"prettier": "^2.2.1",
"ts-node": "^10.9.1"
},
"resolutions": {
"@types/react": "^17.0.2"
},
"msw": {
"workerDirectory": "public"
}
Expand Down
33 changes: 33 additions & 0 deletions playwright/integration/landing.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { test, expect } from "../support/test-utils";
import {
INVALID_COLLECTION_PATH,
HOME_PATH,
COLLECTION_PATH,
} from "~/mocks/mockEnv";
import { server } from "~/mocks/server";

test.afterEach(() => server.resetHandlers());
test.afterAll(() => server.close());

test("View landing page with collection", async ({ page, port }) => {
await page.goto(`http://localhost:${port}${HOME_PATH}`);
const collectionHeading = page.getByRole("heading", {
name: "Recently Added Collections",
level: 2,
});
expect(collectionHeading).toBeVisible();
const collectionLink = await page
.getByRole("link", {
name: /Baseball: A Collection by Mike Benowitz/,
})
.getAttribute("href");
expect(collectionLink).toContain(COLLECTION_PATH);
});

test("Shows error boundary for invalid collection", async ({ page, port }) => {
await page.goto(`http://localhost:${port}${INVALID_COLLECTION_PATH}`);

const alert = page.getByRole("alert");
const errorText = alert.getByText("An error 500 occurred on server");
await expect(errorText).toBeVisible();
});
3 changes: 3 additions & 0 deletions playwright/support/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@ import { parse } from "url";
import { AddressInfo } from "net";
import next from "next";
import path from "path";
import { http } from "msw";

const test = base.extend<{
setCookie(expires?: number): Promise<void>;
port: string;
http: typeof http;
}>({
setCookie: [
async ({ context }, use, _expires) => {
Expand Down Expand Up @@ -57,6 +59,7 @@ const test = base.extend<{
},
{ auto: true },
],
http,
});

export { test, expect };
46 changes: 46 additions & 0 deletions src/components/ErrorBoundary.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import React, { ErrorInfo } from "react";
import Error from "../pages/_error";

interface ErrorBoundaryProps {
children?: React.ReactNode;
}

interface ErrorBoundaryState {
error?: Error;
info?: React.ErrorInfo;
}

class ErrorBoundary extends React.Component<
ErrorBoundaryProps,
ErrorBoundaryState
> {
constructor(props) {
super(props);

this.state = { error: undefined, info: undefined };
}

static getDerivedStateFromError(error: Error) {
return { error };
}

componentDidCatch(error: Error, errorInfo: ErrorInfo) {
// TODO: add logging to New Relic
this.setState({
error,
info: errorInfo,
});
}

render() {
const { error } = this.state;

if (error) {
return <Error statusCode={error} />;
}

return this.props.children;
}
}

export default ErrorBoundary;
14 changes: 9 additions & 5 deletions src/pages/_app.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ import { trackPageview } from "../lib/adobe/Analytics";
import { pageNames } from "../constants/analytics";
import { getQueryDecodedString } from "../util/SearchQueryUtils";
import NewRelicSnippet from "../lib/newrelic/NewRelic";
import ErrorBoundary from "../components/ErrorBoundary";

if (process.env.APP_ENV === "testing") {
require("mocks");
const { initMocks } = await import("mocks");
await initMocks();
}

/**
Expand Down Expand Up @@ -90,10 +92,12 @@ const MyApp = ({ Component, pageProps }: AppProps) => {

<link rel="icon" href={appConfig.favIconPath} />
</Head>
<NewRelicSnippet />
<FeatureFlagProvider>
<Component {...pageProps} />
</FeatureFlagProvider>
<ErrorBoundary>
<NewRelicSnippet />
<FeatureFlagProvider>
<Component {...pageProps} />
</FeatureFlagProvider>
</ErrorBoundary>
</>
);
};
Expand Down
22 changes: 12 additions & 10 deletions src/pages/_error.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,18 @@ import Link from "../components/Link/Link";
const Error = ({ statusCode }) => {
return (
<Layout>
<p>
{statusCode
? `An error ${statusCode} occurred on server`
: "An error occurred on client"}
</p>
<p>
Search&nbsp;
<Link to="/">Digital Research Books Beta</Link>
{"."}
</p>
<div role="alert">
<p>
{statusCode
? `An error ${statusCode} occurred on server`
: "An error occurred on client"}
</p>
<p>
Search&nbsp;
<Link to="/">Digital Research Books Beta</Link>
{"."}
</p>
</div>
</Layout>
);
};
Expand Down
18 changes: 4 additions & 14 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,17 +1,11 @@
{
"compilerOptions": {
"target": "es5",
"lib": [
"dom",
"dom.iterable",
"esnext"
],
"target": "es2017",
"lib": ["dom", "dom.iterable", "esnext"],
"moduleResolution": "node",
"baseUrl": ".",
"paths": {
"~/*": [
"*"
]
"~/*": ["*"]
},
"allowJs": true,
"skipLibCheck": true,
Expand Down Expand Up @@ -46,9 +40,5 @@
"src/components/RequestDigital/RequestDigital.jsx",
"src/__tests__/index.test.tsx"
],
"exclude": [
"node_modules",
"jest.config.ts",
"setupTests.js"
]
"exclude": ["node_modules", "jest.config.ts", "setupTests.js"]
}

0 comments on commit 2cd719e

Please sign in to comment.