From be87132327ea28acd4bf7af09a401bac2374cb64 Mon Sep 17 00:00:00 2001 From: Will Binns-Smith Date: Fri, 1 Mar 2024 16:31:02 -0800 Subject: [PATCH] Turbopack: Trace server app render errors through source maps (#62611) Previously, errors shown in the error overlay, these stir were left untraced through source maps. Test Plan: `TURBOPACK=1 pnpm test-dev test/development/app-render-error-log/app-render-error-log.test.ts` Closes PACK-2608 --- .../server/middleware-turbopack.ts | 2 +- packages/next/src/lib/is-error.ts | 1 + .../lib/router-utils/setup-dev-bundler.ts | 111 ++++++++++++++++-- .../app-render-error-log.test.ts | 2 +- test/turbopack-tests-manifest.json | 5 +- 5 files changed, 105 insertions(+), 16 deletions(-) diff --git a/packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts b/packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts index 911e1e3536..150fd21e2a 100644 --- a/packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts +++ b/packages/next/src/client/components/react-dev-overlay/server/middleware-turbopack.ts @@ -24,7 +24,7 @@ interface TurbopackStackFrame { } const currentSourcesByFile: Map> = new Map() -async function batchedTraceSource( +export async function batchedTraceSource( project: Project, frame: TurbopackStackFrame ) { diff --git a/packages/next/src/lib/is-error.ts b/packages/next/src/lib/is-error.ts index 31ba3e4a47..f9bf9104a3 100644 --- a/packages/next/src/lib/is-error.ts +++ b/packages/next/src/lib/is-error.ts @@ -6,6 +6,7 @@ export interface NextError extends Error { page?: string code?: string | number cancelled?: boolean + digest?: number } export default function isError(err: unknown): err is NextError { diff --git a/packages/next/src/server/lib/router-utils/setup-dev-bundler.ts b/packages/next/src/server/lib/router-utils/setup-dev-bundler.ts index ef6aca7b6d..2ffa2a9a68 100644 --- a/packages/next/src/server/lib/router-utils/setup-dev-bundler.ts +++ b/packages/next/src/server/lib/router-utils/setup-dev-bundler.ts @@ -6,14 +6,14 @@ import type { MiddlewareRouteMatch } from '../../../shared/lib/router/utils/midd import type { PropagateToWorkersField } from './types' import type { NextJsHotReloaderInterface } from '../../dev/hot-reloader-types' -import { createDefineEnv } from '../../../build/swc' +import { createDefineEnv, type Project } from '../../../build/swc' import fs from 'fs' import url from 'url' import path from 'path' import qs from 'querystring' import Watchpack from 'next/dist/compiled/watchpack' import { loadEnvConfig } from '@next/env' -import isError from '../../../lib/is-error' +import isError, { type NextError } from '../../../lib/is-error' import findUp from 'next/dist/compiled/find-up' import { buildCustomRoute } from './filesystem' import * as Log from '../../../build/output/log' @@ -64,13 +64,17 @@ import { getSourceById, parseStack, } from '../../../client/components/react-dev-overlay/server/middleware' -import { createOriginalStackFrame as createOriginalTurboStackFrame } from '../../../client/components/react-dev-overlay/server/middleware-turbopack' +import { + batchedTraceSource, + createOriginalStackFrame as createOriginalTurboStackFrame, +} from '../../../client/components/react-dev-overlay/server/middleware-turbopack' import { devPageFiles } from '../../../build/webpack/plugins/next-types-plugin/shared' import type { LazyRenderServerInstance } from '../router-server' import { HMR_ACTIONS_SENT_TO_BROWSER } from '../../dev/hot-reloader-types' import { PAGE_TYPES } from '../../../lib/page-types' import { createHotReloaderTurbopack } from '../../dev/hot-reloader-turbopack' import { getErrorSource } from '../../../shared/lib/error-source' +import type { StackFrame } from 'stacktrace-parser' export type SetupOpts = { renderServer: LazyRenderServerInstance @@ -953,17 +957,33 @@ async function startWatcher(opts: SetupOpts) { Log[type === 'warning' ? 'warn' : 'error']( `${file} (${lineNumber}:${column}) @ ${methodName}` ) + + let errorToLog if (isEdgeCompiler) { - err = err.message - } - if (type === 'warning') { - Log.warn(err) - } else if (type === 'app-dir') { - logAppDirError(err) - } else if (type) { - Log.error(`${type}:`, err) + errorToLog = err.message + } else if (isError(err) && hotReloader.turbopackProject) { + const stack = await traceTurbopackErrorStack( + hotReloader.turbopackProject, + err, + frames + ) + + const error: NextError = new Error(err.message) + error.stack = stack + error.digest = err.digest + errorToLog = error } else { - Log.error(err) + errorToLog = err + } + + if (type === 'warning') { + Log.warn(errorToLog) + } else if (type === 'app-dir') { + logAppDirError(errorToLog) + } else if (type) { + Log.error(`${type}:`, errorToLog) + } else { + Log.error(errorToLog) } console[type === 'warning' ? 'warn' : 'error'](originalCodeFrame) usedOriginalStack = true @@ -1036,3 +1056,70 @@ export async function setupDevBundler(opts: SetupOpts) { } export type DevBundler = Awaited> + +// Returns a trace rewritten through Turbopack's sourcemaps +async function traceTurbopackErrorStack( + project: Project, + error: Error, + frames: StackFrame[] +): Promise { + let originalFrames = await Promise.all( + frames.map(async (f) => { + try { + const traced = await batchedTraceSource(project, { + file: f.file!, + methodName: f.methodName, + line: f.lineNumber ?? 0, + column: f.column, + isServer: true, + }) + + return traced?.frame ?? f + } catch { + return f + } + }) + ) + + return ( + error.name + + ': ' + + error.message + + '\n' + + originalFrames + .map((f) => { + if (f == null) { + return null + } + + let line = ' at' + if (f.methodName != null) { + line += ' ' + f.methodName + } + + if (f.file != null) { + const file = + f.file.startsWith('/') || + // Built-in "filenames" like `` shouldn't be made relative + f.file.startsWith('<') || + f.file.startsWith('node:') + ? f.file + : `./${f.file}` + + line += ` (${file}` + if (f.lineNumber != null) { + line += ':' + f.lineNumber + + if (f.column != null) { + line += ':' + f.column + } + } + line += ')' + } + + return line + }) + .filter(Boolean) + .join('\n') + ) +} diff --git a/test/development/app-render-error-log/app-render-error-log.test.ts b/test/development/app-render-error-log/app-render-error-log.test.ts index cc3c98de1d..f9b2c7386d 100644 --- a/test/development/app-render-error-log/app-render-error-log.test.ts +++ b/test/development/app-render-error-log/app-render-error-log.test.ts @@ -17,7 +17,7 @@ createNextDescribe( await check(() => cliOutput, /digest:/) expect(cliOutput).toInclude('Error: boom') expect(cliOutput).toInclude('at fn2 (./app/fn.ts') - expect(cliOutput).toInclude('at fn1 (./app/fn.ts') + expect(cliOutput).toMatch(/at (Module\.)?fn1 \(\.\/app\/fn\.ts/) expect(cliOutput).toInclude('at Page (./app/page.tsx') expect(cliOutput).not.toInclude('webpack-internal') diff --git a/test/turbopack-tests-manifest.json b/test/turbopack-tests-manifest.json index e7accd094a..45ed8905e2 100644 --- a/test/turbopack-tests-manifest.json +++ b/test/turbopack-tests-manifest.json @@ -1513,9 +1513,10 @@ "runtimeError": false }, "test/development/app-render-error-log/app-render-error-log.test.ts": { - "passed": [], + "passed": [ + "app-render-error-log should log the correct values on app-render error" + ], "failed": [ - "app-render-error-log should log the correct values on app-render error", "app-render-error-log should log the correct values on app-render error with edge runtime" ], "pending": [],