Reland "Ensure bail out on ssr error in static generation"

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,34 +1237,37 @@ async function renderToStream(
 
   const reactServerErrorsByDigest: Map<string, DigestedError> = 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<unknown> = []
   const htmlRendererErrorHandler = createHTMLErrorHandler(
     !!renderOpts.dev,
     !!renderOpts.nextExport,
     reactServerErrorsByDigest,
     allCapturedErrors,
     silenceLogger,
-    onServerRenderError
+    onHTMLRenderSSRError
   )
 
   let primaryRenderReactServerStream: null | ReadableStream<Uint8Array> = null
@@ -1575,6 +1586,7 @@ async function renderToStream(
 type PrenderToStringResult = {
   stream: ReadableStream<Uint8Array>
   digestErrorsMap: Map<string, DigestedError>
+  ssrErrors: Array<unknown>
   dynamicTracking?: null | DynamicTrackingState
   err?: unknown
 }
@@ -1639,24 +1651,26 @@ async function prerenderToStream(
   const reactServerErrorsByDigest: Map<string, DigestedError> = 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<unknown> = []
@@ -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

packages/next/src/server/app-render/create-error-handler.tsx

@@ -139,19 +139,21 @@ export function createHTMLErrorHandler(
   dev: boolean,
   isNextExport: boolean,
   reactServerErrors: Map<string, DigestedError>,
-  allCapturedError: Array<unknown>,
+  allCapturedErrors: Array<unknown>,
   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)
+  )
+}

test/production/app-dir/client-page-error-bailout/app/layout.js

@@ -0,0 +1,7 @@
+export default function Root({ children }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}

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')
+}

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')
+  })
+})