fix(error-overlay): correct module grouping, hide useless frames (#62206

) ### What? While working on hiding useless frames, I also noticed that we regressed on #44137, meaning it was totally ignored. I haven't tracked down at which point this happened but made it work again in the same PR. This should significantly clear up the shown error stack in the error overlay: <details> <summary><b>Before:</b></summary> <img src="https://github.com/vercel/next.js/assets/18369201/1833abfe-7c0b-4a34-bad8-735799f1cf42"/> <img src="https://github.com/vercel/next.js/assets/18369201/70ecc124-1241-4df9-adfe-7f0c8f47d6d3"/> </details> <details> <summary><b>After:</b></summary> <img src="https://github.com/vercel/next.js/assets/18369201/d0395320-c52c-47a0-a281-f7721410f4da"/> </details> ### Why? Some frames in the error stack are useless/unactionable to the user and make it harder to parse the error. This PR filters out some of them, to make the stack more readable. ### How? The stack traces are run through a `.filter()` before being displayed. Closes NEXT-2505 Closes NEXT-2522
vercel · Feb 21, 2024 · 2a9a7a2 · 2a9a7a2
1 parent 7d44823
commit 2a9a7a2
Show file tree

Hide file tree

Showing 10 changed files with 223 additions and 193 deletions.
diff --git a/...lient/components/react-dev-overlay/internal/container/RuntimeError/GroupedStackFrames.tsx b/...lient/components/react-dev-overlay/internal/container/RuntimeError/GroupedStackFrames.tsx
@@ -5,62 +5,56 @@ import { FrameworkIcon } from './FrameworkIcon'
 function FrameworkGroup({
   framework,
   stackFrames,
-  all,
 }: {
   framework: NonNullable<StackFramesGroup['framework']>
   stackFrames: StackFramesGroup['stackFrames']
-  all: boolean
 }) {
   return (
-    <>
-      <details data-nextjs-collapsed-call-stack-details>
-        <summary
-          tabIndex={10} // Match CallStackFrame tabIndex
+    <details data-nextjs-collapsed-call-stack-details>
+      {/* Match CallStackFrame tabIndex */}
+      <summary tabIndex={10}>
+        <svg
+          data-nextjs-call-stack-chevron-icon
+          fill="none"
+          height="20"
+          width="20"
+          shapeRendering="geometricPrecision"
+          stroke="currentColor"
+          strokeLinecap="round"
+          strokeLinejoin="round"
+          strokeWidth="2"
+          viewBox="0 0 24 24"
         >
-          <svg
-            data-nextjs-call-stack-chevron-icon
-            fill="none"
-            height="20"
-            width="20"
-            shapeRendering="geometricPrecision"
-            stroke="currentColor"
-            strokeLinecap="round"
-            strokeLinejoin="round"
-            strokeWidth="2"
-            viewBox="0 0 24 24"
-          >
-            <path d="M9 18l6-6-6-6" />
-          </svg>
-          <FrameworkIcon framework={framework} />
-          {framework === 'react' ? 'React' : 'Next.js'}
-        </summary>
-
-        {stackFrames.map((frame, index) => (
-          <CallStackFrame key={`call-stack-${index}-${all}`} frame={frame} />
-        ))}
-      </details>
-    </>
+          <path d="M9 18l6-6-6-6" />
+        </svg>
+        <FrameworkIcon framework={framework} />
+        {framework === 'react' ? 'React' : 'Next.js'}
+      </summary>
+      {stackFrames.map((frame, index) => (
+        <CallStackFrame key={`call-stack-${index}`} frame={frame} />
+      ))}
+    </details>
   )
 }
 
 export function GroupedStackFrames({
   groupedStackFrames,
-  all,
+  show,
 }: {
   groupedStackFrames: StackFramesGroup[]
-  all: boolean
+  show: boolean
 }) {
+  if (!show) return
   return (
     <>
       {groupedStackFrames.map((stackFramesGroup, groupIndex) => {
         // Collapse React and Next.js frames
         if (stackFramesGroup.framework) {
           return (
             <FrameworkGroup
-              key={`call-stack-framework-group-${groupIndex}-${all}`}
+              key={`call-stack-framework-group-${groupIndex}`}
               framework={stackFramesGroup.framework}
               stackFrames={stackFramesGroup.stackFrames}
-              all={all}
             />
           )
         }
@@ -69,7 +63,7 @@ export function GroupedStackFrames({
           // Don't group non React and Next.js frames
           stackFramesGroup.stackFrames.map((frame, frameIndex) => (
             <CallStackFrame
-              key={`call-stack-${groupIndex}-${frameIndex}-${all}`}
+              key={`call-stack-${groupIndex}-${frameIndex}`}
               frame={frame}
             />
           ))

diff --git a/...es/next/src/client/components/react-dev-overlay/internal/container/RuntimeError/index.tsx b/...es/next/src/client/components/react-dev-overlay/internal/container/RuntimeError/index.tsx
@@ -4,7 +4,6 @@ import type { ReadyRuntimeError } from '../../helpers/getErrorByType'
 import { noop as css } from '../../helpers/noop-template'
 import type { OriginalStackFrame } from '../../helpers/stack-frame'
 import { groupStackFramesByFramework } from '../../helpers/group-stack-frames-by-framework'
-import { CallStackFrame } from './CallStackFrame'
 import { GroupedStackFrames } from './GroupedStackFrames'
 import { ComponentStackFrameRow } from './ComponentStackFrameRow'
 
@@ -64,21 +63,24 @@ const RuntimeError: React.FC<RuntimeErrorProps> = function RuntimeError({
   ])
 
   const stackFramesGroupedByFramework = React.useMemo(
-    () => groupStackFramesByFramework(visibleCallStackFrames),
-    [visibleCallStackFrames]
+    () => groupStackFramesByFramework(allCallStackFrames),
+    [allCallStackFrames]
+  )
+
+  const leadingFramesGroupedByFramework = React.useMemo(
+    () => groupStackFramesByFramework(leadingFrames),
+    [leadingFrames]
   )
 
   return (
     <React.Fragment>
       {firstFrame ? (
         <React.Fragment>
           <h2>Source</h2>
-          {leadingFrames.map((frame, index) => (
-            <CallStackFrame
-              key={`leading-frame-${index}-${all}`}
-              frame={frame}
-            />
-          ))}
+          <GroupedStackFrames
+            groupedStackFrames={leadingFramesGroupedByFramework}
+            show={all}
+          />
           <CodeFrame
             stackFrame={firstFrame.originalStackFrame!}
             codeFrame={firstFrame.originalCodeFrame!}
@@ -103,7 +105,7 @@ const RuntimeError: React.FC<RuntimeErrorProps> = function RuntimeError({
           <h2>Call Stack</h2>
           <GroupedStackFrames
             groupedStackFrames={stackFramesGroupedByFramework}
-            all={all}
+            show={all}
           />
         </React.Fragment>
       ) : undefined}
@@ -196,7 +198,7 @@ export const styles = css`
   [data-nextjs-collapsed-call-stack-details] summary {
     display: flex;
     align-items: center;
-    margin: var(--size-gap-double) 0;
+    margin-bottom: var(--size-gap);
     list-style: none;
   }
   [data-nextjs-collapsed-call-stack-details] summary::-webkit-details-marker {

diff --git a/...c/client/components/react-dev-overlay/internal/helpers/group-stack-frames-by-framework.ts b/...c/client/components/react-dev-overlay/internal/helpers/group-stack-frames-by-framework.ts
@@ -1,31 +1,11 @@
+import type { SourcePackage } from '../../server/shared'
 import type { OriginalStackFrame } from './stack-frame'
 
 export type StackFramesGroup = {
-  framework?: 'next' | 'react'
+  framework?: SourcePackage | null
   stackFrames: OriginalStackFrame[]
 }
 
-/**
- * Get the origin framework of the stack frame by package name.
- */
-function getFramework(
-  sourcePackage: string | undefined
-): StackFramesGroup['framework'] {
-  if (!sourcePackage) return undefined
-
-  if (
-    /^(react|react-dom|react-is|react-refresh|react-server-dom-webpack|react-server-dom-turbopack|scheduler)$/.test(
-      sourcePackage
-    )
-  ) {
-    return 'react'
-  } else if (sourcePackage === 'next') {
-    return 'next'
-  }
-
-  return undefined
-}
-
 /**
  * Group sequences of stack frames by framework.
  *
@@ -55,7 +35,7 @@ export function groupStackFramesByFramework(
   for (const stackFrame of stackFrames) {
     const currentGroup =
       stackFramesGroupedByFramework[stackFramesGroupedByFramework.length - 1]
-    const framework = getFramework(stackFrame.sourcePackage)
+    const framework = stackFrame.sourcePackage
 
     if (currentGroup && currentGroup.framework === framework) {
       currentGroup.stackFrames.push(stackFrame)

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,37 +1,13 @@
 import type { StackFrame } from 'next/dist/compiled/stacktrace-parser'
-import type { OriginalStackFrameResponse } from '../../server/middleware'
+import type { OriginalStackFrameResponse } from '../../server/shared'
 
-export type OriginalStackFrame =
-  | {
-      error: true
-      reason: string
-      external: false
-      expanded: false
-      sourceStackFrame: StackFrame
-      originalStackFrame: null
-      originalCodeFrame: null
-      sourcePackage?: string
-    }
-  | {
-      error: false
-      reason: null
-      external: false
-      expanded: boolean
-      sourceStackFrame: StackFrame
-      originalStackFrame: StackFrame
-      originalCodeFrame: string | null
-      sourcePackage?: string
-    }
-  | {
-      error: false
-      reason: null
-      external: true
-      expanded: false
-      sourceStackFrame: StackFrame
-      originalStackFrame: null
-      originalCodeFrame: null
-      sourcePackage?: string
-    }
+export interface OriginalStackFrame extends OriginalStackFrameResponse {
+  error: boolean
+  reason: string | null
+  external: boolean
+  expanded: boolean
+  sourceStackFrame: StackFrame
+}
 
 function getOriginalStackFrame(
   source: StackFrame,
@@ -99,6 +75,7 @@ function getOriginalStackFrame(
       sourceStackFrame: source,
       originalStackFrame: null,
       originalCodeFrame: null,
+      sourcePackage: null,
     })
   }
 
@@ -110,6 +87,7 @@ function getOriginalStackFrame(
     sourceStackFrame: source,
     originalStackFrame: null,
     originalCodeFrame: null,
+    sourcePackage: null,
   }))
 }
 

diff --git a/packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts b/packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts
@@ -1,9 +1,7 @@
 import type { IncomingMessage, ServerResponse } from 'http'
-import type { ParsedUrlQuery } from 'querystring'
-import type { OriginalStackFrameResponse } from './middleware'
+import { findSourcePackage, type OriginalStackFrameResponse } from './shared'
 
 import fs, { constants as FS } from 'fs/promises'
-import url from 'url'
 import { codeFrameColumns } from 'next/dist/compiled/babel/code-frame'
 import { launchEditor } from '../internal/helpers/launchEditor'
 
@@ -76,6 +74,8 @@ export async function createOriginalStackFrame(
 ): Promise<OriginalStackFrameResponse | null> {
   const traced = await batchedTraceSource(project, frame)
   if (!traced) {
+    const sourcePackage = findSourcePackage(frame.file)
+    if (sourcePackage) return { sourcePackage }
     return null
   }
 
@@ -96,27 +96,24 @@ export async function createOriginalStackFrame(
             },
             { forceColor: true }
           ),
-  }
-}
-
-function stackFrameFromQuery(query: ParsedUrlQuery): TurbopackStackFrame {
-  return {
-    file: query.file as string,
-    methodName: query.methodName as string,
-    line:
-      typeof query.lineNumber === 'string' ? parseInt(query.lineNumber, 10) : 0,
-    column:
-      typeof query.column === 'string' ? parseInt(query.column, 10) : null,
-    isServer: query.isServer === 'true',
+    sourcePackage: findSourcePackage(traced.frame.file),
   }
 }
 
 export function getOverlayMiddleware(project: Project) {
   return async function (req: IncomingMessage, res: ServerResponse) {
-    const { pathname, query } = url.parse(req.url!, true)
+    const { pathname, searchParams } = new URL(req.url!, 'http://n')
+
+    const frame = {
+      file: searchParams.get('file') as string,
+      methodName: searchParams.get('methodName'),
+      line: parseInt(searchParams.get('lineNumber') ?? '0', 10) || 0,
+      column: parseInt(searchParams.get('column') ?? '0', 10) || 0,
+      isServer: searchParams.get('isServer') === 'true',
+    } satisfies TurbopackStackFrame
+
     if (pathname === '/__nextjs_original-stack-frame') {
-      const frame = stackFrameFromQuery(query)
-      let originalStackFrame
+      let originalStackFrame: OriginalStackFrameResponse | null
       try {
         originalStackFrame = await createOriginalStackFrame(project, frame)
       } catch (e: any) {
@@ -139,17 +136,14 @@ export function getOverlayMiddleware(project: Project) {
       res.end()
       return
     } else if (pathname === '/__nextjs_launch-editor') {
-      const frame = stackFrameFromQuery(query)
-
-      const filePath = frame.file?.toString()
-      if (filePath === undefined) {
+      if (!frame.file) {
         res.statusCode = 400
         res.write('Bad Request')
         res.end()
         return
       }
 
-      const fileExists = await fs.access(filePath, FS.F_OK).then(
+      const fileExists = await fs.access(frame.file, FS.F_OK).then(
         () => true,
         () => false
       )
@@ -161,7 +155,7 @@ export function getOverlayMiddleware(project: Project) {
       }
 
       try {
-        launchEditor(filePath, frame.line ?? 1, frame.column ?? 1)
+        launchEditor(frame.file, frame.line ?? 1, frame.column ?? 1)
       } catch (err) {
         console.log('Failed to launch editor:', err)
         res.statusCode = 500