fix: handle server component replay error in error overlay (#71772)

### What When there's a replay console error showed up, display it in error overlay. #### Server Component Page <img width="480" alt="image" src="https://github.com/user-attachments/assets/7af65809-1e0e-4aa0-bffb-bc95a376fb02"> #### Server Component Page with Client Components <img width="480" alt="image" src="https://github.com/user-attachments/assets/1f8bd507-3e0f-43f3-9240-07c4aec16ab4"> --------- Co-authored-by: Sebastian Markbage <[email protected]>
vercel · Oct 24, 2024 · ab1d9ee · ab1d9ee
1 parent 0dce6bb
commit ab1d9ee
Show file tree

Hide file tree

Showing 9 changed files with 110 additions and 37 deletions.
diff --git a/packages/next/src/client/components/globals/intercept-console-error.ts b/packages/next/src/client/components/globals/intercept-console-error.ts
@@ -13,13 +13,11 @@ export function patchConsoleError() {
 
   window.console.error = (...args: any[]) => {
     let maybeError: unknown
-    let isReplayed: boolean = false
 
     if (process.env.NODE_ENV !== 'production') {
       const replayedError = matchReplayedError(...args)
       if (replayedError) {
         maybeError = replayedError
-        isReplayed = true
       } else {
         // See https://github.com/facebook/react/blob/d50323eb845c5fde0d720cae888bf35dedd05506/packages/react-reconciler/src/ReactFiberErrorLogger.js#L78
         maybeError = args[1]
@@ -33,10 +31,7 @@ export function patchConsoleError() {
         handleClientError(
           // replayed errors have their own complex format string that should be used,
           // but if we pass the error directly, `handleClientError` will ignore it
-          //
-          // TODO: not passing an error here will make `handleClientError`
-          // create a new Error, so we'll lose the stack. we should make it smarter
-          isReplayed ? undefined : maybeError,
+          maybeError,
           args
         )
       }

diff --git a/packages/next/src/client/components/react-dev-overlay/internal/container/Errors.tsx b/packages/next/src/client/components/react-dev-overlay/internal/container/Errors.tsx
@@ -61,11 +61,22 @@ function ErrorDescription({
   error: Error
   hydrationWarning: string | null
 }) {
-  const isUnhandledError = isUnhandledConsoleOrRejection(error)
-  // If there's hydration warning or console error, skip displaying the error name
+  const isUnhandledOrReplayError = isUnhandledConsoleOrRejection(error)
+  // If the error is:
+  // - hydration warning
+  // - captured console error or unhandled rejection
+  // skip displaying the error name
+  const title =
+    isUnhandledOrReplayError || hydrationWarning ? '' : error.name + ': '
+
+  // If it's replayed error, display the environment name
+  const environmentName =
+    'environmentName' in error ? error['environmentName'] : ''
+  const envPrefix = environmentName ? `[ ${environmentName} ] ` : ''
   return (
     <>
-      {isUnhandledError || hydrationWarning ? '' : error.name + ': '}
+      {envPrefix}
+      {title}
       <HotlinkedText
         text={hydrationWarning || error.message}
         matcher={isNextjsLink}

diff --git a/packages/next/src/client/components/react-dev-overlay/internal/helpers/stack-frame.ts b/packages/next/src/client/components/react-dev-overlay/internal/helpers/stack-frame.ts
@@ -1,6 +1,9 @@
 import type { StackFrame } from 'next/dist/compiled/stacktrace-parser'
 import type { OriginalStackFrameResponse } from '../../server/shared'
-import { isWebpackBundled, formatFrameSourceFile } from './webpack-module-path'
+import {
+  isWebpackInternalResource,
+  formatFrameSourceFile,
+} from './webpack-module-path'
 export interface OriginalStackFrame extends OriginalStackFrameResponse {
   error: boolean
   reason: string | null
@@ -106,7 +109,7 @@ export function getOriginalStackFrames(
 export function getFrameSource(frame: StackFrame): string {
   if (!frame.file) return ''
 
-  const isWebpackFrame = isWebpackBundled(frame.file)
+  const isWebpackFrame = isWebpackInternalResource(frame.file)
 
   let str = ''
   // Skip URL parsing for webpack internal file paths.
@@ -137,11 +140,18 @@ export function getFrameSource(frame: StackFrame): string {
     }
   }
 
-  if (!isWebpackBundled(frame.file) && frame.lineNumber != null) {
-    if (frame.column != null) {
-      str += ` (${frame.lineNumber}:${frame.column})`
-    } else {
-      str += ` (${frame.lineNumber})`
+  if (!isWebpackInternalResource(frame.file) && frame.lineNumber != null) {
+    // If the method name is replayed from server, e.g. Page [Server],
+    // and it's formatted to empty string, recover it as <anonymous> to display it.
+    if (!str && frame.methodName.endsWith(' [Server]')) {
+      str = '<anonymous>'
+    }
+    if (str) {
+      if (frame.column != null) {
+        str += ` (${frame.lineNumber}:${frame.column})`
+      } else {
+        str += ` (${frame.lineNumber})`
+      }
     }
   }
   return str

diff --git a/...next/src/client/components/react-dev-overlay/internal/helpers/webpack-module-path.test.ts b/...next/src/client/components/react-dev-overlay/internal/helpers/webpack-module-path.test.ts
@@ -1,32 +1,44 @@
-import { formatFrameSourceFile, isWebpackBundled } from './webpack-module-path'
+import {
+  formatFrameSourceFile,
+  isWebpackInternalResource,
+} from './webpack-module-path'
 
 describe('webpack-module-path', () => {
-  describe('isWebpackBundled', () => {
+  describe('isWebpackInternalResource', () => {
     it('should return true for webpack-internal paths', () => {
-      expect(isWebpackBundled('webpack-internal:///./src/hello.tsx')).toBe(true)
       expect(
-        isWebpackBundled(
+        isWebpackInternalResource('webpack-internal:///./src/hello.tsx')
+      ).toBe(true)
+      expect(
+        isWebpackInternalResource(
           'rsc://React/Server/webpack-internal:///(rsc)/./src/hello.tsx?42'
         )
       ).toBe(true)
       expect(
-        isWebpackBundled(
+        isWebpackInternalResource(
           'rsc://React/Server/webpack:///(rsc)/./src/hello.tsx?42'
         )
       ).toBe(true)
       expect(
-        isWebpackBundled(
+        isWebpackInternalResource(
           'rsc://React/Server/webpack:///(app-pages-browser)/./src/hello.tsx?42'
         )
       ).toBe(true)
-      expect(isWebpackBundled('webpack://_N_E/./src/hello.tsx')).toBe(true)
-      expect(isWebpackBundled('webpack://./src/hello.tsx')).toBe(true)
-      expect(isWebpackBundled('webpack:///./src/hello.tsx')).toBe(true)
+      expect(
+        isWebpackInternalResource(
+          'rsc://React/Server/webpack:///(app-pages-browser)/./src/hello.tsx?42dc'
+        )
+      ).toBe(true)
+      expect(isWebpackInternalResource('webpack://_N_E/./src/hello.tsx')).toBe(
+        true
+      )
+      expect(isWebpackInternalResource('webpack://./src/hello.tsx')).toBe(true)
+      expect(isWebpackInternalResource('webpack:///./src/hello.tsx')).toBe(true)
     })
 
     it('should return false for non-webpack-internal paths', () => {
-      expect(isWebpackBundled('<anonymous>')).toBe(false)
-      expect(isWebpackBundled('file:///src/hello.tsx')).toBe(false)
+      expect(isWebpackInternalResource('<anonymous>')).toBe(false)
+      expect(isWebpackInternalResource('file:///src/hello.tsx')).toBe(false)
     })
   })
 
@@ -50,6 +62,16 @@ describe('webpack-module-path', () => {
           'rsc://React/Server/webpack:///(app-pages-browser)/./src/hello.tsx?42'
         )
       ).toBe('./src/hello.tsx')
+      expect(
+        formatFrameSourceFile(
+          'rsc://React/Server/webpack:///(app-pages-browser)/./src/hello.tsx?42?0'
+        )
+      ).toBe('./src/hello.tsx')
+      expect(
+        formatFrameSourceFile(
+          'rsc://React/Server/webpack:///(app-pages-browser)/./src/hello.tsx?42dc'
+        )
+      ).toBe('./src/hello.tsx')
       expect(formatFrameSourceFile('webpack://_N_E/./src/hello.tsx')).toBe(
         './src/hello.tsx'
       )

diff --git a/...ages/next/src/client/components/react-dev-overlay/internal/helpers/webpack-module-path.ts b/...ages/next/src/client/components/react-dev-overlay/internal/helpers/webpack-module-path.ts
@@ -2,10 +2,11 @@ const replacementRegExes = [
   /^(rsc:\/\/React\/[^/]+\/)/,
   /^webpack-internal:\/\/\/(\([\w-]+\)\/)?/,
   /^(webpack:\/\/\/|webpack:\/\/(_N_E\/)?)(\([\w-]+\)\/)?/,
+  /\?\w+(\?\d+)?$/, // React replay error query param, .e.g. ?c69d?0, ?c69d
   /\?\d+$/, // React's fakeFunctionIdx query param
 ]
 
-export function isWebpackBundled(file: string) {
+export function isWebpackInternalResource(file: string) {
   for (const regex of replacementRegExes) {
     if (regex.test(file)) return true
 
@@ -19,6 +20,7 @@ export function isWebpackBundled(file: string) {
  * Format the webpack internal id to original file path
  * webpack-internal:///./src/hello.tsx => ./src/hello.tsx
  * rsc://React/Server/webpack-internal:///(rsc)/./src/hello.tsx?42 => ./src/hello.tsx
+ * rsc://React/Server/webpack:///app/indirection.tsx?14cb?0 => app/indirection.tsx
  * webpack://_N_E/./src/hello.tsx => ./src/hello.tsx
  * webpack://./src/hello.tsx => ./src/hello.tsx
  * webpack:///./src/hello.tsx => ./src/hello.tsx

diff --git a/test/development/acceptance-app/dynamic-error.test.ts b/test/development/acceptance-app/dynamic-error.test.ts
@@ -33,7 +33,7 @@ describe('dynamic = "error" in devmode', () => {
     await session.assertHasRedbox()
     console.log(await session.getRedboxDescription())
     expect(await session.getRedboxDescription()).toMatchInlineSnapshot(
-      `"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"`
+      `"[ 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"`
     )
 
     await cleanup()

diff --git a/test/development/app-dir/capture-console-error/capture-console-error.test.ts b/test/development/app-dir/capture-console-error/capture-console-error.test.ts
@@ -196,12 +196,12 @@ describe('app-dir - capture-console-error', () => {
         {
           "callStacks": "",
           "count": 1,
-          "description": "[ Server ]  Error: boom",
-          "source": "app/rsc/page.js (2:11) @ Page
+          "description": "[ Server ] Error: boom",
+          "source": "app/rsc/page.js (2:17) @ Page
 
           1 | export default function Page() {
         > 2 |   console.error(new Error('boom'))
-            |           ^
+            |                 ^
           3 |   return <p>rsc</p>
           4 | }
           5 |",
@@ -212,12 +212,12 @@ describe('app-dir - capture-console-error', () => {
         {
           "callStacks": "",
           "count": 1,
-          "description": "[ Server ]  Error: boom",
-          "source": "app/rsc/page.js (2:11) @ error
+          "description": "[ Server ] Error: boom",
+          "source": "app/rsc/page.js (2:17) @ Page
 
           1 | export default function Page() {
         > 2 |   console.error(new Error('boom'))
-            |           ^
+            |                 ^
           3 |   return <p>rsc</p>
           4 | }
           5 |",

diff --git a/test/development/app-dir/dynamic-io-dev-errors/app/no-accessed-data/page.js b/test/development/app-dir/dynamic-io-dev-errors/app/no-accessed-data/page.js
@@ -0,0 +1,4 @@
+export default async function Page() {
+  await new Promise((r) => setTimeout(r, 200))
+  return <p>Page</p>
+}
diff --git a/test/development/app-dir/dynamic-io-dev-errors/dynamic-io-dev-errors.test.ts b/test/development/app-dir/dynamic-io-dev-errors/dynamic-io-dev-errors.test.ts
@@ -1,5 +1,7 @@
 import { nextTestSetup } from 'e2e-utils'
 import {
+  assertHasRedbox,
+  getRedboxCallStack,
   getRedboxDescription,
   hasErrorToast,
   retry,
@@ -20,7 +22,7 @@ describe('Dynamic IO Dev Errors', () => {
       await waitForAndOpenRuntimeError(browser)
 
       expect(await getRedboxDescription(browser)).toMatchInlineSnapshot(
-        `"[ Server ]  Error: Route "/error" used \`Math.random()\` outside of \`"use cache"\` and without explicitly calling \`await connection()\` beforehand. See more info here: https://nextjs.org/docs/messages/next-prerender-random"`
+        `"[ Server ] Error: Route "/error" used \`Math.random()\` outside of \`"use cache"\` and without explicitly calling \`await connection()\` beforehand. See more info here: https://nextjs.org/docs/messages/next-prerender-random"`
       )
     })
   })
@@ -38,8 +40,35 @@ describe('Dynamic IO Dev Errors', () => {
       await waitForAndOpenRuntimeError(browser)
 
       expect(await getRedboxDescription(browser)).toMatchInlineSnapshot(
-        `"[ Server ]  Error: Route "/error" used \`Math.random()\` outside of \`"use cache"\` and without explicitly calling \`await connection()\` beforehand. See more info here: https://nextjs.org/docs/messages/next-prerender-random"`
+        `"[ Server ] Error: Route "/error" used \`Math.random()\` outside of \`"use cache"\` and without explicitly calling \`await connection()\` beforehand. See more info here: https://nextjs.org/docs/messages/next-prerender-random"`
       )
     })
   })
+
+  it('should display error when component accessed data without suspense boundary', async () => {
+    const browser = await next.browser('/no-accessed-data')
+
+    await retry(async () => {
+      expect(await hasErrorToast(browser)).toBe(true)
+      await waitForAndOpenRuntimeError(browser)
+      await assertHasRedbox(browser)
+    })
+
+    const description = await getRedboxDescription(browser)
+    const stack = await getRedboxCallStack(browser)
+    const result = {
+      description,
+      stack,
+    }
+
+    expect(result).toMatchInlineSnapshot(`
+      {
+        "description": "[ Server ] Error: In Route "/no-accessed-data" this component accessed data without a Suspense boundary above it to provide a fallback UI. See more info: https://nextjs.org/docs/messages/next-prerender-data",
+        "stack": "Page [Server]
+      <anonymous> (2:1)
+      Root [Server]
+      <anonymous> (2:1)",
+      }
+    `)
+  })
 })