From e101bd8de46a917dafcd66213ba545ca964c16d2 Mon Sep 17 00:00:00 2001 From: Zack Tanner <1939140+ztanner@users.noreply.github.com> Date: Fri, 16 Aug 2024 16:42:42 -0700 Subject: [PATCH] Reland "Ensure bail out on ssr error in static generation" (#68999) Relands #68764 -- no additional changes were made, the errors were expected due to faulty user code. --- .../next/src/server/app-render/app-render.tsx | 60 ++++++++++++------- .../app-render/create-error-handler.tsx | 24 ++++++-- .../client-page-error-bailout/app/layout.js | 7 +++ .../client-page-error-bailout/app/page.js | 5 ++ .../client-page-error-bailout.test.ts | 29 +++++++++ 5 files changed, 98 insertions(+), 27 deletions(-) create mode 100644 test/production/app-dir/client-page-error-bailout/app/layout.js create mode 100644 test/production/app-dir/client-page-error-bailout/app/page.js create mode 100644 test/production/app-dir/client-page-error-bailout/client-page-error-bailout.test.ts diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index 514a4ecc189a3..c2c629f526150 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -68,6 +68,7 @@ import { createHTMLReactServerErrorHandler, createHTMLErrorHandler, type DigestedError, + isUserLandError, } from './create-error-handler' import { getShortDynamicParamType, @@ -976,7 +977,14 @@ async function renderToHTMLOrFlightImpl( // prerendering phase and the build. if (response.digestErrorsMap.size) { const buildFailingError = response.digestErrorsMap.values().next().value - throw buildFailingError + if (buildFailingError) throw buildFailingError + } + // Pick first userland SSR error, which is also not a RSC error. + if (response.ssrErrors.length) { + const buildFailingError = response.ssrErrors.find((err) => + isUserLandError(err) + ) + if (buildFailingError) throw buildFailingError } const options: RenderResultOptions = { @@ -1229,26 +1237,29 @@ async function renderToStream( const reactServerErrorsByDigest: Map = new Map() const silenceLogger = false + function onHTMLRenderRSCError(err: DigestedError) { + return renderOpts.onInstrumentationRequestError?.( + err, + req, + createErrorContext(ctx, 'react-server-components') + ) + } const serverComponentsErrorHandler = createHTMLReactServerErrorHandler( !!renderOpts.dev, !!renderOpts.nextExport, reactServerErrorsByDigest, silenceLogger, - // RSC rendering error will report as SSR error - // @TODO we should report RSC errors where they happen for instrumentation purposes - // and should omit the error reporter in the SSR layer instead - undefined + onHTMLRenderRSCError ) - function onServerRenderError(err: DigestedError) { - const renderSource = reactServerErrorsByDigest.has(err.digest) - ? 'react-server-components' - : 'server-rendering' + + function onHTMLRenderSSRError(err: DigestedError) { return renderOpts.onInstrumentationRequestError?.( err, req, - createErrorContext(ctx, renderSource) + createErrorContext(ctx, 'server-rendering') ) } + const allCapturedErrors: Array = [] const htmlRendererErrorHandler = createHTMLErrorHandler( !!renderOpts.dev, @@ -1256,7 +1267,7 @@ async function renderToStream( reactServerErrorsByDigest, allCapturedErrors, silenceLogger, - onServerRenderError + onHTMLRenderSSRError ) let primaryRenderReactServerStream: null | ReadableStream = null @@ -1575,6 +1586,7 @@ async function renderToStream( type PrenderToStringResult = { stream: ReadableStream digestErrorsMap: Map + ssrErrors: Array dynamicTracking?: null | DynamicTrackingState err?: unknown } @@ -1639,24 +1651,26 @@ async function prerenderToStream( const reactServerErrorsByDigest: Map = new Map() // We don't report errors during prerendering through our instrumentation hooks const silenceLogger = !!renderOpts.experimental.isRoutePPREnabled + function onHTMLRenderRSCError(err: DigestedError) { + return renderOpts.onInstrumentationRequestError?.( + err, + req, + createErrorContext(ctx, 'react-server-components') + ) + } const serverComponentsErrorHandler = createHTMLReactServerErrorHandler( !!renderOpts.dev, !!renderOpts.nextExport, reactServerErrorsByDigest, silenceLogger, - // RSC rendering error will report as SSR error - // @TODO we should report RSC errors where they happen for instrumentation purposes - // and should omit the error reporter in the SSR layer instead - undefined + onHTMLRenderRSCError ) - function onServerRenderError(err: DigestedError) { - const renderSource = reactServerErrorsByDigest.has(err.digest) - ? 'react-server-components' - : 'server-rendering' + + function onHTMLRenderSSRError(err: DigestedError) { return renderOpts.onInstrumentationRequestError?.( err, req, - createErrorContext(ctx, renderSource) + createErrorContext(ctx, 'server-rendering') ) } const allCapturedErrors: Array = [] @@ -1666,7 +1680,7 @@ async function prerenderToStream( reactServerErrorsByDigest, allCapturedErrors, silenceLogger, - onServerRenderError + onHTMLRenderSSRError ) let dynamicTracking: null | DynamicTrackingState = null @@ -1790,6 +1804,7 @@ async function prerenderToStream( // require the same set so we unify the code path here return { digestErrorsMap: reactServerErrorsByDigest, + ssrErrors: allCapturedErrors, stream: await continueDynamicPrerender(prelude, { getServerInsertedHTML, }), @@ -1837,6 +1852,7 @@ async function prerenderToStream( return { digestErrorsMap: reactServerErrorsByDigest, + ssrErrors: allCapturedErrors, stream: await continueStaticPrerender(htmlStream, { inlinedDataStream: createInlinedDataReadableStream( inlinedReactServerDataStream, @@ -1906,6 +1922,7 @@ async function prerenderToStream( }) return { digestErrorsMap: reactServerErrorsByDigest, + ssrErrors: allCapturedErrors, stream: await continueFizzStream(htmlStream, { inlinedDataStream: createInlinedDataReadableStream( inlinedReactServerDataStream, @@ -2044,6 +2061,7 @@ async function prerenderToStream( // the response in the caller. err, digestErrorsMap: reactServerErrorsByDigest, + ssrErrors: allCapturedErrors, stream: await continueFizzStream(fizzStream, { inlinedDataStream: createInlinedDataReadableStream( // This is intentionally using the readable datastream from the diff --git a/packages/next/src/server/app-render/create-error-handler.tsx b/packages/next/src/server/app-render/create-error-handler.tsx index e3b030dfb793d..9db7cbbc49f18 100644 --- a/packages/next/src/server/app-render/create-error-handler.tsx +++ b/packages/next/src/server/app-render/create-error-handler.tsx @@ -139,19 +139,21 @@ export function createHTMLErrorHandler( dev: boolean, isNextExport: boolean, reactServerErrors: Map, - allCapturedError: Array, + allCapturedErrors: Array, silenceLogger: boolean, - onHTMLRenderError: (err: any) => void + onHTMLRenderSSRError: (err: any) => void ): ErrorHandler { return (err: any, errorInfo: any) => { + let isSSRError = true + // If the error already has a digest, respect the original digest, // so it won't get re-generated into another new error. - if (err.digest) { if (reactServerErrors.has(err.digest)) { // This error is likely an obfuscated error from react-server. // We recover the original error here. err = reactServerErrors.get(err.digest) + isSSRError = false } else { // The error is not from react-server but has a digest // from other means so we don't need to produce a new one @@ -162,7 +164,7 @@ export function createHTMLErrorHandler( ).toString() } - allCapturedError.push(err) + allCapturedErrors.push(err) // If the response was closed, we don't need to log the error. if (isAbortError(err)) return @@ -203,11 +205,21 @@ export function createHTMLErrorHandler( }) } - if (!silenceLogger) { - onHTMLRenderError(err) + if ( + !silenceLogger && + // HTML errors contain RSC errors as well, filter them out before reporting + isSSRError + ) { + onHTMLRenderSSRError(err) } } return err.digest } } + +export function isUserLandError(err: any): boolean { + return ( + !isAbortError(err) && !isBailoutToCSRError(err) && !isNextRouterError(err) + ) +} diff --git a/test/production/app-dir/client-page-error-bailout/app/layout.js b/test/production/app-dir/client-page-error-bailout/app/layout.js new file mode 100644 index 0000000000000..a3a86a5ca1e12 --- /dev/null +++ b/test/production/app-dir/client-page-error-bailout/app/layout.js @@ -0,0 +1,7 @@ +export default function Root({ children }) { + return ( + + {children} + + ) +} diff --git a/test/production/app-dir/client-page-error-bailout/app/page.js b/test/production/app-dir/client-page-error-bailout/app/page.js new file mode 100644 index 0000000000000..22570eae6c578 --- /dev/null +++ b/test/production/app-dir/client-page-error-bailout/app/page.js @@ -0,0 +1,5 @@ +'use client' + +export default function Page() { + throw new Error('client-page-error') +} diff --git a/test/production/app-dir/client-page-error-bailout/client-page-error-bailout.test.ts b/test/production/app-dir/client-page-error-bailout/client-page-error-bailout.test.ts new file mode 100644 index 0000000000000..1c36d86023be2 --- /dev/null +++ b/test/production/app-dir/client-page-error-bailout/client-page-error-bailout.test.ts @@ -0,0 +1,29 @@ +import { nextTestSetup } from 'e2e-utils' + +describe('app-dir - client-page-error-bailout', () => { + const { next, skipped } = nextTestSetup({ + files: __dirname, + skipStart: true, + }) + + if (skipped) { + return + } + + let stderr = '' + beforeAll(() => { + const onLog = (log: string) => { + stderr += log + } + + next.on('stderr', onLog) + }) + + it('should bail out in static generation build', async () => { + await next.build() + expect(stderr).toContain( + 'Error occurred prerendering page "/". Read more: https://nextjs.org/docs/messages/prerender-error' + ) + expect(stderr).toContain('Error: client-page-error') + }) +})