Respect sourcemap's ignore list when printing errors in the terminal

packages/next/src/server/patch-error-inspect.ts

@@ -1,11 +1,24 @@
-import { findSourceMap } from 'module'
+import { findSourceMap, type SourceMapPayload } from 'module'
 import type * as util from 'util'
 import { SourceMapConsumer as SyncSourceMapConsumer } from 'next/dist/compiled/source-map'
 import type { StackFrame } from 'next/dist/compiled/stacktrace-parser'
 import { parseStack } from '../client/components/react-dev-overlay/server/middleware'
 import { getOriginalCodeFrame } from '../client/components/react-dev-overlay/server/shared'
 import { workUnitAsyncStorage } from './app-render/work-unit-async-storage.external'
 
+interface ModernRawSourceMap extends SourceMapPayload {
+  ignoreList?: number[]
+}
+
+interface IgnoreableStackFrame extends StackFrame {
+  ignored: boolean
+}
+
+type SourceMapCache = Map<
+  string,
+  { map: SyncSourceMapConsumer; raw: ModernRawSourceMap }
+>
+
 // TODO: Implement for Edge runtime
 const inspectSymbol = Symbol.for('nodejs.util.inspect.custom')
 
@@ -56,30 +69,39 @@ function shouldIgnoreListByDefault(file: string): boolean {
 
 function getSourcemappedFrameIfPossible(
   frame: StackFrame,
-  sourcemapConsumers: Map<string, SyncSourceMapConsumer>
+  sourceMapCache: SourceMapCache
 ): {
-  stack: StackFrame
+  stack: IgnoreableStackFrame
   // DEV only
   code: string | null
 } | null {
   if (frame.file === null) {
     return null
   }
 
-  let sourcemap = sourcemapConsumers.get(frame.file)
-  if (sourcemap === undefined) {
-    const moduleSourcemap = findSourceMap(frame.file)
-    if (moduleSourcemap === undefined) {
+  const sourceMapCacheEntry = sourceMapCache.get(frame.file)
+  let sourceMap: SyncSourceMapConsumer
+  let rawSourceMap: ModernRawSourceMap
+  if (sourceMapCacheEntry === undefined) {
+    const moduleSourceMap = findSourceMap(frame.file)
+    if (moduleSourceMap === undefined) {
       return null
     }
-    sourcemap = new SyncSourceMapConsumer(
+    rawSourceMap = moduleSourceMap.payload
+    sourceMap = new SyncSourceMapConsumer(
       // @ts-expect-error -- Module.SourceMap['version'] is number but SyncSourceMapConsumer wants a string
-      moduleSourcemap.payload
+      rawSourceMap
     )
-    sourcemapConsumers.set(frame.file, sourcemap)
+    sourceMapCache.set(frame.file, {
+      map: sourceMap,
+      raw: rawSourceMap,
+    })
+  } else {
+    sourceMap = sourceMapCacheEntry.map
+    rawSourceMap = sourceMapCacheEntry.raw
   }
 
-  const sourcePosition = sourcemap.originalPositionFor({
+  const sourcePosition = sourceMap.originalPositionFor({
     column: frame.column ?? 0,
     line: frame.lineNumber ?? 1,
   })
@@ -89,12 +111,16 @@ function getSourcemappedFrameIfPossible(
   }
 
   const sourceContent: string | null =
-    sourcemap.sourceContentFor(
+    sourceMap.sourceContentFor(
       sourcePosition.source,
       /* returnNullOnMissing */ true
     ) ?? null
 
-  const originalFrame: StackFrame = {
+  // TODO: O(n^2). Consider moving `ignoreList` into a Set
+  const sourceIndex = rawSourceMap.sources.indexOf(sourcePosition.source)
+  const ignored = rawSourceMap.ignoreList?.includes(sourceIndex) ?? false
+
+  const originalFrame: IgnoreableStackFrame = {
     methodName:
       sourcePosition.name ||
       // default is not a valid identifier in JS so webpack uses a custom variable when it's an unnamed default export
@@ -107,6 +133,7 @@ function getSourcemappedFrameIfPossible(
     lineNumber: sourcePosition.line,
     // TODO: c&p from async createOriginalStackFrame but why not frame.arguments?
     arguments: [],
+    ignored,
   }
 
   const codeFrame =
@@ -138,7 +165,7 @@ function parseAndSourceMap(error: Error): string {
   }
 
   const unsourcemappedStack = parseStack(unparsedStack)
-  const sourcemapConsumers = new Map<string, SyncSourceMapConsumer>()
+  const sourceMapCache: SourceMapCache = new Map()
 
   let sourceMappedStack = ''
   let sourceFrameDEV: null | string = null
@@ -148,22 +175,25 @@ function parseAndSourceMap(error: Error): string {
     } else if (!shouldIgnoreListByDefault(frame.file)) {
       const sourcemappedFrame = getSourcemappedFrameIfPossible(
         frame,
-        sourcemapConsumers
+        sourceMapCache
       )
 
       if (sourcemappedFrame === null) {
         sourceMappedStack += '\n' + frameToString(frame)
       } else {
-        // TODO: Use the first frame that's not ignore-listed
         if (
           process.env.NODE_ENV !== 'production' &&
           sourcemappedFrame.code !== null &&
-          sourceFrameDEV === null
+          sourceFrameDEV === null &&
+          // TODO: Is this the right choice?
+          !sourcemappedFrame.stack.ignored
         ) {
           sourceFrameDEV = sourcemappedFrame.code
         }
-        // TODO: Hide if ignore-listed but consider what happens if every frame is ignore listed.
-        sourceMappedStack += '\n' + frameToString(sourcemappedFrame.stack)
+        if (!sourcemappedFrame.stack.ignored) {
+          // TODO: Consider what happens if every frame is ignore listed.
+          sourceMappedStack += '\n' + frameToString(sourcemappedFrame.stack)
+        }
       }
     }
   }

test/e2e/app-dir/server-source-maps/fixtures/default/.gitignore

@@ -0,0 +1 @@
+node_modules/

test/e2e/app-dir/server-source-maps/fixtures/default/app/rsc-error-log-ignore-listed/page.js

@@ -0,0 +1,11 @@
+import { run } from 'internal-pkg'
+
+function logError() {
+  const error = new Error('Boom')
+  console.error(error)
+}
+
+export default function Page() {
+  run(() => logError())
+  return null
+}

test/e2e/app-dir/server-source-maps/fixtures/default/internal-pkg/index.js

@@ -0,0 +1,3 @@
+export function run(fn) {
+  return fn();
+}

test/e2e/app-dir/server-source-maps/fixtures/default/internal-pkg/package.json

@@ -0,0 +1,10 @@
+{
+  "name": "internal-pkg",
+  "private": true,
+  "type": "module",
+  "exports": {
+    ".": {
+      "default": "./index.js"
+    }
+  }
+}

test/e2e/app-dir/server-source-maps/fixtures/default/package.json

@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "internal-pkg": "file:./internal-pkg"
+  }
+}

test/e2e/app-dir/server-source-maps/server-source-maps.test.ts

@@ -2,7 +2,22 @@ import * as path from 'path'
 import { nextTestSetup } from 'e2e-utils'
 
 describe('app-dir - server source maps', () => {
+  const dependencies = (global as any).isNextDeploy
+    ? // `link` is incompatible with the npm version used when this test is deployed
+      {
+        'internal-pkg': 'file:./internal-pkg',
+      }
+    : // But link: is not suitable for this test since this makes internal-pkg
+      // not appear in node_modules.
+      // file: is not compatible with our test infra.
+      // You can manually test though by running
+      // pnpm install --ignore-workspace in the fixture directory
+      // and running pnpm next dev <fixture> inside the monorepo
+      {
+        'internal-pkg': 'link:./internal-pkg',
+      }
   const { skipped, next, isNextDev } = nextTestSetup({
+    dependencies,
     files: path.join(__dirname, 'fixtures/default'),
     // Deploy tests don't have access to runtime logs.
     // Manually verify that the runtime logs match.
@@ -40,4 +55,8 @@ describe('app-dir - server source maps', () => {
       isNextDev ? 'Error [MyError]: Bar' : 'Error [MyError]: Bar'
     )
   })
+
+  it('respects ignore-listed frames', async () => {
+    await next.render('/rsc-error-log-ignore-listed')
+  })
 })