From a78163dc613f168f991b29018adbc3472004d38b Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 11 Oct 2022 11:17:10 +0200 Subject: [PATCH] Global layouts error boundary (#41305) --- .../client/components/app-router.client.tsx | 25 +++- .../next/client/components/error-boundary.tsx | 108 ++++++++++++++++ .../components/layout-router.client.tsx | 62 +-------- .../error.js | 0 .../page.js | 0 .../app/error/global-error-boundary/page.js | 20 +++ test/e2e/app-dir/index.test.ts | 121 ++++++++++++------ 7 files changed, 228 insertions(+), 108 deletions(-) create mode 100644 packages/next/client/components/error-boundary.tsx rename test/e2e/app-dir/app/app/error/{clientcomponent => client-component}/error.js (100%) rename test/e2e/app-dir/app/app/error/{clientcomponent => client-component}/page.js (100%) create mode 100644 test/e2e/app-dir/app/app/error/global-error-boundary/page.js diff --git a/packages/next/client/components/app-router.client.tsx b/packages/next/client/components/app-router.client.tsx index 2f6daf61bf67..4f718b842bcb 100644 --- a/packages/next/client/components/app-router.client.tsx +++ b/packages/next/client/components/app-router.client.tsx @@ -28,6 +28,7 @@ import { // LayoutSegmentsContext, } from './hooks-client-context' import { useReducerWithReduxDevtools } from './use-reducer-with-devtools' +import { ErrorBoundary, GlobalErrorComponent } from './error-boundary' function urlToUrlWithoutFlightMarker(url: string): URL { const urlWithoutFlightParameters = new URL(url, location.origin) @@ -91,20 +92,22 @@ let initialParallelRoutes: CacheNode['parallelRoutes'] = const prefetched = new Set() +type AppRouterProps = { + initialTree: FlightRouterState + initialCanonicalUrl: string + children: ReactNode + assetPrefix: string +} + /** * The global router that wraps the application components. */ -export default function AppRouter({ +function Router({ initialTree, initialCanonicalUrl, children, assetPrefix, -}: { - initialTree: FlightRouterState - initialCanonicalUrl: string - children: ReactNode - assetPrefix: string -}) { +}: AppRouterProps) { const initialState = useMemo(() => { return { tree: initialTree, @@ -367,3 +370,11 @@ export default function AppRouter({ ) } + +export default function AppRouter(props: AppRouterProps) { + return ( + + + + ) +} diff --git a/packages/next/client/components/error-boundary.tsx b/packages/next/client/components/error-boundary.tsx new file mode 100644 index 000000000000..649a7bd73721 --- /dev/null +++ b/packages/next/client/components/error-boundary.tsx @@ -0,0 +1,108 @@ +import React from 'react' + +export type ErrorComponent = React.ComponentType<{ + error: Error + reset: () => void +}> +interface ErrorBoundaryProps { + errorComponent: ErrorComponent +} + +/** + * Handles errors through `getDerivedStateFromError`. + * Renders the provided error component and provides a way to `reset` the error boundary state. + */ +class ErrorBoundaryHandler extends React.Component< + ErrorBoundaryProps, + { error: Error | null } +> { + constructor(props: ErrorBoundaryProps) { + super(props) + this.state = { error: null } + } + + static getDerivedStateFromError(error: Error) { + return { error } + } + + reset = () => { + this.setState({ error: null }) + } + + render() { + if (this.state.error) { + return ( + + ) + } + + return this.props.children + } +} + +/** + * Renders error boundary with the provided "errorComponent" property as the fallback. + * If no "errorComponent" property is provided it renders the children without an error boundary. + */ +export function ErrorBoundary({ + errorComponent, + children, +}: ErrorBoundaryProps & { children: React.ReactNode }): JSX.Element { + if (errorComponent) { + return ( + + {children} + + ) + } + + return <>{children} +} + +const styles: { [k: string]: React.CSSProperties } = { + error: { + fontFamily: + '-apple-system, BlinkMacSystemFont, Roboto, "Segoe UI", "Fira Sans", Avenir, "Helvetica Neue", "Lucida Grande", sans-serif', + height: '100vh', + textAlign: 'center', + display: 'flex', + flexDirection: 'column', + alignItems: 'center', + justifyContent: 'center', + }, + + desc: { + display: 'inline-block', + textAlign: 'left', + lineHeight: '49px', + height: '49px', + verticalAlign: 'middle', + }, + h2: { + fontSize: '14px', + fontWeight: 'normal', + lineHeight: '49px', + margin: 0, + padding: 0, + }, +} + +export function GlobalErrorComponent() { + return ( + + +
+
+

+ Application error: a client-side exception has occurred (see the + browser console for more information). +

+
+
+ + + ) +} diff --git a/packages/next/client/components/layout-router.client.tsx b/packages/next/client/components/layout-router.client.tsx index 447a82f15d3d..5a5b53e4ceb2 100644 --- a/packages/next/client/components/layout-router.client.tsx +++ b/packages/next/client/components/layout-router.client.tsx @@ -20,6 +20,7 @@ import type { FlightSegmentPath, // FlightDataPath, } from '../../server/app-render' +import type { ErrorComponent } from './error-boundary' import { LayoutRouterContext, GlobalLayoutRouterContext, @@ -28,7 +29,7 @@ import { } from '../../shared/lib/app-router-context' import { fetchServerResponse } from './app-router.client' import { createInfinitePromise } from './infinite-promise' - +import { ErrorBoundary } from './error-boundary' import { matchSegment } from './match-segments' /** @@ -364,65 +365,6 @@ function NotFoundBoundary({ notFound, children }: NotFoundBoundaryProps) { ) } -type ErrorComponent = React.ComponentType<{ error: Error; reset: () => void }> -interface ErrorBoundaryProps { - errorComponent: ErrorComponent -} - -/** - * Handles errors through `getDerivedStateFromError`. - * Renders the provided error component and provides a way to `reset` the error boundary state. - */ -class ErrorBoundaryHandler extends React.Component< - ErrorBoundaryProps, - { error: Error | null } -> { - constructor(props: ErrorBoundaryProps) { - super(props) - this.state = { error: null } - } - - static getDerivedStateFromError(error: Error) { - return { error } - } - - reset = () => { - this.setState({ error: null }) - } - - render() { - if (this.state.error) { - return ( - - ) - } - - return this.props.children - } -} - -/** - * Renders error boundary with the provided "errorComponent" property as the fallback. - * If no "errorComponent" property is provided it renders the children without an error boundary. - */ -function ErrorBoundary({ - errorComponent, - children, -}: ErrorBoundaryProps & { children: React.ReactNode }): JSX.Element { - if (errorComponent) { - return ( - - {children} - - ) - } - - return <>{children} -} - /** * OuterLayoutRouter handles the current segment as well as rendering of other segments. * It can be rendered next to each other with a different `parallelRouterKey`, allowing for Parallel routes. diff --git a/test/e2e/app-dir/app/app/error/clientcomponent/error.js b/test/e2e/app-dir/app/app/error/client-component/error.js similarity index 100% rename from test/e2e/app-dir/app/app/error/clientcomponent/error.js rename to test/e2e/app-dir/app/app/error/client-component/error.js diff --git a/test/e2e/app-dir/app/app/error/clientcomponent/page.js b/test/e2e/app-dir/app/app/error/client-component/page.js similarity index 100% rename from test/e2e/app-dir/app/app/error/clientcomponent/page.js rename to test/e2e/app-dir/app/app/error/client-component/page.js diff --git a/test/e2e/app-dir/app/app/error/global-error-boundary/page.js b/test/e2e/app-dir/app/app/error/global-error-boundary/page.js new file mode 100644 index 000000000000..5f4e73da9dc1 --- /dev/null +++ b/test/e2e/app-dir/app/app/error/global-error-boundary/page.js @@ -0,0 +1,20 @@ +'client' + +import { useState } from 'react' + +export default function Page() { + const [clicked, setClicked] = useState(false) + if (clicked) { + throw new Error('this is a test') + } + return ( + + ) +} diff --git a/test/e2e/app-dir/index.test.ts b/test/e2e/app-dir/index.test.ts index 33fd7ebcbbd7..38270f263db1 100644 --- a/test/e2e/app-dir/index.test.ts +++ b/test/e2e/app-dir/index.test.ts @@ -1,7 +1,14 @@ import { createNext, FileRef } from 'e2e-utils' import crypto from 'crypto' import { NextInstance } from 'test/lib/next-modes/base' -import { check, fetchViaHTTP, renderViaHTTP, waitFor } from 'next-test-utils' +import { + check, + fetchViaHTTP, + getRedboxHeader, + hasRedbox, + renderViaHTTP, + waitFor, +} from 'next-test-utils' import path from 'path' import cheerio from 'cheerio' import webdriver from 'next-webdriver' @@ -1494,57 +1501,89 @@ describe('app dir', () => { ) }) - // TODO-APP: This is disabled for development as the error overlay needs to be reworked. - ;(isDev ? describe.skip : describe)('error component', () => { + describe('error component', () => { it('should trigger error component when an error happens during rendering', async () => { - const browser = await webdriver(next.url, '/error/clientcomponent') - await browser - .elementByCss('#error-trigger-button') - .click() - .waitForElementByCss('#error-boundary-message') - - expect( - await browser.elementByCss('#error-boundary-message').text() - ).toBe('An error occurred: this is a test') - }) - - it('should allow resetting error boundary', async () => { - const browser = await webdriver(next.url, '/error/clientcomponent') - - // Try triggering and resetting a few times in a row - for (let i = 0; i < 5; i++) { + const browser = await webdriver(next.url, '/error/client-component') + await browser.elementByCss('#error-trigger-button').click() + + if (isDev) { + expect(await hasRedbox(browser)).toBe(true) + console.log('getRedboxHeader', await getRedboxHeader(browser)) + // expect(await getRedboxHeader(browser)).toMatch(/An error occurred: this is a test/) + } else { await browser - .elementByCss('#error-trigger-button') - .click() - .waitForElementByCss('#error-boundary-message') - expect( - await browser.elementByCss('#error-boundary-message').text() + await browser + .waitForElementByCss('#error-boundary-message') + .elementByCss('#error-boundary-message') + .text() ).toBe('An error occurred: this is a test') - - await browser - .elementByCss('#reset') - .click() - .waitForElementByCss('#error-trigger-button') - - expect( - await browser.elementByCss('#error-trigger-button').text() - ).toBe('Trigger Error!') } }) - it('should hydrate empty shell to handle server-side rendering errors', async () => { + it('should use default error boundary for prod and overlay for dev when no error component specified', async () => { const browser = await webdriver( next.url, - '/error/ssr-error-client-component' + '/error/global-error-boundary' ) - const logs = await browser.log() - const errors = logs - .filter((x) => x.source === 'error') - .map((x) => x.message) - .join('\n') - expect(errors).toInclude('Error during SSR') + await browser.elementByCss('#error-trigger-button').click() + // .waitForElementByCss('body') + + if (isDev) { + expect(await hasRedbox(browser)).toBe(true) + console.log('getRedboxHeader', await getRedboxHeader(browser)) + // expect(await getRedboxHeader(browser)).toMatch(/An error occurred: this is a test/) + } else { + expect( + await browser + .waitForElementByCss('body') + .elementByCss('body') + .text() + ).toBe( + 'Application error: a client-side exception has occurred (see the browser console for more information).' + ) + } }) + + if (!isDev) { + it('should allow resetting error boundary', async () => { + const browser = await webdriver(next.url, '/error/client-component') + + // Try triggering and resetting a few times in a row + for (let i = 0; i < 5; i++) { + await browser + .elementByCss('#error-trigger-button') + .click() + .waitForElementByCss('#error-boundary-message') + + expect( + await browser.elementByCss('#error-boundary-message').text() + ).toBe('An error occurred: this is a test') + + await browser + .elementByCss('#reset') + .click() + .waitForElementByCss('#error-trigger-button') + + expect( + await browser.elementByCss('#error-trigger-button').text() + ).toBe('Trigger Error!') + } + }) + + it('should hydrate empty shell to handle server-side rendering errors', async () => { + const browser = await webdriver( + next.url, + '/error/ssr-error-client-component' + ) + const logs = await browser.log() + const errors = logs + .filter((x) => x.source === 'error') + .map((x) => x.message) + .join('\n') + expect(errors).toInclude('Error during SSR') + }) + } }) describe('known bugs', () => {