From b9760b2910c720ee0b4700917cd02575f6bbd7d6 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 12 Jul 2023 03:54:07 +0200 Subject: [PATCH] Support global-error for ssr fallback (#52573) Previously `global-error` only caught the error on client side, this PR adds the support for catching the errors thrown during client components SSR or server components RSC rendering. Closes #46572 Closes #50119 Closes #50723 --- .../build/webpack/loaders/next-app-loader.ts | 12 ++- .../src/client/components/error-boundary.tsx | 4 +- .../next/src/server/app-render/app-render.tsx | 90 ++++++++++++++----- .../server/app-render/use-flight-response.tsx | 8 +- .../app/{throw => client}/page.js | 0 .../app-dir/global-error/app/global-error.js | 4 +- test/e2e/app-dir/global-error/app/layout.js | 2 + .../global-error/app/ssr/client/page.js | 5 ++ .../global-error/app/ssr/server/page.js | 3 + .../app-dir/global-error/global-error.test.ts | 28 ------ test/e2e/app-dir/global-error/index.test.ts | 61 +++++++++++++ 11 files changed, 161 insertions(+), 56 deletions(-) rename test/e2e/app-dir/global-error/app/{throw => client}/page.js (100%) create mode 100644 test/e2e/app-dir/global-error/app/ssr/client/page.js create mode 100644 test/e2e/app-dir/global-error/app/ssr/server/page.js delete mode 100644 test/e2e/app-dir/global-error/global-error.test.ts create mode 100644 test/e2e/app-dir/global-error/index.test.ts diff --git a/packages/next/src/build/webpack/loaders/next-app-loader.ts b/packages/next/src/build/webpack/loaders/next-app-loader.ts index 8505bc459cb87..74377e14dd3f8 100644 --- a/packages/next/src/build/webpack/loaders/next-app-loader.ts +++ b/packages/next/src/build/webpack/loaders/next-app-loader.ts @@ -666,9 +666,15 @@ const nextAppLoader: AppLoader = async function nextAppLoader() { const result = ` export ${treeCodeResult.treeCode} export ${treeCodeResult.pages} - export { default as GlobalError } from ${JSON.stringify( - treeCodeResult.globalError || 'next/dist/client/components/error-boundary' - )} + + ${ + treeCodeResult.globalError + ? `export { default as GlobalError } from ${JSON.stringify( + treeCodeResult.globalError + )}` + : `export { GlobalError } from 'next/dist/client/components/error-boundary'` + } + export const originalPathname = ${JSON.stringify(page)} export const __next_app__ = { require: __webpack_require__, diff --git a/packages/next/src/client/components/error-boundary.tsx b/packages/next/src/client/components/error-boundary.tsx index ded99a39acddd..fca1aa41ca089 100644 --- a/packages/next/src/client/components/error-boundary.tsx +++ b/packages/next/src/client/components/error-boundary.tsx @@ -99,10 +99,10 @@ export class ErrorBoundaryHandler extends React.Component< } } -export default function GlobalError({ error }: { error: any }) { +export function GlobalError({ error }: { error: any }) { const digest: string | undefined = error?.digest return ( - +
diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index 3d46c137c7f62..264a0d06f0714 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -1224,13 +1224,18 @@ export async function renderToHTMLOrFlight( const GlobalError = /** GlobalError can be either the default error boundary or the overwritten app/global-error.js **/ - ComponentMod.GlobalError as typeof import('../../client/components/error-boundary').default + ComponentMod.GlobalError as typeof import('../../client/components/error-boundary').GlobalError let serverComponentsInlinedTransformStream: TransformStream< Uint8Array, Uint8Array > = new TransformStream() + let serverErrorComponentsInlinedTransformStream: TransformStream< + Uint8Array, + Uint8Array + > = new TransformStream() + // Get the nonce from the incoming request if it has one. const csp = req.headers['content-security-policy'] let nonce: string | undefined @@ -1245,8 +1250,11 @@ export async function renderToHTMLOrFlight( rscChunks: [], } - if (!clientReferenceManifest) { - console.log(req.url) + const serverErrorComponentsRenderOpts = { + transformStream: serverErrorComponentsInlinedTransformStream, + clientReferenceManifest, + serverContexts, + rscChunks: [], } const validateRootLayout = dev @@ -1511,7 +1519,7 @@ export async function renderToHTMLOrFlight( }) const result = await continueFromInitialStream(renderStream, { - dataStream: serverComponentsInlinedTransformStream?.readable, + dataStream: serverComponentsInlinedTransformStream.readable, generateStaticHTML: staticGenerationStore.isStaticGeneration || generateStaticHTML, getServerInsertedHTML, @@ -1553,24 +1561,61 @@ export async function renderToHTMLOrFlight( res.setHeader('Location', getURLFromRedirectError(err)) } + const defaultErrorComponent = ( + + + {/* @ts-expect-error allow to use async server component */} + + {appUsingSizeAdjust ? : null} + + + + ) + + const useDefaultError = + res.statusCode < 400 || + res.statusCode === 404 || + res.statusCode === 307 + const serverErrorElement = useDefaultError + ? defaultErrorComponent + : React.createElement( + createServerComponentRenderer( + async () => { + // only pass plain object to client + return ( + <> + {/* @ts-expect-error allow to use async server component */} + + + + ) + }, + ComponentMod, + serverErrorComponentsRenderOpts, + serverComponentsErrorHandler, + nonce + ) + ) + const renderStream = await renderToInitialStream({ ReactDOMServer: require('react-dom/server.edge'), - element: ( - - - {/* @ts-expect-error allow to use async server component */} - - {appUsingSizeAdjust ? : null} - - - - ), + element: serverErrorElement, streamOptions: { nonce, // Include hydration scripts in the HTML @@ -1590,7 +1635,10 @@ export async function renderToHTMLOrFlight( }) return await continueFromInitialStream(renderStream, { - dataStream: serverComponentsInlinedTransformStream?.readable, + dataStream: (useDefaultError + ? serverComponentsInlinedTransformStream + : serverErrorComponentsInlinedTransformStream + ).readable, generateStaticHTML: staticGenerationStore.isStaticGeneration, getServerInsertedHTML, serverInsertedHTMLToHead: true, diff --git a/packages/next/src/server/app-render/use-flight-response.tsx b/packages/next/src/server/app-render/use-flight-response.tsx index 1cc5b67f62d00..f441aa87e25cf 100644 --- a/packages/next/src/server/app-render/use-flight-response.tsx +++ b/packages/next/src/server/app-render/use-flight-response.tsx @@ -59,7 +59,13 @@ export function useFlightResponse( ) } if (done) { - flightResponseRef.current = null + // Add a setTimeout here because the error component is too small, the first forwardReader.read() read will return the full chunk + // and then it immediately set flightResponseRef.current as null. + // react renders the component twice, the second render will run into the state with useFlightResponse where flightResponseRef.current is null, + // so it tries to render the flight payload again + setTimeout(() => { + flightResponseRef.current = null + }) writer.close() } else { const responsePartial = decodeText(value, textDecoder) diff --git a/test/e2e/app-dir/global-error/app/throw/page.js b/test/e2e/app-dir/global-error/app/client/page.js similarity index 100% rename from test/e2e/app-dir/global-error/app/throw/page.js rename to test/e2e/app-dir/global-error/app/client/page.js diff --git a/test/e2e/app-dir/global-error/app/global-error.js b/test/e2e/app-dir/global-error/app/global-error.js index 5440155259e6c..d960556d75cd5 100644 --- a/test/e2e/app-dir/global-error/app/global-error.js +++ b/test/e2e/app-dir/global-error/app/global-error.js @@ -5,7 +5,9 @@ export default function GlobalError({ error }) { -
{`Error message: ${error?.message}`}
+

Global Error

+

{`Global error: ${error?.message}`}

+ {error?.digest &&

{error?.digest}

} ) diff --git a/test/e2e/app-dir/global-error/app/layout.js b/test/e2e/app-dir/global-error/app/layout.js index 762515029332e..440cc5cc28b51 100644 --- a/test/e2e/app-dir/global-error/app/layout.js +++ b/test/e2e/app-dir/global-error/app/layout.js @@ -6,3 +6,5 @@ export default function Layout({ children }) { ) } + +export const revalidate = 0 diff --git a/test/e2e/app-dir/global-error/app/ssr/client/page.js b/test/e2e/app-dir/global-error/app/ssr/client/page.js new file mode 100644 index 0000000000000..1aab5b332230a --- /dev/null +++ b/test/e2e/app-dir/global-error/app/ssr/client/page.js @@ -0,0 +1,5 @@ +'use client' + +export default function page() { + throw new Error('client page error') +} diff --git a/test/e2e/app-dir/global-error/app/ssr/server/page.js b/test/e2e/app-dir/global-error/app/ssr/server/page.js new file mode 100644 index 0000000000000..a8a95ce72a343 --- /dev/null +++ b/test/e2e/app-dir/global-error/app/ssr/server/page.js @@ -0,0 +1,3 @@ +export default function page() { + throw new Error('server page error') +} diff --git a/test/e2e/app-dir/global-error/global-error.test.ts b/test/e2e/app-dir/global-error/global-error.test.ts deleted file mode 100644 index 01b320b34fde0..0000000000000 --- a/test/e2e/app-dir/global-error/global-error.test.ts +++ /dev/null @@ -1,28 +0,0 @@ -import { getRedboxHeader, hasRedbox } from 'next-test-utils' -import { createNextDescribe } from 'e2e-utils' - -createNextDescribe( - 'app dir - global error', - { - files: __dirname, - }, - ({ next, isNextDev }) => { - it('should trigger error component when an error happens during rendering', async () => { - const browser = await next.browser('/throw') - await browser - .waitForElementByCss('#error-trigger-button') - .elementByCss('#error-trigger-button') - .click() - - if (isNextDev) { - expect(await hasRedbox(browser, true)).toBe(true) - expect(await getRedboxHeader(browser)).toMatch(/Error: Client error/) - } else { - await browser - expect(await browser.elementByCss('#error').text()).toBe( - 'Error message: Client error' - ) - } - }) - } -) diff --git a/test/e2e/app-dir/global-error/index.test.ts b/test/e2e/app-dir/global-error/index.test.ts new file mode 100644 index 0000000000000..4ff397d501f60 --- /dev/null +++ b/test/e2e/app-dir/global-error/index.test.ts @@ -0,0 +1,61 @@ +import { getRedboxHeader, hasRedbox } from 'next-test-utils' +import { createNextDescribe } from 'e2e-utils' + +async function testDev(browser, errorRegex) { + expect(await hasRedbox(browser, true)).toBe(true) + expect(await getRedboxHeader(browser)).toMatch(errorRegex) +} + +createNextDescribe( + 'app dir - global error', + { + files: __dirname, + }, + ({ next, isNextDev }) => { + it('should trigger error component when an error happens during rendering', async () => { + const browser = await next.browser('/client') + await browser + .waitForElementByCss('#error-trigger-button') + .elementByCss('#error-trigger-button') + .click() + + if (isNextDev) { + await testDev(browser, /Error: Client error/) + } else { + await browser + expect(await browser.elementByCss('#error').text()).toBe( + 'Global error: Client error' + ) + } + }) + + it('should render global error for error in server components', async () => { + const browser = await next.browser('/ssr/server') + + if (isNextDev) { + await testDev(browser, /Error: server page error/) + } else { + expect(await browser.elementByCss('h1').text()).toBe('Global Error') + expect(await browser.elementByCss('#error').text()).toBe( + 'Global error: An error occurred in the Server Components render. The specific message is omitted in production builds to avoid leaking sensitive details. A digest property is included on this error instance which may provide additional details about the nature of the error.' + ) + expect(await browser.elementByCss('#digest').text()).toMatch(/\w+/) + } + }) + + it('should render global error for error in client components', async () => { + const browser = await next.browser('/ssr/client') + + if (isNextDev) { + await testDev(browser, /Error: client page error/) + } else { + expect(await browser.elementByCss('h1').text()).toBe('Global Error') + expect(await browser.elementByCss('#error').text()).toBe( + 'Global error: client page error' + ) + + expect(await browser.hasElementByCssSelector('#digest')).toBeFalsy() + } + }) + } +)