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: #37252 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Ian Sutherland <ian@iansutherland.ca> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
nodejs · May 27, 2021 · de1de70 · de1de70
1 parent e08fa08
commit de1de70
Show file tree

Hide file tree

Showing 8 changed files with 41 additions and 49 deletions.
diff --git 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 : '<anonymous>'}`;
+          // 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
@@ -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.<anonymous> (*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.<anonymous> (*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
@@ -3,10 +3,8 @@
 	^
 
 ReferenceError: alert is not defined
-    at Object.<anonymous> (*tabs.coffee:39:5)
-        -> *tabs.coffee:26:2*
-    at Object.<anonymous> (*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
@@ -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.<anonymous> (*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
@@ -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.<anonymous> (*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
@@ -3,10 +3,8 @@
                        ^
 
 Error: an error
-    at Object.createElement (*icu.js:5:7)
-        -> *icu.jsx:3:23*
-    at Object.<anonymous> (*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
@@ -3,8 +3,6 @@
         ^
 
 Error: goodbye
-    at *uglify-throw.js:1:43
-        -> at Hello *uglify-throw-original.js:5:9*
-    at Immediate.<anonymous> (*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
@@ -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/)
   );
 }