Display the stitched error instead of react error (vercel#72106)

huozhi · stipsan · commit 1c8cc6df449a · 2024-11-07T00:02:04.000+01:00
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
@@ -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 ? (
           <html>
             <head></head>
             <body></body>
@@ -78,8 +70,10 @@ export default class ReactDevOverlay extends React.PureComponent<
               {hasRuntimeErrors ? (
                 <Errors
                   isAppDir={true}
-                  initialDisplayState={reactError ? 'fullscreen' : 'minimized'}
-                  errors={reactError ? [reactError] : state.errors}
+                  initialDisplayState={
+                    isReactError ? 'fullscreen' : 'minimized'
+                  }
+                  errors={state.errors}
                   versionInfo={state.versionInfo}
                   hasStaticIndicator={hasStaticIndicator}
                   debugInfo={debugInfo}
diff --git 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
@@ -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 <Component />
-        }
-
-        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
@@ -0,0 +1,11 @@
+'use client'
+
+import Foo from '../foo'
+
+export default function BrowserOnly() {
+  return (
+    <div>
+      <Foo />
+    </div>
+  )
+}
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
@@ -0,0 +1,16 @@
+'use client'
+
+import dynamic from 'next/dynamic'
+
+const BrowserOnly = dynamic(() => import('./browser-only'), {
+  ssr: false,
+})
+
+// Intermediate component for testing owner stack
+function Inner() {
+  return <BrowserOnly />
+}
+
+export default function Page() {
+  return <Inner />
+}
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
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
@@ -0,0 +1,8 @@
+import { ReactNode } from 'react'
+export default function Root({ children }: { children: ReactNode }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}
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
@@ -0,0 +1,14 @@
+import Foo from '../foo'
+
+// Intermediate component for testing owner stack
+function Inner() {
+  return <Foo />
+}
+
+export default function Page() {
+  return (
+    <div>
+      <Inner />
+    </div>
+  )
+}
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
@@ -0,0 +1,16 @@
+'use client'
+
+import Foo from '../foo'
+
+// Intermediate component for testing owner stack
+function Inner() {
+  return <Foo />
+}
+
+export default function Page() {
+  return (
+    <div>
+      <Inner />
+    </div>
+  )
+}
diff --git a/test/development/app-dir/owner-stack-invalid-element-type/invalid-element-type.test.ts b/test/development/app-dir/owner-stack-invalid-element-type/invalid-element-type.test.ts
@@ -0,0 +1,133 @@
+process.env.TEST_OWNER_STACK = 'false'
+
+import { nextTestSetup } from 'e2e-utils'
+import {
+  assertHasRedbox,
+  getRedboxSource,
+  getStackFramesContent,
+} from 'next-test-utils'
+
+// TODO: When owner stack is enabled by default, remove the condition and only keep one test
+const isOwnerStackEnabled =
+  process.env.TEST_OWNER_STACK !== 'false' ||
+  process.env.__NEXT_EXPERIMENTAL_PPR === 'true'
+
+;(isOwnerStackEnabled ? describe.skip : describe)(
+  'app-dir - invalid-element-type',
+  () => {
+    const { next } = nextTestSetup({
+      files: __dirname,
+    })
+
+    it('should catch invalid element from a browser 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(`""`)
+        expect(source).toMatchInlineSnapshot(`
+          "app/browser/browser-only.js (8:7) @ BrowserOnly
+
+             6 |   return (
+             7 |     <div>
+          >  8 |       <Foo />
+               |       ^
+             9 |     </div>
+            10 |   )
+            11 | }"
+        `)
+      } else {
+        expect(stackFramesContent).toMatchInlineSnapshot(`""`)
+        // FIXME: the methodName should be `@ BrowserOnly` instead of `@ Foo`
+        expect(source).toMatchInlineSnapshot(`
+          "app/browser/browser-only.js (8:8) @ Foo
+
+             6 |   return (
+             7 |     <div>
+          >  8 |       <Foo />
+               |        ^
+             9 |     </div>
+            10 |   )
+            11 | }"
+        `)
+      }
+    })
+
+    it('should catch invalid element from a 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 (5:10) @ Inner
+
+            3 | // Intermediate component for testing owner stack
+            4 | function Inner() {
+          > 5 |   return <Foo />
+              |          ^
+            6 | }
+            7 |
+            8 | export default function Page() {"
+        `)
+      } else {
+        expect(stackFramesContent).toMatchInlineSnapshot(`""`)
+        // FIXME: the methodName should be `@ Inner` instead of `@ Foo`
+        expect(source).toMatchInlineSnapshot(`
+          "app/rsc/page.js (5:11) @ Foo
+
+            3 | // Intermediate component for testing owner stack
+            4 | function Inner() {
+          > 5 |   return <Foo />
+              |           ^
+            6 | }
+            7 |
+            8 | export default function Page() {"
+        `)
+      }
+    })
+
+    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 (7:10) @ Inner
+
+             5 | // Intermediate component for testing owner stack
+             6 | function Inner() {
+          >  7 |   return <Foo />
+               |          ^
+             8 | }
+             9 |
+            10 | export default function Page() {"
+        `)
+      } else {
+        expect(stackFramesContent).toMatchInlineSnapshot(`""`)
+        // FIXME: the methodName should be `@ Inner` instead of `@ Foo`
+        expect(source).toMatchInlineSnapshot(`
+          "app/ssr/page.js (7:11) @ Foo
+
+             5 | // Intermediate component for testing owner stack
+             6 | function Inner() {
+          >  7 |   return <Foo />
+               |           ^
+             8 | }
+             9 |
+            10 | export default function Page() {"
+        `)
+      }
+    })
+  }
+)
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
@@ -0,0 +1,10 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {
+  experimental: {
+    reactOwnerStack: process.env.TEST_OWNER_STACK !== 'false',
+  },
+}
+
+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
diff --git a/test/lib/next-test-utils.ts b/test/lib/next-test-utils.ts
