Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clickable stack trace links in logged terminal errors #71940

Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Clickable stack trace links in logged terminal errors
  • Loading branch information
eps1lon committed Nov 1, 2024
commit 8675caf548744f4f12f0fd98e79b54b99c7c095c
6 changes: 6 additions & 0 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
@@ -1131,6 +1131,12 @@ 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: '[absolute-resource-path]',
webassemblyModuleFilename: 'static/wasm/[modulehash].wasm',
hashFunction: 'xxhash64',
hashDigestLength: 16,
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
@@ -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')
)
}
@@ -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 (
@@ -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

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 { 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'
@@ -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(), new URL(frame.file).pathname)
unstubbable marked this conversation as resolved.
Show resolved Hide resolved
: 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 {
13 changes: 10 additions & 3 deletions test/development/middleware-errors/index.test.ts
Original file line number Diff line number Diff line change
@@ -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
@@ -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__ '
)
})

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
@@ -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', () => {
@@ -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 | ^' +
@@ -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')" +
@@ -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 |'
)
})
@@ -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 |'
)
})