Skip to content

Commit af6f6aa

Browse files
authored
Sourcemap errors in terminal by default (#71444)
1 parent a8de873 commit af6f6aa

File tree

13 files changed

+327
-3
lines changed

13 files changed

+327
-3
lines changed

packages/next/src/client/components/react-dev-overlay/server/shared.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ export function findSourcePackage({
5050
export function getOriginalCodeFrame(
5151
frame: StackFrame,
5252
source: string | null
53-
): string | null | undefined {
53+
): string | null {
5454
if (!source || isInternal(frame.file)) {
5555
return null
5656
}
@@ -65,6 +65,7 @@ export function getOriginalCodeFrame(
6565
column: frame.column ?? 0,
6666
},
6767
},
68+
// TODO: Only in TTY
6869
{ forceColor: true }
6970
)
7071
}
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import { patchErrorInspect } from '../patch-error-inspect'
2+
3+
patchErrorInspect()

packages/next/src/server/node-environment.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
// This file should be imported before any others. It sets up the environment
22
// for later imports to work properly.
33

4+
// Import before anything else so that unexpected errors in other extensions are properly formatted.
5+
import './node-environment-extensions/error-inspect'
6+
47
import './node-environment-baseline'
58
import './node-environment-extensions/random'
69
import './node-environment-extensions/date'
Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,228 @@
1+
import { findSourceMap } from 'module'
2+
import type * as util from 'util'
3+
import { SourceMapConsumer as SyncSourceMapConsumer } from 'next/dist/compiled/source-map'
4+
import type { StackFrame } from 'next/dist/compiled/stacktrace-parser'
5+
import { parseStack } from '../client/components/react-dev-overlay/server/middleware'
6+
import { getOriginalCodeFrame } from '../client/components/react-dev-overlay/server/shared'
7+
8+
// TODO: Implement for Edge runtime
9+
const inspectSymbol = Symbol.for('nodejs.util.inspect.custom')
10+
11+
function frameToString(frame: StackFrame): string {
12+
let sourceLocation = frame.lineNumber !== null ? `:${frame.lineNumber}` : ''
13+
if (frame.column !== null && sourceLocation !== '') {
14+
sourceLocation += `:${frame.column}`
15+
}
16+
return frame.methodName
17+
? ` at ${frame.methodName} (${frame.file}${sourceLocation})`
18+
: ` at ${frame.file}${frame.lineNumber}:${frame.column}`
19+
}
20+
21+
function computeErrorName(error: Error): string {
22+
// TODO: Node.js seems to use a different algorithm
23+
// class ReadonlyRequestCookiesError extends Error {}` would read `ReadonlyRequestCookiesError: [...]`
24+
// in the stack i.e. seems like under certain conditions it favors the constructor name.
25+
return error.name || 'Error'
26+
}
27+
28+
function prepareUnsourcemappedStackTrace(
29+
error: Error,
30+
structuredStackTrace: any[]
31+
): string {
32+
const name = computeErrorName(error)
33+
const message = error.message || ''
34+
let stack = name + ': ' + message
35+
for (let i = 0; i < structuredStackTrace.length; i++) {
36+
stack += '\n at ' + structuredStackTrace[i].toString()
37+
}
38+
return stack
39+
}
40+
41+
function shouldIgnoreListByDefault(file: string): boolean {
42+
return (
43+
// TODO: Solve via `ignoreList` instead. Tricky because node internals are not part of the sourcemap.
44+
file.startsWith('node:')
45+
// C&P from setup-dev-bundler
46+
// TODO: Taken from setup-dev-bundler but these seem too broad
47+
// file.includes('web/adapter') ||
48+
// file.includes('web/globals') ||
49+
// file.includes('sandbox/context') ||
50+
// TODO: Seems too aggressive?
51+
// file.includes('<anonymous>') ||
52+
// file.startsWith('eval')
53+
)
54+
}
55+
56+
function getSourcemappedFrameIfPossible(
57+
frame: StackFrame,
58+
sourcemapConsumers: Map<string, SyncSourceMapConsumer>
59+
): {
60+
stack: StackFrame
61+
// DEV only
62+
code: string | null
63+
} | null {
64+
if (frame.file === null) {
65+
return null
66+
}
67+
68+
let sourcemap = sourcemapConsumers.get(frame.file)
69+
if (sourcemap === undefined) {
70+
const moduleSourcemap = findSourceMap(frame.file)
71+
if (moduleSourcemap === undefined) {
72+
return null
73+
}
74+
sourcemap = new SyncSourceMapConsumer(
75+
// @ts-expect-error -- Module.SourceMap['version'] is number but SyncSourceMapConsumer wants a string
76+
moduleSourcemap.payload
77+
)
78+
sourcemapConsumers.set(frame.file, sourcemap)
79+
}
80+
81+
const sourcePosition = sourcemap.originalPositionFor({
82+
column: frame.column ?? 0,
83+
line: frame.lineNumber ?? 1,
84+
})
85+
86+
if (sourcePosition.source === null) {
87+
return null
88+
}
89+
90+
const sourceContent: string | null =
91+
sourcemap.sourceContentFor(
92+
sourcePosition.source,
93+
/* returnNullOnMissing */ true
94+
) ?? null
95+
96+
const originalFrame: StackFrame = {
97+
methodName:
98+
sourcePosition.name ||
99+
// default is not a valid identifier in JS so webpack uses a custom variable when it's an unnamed default export
100+
// Resolve it back to `default` for the method name if the source position didn't have the method.
101+
frame.methodName
102+
?.replace('__WEBPACK_DEFAULT_EXPORT__', 'default')
103+
?.replace('__webpack_exports__.', ''),
104+
column: sourcePosition.column,
105+
file: sourcePosition.source,
106+
lineNumber: sourcePosition.line,
107+
// TODO: c&p from async createOriginalStackFrame but why not frame.arguments?
108+
arguments: [],
109+
}
110+
111+
const codeFrame =
112+
process.env.NODE_ENV !== 'production'
113+
? getOriginalCodeFrame(originalFrame, sourceContent)
114+
: null
115+
116+
return {
117+
stack: originalFrame,
118+
code: codeFrame,
119+
}
120+
}
121+
122+
function parseAndSourceMap(error: Error): string {
123+
// We overwrote Error.prepareStackTrace earlier so error.stack is not sourcemapped.
124+
let unparsedStack = String(error.stack)
125+
// We could just read it from `error.stack`.
126+
// This works around cases where a 3rd party `Error.prepareStackTrace` implementation
127+
// doesn't implement the name computation correctly.
128+
const errorName = computeErrorName(error)
129+
130+
let idx = unparsedStack.indexOf('react-stack-bottom-frame')
131+
if (idx !== -1) {
132+
idx = unparsedStack.lastIndexOf('\n', idx)
133+
}
134+
if (idx !== -1) {
135+
// Cut off everything after the bottom frame since it'll be React internals.
136+
unparsedStack = unparsedStack.slice(0, idx)
137+
}
138+
139+
const unsourcemappedStack = parseStack(unparsedStack)
140+
const sourcemapConsumers = new Map<string, SyncSourceMapConsumer>()
141+
142+
let sourceMappedStack = ''
143+
let sourceFrameDEV: null | string = null
144+
for (const frame of unsourcemappedStack) {
145+
if (frame.file === null) {
146+
sourceMappedStack += '\n' + frameToString(frame)
147+
} else if (!shouldIgnoreListByDefault(frame.file)) {
148+
const sourcemappedFrame = getSourcemappedFrameIfPossible(
149+
frame,
150+
sourcemapConsumers
151+
)
152+
153+
if (sourcemappedFrame === null) {
154+
sourceMappedStack += '\n' + frameToString(frame)
155+
} else {
156+
// TODO: Use the first frame that's not ignore-listed
157+
if (
158+
process.env.NODE_ENV !== 'production' &&
159+
sourcemappedFrame.code !== null &&
160+
sourceFrameDEV !== null
161+
) {
162+
sourceFrameDEV = sourcemappedFrame.code
163+
}
164+
// TODO: Hide if ignore-listed but consider what happens if every frame is ignore listed.
165+
sourceMappedStack += '\n' + frameToString(sourcemappedFrame.stack)
166+
}
167+
}
168+
}
169+
170+
return (
171+
errorName +
172+
': ' +
173+
error.message +
174+
sourceMappedStack +
175+
(sourceFrameDEV !== null ? '\n' + sourceFrameDEV : '')
176+
)
177+
}
178+
179+
export function patchErrorInspect() {
180+
Error.prepareStackTrace = prepareUnsourcemappedStackTrace
181+
182+
// @ts-expect-error -- TODO upstream types
183+
// eslint-disable-next-line no-extend-native -- We're not extending but overriding.
184+
Error.prototype[inspectSymbol] = function (
185+
depth: number,
186+
inspectOptions: util.InspectOptions,
187+
inspect: typeof util.inspect
188+
): string {
189+
// Create a new Error object with the source mapping applied and then use native
190+
// Node.js formatting on the result.
191+
const newError =
192+
this.cause !== undefined
193+
? // Setting an undefined `cause` would print `[cause]: undefined`
194+
new Error(this.message, { cause: this.cause })
195+
: new Error(this.message)
196+
197+
// TODO: Ensure `class MyError extends Error {}` prints `MyError` as the name
198+
newError.stack = parseAndSourceMap(this)
199+
200+
for (const key in this) {
201+
if (!Object.prototype.hasOwnProperty.call(newError, key)) {
202+
// @ts-expect-error -- We're copying all enumerable properties.
203+
// So they definitely exist on `this` and obviously have no type on `newError` (yet)
204+
newError[key] = this[key]
205+
}
206+
}
207+
208+
const originalCustomInspect = (newError as any)[inspectSymbol]
209+
// Prevent infinite recursion.
210+
// { customInspect: false } would result in `error.cause` not using our inspect.
211+
Object.defineProperty(newError, inspectSymbol, {
212+
value: undefined,
213+
enumerable: false,
214+
writable: true,
215+
})
216+
try {
217+
return inspect(newError, {
218+
...inspectOptions,
219+
depth:
220+
(inspectOptions.depth ??
221+
// Default in Node.js
222+
2) - depth,
223+
})
224+
} finally {
225+
;(newError as any)[inspectSymbol] = originalCustomInspect
226+
}
227+
}
228+
}

test/e2e/app-dir/actions/app-action-size-limit-invalid.test.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { nextTestSetup } from 'e2e-utils'
22
import { retry } from 'next-test-utils'
3+
import stripAnsi from 'strip-ansi'
34
import { accountForOverhead } from './account-for-overhead'
45

56
const CONFIG_ERROR =
@@ -26,7 +27,7 @@ describe('app-dir action size limit invalid config', () => {
2627

2728
beforeAll(() => {
2829
const onLog = (log: string) => {
29-
logs.push(log.trim())
30+
logs.push(stripAnsi(log.trim()))
3031
}
3132

3233
next.on('stdout', onLog)
@@ -115,7 +116,7 @@ describe('app-dir action size limit invalid config', () => {
115116

116117
await retry(() => {
117118
expect(logs).toContainEqual(
118-
expect.stringContaining('[Error]: Body exceeded 1.5mb limit')
119+
expect.stringContaining('Error: Body exceeded 1.5mb limit')
119120
)
120121
expect(logs).toContainEqual(
121122
expect.stringContaining(

test/e2e/app-dir/dynamic-io-errors/dynamic-io-errors.prospective-fallback.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ describe(`Dynamic IO Prospective Fallback`, () => {
2727
// we expect the build to fail
2828
}
2929

30+
// TODO: Assert on component stack
3031
expect(next.cliOutput).toContain(
3132
'Route "/blog/[slug]": A component accessed data, headers, params, searchParams, or a short-lived cache without a Suspense boundary nor a "use cache" above it.'
3233
)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
module.exports = {
22
experimental: {
33
dynamicIO: true,
4+
serverSourceMaps: true,
45
},
56
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
export default function Root({ children }) {
2+
return (
3+
<html>
4+
<body>{children}</body>
5+
</html>
6+
)
7+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
export const dynamic = 'force-dynamic'
2+
3+
function logError(cause) {
4+
const error = new Error('Boom', { cause })
5+
console.error(error)
6+
}
7+
8+
export default function Page() {
9+
const error = new Error('Boom')
10+
logError(error)
11+
return null
12+
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
export const dynamic = 'force-dynamic'
2+
3+
class UnnamedError extends Error {}
4+
class NamedError extends Error {
5+
name = 'MyError'
6+
}
7+
8+
export default function Page() {
9+
console.error(new UnnamedError('Foo'))
10+
console.error(new NamedError('Bar'))
11+
return null
12+
}

0 commit comments

Comments
 (0)