From 9417fd0bc81edf500dde21a47f304ccbb19ffba1 Mon Sep 17 00:00:00 2001 From: bcoe Date: Sat, 6 Feb 2021 11:11:35 -0800 Subject: [PATCH] errors: align source-map stacks with spec Reformat stack traces when --enable-source-maps flag is set to format more likely to fit https://github.com/tc39/proposal-error-stacks PR-URL: https://github.com/nodejs/node/pull/37252 Reviewed-By: Rich Trott Reviewed-By: Ian Sutherland Reviewed-By: Gus Caplan Reviewed-By: Zeyu Yang Reviewed-By: Benjamin Gruenbaum Reviewed-By: Joyee Cheung --- .../source_map/prepare_stack_trace.js | 25 ++++++++++++------- .../message/source_map_enclosing_function.out | 15 ++++------- .../source_map_reference_error_tabs.out | 6 ++--- test/message/source_map_throw_catch.out | 6 ++--- test/message/source_map_throw_first_tick.out | 6 ++--- test/message/source_map_throw_icu.out | 6 ++--- .../source_map_throw_set_immediate.out | 6 ++--- test/parallel/test-source-map-enable.js | 20 +++++++-------- 8 files changed, 41 insertions(+), 49 deletions(-) diff --git a/lib/internal/source_map/prepare_stack_trace.js b/lib/internal/source_map/prepare_stack_trace.js index c32f9780c2fb3f..6b8d4e566ff1b1 100644 --- a/lib/internal/source_map/prepare_stack_trace.js +++ b/lib/internal/source_map/prepare_stack_trace.js @@ -51,8 +51,7 @@ const prepareStackTrace = (globalThis, error, trace) => { let lastSourceMap; let lastFileName; const preparedTrace = ArrayPrototypeJoin(ArrayPrototypeMap(trace, (t, i) => { - let str = i !== 0 ? '\n at ' : ''; - str = `${str}${t}`; + const str = i !== 0 ? '\n at ' : ''; try { // A stack trace will often have several call sites in a row within the // same file, cache the source map and file content accordingly: @@ -81,21 +80,29 @@ const prepareStackTrace = (globalThis, error, trace) => { originalColumn ); } - // Show both original and transpiled stack trace information: - const prefix = (name && name !== t.getFunctionName()) ? - `\n -> at ${name}` : - '\n ->'; + // Construct call site name based on: v8.dev/docs/stack-trace-api: + const fnName = t.getFunctionName() ?? t.getMethodName(); + const originalName = `${t.getTypeName() !== 'global' ? + `${t.getTypeName()}.` : ''}${fnName ? fnName : ''}`; + // The original call site may have a different symbol name + // associated with it, use it: + const prefix = (name && name !== originalName) ? + `${name}` : + `${originalName ? originalName : ''}`; + const hasName = !!(name || originalName); const originalSourceNoScheme = StringPrototypeStartsWith(originalSource, 'file://') ? fileURLToPath(originalSource) : originalSource; - str += `${prefix} (${originalSourceNoScheme}:${originalLine + 1}:` + - `${originalColumn + 1})`; + // Replace the transpiled call site with the original: + return `${str}${prefix}${hasName ? ' (' : ''}` + + `${originalSourceNoScheme}:${originalLine + 1}:` + + `${originalColumn + 1}${hasName ? ')' : ''}`; } } } catch (err) { debug(err.stack); } - return str; + return `${str}${t}`; }), ''); return `${errorSource}${errorString}\n at ${preparedTrace}`; }; diff --git a/test/message/source_map_enclosing_function.out b/test/message/source_map_enclosing_function.out index 370e3a61f3a597..057c1e762993c9 100644 --- a/test/message/source_map_enclosing_function.out +++ b/test/message/source_map_enclosing_function.out @@ -3,16 +3,11 @@ ^ Error: an error! - at functionD (*enclosing-call-site-min.js:1:156) - -> (*enclosing-call-site.js:16:17) - at functionC (*enclosing-call-site-min.js:1:97) - -> (*enclosing-call-site.js:10:3) - at functionB (*enclosing-call-site-min.js:1:60) - -> (*enclosing-call-site.js:6:3) - at functionA (*enclosing-call-site-min.js:1:26) - -> (*enclosing-call-site.js:2:3) - at Object. (*enclosing-call-site-min.js:1:199) - -> (*enclosing-call-site.js:24:3) + at functionD (*enclosing-call-site.js:16:17) + at functionC (*enclosing-call-site.js:10:3) + at functionB (*enclosing-call-site.js:6:3) + at functionA (*enclosing-call-site.js:2:3) + at Object. (*enclosing-call-site.js:24:3) at Module._compile (internal/modules/cjs/loader.js:*) at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) diff --git a/test/message/source_map_reference_error_tabs.out b/test/message/source_map_reference_error_tabs.out index b2a2541c8c5ee3..275c703c661212 100644 --- a/test/message/source_map_reference_error_tabs.out +++ b/test/message/source_map_reference_error_tabs.out @@ -3,10 +3,8 @@ ^ ReferenceError: alert is not defined - at Object. (*tabs.coffee:39:5) - -> *tabs.coffee:26:2* - at Object. (*tabs.coffee:53:4) - -> *tabs.coffee:1:14* + at *tabs.coffee:26:2* + at *tabs.coffee:1:14* at Module._compile (internal/modules/cjs/loader.js:* at Object.Module._extensions..js (internal/modules/cjs/loader.js:* at Module.load (internal/modules/cjs/loader.js:* diff --git a/test/message/source_map_throw_catch.out b/test/message/source_map_throw_catch.out index 94533eaa8dbb80..b2123e6eebd1b4 100644 --- a/test/message/source_map_throw_catch.out +++ b/test/message/source_map_throw_catch.out @@ -3,10 +3,8 @@ reachable throw Error('an exception'); ^ Error: an exception - at branch (*typescript-throw.js:20:15) - -> *typescript-throw.ts:18:11* - at Object. (*typescript-throw.js:26:1) - -> *typescript-throw.ts:24:1* + at *typescript-throw.ts:18:11* + at *typescript-throw.ts:24:1* at Module._compile (internal/modules/cjs/loader.js:*) at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) diff --git a/test/message/source_map_throw_first_tick.out b/test/message/source_map_throw_first_tick.out index e561d9818393f3..e4a8e9ff25bb3e 100644 --- a/test/message/source_map_throw_first_tick.out +++ b/test/message/source_map_throw_first_tick.out @@ -3,10 +3,8 @@ reachable throw Error('an exception'); ^ Error: an exception - at branch (*typescript-throw.js:20:15) - -> *typescript-throw.ts:18:11* - at Object. (*typescript-throw.js:26:1) - -> *typescript-throw.ts:24:1* + at *typescript-throw.ts:18:11* + at *typescript-throw.ts:24:1* at Module._compile (internal/modules/cjs/loader.js:*) at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) diff --git a/test/message/source_map_throw_icu.out b/test/message/source_map_throw_icu.out index af437d08e95e94..9ceb20125f7d9a 100644 --- a/test/message/source_map_throw_icu.out +++ b/test/message/source_map_throw_icu.out @@ -3,10 +3,8 @@ ^ Error: an error - at Object.createElement (*icu.js:5:7) - -> *icu.jsx:3:23* - at Object. (*icu.js:8:82) - -> *icu.jsx:9:5* + at *icu.jsx:3:23* + at *icu.jsx:9:5* at Module._compile (internal/modules/cjs/loader.js:* at Object.Module._extensions..js (internal/modules/cjs/loader.js:* at Module.load (internal/modules/cjs/loader.js:* diff --git a/test/message/source_map_throw_set_immediate.out b/test/message/source_map_throw_set_immediate.out index 15d91388440f45..08d7f76fcfcc2a 100644 --- a/test/message/source_map_throw_set_immediate.out +++ b/test/message/source_map_throw_set_immediate.out @@ -3,8 +3,6 @@ ^ Error: goodbye - at *uglify-throw.js:1:43 - -> at Hello *uglify-throw-original.js:5:9* - at Immediate. (*uglify-throw.js:1:60) - -> *uglify-throw-original.js:9:3* + at Hello *uglify-throw-original.js:5:9* + at *uglify-throw-original.js:9:3* at processImmediate (internal/timers.js:*) diff --git a/test/parallel/test-source-map-enable.js b/test/parallel/test-source-map-enable.js index 6435aa99d2c752..9df38cba6cac91 100644 --- a/test/parallel/test-source-map-enable.js +++ b/test/parallel/test-source-map-enable.js @@ -155,11 +155,11 @@ function nextdir() { require.resolve('../fixtures/source-map/uglify-throw.js') ]); assert.strictEqual( - output.stderr.toString().match(/->.*uglify-throw-original\.js:5:9/), + output.stderr.toString().match(/.*uglify-throw-original\.js:5:9/), null ); assert.strictEqual( - output.stderr.toString().match(/->.*uglify-throw-original\.js:9:3/), + output.stderr.toString().match(/.*uglify-throw-original\.js:9:3/), null ); } @@ -172,11 +172,11 @@ function nextdir() { ]); assert.match( output.stderr.toString(), - /->.*uglify-throw-original\.js:5:9/ + /.*uglify-throw-original\.js:5:9/ ); assert.match( output.stderr.toString(), - /->.*uglify-throw-original\.js:9:3/ + /.*uglify-throw-original\.js:9:3/ ); assert.match(output.stderr.toString(), /at Hello/); } @@ -187,8 +187,8 @@ function nextdir() { '--enable-source-maps', require.resolve('../fixtures/source-map/typescript-throw.js') ]); - assert.ok(output.stderr.toString().match(/->.*typescript-throw\.ts:18:11/)); - assert.ok(output.stderr.toString().match(/->.*typescript-throw\.ts:24:1/)); + assert.ok(output.stderr.toString().match(/.*typescript-throw\.ts:18:11/)); + assert.ok(output.stderr.toString().match(/.*typescript-throw\.ts:24:1/)); } // Applies source-maps generated by babel to stack trace. @@ -198,7 +198,7 @@ function nextdir() { require.resolve('../fixtures/source-map/babel-throw.js') ]); assert.ok( - output.stderr.toString().match(/->.*babel-throw-original\.js:18:31/) + output.stderr.toString().match(/.*babel-throw-original\.js:18:31/) ); } @@ -209,10 +209,10 @@ function nextdir() { require.resolve('../fixtures/source-map/istanbul-throw.js') ]); assert.ok( - output.stderr.toString().match(/->.*istanbul-throw-original\.js:5:9/) + output.stderr.toString().match(/.*istanbul-throw-original\.js:5:9/) ); assert.ok( - output.stderr.toString().match(/->.*istanbul-throw-original\.js:9:3/) + output.stderr.toString().match(/.*istanbul-throw-original\.js:9:3/) ); } @@ -223,7 +223,7 @@ function nextdir() { require.resolve('../fixtures/source-map/babel-esm.mjs') ]); assert.ok( - output.stderr.toString().match(/->.*babel-esm-original\.mjs:9:29/) + output.stderr.toString().match(/.*babel-esm-original\.mjs:9:29/) ); }