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 Dec 23, 2023
1 parent 2caf188 commit 24e4b3f
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 59 deletions.
86 changes: 45 additions & 41 deletions packages/next/src/server/app-render/app-render.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -702,29 +702,27 @@ async function renderToHTMLOrFlightImpl(
// response directly.
const onHeadersFinished = new DetachedPromise<void>()

const renderToStream = getTracer().wrap(
type RenderToStream = (options: {
/**
* 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
}) => Promise<{ ok: boolean; stream: RenderResultResponse }>

const renderToStream: RenderToStream = getTracer().wrap(
AppRenderSpan.getBodyResult,
{
spanName: `render route (app) ${pagePath}`,
attributes: {
'next.route': pagePath,
},
},
async ({
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
}) => {
async ({ asNotFound, tree, formState }) => {
const polyfills: JSX.IntrinsicElements['script'][] =
buildManifest.polyfillFiles
.filter(
Expand Down Expand Up @@ -821,7 +819,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, ok: true }
}

const options: ContinueStreamOptions = {
Expand All @@ -842,10 +840,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: stream, ok: true }
} catch (err: any) {
if (
err.code === 'NEXT_STATIC_GEN_BAILOUT' ||
Expand Down Expand Up @@ -1002,16 +1002,19 @@ async function renderToHTMLOrFlightImpl(
},
})

return await continueFizzStream(fizzStream, {
inlinedDataStream:
serverErrorComponentsRenderOpts.inlinedDataTransformStream
.readable,
isStaticGeneration,
getServerInsertedHTML: () => getServerInsertedHTML([]),
serverInsertedHTMLToHead: true,
validateRootLayout,
suffix: undefined,
})
return {
stream: await continueFizzStream(fizzStream, {
inlinedDataStream:
serverErrorComponentsRenderOpts.inlinedDataTransformStream
.readable,
isStaticGeneration,
getServerInsertedHTML: () => getServerInsertedHTML([]),
serverInsertedHTMLToHead: true,
validateRootLayout,
suffix: undefined,
}),
ok: false,
}
} catch (finalErr: any) {
if (
process.env.NODE_ENV === 'development' &&
Expand Down Expand Up @@ -1044,14 +1047,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 @@ -1066,7 +1068,7 @@ async function renderToHTMLOrFlightImpl(
metadata,
}

let response: RenderResultResponse = await renderToStream({
let response = await renderToStream({
asNotFound: isNotFoundPath,
tree: loaderTree,
formState,
Expand All @@ -1086,7 +1088,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 @@ -1095,7 +1097,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 @@ -1122,6 +1124,8 @@ async function renderToHTMLOrFlightImpl(
renderOpts.experimental.ppr &&
// and a call to `postpone` happened
staticGenerationStore.postponeWasTriggered &&
// and we didn't encounter any errors during the build
response.ok &&
// but there's no postpone state
!metadata.postponed
) {
Expand Down Expand Up @@ -1185,7 +1189,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 24e4b3f

Please sign in to comment.