Skip to content

Commit

Permalink
fix: don't warn about missing postponed data if the build errored
Browse files Browse the repository at this point in the history
  • Loading branch information
wyattjoh committed Jan 8, 2024
1 parent 0a34e04 commit 9f7c091
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 59 deletions.
98 changes: 57 additions & 41 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,23 @@ async function renderToHTMLOrFlightImpl(
// response directly.
const onHeadersFinished = new DetachedPromise<void>()

type RenderToStreamResult = {
stream: RenderResultResponse
err?: Error
}

type RenderToStreamOptions = {
/**
* This option is used to indicate that the page should be rendered as
* if it was not found. When it's enabled, instead of rendering the
* page component, it renders the not-found segment.
*
*/
asNotFound: boolean
tree: LoaderTree
formState: any
}

const renderToStream = getTracer().wrap(
AppRenderSpan.getBodyResult,
{
Expand All @@ -816,17 +833,7 @@ async function renderToHTMLOrFlightImpl(
asNotFound,
tree,
formState,
}: {
/**
* This option is used to indicate that the page should be rendered as
* if it was not found. When it's enabled, instead of rendering the
* page component, it renders the not-found segment.
*
*/
asNotFound: boolean
tree: LoaderTree
formState: any
}) => {
}: RenderToStreamOptions): Promise<RenderToStreamResult> => {
const polyfills: JSX.IntrinsicElements['script'][] =
buildManifest.polyfillFiles
.filter(
Expand Down Expand Up @@ -946,7 +953,7 @@ async function renderToHTMLOrFlightImpl(

// We don't need to "continue" this stream now as it's continued when
// we resume the stream.
return stream
return { stream }
}

const options: ContinueStreamOptions = {
Expand All @@ -966,10 +973,12 @@ async function renderToHTMLOrFlightImpl(
}

if (renderOpts.postponed) {
return await continuePostponedFizzStream(stream, options)
stream = await continuePostponedFizzStream(stream, options)
} else {
stream = await continueFizzStream(stream, options)
}

return await continueFizzStream(stream, options)
return { stream }
} catch (err: any) {
if (
err.code === 'NEXT_STATIC_GEN_BAILOUT' ||
Expand All @@ -987,8 +996,9 @@ async function renderToHTMLOrFlightImpl(
throw err
}

const isBailoutCSR = isBailoutCSRError(err)
if (isBailoutCSR) {
// True if this error was a bailout to client side rendering error.
const shouldBailoutToCSR = isBailoutCSRError(err)
if (shouldBailoutToCSR) {
warn(
`Entire page ${pagePath} deopted into client-side rendering. https://nextjs.org/docs/messages/deopted-into-client-rendering`,
pagePath
Expand Down Expand Up @@ -1019,7 +1029,7 @@ async function renderToHTMLOrFlightImpl(
}

const is404 = res.statusCode === 404
if (!is404 && !hasRedirectError && !isBailoutCSR) {
if (!is404 && !hasRedirectError && !shouldBailoutToCSR) {
res.statusCode = 500
}

Expand Down Expand Up @@ -1077,14 +1087,19 @@ async function renderToHTMLOrFlightImpl(
},
})

return await continueFizzStream(fizzStream, {
inlinedDataStream: errorInlinedDataTransformStream.readable,
isStaticGeneration,
getServerInsertedHTML: () => getServerInsertedHTML([]),
serverInsertedHTMLToHead: true,
validateRootLayout,
suffix: undefined,
})
return {
// Returning the error that was thrown so it can be used to handle
// the response in the caller.
err,
stream: await continueFizzStream(fizzStream, {
inlinedDataStream: errorInlinedDataTransformStream.readable,
isStaticGeneration,
getServerInsertedHTML: () => getServerInsertedHTML([]),
serverInsertedHTMLToHead: true,
validateRootLayout,
suffix: undefined,
}),
}
} catch (finalErr: any) {
if (
process.env.NODE_ENV === 'development' &&
Expand Down Expand Up @@ -1117,14 +1132,13 @@ async function renderToHTMLOrFlightImpl(
if (actionRequestResult) {
if (actionRequestResult.type === 'not-found') {
const notFoundLoaderTree = createNotFoundLoaderTree(loaderTree)
return new RenderResult(
await renderToStream({
asNotFound: true,
tree: notFoundLoaderTree,
formState,
}),
{ metadata }
)
const response = await renderToStream({
asNotFound: true,
tree: notFoundLoaderTree,
formState,
})

return new RenderResult(response.stream, { metadata })
} else if (actionRequestResult.type === 'done') {
if (actionRequestResult.result) {
actionRequestResult.result.assignMetadata(metadata)
Expand All @@ -1139,7 +1153,7 @@ async function renderToHTMLOrFlightImpl(
metadata,
}

let response: RenderResultResponse = await renderToStream({
let response = await renderToStream({
asNotFound: isNotFoundPath,
tree: loaderTree,
formState,
Expand All @@ -1159,7 +1173,7 @@ async function renderToHTMLOrFlightImpl(
}

// Create the new render result for the response.
const result = new RenderResult(response, options)
const result = new RenderResult(response.stream, options)

// If we aren't performing static generation, we can return the result now.
if (!isStaticGeneration) {
Expand All @@ -1168,7 +1182,7 @@ async function renderToHTMLOrFlightImpl(

// If this is static generation, we should read this in now rather than
// sending it back to be sent to the client.
response = await result.toUnchunkedString(true)
response.stream = await result.toUnchunkedString(true)

// Timeout after 1.5 seconds for the headers to write. If it takes
// longer than this it's more likely that the stream has stalled and
Expand All @@ -1190,13 +1204,15 @@ async function renderToHTMLOrFlightImpl(
// it from rejecting again (which is a no-op anyways).
clearTimeout(timeout)

// If PPR is enabled and the postpone was triggered but lacks the postponed
// state information then we should error out unless the client side rendering
// bailout error was also emitted which indicates that part of the stream was
// not rendered.
if (
// if PPR is enabled
renderOpts.experimental.ppr &&
// and a call to `postpone` happened
staticGenerationStore.postponeWasTriggered &&
// but there's no postpone state
!metadata.postponed
!metadata.postponed &&
(!response.err || !isBailoutCSRError(response.err))
) {
// a call to postpone was made but was caught and not detected by Next.js. We should fail the build immediately
// as we won't be able to generate the static part
Expand Down Expand Up @@ -1258,7 +1274,7 @@ async function renderToHTMLOrFlightImpl(
}
}

return new RenderResult(response, options)
return new RenderResult(response.stream, options)
}

export type AppPageRender = (
Expand Down
18 changes: 0 additions & 18 deletions test/e2e/app-dir/ppr-errors/ppr-errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,24 +66,6 @@ describe('ppr build errors', () => {
'Error occurred prerendering page "/no-suspense-boundary"'
)
})

it('should fail the build & surface any errors that were thrown by user code', async () => {
// in the case of catching a postpone and throwing a new error, we log the error that the user threw to help with debugging
expect(stderr).toContain(
'Prerendering /no-suspense-boundary-re-throwing-error needs to partially bail out because something dynamic was used. '
)
expect(stderr).toContain(
'The following error was thrown during build, and may help identify the source of the issue:'
)
expect(stderr).toContain(
"Error: Throwing a new error from 'no-suspense-boundary-re-throwing-error'"
)

// the regular pre-render error should not be thrown as well, as we've already logged a more specific error
expect(stderr).not.toContain(
'Error occurred prerendering page "/no-suspense-boundary-re-throwing-error"'
)
})
})
})

Expand Down

0 comments on commit 9f7c091

Please sign in to comment.