Skip to content

Commit

Permalink
Clickable stack trace links in logged terminal errors (vercel#71940)
Browse files Browse the repository at this point in the history
Co-authored-by: Hendrik Liebau <mail@hendrik-liebau.de>
  • Loading branch information
2 people authored and stipsan committed Nov 6, 2024
1 parent 209c61d commit 322196a
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 59 deletions.
8 changes: 8 additions & 0 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1131,6 +1131,14 @@ export default async function getBaseWebpackConfig(
}.js`,
strictModuleExceptionHandling: true,
crossOriginLoading: crossOrigin,
// if `sources[number]` is not an absolute path, it's is resolved
// relative to the location of the source map file (https://tc39.es/source-map/#resolving-sources).
// However, Webpack's `resource-path` is relative to the app dir.
// TODO: Either `sourceRoot` should be populated with the root and then we can use `[resource-path]`
// or we need a way to resolve return `path.relative(sourceMapLocation, info.resourcePath)`
devtoolModuleFilenameTemplate: dev
? '[absolute-resource-path]'
: undefined,
webassemblyModuleFilename: 'static/wasm/[modulehash].wasm',
hashFunction: 'xxhash64',
hashDigestLength: 16,
Expand Down
32 changes: 20 additions & 12 deletions packages/next/src/build/webpack/config/blocks/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,9 @@ import DevToolsIgnorePlugin from '../../plugins/devtools-ignore-list-plugin'
import EvalSourceMapDevToolPlugin from '../../plugins/eval-source-map-dev-tool-plugin'

function shouldIgnorePath(modulePath: string): boolean {
// TODO: How to ignore list 'webpack:///../../../src/shared/lib/is-thenable.ts'
return (
modulePath.includes('node_modules') ||
// would filter 'webpack://_N_E/./app/page.tsx'
// modulePath.startsWith('webpack://_N_E/') ||
// e.g. 'webpack:///external commonjs "next/dist/compiled/next-server/app-page.runtime.dev.js"'
// Only relevant for when Next.js is symlinked e.g. in the Next.js monorepo
modulePath.includes('next/dist')
)
}
Expand Down Expand Up @@ -42,14 +39,7 @@ export const base = curry(function base(
// original source, including columns and original variable names.
// This is desirable so the in-browser debugger can correctly pause
// and show scoped variables with their original names.
// We're using a fork of `eval-source-map`
config.devtool = false
config.plugins ??= []
config.plugins.push(
new EvalSourceMapDevToolPlugin({
shouldIgnorePath,
})
)
config.devtool = 'eval-source-map'
}
} else {
if (
Expand All @@ -76,6 +66,24 @@ export const base = curry(function base(
config.module = { rules: [] }
}

config.plugins ??= []
if (config.devtool === 'source-map') {
config.plugins.push(
new DevToolsIgnorePlugin({
shouldIgnorePath,
})
)
} else if (config.devtool === 'eval-source-map') {
// We're using a fork of `eval-source-map`
config.devtool = false
config.plugins.push(
new EvalSourceMapDevToolPlugin({
moduleFilenameTemplate: config.output?.devtoolModuleFilenameTemplate,
shouldIgnorePath,
})
)
}

// TODO: add codemod for "Should not import the named export" with JSON files
// config.module.strictExportPresence = !isWebpack5

Expand Down
18 changes: 16 additions & 2 deletions packages/next/src/server/patch-error-inspect.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { findSourceMap, type SourceMapPayload } from 'module'
import * as path from 'path'
import * as url from 'url'
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'
Expand Down Expand Up @@ -27,9 +29,21 @@ function frameToString(frame: StackFrame): string {
if (frame.column !== null && sourceLocation !== '') {
sourceLocation += `:${frame.column}`
}

const filePath =
frame.file !== null &&
frame.file.startsWith('file://') &&
URL.canParse(frame.file)
? // If not relative to CWD, the path is ambiguous to IDEs and clicking will prompt to select the file first.
// In a multi-app repo, this leads to potentially larger file names but will make clicking snappy.
// There's no tradeoff for the cases where `dir` in `next dev [dir]` is omitted
// since relative to cwd is both the shortest and snappiest.
path.relative(process.cwd(), url.fileURLToPath(frame.file))
: frame.file

return frame.methodName
? ` at ${frame.methodName} (${frame.file}${sourceLocation})`
: ` at ${frame.file}${frame.lineNumber}:${frame.column}`
? ` at ${frame.methodName} (${filePath}${sourceLocation})`
: ` at ${filePath}${frame.lineNumber}:${frame.column}`
}

function computeErrorName(error: Error): string {
Expand Down
13 changes: 10 additions & 3 deletions test/development/middleware-errors/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,11 @@ describe('middleware - development errors', () => {
'\n at eval (./middleware.js:4:9)' +
'\n at <unknown> (./middleware.js:4:9'
: '\n ⨯ Error [ReferenceError]: test is not defined' +
// TODO: Redundant and not clickable
'\n at eval (file://webpack-internal:///(middleware)/./middleware.js)' +
'\n at eval (webpack:///middleware.js?3bcb:4:8)'
'\n at eval (middleware.js:4:8)' +
// TODO: Should be ignore-listed
'\n at fn (node_modules'
)
expect(stripAnsi(next.cliOutput)).toContain(
isTurbopack
Expand Down Expand Up @@ -181,14 +184,18 @@ describe('middleware - development errors', () => {
await retry(() => {
expect(stripAnsi(next.cliOutput)).toContain(`Error: booooom!`)
})
// TODO: assert on full, ignore-listed stack
expect(stripAnsi(next.cliOutput)).toContain(
isTurbopack
? '\n ⨯ middleware.js (3:13) @ [project]/middleware.js [middleware] (ecmascript)' +
'\n ⨯ Error: booooom!' +
'\n at <unknown> ([project]/middleware.js [middleware] (ecmascript) (./middleware.js:3:13)'
: '\n ⨯ Error: booooom!' +
'\n at <unknown> (webpack:///middleware.js'
// TODO: Should be anonymous method without a method name
'\n at <unknown> (middleware.js:3)' +
// TODO: Should be ignore-listed
'\n at eval (middleware.js:3:12)' +
'\n at (middleware)/./middleware.js (.next/server/middleware.js:40:1)' +
'\n at __webpack_require__ '
)
})

Expand Down
53 changes: 11 additions & 42 deletions test/e2e/app-dir/server-source-maps/server-source-maps.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,39 +4,8 @@ import { nextTestSetup } from 'e2e-utils'
import stripAnsi from 'strip-ansi'
import { retry } from 'next-test-utils'

/**
* TODO: Remove this function once we log the file name on disk by adjusting `moduleFileNameTemplate`.
* in: webpack:///app/ssr-error-log-ignore-listed/page.js?1234:5:16
* out: webpack:///app/ssr-error-log-ignore-listed/page.js?[search]:5:16
*/
function normalizeWebpackLocation(frame: string) {
const webpackQueryStringtMatch = frame.match(/\?\w+:(\d+):(\d+)\)$/)
if (webpackQueryStringtMatch === null) {
return frame
}

return frame.replace(
webpackQueryStringtMatch[0],
`?[search]:${webpackQueryStringtMatch[1]}:${webpackQueryStringtMatch[2]})`
)
}

function normalizeCliOutput(output: string) {
return stripAnsi(output)
.split('\n')
.map((line) => {
return (
normalizeWebpackLocation(line)
.replaceAll(
path.join(__dirname, 'fixtures/default'),
'[fixture-root]'
)
// in: webpack://[fixture-root]/node_modules/.pnpm/internal-pkg@file+internal-pkg/node_modules/internal-pkg/index.js?[search]:2:0
// out: webpack://[fixture-root]/node_modules/[pnpm]/internal-pkg/index.js?[search]:2:0
.replace(/\/.pnpm\/[^/]+\/node_modules/g, '/[pnpm]')
)
})
.join('\n')
}

describe('app-dir - server source maps', () => {
Expand Down Expand Up @@ -74,9 +43,9 @@ describe('app-dir - server source maps', () => {
'\n 5 |' +
'\n'
: '\nError: Boom' +
'\n at logError (webpack:///app/rsc-error-log/page.js?[search]:2:16)' +
'\n at logError (app/rsc-error-log/page.js:2:16)' +
// FIXME: Method name should be "Page"
'\n at logError (webpack:///app/rsc-error-log/page.js?[search]:7:2)' +
'\n at logError (app/rsc-error-log/page.js:7:2)' +
'\n 1 | function logError() {' +
"\n> 2 | const error = new Error('Boom')" +
'\n | ^' +
Expand Down Expand Up @@ -118,17 +87,17 @@ describe('app-dir - server source maps', () => {
'\n 10 | }' +
'\n'
: '\nError: Boom' +
'\n at logError (webpack:///app/rsc-error-log-cause/page.js?[search]:2:16)' +
'\n at logError (app/rsc-error-log-cause/page.js:2:16)' +
// FIXME: Method name should be "Page"
'\n at logError (webpack:///app/rsc-error-log-cause/page.js?[search]:8:2)' +
'\n at logError (app/rsc-error-log-cause/page.js:8:2)' +
'\n 1 | function logError(cause) {' +
"\n> 2 | const error = new Error('Boom', { cause })" +
'\n | ^' +
'\n 3 | console.error(error)' +
'\n 4 | }' +
'\n 5 | {' +
'\n [cause]: Error: Boom' +
'\n at Page (webpack:///app/rsc-error-log-cause/page.js?[search]:7:16)' +
'\n at Page (app/rsc-error-log-cause/page.js:7:16)' +
'\n 5 |' +
'\n 6 | export default function Page() {' +
"\n > 7 | const error = new Error('Boom')" +
Expand Down Expand Up @@ -158,10 +127,10 @@ describe('app-dir - server source maps', () => {
? // FIXME: Turbopack resolver bug
"Module not found: Can't resolve 'internal-pkg'"
: '\nError: Boom' +
'\n at logError (webpack:///app/ssr-error-log-ignore-listed/page.js?[search]:5:16)' +
'\n at logError (app/ssr-error-log-ignore-listed/page.js:5:16)' +
// FIXME: Method name should be "Page"
'\n at logError (webpack:///app/ssr-error-log-ignore-listed/page.js?[search]:10:12)' +
'\n at Page (webpack:///app/ssr-error-log-ignore-listed/page.js?[search]:10:6)' +
'\n at logError (app/ssr-error-log-ignore-listed/page.js:10:12)' +
'\n at Page (app/ssr-error-log-ignore-listed/page.js:10:6)' +
'\n 3 |'
)
})
Expand All @@ -185,10 +154,10 @@ describe('app-dir - server source maps', () => {
? // FIXME: Turbopack resolver bug
"Module not found: Can't resolve 'internal-pkg'"
: '\nError: Boom' +
'\n at logError (webpack:///app/rsc-error-log-ignore-listed/page.js?[search]:4:16)' +
'\n at logError (app/rsc-error-log-ignore-listed/page.js:4:16)' +
// FIXME: Method name should be "Page"
'\n at logError (webpack:///app/rsc-error-log-ignore-listed/page.js?[search]:9:12)' +
'\n at Page (webpack:///app/rsc-error-log-ignore-listed/page.js?[search]:9:6)' +
'\n at logError (app/rsc-error-log-ignore-listed/page.js:9:12)' +
'\n at Page (app/rsc-error-log-ignore-listed/page.js:9:6)' +
'\n 2 |'
)
})
Expand Down

0 comments on commit 322196a

Please sign in to comment.