Skip to content

Commit

Permalink
Respect sourcemap's ignore list when printing errors in the terminal
Browse files Browse the repository at this point in the history
  • Loading branch information
@eps1lon
eps1lon committed Oct 27, 2024
1 parent 7ae4105 commit 7fcb95c
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 27 deletions.
68 changes: 49 additions & 19 deletions packages/next/src/server/patch-error-inspect.ts
View file
Original file line number Diff line number Diff line change
@@ -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')

Expand Down Expand Up @@ -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,
})
Expand All @@ -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
Expand All @@ -107,6 +133,7 @@ function getSourcemappedFrameIfPossible(
lineNumber: sourcePosition.line,
// TODO: c&p from async createOriginalStackFrame but why not frame.arguments?
arguments: [],
ignored,
}

const codeFrame =
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
}
}
}
Expand Down
10 changes: 2 additions & 8 deletions test/e2e/app-dir/server-source-maps/server-source-maps.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ describe('app-dir - server source maps', () => {
// FIXME: Turbopack resolver bug
// FIXME: Turbopack build? bugs taint the whole dev server
;(isTurbopack ? it.skip : it)(
'stack frames are not ignore-listed in ssr',
'stack frames are ignore-listed in ssr',
async () => {
await next.render('/ssr-error-log-ignore-listed')

Expand All @@ -161,9 +161,6 @@ describe('app-dir - server source maps', () => {
'\n at logError (webpack:///app/ssr-error-log-ignore-listed/page.js?[search]:5:16)' +
// FIXME: Method name should be "Page"
'\n at logError (webpack:///app/ssr-error-log-ignore-listed/page.js?[search]:10:12)' +
(process.env.NEXT_TEST_CI
? '\n at run (webpack:///node_modules/[pnpm]/internal-pkg/index.js?[search]:2:0)'
: '\n at run (webpack://[fixture-root]/node_modules/[pnpm]/internal-pkg/index.js?[search]:2:0)') +
'\n at Page (webpack:///app/ssr-error-log-ignore-listed/page.js?[search]:10:6)' +
'\n 3 |'
)
Expand All @@ -177,7 +174,7 @@ describe('app-dir - server source maps', () => {
// FIXME: Turbopack resolver bug
// FIXME: Turbopack build? bugs taint the whole dev server
;(isTurbopack ? it.skip : it)(
'stack frames are not ignore-listed in rsc',
'stack frames are ignore-listed in rsc',
async () => {
await next.render('/rsc-error-log-ignore-listed')

Expand All @@ -191,9 +188,6 @@ describe('app-dir - server source maps', () => {
'\n at logError (webpack:///app/rsc-error-log-ignore-listed/page.js?[search]:4:16)' +
// FIXME: Method name should be "Page"
'\n at logError (webpack:///app/rsc-error-log-ignore-listed/page.js?[search]:9:12)' +
(process.env.NEXT_TEST_CI
? '\n at run (webpack:///node_modules/[pnpm]/internal-pkg/index.js?[search]:2:0)'
: '\n at run (webpack://[fixture-root]/node_modules/[pnpm]/internal-pkg/index.js?[search]:2:0)') +
'\n at Page (webpack:///app/rsc-error-log-ignore-listed/page.js?[search]:9:6)' +
'\n 2 |'
)
Expand Down

0 comments on commit 7fcb95c

Please sign in to comment.