Skip to content

Commit

Permalink
Ensure bail out on ssr error in static generation (#68764)
Browse files Browse the repository at this point in the history
### What

It's regression introduced in #67703 where throwing error in client
component page is not bailing out anymore in static generation. It was
supposed to bail out the build, found by @gnoff

Closes NDX-52

### Why

We're throwing the all the collected render errors for both RSC and SSR
rendering error. But in #67703 we accidentally removed the collection of
SSR rendering error into digested errors map

```js
   if (response.digestErrorsMap.size) {
      const buildFailingError = response.digestErrorsMap.values().next().value
      throw buildFailingError
    }
```

We add another check after above for SSR errors, if they're presented
we'll bail with the 1st SSR error.
  • Loading branch information
huozhi committed Aug 13, 2024
1 parent 7ab9f98 commit 7d952ed
Show file tree
Hide file tree
Showing 5 changed files with 98 additions and 27 deletions.
60 changes: 39 additions & 21 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import {
createHTMLReactServerErrorHandler,
createHTMLErrorHandler,
type DigestedError,
isUserLandError,
} from './create-error-handler'
import {
getShortDynamicParamType,
Expand Down Expand Up @@ -968,7 +969,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 = {
Expand Down Expand Up @@ -1221,34 +1229,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
Expand Down Expand Up @@ -1567,6 +1578,7 @@ async function renderToStream(
type PrenderToStringResult = {
stream: ReadableStream<Uint8Array>
digestErrorsMap: Map<string, DigestedError>
ssrErrors: Array<unknown>
dynamicTracking?: null | DynamicTrackingState
err?: unknown
}
Expand Down Expand Up @@ -1631,24 +1643,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> = []
Expand All @@ -1658,7 +1672,7 @@ async function prerenderToStream(
reactServerErrorsByDigest,
allCapturedErrors,
silenceLogger,
onServerRenderError
onHTMLRenderSSRError
)

let dynamicTracking: null | DynamicTrackingState = null
Expand Down Expand Up @@ -1782,6 +1796,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,
}),
Expand Down Expand Up @@ -1829,6 +1844,7 @@ async function prerenderToStream(

return {
digestErrorsMap: reactServerErrorsByDigest,
ssrErrors: allCapturedErrors,
stream: await continueStaticPrerender(htmlStream, {
inlinedDataStream: createInlinedDataReadableStream(
inlinedReactServerDataStream,
Expand Down Expand Up @@ -1898,6 +1914,7 @@ async function prerenderToStream(
})
return {
digestErrorsMap: reactServerErrorsByDigest,
ssrErrors: allCapturedErrors,
stream: await continueFizzStream(htmlStream, {
inlinedDataStream: createInlinedDataReadableStream(
inlinedReactServerDataStream,
Expand Down Expand Up @@ -2036,6 +2053,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
Expand Down
24 changes: 18 additions & 6 deletions packages/next/src/server/app-render/create-error-handler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function Root({ children }) {
return (
<html>
<body>{children}</body>
</html>
)
}
5 changes: 5 additions & 0 deletions test/production/app-dir/client-page-error-bailout/app/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
'use client'

export default function Page() {
throw new Error('client-page-error')
}
Original file line number Diff line number Diff line change
@@ -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')
})
})

0 comments on commit 7d952ed

Please sign in to comment.