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.

Sign up for GitHub

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
merged 3 commits into from
Nov 1, 2024
Merged

Clickable stack trace links in logged terminal errors #71940

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8675caf
Clickable stack trace links in logged terminal errors
eps1lon Oct 27, 2024
cfa965f
dev only to no leak machine information
eps1lon Oct 27, 2024
679929a
Prefer `url.fileURLToPath `
eps1lon Oct 28, 2024
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
8 changes: 8 additions & 0 deletions packages/next/src/build/webpack-config.ts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
Loading