From 0731c439913bb3ec8a8cb1619be5d3fbcf699ef8 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Fri, 26 Jan 2024 13:05:20 +0100 Subject: [PATCH] Add stack trace to client rendering bailout error --- packages/next/src/lib/format-server-error.ts | 16 +++++++++++ .../next/src/server/app-render/app-render.tsx | 4 ++- .../app-render/create-error-handler.tsx | 3 +-- .../app/layout-no-suspense.js | 8 ------ .../app/layout-suspense.js | 12 +++++++++ .../app/layout.js | 6 +---- .../app/page.js | 13 +++++++-- .../missing-suspense-with-csr-bailout.test.ts | 27 ++++++++++--------- 8 files changed, 59 insertions(+), 30 deletions(-) delete mode 100644 test/e2e/app-dir/missing-suspense-with-csr-bailout/app/layout-no-suspense.js create mode 100644 test/e2e/app-dir/missing-suspense-with-csr-bailout/app/layout-suspense.js diff --git a/packages/next/src/lib/format-server-error.ts b/packages/next/src/lib/format-server-error.ts index 6723b9125d11e..3604ad67133ae 100644 --- a/packages/next/src/lib/format-server-error.ts +++ b/packages/next/src/lib/format-server-error.ts @@ -22,6 +22,22 @@ function setMessage(error: Error, message: string): void { } } +/** + * Input: + * Error: Something went wrong + at funcName (/path/to/file.js:10:5) + at anotherFunc (/path/to/file.js:15:10) + + * Output: + at funcName (/path/to/file.js:10:5) + at anotherFunc (/path/to/file.js:15:10) + */ +export function getStackWithoutErrorMessage(error: Error): string { + const stack = error.stack + if (!stack) return '' + return stack.replace(/^[^\n]*\n/, '') +} + export function formatServerError(error: Error): void { if (typeof error?.message !== 'string') return diff --git a/packages/next/src/server/app-render/app-render.tsx b/packages/next/src/server/app-render/app-render.tsx index 7642827f4f4ac..2ee8dd6259d9c 100644 --- a/packages/next/src/server/app-render/app-render.tsx +++ b/packages/next/src/server/app-render/app-render.tsx @@ -81,6 +81,7 @@ import { isDynamicServerError } from '../../client/components/hooks-server-conte import { useFlightResponse } from './use-flight-response' import { isStaticGenBailoutError } from '../../client/components/static-generation-bailout' import { isInterceptionRouteAppPath } from '../future/helpers/interception-routes' +import { getStackWithoutErrorMessage } from '../../lib/format-server-error' export type GetDynamicParamFromSegment = ( // [slug] / [[slug]] / [...slug] @@ -1014,8 +1015,9 @@ async function renderToHTMLOrFlightImpl( console.log() if (renderOpts.experimental.missingSuspenseWithCSRBailout) { + const stack = getStackWithoutErrorMessage(err) error( - `${err.reason} should be wrapped in a suspense boundary at page "${pagePath}". Read more: https://nextjs.org/docs/messages/missing-suspense-with-csr-bailout` + `${err.reason} should be wrapped in a suspense boundary at page "${pagePath}". Read more: https://nextjs.org/docs/messages/missing-suspense-with-csr-bailout\n${stack}` ) throw err diff --git a/packages/next/src/server/app-render/create-error-handler.tsx b/packages/next/src/server/app-render/create-error-handler.tsx index fde69072ac555..88c717dea9c07 100644 --- a/packages/next/src/server/app-render/create-error-handler.tsx +++ b/packages/next/src/server/app-render/create-error-handler.tsx @@ -77,8 +77,7 @@ export function createErrorHandler({ const { logAppDirError } = require('../dev/log-app-dir-error') as typeof import('../dev/log-app-dir-error') logAppDirError(err) - } - if (process.env.NODE_ENV === 'production') { + } else { console.error(err) } } diff --git a/test/e2e/app-dir/missing-suspense-with-csr-bailout/app/layout-no-suspense.js b/test/e2e/app-dir/missing-suspense-with-csr-bailout/app/layout-no-suspense.js deleted file mode 100644 index f3791f288f9d7..0000000000000 --- a/test/e2e/app-dir/missing-suspense-with-csr-bailout/app/layout-no-suspense.js +++ /dev/null @@ -1,8 +0,0 @@ -export default function Layout({ children }) { - return ( - - - {children} - - ) -} diff --git a/test/e2e/app-dir/missing-suspense-with-csr-bailout/app/layout-suspense.js b/test/e2e/app-dir/missing-suspense-with-csr-bailout/app/layout-suspense.js new file mode 100644 index 0000000000000..6e39f296f6568 --- /dev/null +++ b/test/e2e/app-dir/missing-suspense-with-csr-bailout/app/layout-suspense.js @@ -0,0 +1,12 @@ +import { Suspense } from 'react' + +export default function Layout({ children }) { + return ( + + + + Loading...}>{children} + + + ) +} diff --git a/test/e2e/app-dir/missing-suspense-with-csr-bailout/app/layout.js b/test/e2e/app-dir/missing-suspense-with-csr-bailout/app/layout.js index 6e39f296f6568..f3791f288f9d7 100644 --- a/test/e2e/app-dir/missing-suspense-with-csr-bailout/app/layout.js +++ b/test/e2e/app-dir/missing-suspense-with-csr-bailout/app/layout.js @@ -1,12 +1,8 @@ -import { Suspense } from 'react' - export default function Layout({ children }) { return ( - - Loading...}>{children} - + {children} ) } diff --git a/test/e2e/app-dir/missing-suspense-with-csr-bailout/app/page.js b/test/e2e/app-dir/missing-suspense-with-csr-bailout/app/page.js index db29d22f58baf..b406393369d77 100644 --- a/test/e2e/app-dir/missing-suspense-with-csr-bailout/app/page.js +++ b/test/e2e/app-dir/missing-suspense-with-csr-bailout/app/page.js @@ -2,7 +2,16 @@ import { useSearchParams } from 'next/navigation' -export default function Page() { +function SearchParams() { useSearchParams() - return
Page
+ return null +} + +export default function Page() { + return ( + <> + +
Page
+ + ) } diff --git a/test/e2e/app-dir/missing-suspense-with-csr-bailout/missing-suspense-with-csr-bailout.test.ts b/test/e2e/app-dir/missing-suspense-with-csr-bailout/missing-suspense-with-csr-bailout.test.ts index 3d8bf4402fb53..326ef4e909a29 100644 --- a/test/e2e/app-dir/missing-suspense-with-csr-bailout/missing-suspense-with-csr-bailout.test.ts +++ b/test/e2e/app-dir/missing-suspense-with-csr-bailout/missing-suspense-with-csr-bailout.test.ts @@ -19,31 +19,36 @@ createNextDescribe( describe('useSearchParams', () => { const message = `useSearchParams() should be wrapped in a suspense boundary at page "/".` + it('should fail build if useSearchParams is not wrapped in a suspense boundary', async () => { + const { exitCode } = await next.build() + expect(exitCode).toBe(1) + expect(next.cliOutput).toContain(message) + // Can show the trace where the searchParams hook is used + expect(next.cliOutput).toMatch(/at.*server[\\/]app[\\/]page.js/) + }) + it('should pass build if useSearchParams is wrapped in a suspense boundary', async () => { + await next.renameFile('app/layout.js', 'app/layout-no-suspense.js') + await next.renameFile('app/layout-suspense.js', 'app/layout.js') + await expect(next.build()).resolves.toEqual({ exitCode: 0, cliOutput: expect.not.stringContaining(message), }) - }) - it('should fail build if useSearchParams is not wrapped in a suspense boundary', async () => { await next.renameFile('app/layout.js', 'app/layout-suspense.js') await next.renameFile('app/layout-no-suspense.js', 'app/layout.js') - - await expect(next.build()).resolves.toEqual({ - exitCode: 1, - cliOutput: expect.stringContaining(message), - }) - - await next.renameFile('app/layout.js', 'app/layout-no-suspense.js') - await next.renameFile('app/layout-suspense.js', 'app/layout.js') }) }) describe('next/dynamic', () => { beforeEach(async () => { + await next.renameFile('app/page.js', 'app/_page.js') await next.start() }) + afterEach(async () => { + await next.renameFile('app/_page.js', 'app/page.js') + }) it('does not emit errors related to bailing out of client side rendering', async () => { const browser = await next.browser('/dynamic', { @@ -53,8 +58,6 @@ createNextDescribe( try { await browser.waitForElementByCss('#dynamic') - // await new Promise((resolve) => setTimeout(resolve, 1000)) - expect(await browser.log()).not.toContainEqual( expect.objectContaining({ source: 'error',