diff --git a/packages/next/src/client/components/react-dev-overlay/app/ReactDevOverlay.tsx b/packages/next/src/client/components/react-dev-overlay/app/ReactDevOverlay.tsx index 75cfeb06eb94b..2b46347d4fa2d 100644 --- a/packages/next/src/client/components/react-dev-overlay/app/ReactDevOverlay.tsx +++ b/packages/next/src/client/components/react-dev-overlay/app/ReactDevOverlay.tsx @@ -1,9 +1,8 @@ import React from 'react' -import { ACTION_UNHANDLED_ERROR, type OverlayState } from '../shared' +import type { OverlayState } from '../shared' import { ShadowPortal } from '../internal/components/ShadowPortal' import { BuildError } from '../internal/container/BuildError' -import { Errors, type SupportedErrorEvent } from '../internal/container/Errors' -import { parseStack } from '../internal/helpers/parse-stack' +import { Errors } from '../internal/container/Errors' import { StaticIndicator } from '../internal/container/StaticIndicator' import { Base } from '../internal/styles/Base' import { ComponentStyles } from '../internal/styles/ComponentStyles' @@ -13,7 +12,7 @@ import type { Dispatcher } from './hot-reloader-client' import { RuntimeErrorHandler } from '../internal/helpers/runtime-error-handler' interface ReactDevOverlayState { - reactError: SupportedErrorEvent | null + isReactError: boolean } export default class ReactDevOverlay extends React.PureComponent< { @@ -23,27 +22,20 @@ export default class ReactDevOverlay extends React.PureComponent< }, ReactDevOverlayState > { - state = { reactError: null } + state = { isReactError: false } static getDerivedStateFromError(error: Error): ReactDevOverlayState { - if (!error.stack) return { reactError: null } + if (!error.stack) return { isReactError: false } RuntimeErrorHandler.hadRuntimeError = true return { - reactError: { - id: 0, - event: { - type: ACTION_UNHANDLED_ERROR, - reason: error, - frames: parseStack(error.stack), - }, - }, + isReactError: true, } } render() { const { state, children, dispatcher } = this.props - const { reactError } = this.state + const { isReactError } = this.state const hasBuildError = state.buildError != null const hasRuntimeErrors = Boolean(state.errors.length) @@ -52,7 +44,7 @@ export default class ReactDevOverlay extends React.PureComponent< return ( <> - {reactError ? ( + {isReactError ? ( @@ -78,8 +70,10 @@ export default class ReactDevOverlay extends React.PureComponent< {hasRuntimeErrors ? ( (err: T): Error | T { Object.assign(newError, err) newError.stack = newStack + // Avoid duplicate overriding stack frames + appendOwnerStack(newError) + + return newError +} + +export function appendOwnerStack(error: Error) { + let stack = error.stack || '' // Avoid duplicate overriding stack frames const ownerStack = (React as any).captureOwnerStack() - if (ownerStack && newStack.endsWith(ownerStack) === false) { - newStack += ownerStack + if (ownerStack && stack.endsWith(ownerStack) === false) { + stack += ownerStack // Override stack - newError.stack = newStack + error.stack = stack } - - return newError } diff --git a/packages/next/src/client/react-client-callbacks/shared.ts b/packages/next/src/client/react-client-callbacks/shared.ts index 645e4b473332e..9347e75a89fd2 100644 --- a/packages/next/src/client/react-client-callbacks/shared.ts +++ b/packages/next/src/client/react-client-callbacks/shared.ts @@ -4,18 +4,21 @@ import type { HydrationOptions } from 'react-dom/client' import { isBailoutToCSRError } from '../../shared/lib/lazy-dynamic/bailout-to-csr' import { reportGlobalError } from './report-global-error' import { getReactStitchedError } from '../components/react-dev-overlay/internal/helpers/stitched-error' +import isError from '../../lib/is-error' export const onRecoverableError: HydrationOptions['onRecoverableError'] = ( - err, + error, errorInfo ) => { - const stitchedError = getReactStitchedError(err) + // x-ref: https://github.com/facebook/react/pull/28736 + const cause = isError(error) && 'cause' in error ? error.cause : error + const stitchedError = getReactStitchedError(cause) // In development mode, pass along the component stack to the error if (process.env.NODE_ENV === 'development' && errorInfo.componentStack) { ;(stitchedError as any)._componentStack = errorInfo.componentStack } // Skip certain custom errors which are not expected to be reported on client - if (isBailoutToCSRError(err)) return + if (isBailoutToCSRError(cause)) return reportGlobalError(stitchedError) } diff --git a/test/development/acceptance-app/dynamic-error.test.ts b/test/development/acceptance-app/dynamic-error.test.ts index 1e7e53583598f..258945892e6b4 100644 --- a/test/development/acceptance-app/dynamic-error.test.ts +++ b/test/development/acceptance-app/dynamic-error.test.ts @@ -7,31 +7,30 @@ import { outdent } from 'outdent' describe('dynamic = "error" in devmode', () => { const { next } = nextTestSetup({ files: new FileRef(path.join(__dirname, 'fixtures', 'default-template')), - skipStart: true, }) it('should show error overlay when dynamic is forced', async () => { - const { session, cleanup } = await sandbox(next, undefined, '/server') - - // dynamic = "error" and force dynamic - await session.patch( - 'app/server/page.js', - outdent` - import { cookies } from 'next/headers'; - - import Component from '../../index' - - export default async function Page() { - await cookies() - return - } - - export const dynamic = "error" - ` + const { session, cleanup } = await sandbox( + next, + new Map([ + [ + 'app/server/page.js', + outdent` + import { cookies } from 'next/headers'; + + export default async function Page() { + await cookies() + return null + } + + export const dynamic = "error" + `, + ], + ]), + '/server' ) await session.assertHasRedbox() - console.log(await session.getRedboxDescription()) expect(await session.getRedboxDescription()).toMatchInlineSnapshot( `"[ Server ] Error: Route /server with \`dynamic = "error"\` couldn't be rendered statically because it used \`cookies\`. See more info here: https://nextjs.org/docs/app/building-your-application/rendering/static-and-dynamic#dynamic-rendering"` ) diff --git a/test/development/app-dir/owner-stack-invalid-element-type/app/browser/browser-only.js b/test/development/app-dir/owner-stack-invalid-element-type/app/browser/browser-only.js new file mode 100644 index 0000000000000..dc00bde91399b --- /dev/null +++ b/test/development/app-dir/owner-stack-invalid-element-type/app/browser/browser-only.js @@ -0,0 +1,11 @@ +'use client' + +import Foo from '../foo' + +export default function BrowserOnly() { + return ( +
+ +
+ ) +} diff --git a/test/development/app-dir/owner-stack-invalid-element-type/app/browser/page.js b/test/development/app-dir/owner-stack-invalid-element-type/app/browser/page.js new file mode 100644 index 0000000000000..77875a1db1096 --- /dev/null +++ b/test/development/app-dir/owner-stack-invalid-element-type/app/browser/page.js @@ -0,0 +1,11 @@ +'use client' + +import dynamic from 'next/dynamic' + +const BrowserOnly = dynamic(() => import('./browser-only'), { + ssr: false, +}) + +export default function Page() { + return +} diff --git a/test/development/app-dir/owner-stack-invalid-element-type/app/foo.js b/test/development/app-dir/owner-stack-invalid-element-type/app/foo.js new file mode 100644 index 0000000000000..e69de29bb2d1d diff --git a/test/development/app-dir/owner-stack-invalid-element-type/app/layout.tsx b/test/development/app-dir/owner-stack-invalid-element-type/app/layout.tsx new file mode 100644 index 0000000000000..888614deda3ba --- /dev/null +++ b/test/development/app-dir/owner-stack-invalid-element-type/app/layout.tsx @@ -0,0 +1,8 @@ +import { ReactNode } from 'react' +export default function Root({ children }: { children: ReactNode }) { + return ( + + {children} + + ) +} diff --git a/test/development/app-dir/owner-stack-invalid-element-type/app/rsc/page.js b/test/development/app-dir/owner-stack-invalid-element-type/app/rsc/page.js new file mode 100644 index 0000000000000..3b835737b3983 --- /dev/null +++ b/test/development/app-dir/owner-stack-invalid-element-type/app/rsc/page.js @@ -0,0 +1,9 @@ +import Foo from '../foo' + +export default function Page() { + return ( +
+ +
+ ) +} diff --git a/test/development/app-dir/owner-stack-invalid-element-type/app/ssr/page.js b/test/development/app-dir/owner-stack-invalid-element-type/app/ssr/page.js new file mode 100644 index 0000000000000..297ed0e3ef260 --- /dev/null +++ b/test/development/app-dir/owner-stack-invalid-element-type/app/ssr/page.js @@ -0,0 +1,11 @@ +'use client' + +import Foo from '../foo' + +export default function Page() { + return ( +
+ +
+ ) +} diff --git a/test/development/app-dir/owner-stack-invalid-element-type/next.config.js b/test/development/app-dir/owner-stack-invalid-element-type/next.config.js new file mode 100644 index 0000000000000..d14b1bf8fdc37 --- /dev/null +++ b/test/development/app-dir/owner-stack-invalid-element-type/next.config.js @@ -0,0 +1,10 @@ +/** + * @type {import('next').NextConfig} + */ +const nextConfig = { + experimental: { + reactOwnerStack: true, + }, +} + +module.exports = nextConfig diff --git a/test/development/app-dir/owner-stack-invalid-element-type/owner-stack-invalid-element-type.test.ts b/test/development/app-dir/owner-stack-invalid-element-type/owner-stack-invalid-element-type.test.ts new file mode 100644 index 0000000000000..6a97c1b257f0d --- /dev/null +++ b/test/development/app-dir/owner-stack-invalid-element-type/owner-stack-invalid-element-type.test.ts @@ -0,0 +1,124 @@ +import { nextTestSetup } from 'e2e-utils' +import { + assertHasRedbox, + getRedboxSource, + getStackFramesContent, +} from 'next-test-utils' + +describe('app-dir - owner-stack-invalid-element-type', () => { + const { next } = nextTestSetup({ + files: __dirname, + }) + + it('should catch invalid element from on client-only component', async () => { + const browser = await next.browser('/browser') + + await assertHasRedbox(browser) + const source = await getRedboxSource(browser) + + const stackFramesContent = await getStackFramesContent(browser) + if (process.env.TURBOPACK) { + expect(stackFramesContent).toMatchInlineSnapshot( + `"at Page (app/browser/page.js (10:10))"` + ) + expect(source).toMatchInlineSnapshot(` + "app/browser/browser-only.js (8:7) @ BrowserOnly + + 6 | return ( + 7 |
+ > 8 | + | ^ + 9 |
+ 10 | ) + 11 | }" + `) + } else { + expect(stackFramesContent).toMatchInlineSnapshot( + `"at BrowserOnly (app/browser/page.js (10:11))"` + ) + expect(source).toMatchInlineSnapshot(` + "app/browser/browser-only.js (8:8) @ Foo + + 6 | return ( + 7 |
+ > 8 | + | ^ + 9 |
+ 10 | ) + 11 | }" + `) + } + }) + + it('should catch invalid element from on rsc component', async () => { + const browser = await next.browser('/rsc') + + await assertHasRedbox(browser) + const stackFramesContent = await getStackFramesContent(browser) + const source = await getRedboxSource(browser) + + if (process.env.TURBOPACK) { + expect(stackFramesContent).toMatchInlineSnapshot(`""`) + expect(source).toMatchInlineSnapshot(` + "app/rsc/page.js (6:7) @ Page + + 4 | return ( + 5 |
+ > 6 | + | ^ + 7 |
+ 8 | ) + 9 | }" + `) + } else { + expect(stackFramesContent).toMatchInlineSnapshot(`""`) + expect(source).toMatchInlineSnapshot(` + "app/rsc/page.js (6:8) @ Foo + + 4 | return ( + 5 |
+ > 6 | + | ^ + 7 |
+ 8 | ) + 9 | }" + `) + } + }) + + it('should catch invalid element from on ssr client component', async () => { + const browser = await next.browser('/ssr') + + await assertHasRedbox(browser) + + const stackFramesContent = await getStackFramesContent(browser) + const source = await getRedboxSource(browser) + if (process.env.TURBOPACK) { + expect(stackFramesContent).toMatchInlineSnapshot(`""`) + expect(source).toMatchInlineSnapshot(` + "app/ssr/page.js (8:7) @ Page + + 6 | return ( + 7 |
+ > 8 | + | ^ + 9 |
+ 10 | ) + 11 | }" + `) + } else { + expect(stackFramesContent).toMatchInlineSnapshot(`""`) + expect(source).toMatchInlineSnapshot(` + "app/ssr/page.js (8:8) @ Foo + + 6 | return ( + 7 |
+ > 8 | + | ^ + 9 |
+ 10 | ) + 11 | }" + `) + } + }) +}) diff --git a/test/development/app-dir/owner-stack-react-missing-key-prop/app/layout.tsx b/test/development/app-dir/owner-stack-react-missing-key-prop/app/layout.tsx new file mode 100644 index 0000000000000..888614deda3ba --- /dev/null +++ b/test/development/app-dir/owner-stack-react-missing-key-prop/app/layout.tsx @@ -0,0 +1,8 @@ +import { ReactNode } from 'react' +export default function Root({ children }: { children: ReactNode }) { + return ( + + {children} + + ) +} diff --git a/test/development/app-dir/owner-stack-react-missing-key-prop/app/rsc/page.tsx b/test/development/app-dir/owner-stack-react-missing-key-prop/app/rsc/page.tsx new file mode 100644 index 0000000000000..8b07887166394 --- /dev/null +++ b/test/development/app-dir/owner-stack-react-missing-key-prop/app/rsc/page.tsx @@ -0,0 +1,11 @@ +const list = [1, 2, 3] + +export default function Page() { + return ( +
+ {list.map((item, index) => ( + {item} + ))} +
+ ) +} diff --git a/test/development/app-dir/owner-stack-react-missing-key-prop/app/ssr/page.tsx b/test/development/app-dir/owner-stack-react-missing-key-prop/app/ssr/page.tsx new file mode 100644 index 0000000000000..eea0bc9a463c9 --- /dev/null +++ b/test/development/app-dir/owner-stack-react-missing-key-prop/app/ssr/page.tsx @@ -0,0 +1,13 @@ +'use client' + +const list = [1, 2, 3] + +export default function Page() { + return ( +
+ {list.map((item, index) => ( +

{item}

+ ))} +
+ ) +} diff --git a/test/development/app-dir/owner-stack-react-missing-key-prop/next.config.js b/test/development/app-dir/owner-stack-react-missing-key-prop/next.config.js new file mode 100644 index 0000000000000..d14b1bf8fdc37 --- /dev/null +++ b/test/development/app-dir/owner-stack-react-missing-key-prop/next.config.js @@ -0,0 +1,10 @@ +/** + * @type {import('next').NextConfig} + */ +const nextConfig = { + experimental: { + reactOwnerStack: true, + }, +} + +module.exports = nextConfig diff --git a/test/development/app-dir/owner-stack-react-missing-key-prop/owner-stack-react-missing-key-prop.test.ts b/test/development/app-dir/owner-stack-react-missing-key-prop/owner-stack-react-missing-key-prop.test.ts new file mode 100644 index 0000000000000..2ba38962cbf38 --- /dev/null +++ b/test/development/app-dir/owner-stack-react-missing-key-prop/owner-stack-react-missing-key-prop.test.ts @@ -0,0 +1,91 @@ +import { nextTestSetup } from 'e2e-utils' +import { + getRedboxSource, + waitForAndOpenRuntimeError, + getStackFramesContent, +} from 'next-test-utils' + +describe('owner-stack-react-missing-key-prop', () => { + const { next } = nextTestSetup({ + files: __dirname, + }) + + it('should catch invalid element from on rsc component', async () => { + const browser = await next.browser('/rsc') + await waitForAndOpenRuntimeError(browser) + + const stackFramesContent = await getStackFramesContent(browser) + const source = await getRedboxSource(browser) + + if (process.env.TURBOPACK) { + expect(stackFramesContent).toMatchInlineSnapshot( + `"at Page (app/rsc/page.tsx (6:13))"` + ) + expect(source).toMatchInlineSnapshot(` + "app/rsc/page.tsx (7:9) @ + + 5 |
+ 6 | {list.map((item, index) => ( + > 7 | {item} + | ^ + 8 | ))} + 9 |
+ 10 | )" + `) + } else { + expect(stackFramesContent).toMatchInlineSnapshot( + `"at map (app/rsc/page.tsx (6:13))"` + ) + expect(source).toMatchInlineSnapshot(` + "app/rsc/page.tsx (7:10) @ span + + 5 |
+ 6 | {list.map((item, index) => ( + > 7 | {item} + | ^ + 8 | ))} + 9 |
+ 10 | )" + `) + } + }) + + it('should catch invalid element from on ssr client component', async () => { + const browser = await next.browser('/ssr') + await waitForAndOpenRuntimeError(browser) + + const stackFramesContent = await getStackFramesContent(browser) + const source = await getRedboxSource(browser) + if (process.env.TURBOPACK) { + expect(stackFramesContent).toMatchInlineSnapshot( + `"at Page (app/ssr/page.tsx (8:13))"` + ) + expect(source).toMatchInlineSnapshot(` + "app/ssr/page.tsx (9:9) @ + + 7 |
+ 8 | {list.map((item, index) => ( + > 9 |

{item}

+ | ^ + 10 | ))} + 11 |
+ 12 | )" + `) + } else { + expect(stackFramesContent).toMatchInlineSnapshot( + `"at map (app/ssr/page.tsx (8:13))"` + ) + expect(source).toMatchInlineSnapshot(` + "app/ssr/page.tsx (9:10) @ p + + 7 |
+ 8 | {list.map((item, index) => ( + > 9 |

{item}

+ | ^ + 10 | ))} + 11 |
+ 12 | )" + `) + } + }) +}) diff --git a/test/development/app-dir/owner-stack/owner-stack.test.ts b/test/development/app-dir/owner-stack/owner-stack.test.ts index 1257740989d9d..db1b383339ff6 100644 --- a/test/development/app-dir/owner-stack/owner-stack.test.ts +++ b/test/development/app-dir/owner-stack/owner-stack.test.ts @@ -4,6 +4,7 @@ import { assertNoRedbox, waitForAndOpenRuntimeError, getRedboxDescription, + getStackFramesContent, } from 'next-test-utils' // TODO: parse the location and assert them in the future @@ -17,33 +18,6 @@ function normalizeStackTrace(trace: string) { .trim() } -async function getStackFramesContent(browser) { - const stackFrameElements = await browser.elementsByCss( - '[data-nextjs-call-stack-frame]' - ) - const stackFramesContent = ( - await Promise.all( - stackFrameElements.map(async (frame) => { - const functionNameEl = await frame.$('[data-nextjs-frame-expanded]') - const sourceEl = await frame.$('[data-has-source]') - const functionName = functionNameEl - ? await functionNameEl.innerText() - : '' - const source = sourceEl ? await sourceEl.innerText() : '' - - if (!functionName) { - return '' - } - return `at ${functionName} (${source})` - }) - ) - ) - .filter(Boolean) - .join('\n') - - return stackFramesContent -} - describe('app-dir - owner-stack', () => { const { next } = nextTestSetup({ files: __dirname, diff --git a/test/lib/next-test-utils.ts b/test/lib/next-test-utils.ts index 0304d2464cbea..81977645a77c8 100644 --- a/test/lib/next-test-utils.ts +++ b/test/lib/next-test-utils.ts @@ -1492,3 +1492,32 @@ export const checkLink = ( rel: string, content: string | string[] ) => checkMeta(browser, rel, content, 'rel', 'link', 'href') + +export async function getStackFramesContent( + browser: BrowserInterface +): Promise { + const stackFrameElements = await browser.elementsByCss( + '[data-nextjs-call-stack-frame]' + ) + const stackFramesContent = ( + await Promise.all( + stackFrameElements.map(async (frame) => { + const functionNameEl = await frame.$('[data-nextjs-frame-expanded]') + const sourceEl = await frame.$('[data-has-source]') + const functionName = functionNameEl + ? await functionNameEl.innerText() + : '' + const source = sourceEl ? await sourceEl.innerText() : '' + + if (!functionName) { + return '' + } + return `at ${functionName} (${source})` + }) + ) + ) + .filter(Boolean) + .join('\n') + + return stackFramesContent +}