From 407cc4e3d37d6edbc27522b55460567a4a164eb7 Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Wed, 25 Oct 2023 22:16:42 -0700 Subject: [PATCH 1/3] fail build if there are any errors --- packages/next/src/export/routes/app-page.ts | 163 ++++++++---------- packages/next/src/export/routes/app-route.ts | 2 +- .../next/src/server/app-render/app-render.tsx | 31 ++-- .../app-render}/is-dynamic-usage-error.ts | 0 packages/next/src/server/app-render/types.ts | 1 - .../node/cookies-error-no-throw/page.jsx | 26 --- .../app/suspense/node/cookies-error/page.jsx | 6 +- .../app/suspense/node/fetch-error/page.jsx | 7 +- test/e2e/app-dir/ppr/ppr.test.ts | 6 - 9 files changed, 87 insertions(+), 155 deletions(-) rename packages/next/src/{export/helpers => server/app-render}/is-dynamic-usage-error.ts (100%) delete mode 100644 test/e2e/app-dir/ppr/app/suspense/node/cookies-error-no-throw/page.jsx diff --git a/packages/next/src/export/routes/app-page.ts b/packages/next/src/export/routes/app-page.ts index c490ca03384ab..0da9c47a0027b 100644 --- a/packages/next/src/export/routes/app-page.ts +++ b/packages/next/src/export/routes/app-page.ts @@ -13,7 +13,6 @@ import { NEXT_URL, NEXT_ROUTER_PREFETCH, } from '../../client/components/app-router-headers' -import { isDynamicUsageError } from '../helpers/is-dynamic-usage-error' import { NEXT_CACHE_TAGS_HEADER } from '../../lib/constants' import { hasNextSupport } from '../../telemetry/ci-info' import { lazyRenderAppPage } from '../../server/future/route-modules/app-page/module.render' @@ -80,8 +79,35 @@ export async function exportAppPage( pathname = '/404' } - try { - if (isAppPrefetch) { + if (isAppPrefetch) { + await generatePrefetchRsc( + req, + path, + res, + pathname, + htmlFilepath, + renderOpts, + fileWriter + ) + + return { revalidate: 0 } + } + + const result = await lazyRenderAppPage(req, res, pathname, query, renderOpts) + const html = result.toUnchunkedString() + const { metadata } = result + const flightData = metadata.pageData + const revalidate = metadata.revalidate ?? false + const postponed = metadata.postponed + + if (revalidate === 0) { + if (isDynamicError) { + throw new Error( + `Page with dynamic = "error" encountered dynamic data method on ${path}.` + ) + } + + if (!(renderOpts as any).store.staticPrefetchBailout) { await generatePrefetchRsc( req, path, @@ -91,109 +117,60 @@ export async function exportAppPage( renderOpts, fileWriter ) - - return { revalidate: 0 } } - const result = await lazyRenderAppPage( - req, - res, - pathname, - query, - renderOpts - ) - const html = result.toUnchunkedString() - const { metadata } = result - const flightData = metadata.pageData - const revalidate = metadata.revalidate ?? false - const postponed = metadata.postponed - - if (revalidate === 0) { - if (isDynamicError) { - throw new Error( - `Page with dynamic = "error" encountered dynamic data method on ${path}.` - ) - } - - if (!(renderOpts as any).store.staticPrefetchBailout) { - await generatePrefetchRsc( - req, - path, - res, - pathname, - htmlFilepath, - renderOpts, - fileWriter - ) - } - - const { staticBailoutInfo = {} } = metadata - - if (revalidate === 0 && debugOutput && staticBailoutInfo?.description) { - const err = new Error( - `Static generation failed due to dynamic usage on ${path}, reason: ${staticBailoutInfo.description}` - ) + const { staticBailoutInfo = {} } = metadata - // Update the stack if it was provided via the bailout info. - const { stack } = staticBailoutInfo - if (stack) { - err.stack = err.message + stack.substring(stack.indexOf('\n')) - } + if (revalidate === 0 && debugOutput && staticBailoutInfo?.description) { + const err = new Error( + `Static generation failed due to dynamic usage on ${path}, reason: ${staticBailoutInfo.description}` + ) - console.warn(err) + // Update the stack if it was provided via the bailout info. + const { stack } = staticBailoutInfo + if (stack) { + err.stack = err.message + stack.substring(stack.indexOf('\n')) } - return { revalidate: 0 } + console.warn(err) } - let headers: OutgoingHttpHeaders | undefined - if (metadata.fetchTags) { - headers = { [NEXT_CACHE_TAGS_HEADER]: metadata.fetchTags } - } + return { revalidate: 0 } + } - // Writing static HTML to a file. - await fileWriter( - ExportedAppPageFiles.HTML, - htmlFilepath, - html ?? '', - 'utf8' - ) + let headers: OutgoingHttpHeaders | undefined + if (metadata.fetchTags) { + headers = { [NEXT_CACHE_TAGS_HEADER]: metadata.fetchTags } + } - // Writing the request metadata to a file. - const meta: RouteMetadata = { - status: undefined, - headers, - postponed, - } + // Writing static HTML to a file. + await fileWriter(ExportedAppPageFiles.HTML, htmlFilepath, html ?? '', 'utf8') - await fileWriter( - ExportedAppPageFiles.META, - htmlFilepath.replace(/\.html$/, '.meta'), - JSON.stringify(meta, null, 2) - ) + // Writing the request metadata to a file. + const meta: RouteMetadata = { + status: undefined, + headers, + postponed, + } - // Writing the RSC payload to a file. - await fileWriter( - ExportedAppPageFiles.FLIGHT, - htmlFilepath.replace(/\.html$/, '.rsc'), - flightData - ) + await fileWriter( + ExportedAppPageFiles.META, + htmlFilepath.replace(/\.html$/, '.meta'), + JSON.stringify(meta, null, 2) + ) - return { - // Only include the metadata if the environment has next support. - metadata: hasNextSupport ? meta : undefined, - hasEmptyPrelude: Boolean(postponed) && html === '', - hasPostponed: Boolean(postponed), - revalidate, - } - } catch (err: any) { - // if the error isn't a special dynamic usage error (caught by Next) - // we also do not throw the error if it occurred while attempting a postpone - // since those will be captured and logged during build/ISR - if (!isDynamicUsageError(err) && !renderOpts.hasPostponeErrors) { - throw err - } + // Writing the RSC payload to a file. + await fileWriter( + ExportedAppPageFiles.FLIGHT, + htmlFilepath.replace(/\.html$/, '.rsc'), + flightData + ) - return { revalidate: 0, hasEmptyPrelude: true } + return { + // Only include the metadata if the environment has next support. + metadata: hasNextSupport ? meta : undefined, + hasEmptyPrelude: Boolean(postponed) && html === '', + hasPostponed: Boolean(postponed), + revalidate, } } diff --git a/packages/next/src/export/routes/app-route.ts b/packages/next/src/export/routes/app-route.ts index 9cc370d0e37d0..98b96ec12de38 100644 --- a/packages/next/src/export/routes/app-route.ts +++ b/packages/next/src/export/routes/app-route.ts @@ -16,7 +16,7 @@ import type { MockedRequest, MockedResponse, } from '../../server/lib/mock-request' -import { isDynamicUsageError } from '../helpers/is-dynamic-usage-error' +import { isDynamicUsageError } from '../../server/app-render/is-dynamic-usage-error' import { SERVER_DIRECTORY } from '../../shared/lib/constants' import { hasNextSupport } from '../../telemetry/ci-info' diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index ae4d18a431430..6cafad3a20d3e 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -72,6 +72,8 @@ import { createComponentTree } from './create-component-tree' import { getAssetQueryString } from './get-asset-query-string' import { setReferenceManifestsSingleton } from './action-encryption-utils' import { createStaticRenderer } from './static/static-renderer' +import { isPostpone } from '../lib/router-utils/is-postpone' +import { isDynamicUsageError } from './is-dynamic-usage-error' export type GetDynamicParamFromSegment = ( // [slug] / [[slug]] / [...slug] @@ -999,33 +1001,24 @@ async function renderToHTMLOrFlightImpl( if (staticGenerationStore.isStaticGeneration) { const htmlResult = await renderResult.toUnchunkedString(true) - if (renderOpts.ppr && postponeErrors.length > 0) { - renderOpts.hasPostponeErrors = true - } - if ( renderOpts.ppr && staticGenerationStore.postponeWasTriggered && !extraRenderResultMeta.postponed ) { - warn('') - warn( - `${urlPathname} opted out of partial prerendering because the postpone signal was intercepted by a try/catch in your application code.` + throw new Error( + `Postpone signal was caught while rendering ${urlPathname}. These errors should not be caught during static generation. Learn more: https://nextjs.org/docs/messages/ppr-postpone-errors` ) + } - if (postponeErrors.length > 0) { - warn( - 'The following errors were re-thrown, and might help find the location of the try/catch that triggered this.' - ) - for (let i = 0; i < postponeErrors.length; i++) { - error(`${postponeErrors[i].stack?.split('\n').join('\n ')}`) - } + for (const err of capturedErrors) { + if (!isDynamicUsageError(err) && !isPostpone(err)) { + throw err + } + + if (isDynamicUsageError(err)) { + staticGenerationStore.revalidate = 0 } - } - // if we encountered any unexpected errors during build - // we fail the prerendering phase and the build - if (capturedErrors.length > 0) { - throw capturedErrors[0] } if (staticGenerationStore.forceStatic === false) { diff --git a/packages/next/src/export/helpers/is-dynamic-usage-error.ts b/packages/next/src/server/app-render/is-dynamic-usage-error.ts similarity index 100% rename from packages/next/src/export/helpers/is-dynamic-usage-error.ts rename to packages/next/src/server/app-render/is-dynamic-usage-error.ts diff --git a/packages/next/src/server/app-render/types.ts b/packages/next/src/server/app-render/types.ts index 10a656e2b6b9e..62d8d745c57c1 100644 --- a/packages/next/src/server/app-render/types.ts +++ b/packages/next/src/server/app-render/types.ts @@ -137,7 +137,6 @@ export interface RenderOptsPartial { isPrefetch?: boolean ppr: boolean postponed?: string - hasPostponeErrors?: boolean } export type RenderOpts = LoadComponentsReturnType & diff --git a/test/e2e/app-dir/ppr/app/suspense/node/cookies-error-no-throw/page.jsx b/test/e2e/app-dir/ppr/app/suspense/node/cookies-error-no-throw/page.jsx deleted file mode 100644 index 80f1510fac293..0000000000000 --- a/test/e2e/app-dir/ppr/app/suspense/node/cookies-error-no-throw/page.jsx +++ /dev/null @@ -1,26 +0,0 @@ -import React, { Suspense } from 'react' -import { cookies } from 'next/headers' - -export default async function Page() { - return ( - <> -

Dynamic Component Catching Errors

-

- This shows the dynamic component that reads cookies but wraps the read - in a try/catch. This test does not re-throw the caught error. -

-
- Loading...
}> - - - - - ) -} - -async function Foobar() { - try { - cookies() - } catch (err) {} - return null -} diff --git a/test/e2e/app-dir/ppr/app/suspense/node/cookies-error/page.jsx b/test/e2e/app-dir/ppr/app/suspense/node/cookies-error/page.jsx index b8457f0503bb0..77a3e787ba5a9 100644 --- a/test/e2e/app-dir/ppr/app/suspense/node/cookies-error/page.jsx +++ b/test/e2e/app-dir/ppr/app/suspense/node/cookies-error/page.jsx @@ -9,7 +9,7 @@ export default async function Page() {

Dynamic Component Catching Errors

This shows the dynamic component that reads cookies but wraps the read - in a try/catch. This test re-throws the error that is caught. + in a try/catch.

}> @@ -27,8 +27,6 @@ export default async function Page() { async function Foobar() { try { cookies() - } catch (err) { - throw new Error('You are not signed in') - } + } catch (err) {} return null } diff --git a/test/e2e/app-dir/ppr/app/suspense/node/fetch-error/page.jsx b/test/e2e/app-dir/ppr/app/suspense/node/fetch-error/page.jsx index 72f8e62e6f5cd..dff12e37fe4a2 100644 --- a/test/e2e/app-dir/ppr/app/suspense/node/fetch-error/page.jsx +++ b/test/e2e/app-dir/ppr/app/suspense/node/fetch-error/page.jsx @@ -12,16 +12,13 @@ export default async function Page() { try { await getData() - } catch (err) { - throw new Error('Fetch failed') - } + } catch (err) {} return ( <>

Dynamic Component Catching Errors

- This shows the dynamic component that reads cookies but wraps the read - in a try/catch. + This shows the dynamic component that wraps a data fetch in a try/catch

}> diff --git a/test/e2e/app-dir/ppr/ppr.test.ts b/test/e2e/app-dir/ppr/ppr.test.ts index 8f26cad39cd88..8950af04b748c 100644 --- a/test/e2e/app-dir/ppr/ppr.test.ts +++ b/test/e2e/app-dir/ppr/ppr.test.ts @@ -19,12 +19,6 @@ createNextDescribe( expect(next.cliOutput).toContain( '/suspense/node/cookies-error opted out of partial prerendering because the postpone signal was intercepted by a try/catch in your application code.' ) - - // fetch-error re-throws the error after catching it, so we want to make sure that's retained in the logs - expect(next.cliOutput).toContain( - 'The following errors were re-thrown, and might help find the location of the try/catch that triggered this.' - ) - expect(next.cliOutput).toContain('Error: You are not signed in') }) } describe.each([ From d3626a496f59648a58aab801a633f26d49a53e7f Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Wed, 25 Oct 2023 22:53:50 -0700 Subject: [PATCH 2/3] move test --- .../next/src/server/app-render/app-render.tsx | 2 +- test/e2e/app-dir/ppr-errors/app/layout.jsx | 9 +++++++ test/e2e/app-dir/ppr-errors/app/page.jsx | 26 +++++++++++++++++++ test/e2e/app-dir/ppr-errors/next.config.js | 10 +++++++ test/e2e/app-dir/ppr-errors/ppr.test.ts | 10 +++++++ test/e2e/app-dir/ppr/ppr.test.ts | 14 ---------- 6 files changed, 56 insertions(+), 15 deletions(-) create mode 100644 test/e2e/app-dir/ppr-errors/app/layout.jsx create mode 100644 test/e2e/app-dir/ppr-errors/app/page.jsx create mode 100644 test/e2e/app-dir/ppr-errors/next.config.js create mode 100644 test/e2e/app-dir/ppr-errors/ppr.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 6cafad3a20d3e..34ba7fcad8c30 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -61,7 +61,7 @@ import { validateURL } from './validate-url' import { createFlightRouterStateFromLoaderTree } from './create-flight-router-state-from-loader-tree' import { handleAction } from './action-handler' import { NEXT_DYNAMIC_NO_SSR_CODE } from '../../shared/lib/lazy-dynamic/no-ssr-error' -import { warn, error } from '../../build/output/log' +import { warn } from '../../build/output/log' import { appendMutableCookies } from '../web/spec-extension/adapters/request-cookies' import { createServerInsertedHTML } from './server-inserted-html' import { getRequiredScripts } from './required-scripts' diff --git a/test/e2e/app-dir/ppr-errors/app/layout.jsx b/test/e2e/app-dir/ppr-errors/app/layout.jsx new file mode 100644 index 0000000000000..c4215a0c905a5 --- /dev/null +++ b/test/e2e/app-dir/ppr-errors/app/layout.jsx @@ -0,0 +1,9 @@ +import React from 'react' + +export default function Root({ children }) { + return ( + + {children} + + ) +} diff --git a/test/e2e/app-dir/ppr-errors/app/page.jsx b/test/e2e/app-dir/ppr-errors/app/page.jsx new file mode 100644 index 0000000000000..dafdf6677c1b8 --- /dev/null +++ b/test/e2e/app-dir/ppr-errors/app/page.jsx @@ -0,0 +1,26 @@ +import React, { Suspense } from 'react' +import { cookies } from 'next/headers' + +export default async function Page() { + return ( + <> +

Dynamic Component Catching Errors

+

+ This shows the dynamic component that reads cookies but wraps the read + in a try/catch. +

+
+ Loading...
}> + + +
+ + ) +} + +async function Foobar() { + try { + cookies() + } catch (err) {} + return null +} diff --git a/test/e2e/app-dir/ppr-errors/next.config.js b/test/e2e/app-dir/ppr-errors/next.config.js new file mode 100644 index 0000000000000..016ac8833b57f --- /dev/null +++ b/test/e2e/app-dir/ppr-errors/next.config.js @@ -0,0 +1,10 @@ +/** + * @type {import('next').NextConfig} + */ +const nextConfig = { + experimental: { + ppr: true, + }, +} + +module.exports = nextConfig diff --git a/test/e2e/app-dir/ppr-errors/ppr.test.ts b/test/e2e/app-dir/ppr-errors/ppr.test.ts new file mode 100644 index 0000000000000..c80353e8f1eab --- /dev/null +++ b/test/e2e/app-dir/ppr-errors/ppr.test.ts @@ -0,0 +1,10 @@ +import { nextBuild } from 'next-test-utils' + +describe('ppr build errors', () => { + it('should fail the build', async () => { + const out = await nextBuild(__dirname, [], { stderr: true }) + expect(out.stderr).toContain( + 'Postpone signal was caught while rendering /. These errors should not be caught during static generation.' + ) + }) +}) diff --git a/test/e2e/app-dir/ppr/ppr.test.ts b/test/e2e/app-dir/ppr/ppr.test.ts index 8950af04b748c..59eb1ddc92e40 100644 --- a/test/e2e/app-dir/ppr/ppr.test.ts +++ b/test/e2e/app-dir/ppr/ppr.test.ts @@ -7,20 +7,6 @@ createNextDescribe( skipDeployment: true, }, ({ next, isNextDev, isNextStart }) => { - if (isNextStart) { - it("should log a warning when `unstable_postpone` is called but there's no postpone state", async () => { - // Three different pages wrap APIs that use `unstable_postpone` in a try catch to trigger these errors - expect(next.cliOutput).toContain( - '/suspense/node/cookies-error-no-throw opted out of partial prerendering because the postpone signal was intercepted by a try/catch in your application code.' - ) - expect(next.cliOutput).toContain( - '/suspense/node/fetch-error opted out of partial prerendering because the postpone signal was intercepted by a try/catch in your application code.' - ) - expect(next.cliOutput).toContain( - '/suspense/node/cookies-error opted out of partial prerendering because the postpone signal was intercepted by a try/catch in your application code.' - ) - }) - } describe.each([ { pathname: '/suspense/node' }, { pathname: '/suspense/node/nested/1' }, From 441735e3dc11c31b4e14e1f44a530fcf3872fafd Mon Sep 17 00:00:00 2001 From: Zack Tanner Date: Wed, 25 Oct 2023 23:00:23 -0700 Subject: [PATCH 3/3] remove stuff --- .../app/suspense/node/cookies-error/page.jsx | 32 ------------------- .../app/suspense/node/fetch-error/page.jsx | 30 ----------------- 2 files changed, 62 deletions(-) delete mode 100644 test/e2e/app-dir/ppr/app/suspense/node/cookies-error/page.jsx delete mode 100644 test/e2e/app-dir/ppr/app/suspense/node/fetch-error/page.jsx diff --git a/test/e2e/app-dir/ppr/app/suspense/node/cookies-error/page.jsx b/test/e2e/app-dir/ppr/app/suspense/node/cookies-error/page.jsx deleted file mode 100644 index 77a3e787ba5a9..0000000000000 --- a/test/e2e/app-dir/ppr/app/suspense/node/cookies-error/page.jsx +++ /dev/null @@ -1,32 +0,0 @@ -import React, { Suspense } from 'react' -import { cookies } from 'next/headers' - -import { Dynamic } from '../../../../components/dynamic' - -export default async function Page() { - return ( - <> -

Dynamic Component Catching Errors

-

- This shows the dynamic component that reads cookies but wraps the read - in a try/catch. -

-
- }> - - - - Loading...
}> - - -
- - ) -} - -async function Foobar() { - try { - cookies() - } catch (err) {} - return null -} diff --git a/test/e2e/app-dir/ppr/app/suspense/node/fetch-error/page.jsx b/test/e2e/app-dir/ppr/app/suspense/node/fetch-error/page.jsx deleted file mode 100644 index dff12e37fe4a2..0000000000000 --- a/test/e2e/app-dir/ppr/app/suspense/node/fetch-error/page.jsx +++ /dev/null @@ -1,30 +0,0 @@ -import React, { Suspense } from 'react' - -import { Dynamic } from '../../../../components/dynamic' - -export default async function Page() { - const getData = () => - fetch('https://example.vercel.sh', { - cache: 'no-store', - }) - .then((res) => res.text()) - .then((text) => new Promise((res) => setTimeout(() => res(text), 1000))) - - try { - await getData() - } catch (err) {} - - return ( - <> -

Dynamic Component Catching Errors

-

- This shows the dynamic component that wraps a data fetch in a try/catch -

-
- }> - - -
- - ) -}