From 8675caf548744f4f12f0fd98e79b54b99c7c095c Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sun, 27 Oct 2024 22:20:33 +0100 Subject: [PATCH 1/3] Clickable stack trace links in logged terminal errors --- packages/next/src/build/webpack-config.ts | 6 +++ .../src/build/webpack/config/blocks/base.ts | 32 ++++++----- .../next/src/server/patch-error-inspect.ts | 18 ++++++- .../middleware-errors/index.test.ts | 13 +++-- .../server-source-maps.test.ts | 53 ++++--------------- 5 files changed, 63 insertions(+), 59 deletions(-) diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index d61d491872c23..58d5c3ae519c6 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -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, diff --git a/packages/next/src/build/webpack/config/blocks/base.ts b/packages/next/src/build/webpack/config/blocks/base.ts index c8410ce3b0b8d..d7b2ed0501668 100644 --- a/packages/next/src/build/webpack/config/blocks/base.ts +++ b/packages/next/src/build/webpack/config/blocks/base.ts @@ -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 diff --git a/packages/next/src/server/patch-error-inspect.ts b/packages/next/src/server/patch-error-inspect.ts index 900aa180ffbe1..522e350f32c15 100644 --- a/packages/next/src/server/patch-error-inspect.ts +++ b/packages/next/src/server/patch-error-inspect.ts @@ -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) + : 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 { diff --git a/test/development/middleware-errors/index.test.ts b/test/development/middleware-errors/index.test.ts index 2fc1beef55848..1dd3d45f1ee61 100644 --- a/test/development/middleware-errors/index.test.ts +++ b/test/development/middleware-errors/index.test.ts @@ -138,8 +138,11 @@ describe('middleware - development errors', () => { '\n at eval (./middleware.js:4:9)' + '\n at (./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 ([project]/middleware.js [middleware] (ecmascript) (./middleware.js:3:13)' : '\n ⨯ Error: booooom!' + - '\n at (webpack:///middleware.js' + // TODO: Should be anonymous method without a method name + '\n at (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__ ' ) }) diff --git a/test/e2e/app-dir/server-source-maps/server-source-maps.test.ts b/test/e2e/app-dir/server-source-maps/server-source-maps.test.ts index 45aba1155c338..9f794beb20331 100644 --- a/test/e2e/app-dir/server-source-maps/server-source-maps.test.ts +++ b/test/e2e/app-dir/server-source-maps/server-source-maps.test.ts @@ -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,9 +87,9 @@ 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 | ^' + @@ -128,7 +97,7 @@ describe('app-dir - server source maps', () => { '\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 |' ) }) From cfa965ff114c9a502b976410ae38277d1d0efcc9 Mon Sep 17 00:00:00 2001 From: eps1lon Date: Sun, 27 Oct 2024 23:12:18 +0100 Subject: [PATCH 2/3] dev only to no leak machine information --- packages/next/src/build/webpack-config.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/next/src/build/webpack-config.ts b/packages/next/src/build/webpack-config.ts index 58d5c3ae519c6..3fa6a051fb746 100644 --- a/packages/next/src/build/webpack-config.ts +++ b/packages/next/src/build/webpack-config.ts @@ -1136,7 +1136,9 @@ export default async function getBaseWebpackConfig( // 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]', + devtoolModuleFilenameTemplate: dev + ? '[absolute-resource-path]' + : undefined, webassemblyModuleFilename: 'static/wasm/[modulehash].wasm', hashFunction: 'xxhash64', hashDigestLength: 16, From 679929ad8b470ee0804c8eb655c9899c75a5c89a Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Mon, 28 Oct 2024 13:34:41 +0100 Subject: [PATCH 3/3] Prefer `url.fileURLToPath ` Co-authored-by: Hendrik Liebau --- packages/next/src/server/patch-error-inspect.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/next/src/server/patch-error-inspect.ts b/packages/next/src/server/patch-error-inspect.ts index 522e350f32c15..2e96e413bb6b4 100644 --- a/packages/next/src/server/patch-error-inspect.ts +++ b/packages/next/src/server/patch-error-inspect.ts @@ -1,6 +1,6 @@ import { findSourceMap, type SourceMapPayload } from 'module' import * as path from 'path' -import { URL } from 'url' +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' @@ -38,7 +38,7 @@ function frameToString(frame: StackFrame): string { // 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) + path.relative(process.cwd(), url.fileURLToPath(frame.file)) : frame.file return frame.methodName