Core: Exclude or grey internal frames from stack traces in TAP reporter

Krinkle · Krinkle · commit 6388b2f099d5 · 2025-01-18T03:42:53.000Z
Cherry-picked from 9fed286 (3.0.0-dev): > Core: Exclude or grey internal frames from stack traces in TAP reporter > Internal frames are those from qunit.js, or Node.js runtime. > > * Remove any internal frames from the top of the stack. > * Grey out later internal frames anywhere in the stack. > > This change is applied to the TAP reporter, which the QUnit CLI uses > by default. Cherry-picked from 95105aa (3.0.0-dev): > Fix fragile code in stracktrace.js that previously worked only because > of Babel transformations masking a violation of the Temporal Dead Zone > between `const fileName` and the functions it uses to compute that > value.
diff --git a/src/core/stacktrace.js b/src/core/stacktrace.js
@@ -2,8 +2,8 @@
 //
 // This should reduce a raw stack trace like this:
 //
-// > foo.broken()@/src/foo.js
-// > Bar@/src/bar.js
+// > foo.broken()@/example/foo.js
+// > Bar@/example/bar.js
 // > @/test/bar.test.js
 // > @/lib/qunit.js:500:12
 // > @/lib/qunit.js:100:28
@@ -13,8 +13,8 @@
 //
 // and shorten it to show up until the end of the user's bar.test.js code.
 //
-// > foo.broken()@/src/foo.js
-// > Bar@/src/bar.js
+// > foo.broken()@/example/foo.js
+// > Bar@/example/bar.js
 // > @/test/bar.test.js
 //
 // QUnit will obtain one example trace (once per process/pageload suffices),
@@ -31,22 +31,89 @@
 //
 // See also:
 // - https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Error/Stack
-//
-const fileName = (sourceFromStacktrace(0) || '')
-  // Global replace, because a frame like localhost:4000/lib/qunit.js:1234:50,
-  // would otherwise (harmlessly, but uselessly) remove only the port (first match).
-  // https://github.com/qunitjs/qunit/issues/1769
-  .replace(/(:\d+)+\)?/g, '')
-  // Remove anything prior to the last slash (Unix/Windows) from the last frame,
-  // leaving only "qunit.js".
-  .replace(/.+[/\\]/, '');
+
+function qunitFileName () {
+  let error = new Error();
+  if (!error.stack) {
+    // Copy of sourceFromStacktrace() to avoid circular dependency
+    // Support: IE 9-11
+    try {
+      throw error;
+    } catch (err) {
+      error = err;
+    }
+  }
+  return (error.stack || '')
+    // Copy of extractStacktrace() to avoid circular dependency
+    // Support: V8/Chrome
+    .replace(/^error$\n/im, '')
+    .split('\n')[0]
+    // Global replace, because a frame like localhost:4000/lib/qunit.js:1234:50,
+    // would otherwise (harmlessly, but uselessly) remove only the port (first match).
+    // https://github.com/qunitjs/qunit/issues/1769
+    .replace(/(:\d+)+\)?/g, '')
+    // Remove anything prior to the last slash (Unix/Windows) from the last frame,
+    // leaving only "qunit.js".
+    .replace(/.+[/\\]/, '');
+}
+
+const fileName = qunitFileName();
+
+/**
+ * Responsibilities:
+ * - For internal errors from QUnit itself, remove the first qunit.js frames.
+ * - For errors in Node.js, format any remaining qunit.js and node:internal
+ *   frames as internal (i.e. grey out).
+ *
+ * @param {string} Error#stack
+ * @param {Function} formatInternal Format a string in an "internal" color
+ * @param {string|null} [eToString] Error#toString() to help remove
+ *  noise from Node.js/V8 stack traces.
+ */
+export function annotateStacktrace (stack, formatInternal, eToString = null) {
+  const frames = stack.split('\n');
+  const annotated = [];
+  if (eToString && eToString.indexOf(frames[0]) !== -1) {
+    // In Firefox and Safari e.stack starts with frame 0, but in V8 (Chrome/Node.js),
+    // e.stack starts first stringified message. Preserve this separately,
+    // so that, below, we can distinguish between internal frames on top
+    // (to remove) vs later internal frames (to format differently).
+    annotated.push(frames.shift());
+  }
+  let initialInternal = true;
+  for (let i = 0; i < frames.length; i++) {
+    const frame = frames[i];
+    const isInternal = (
+      (fileName && frame.indexOf(fileName) !== -1)
+      // Support Node 16+: ESM-style
+      // "at wrap (node:internal/modules/cjs/loader:1)"
+      || frame.indexOf('node:internal/') !== -1
+      // Support Node 10-14 (CJS-style)
+      // "at load (internal/modules/cjs/loader.js:7)"
+      || frame.match(/^\s+at .+\(internal[^)]*\)$/)
+    );
+    if (!isInternal) {
+      initialInternal = false;
+    }
+    // Remove initial internal frames entirely.
+    if (!initialInternal) {
+      annotated.push(isInternal ? formatInternal(frame) : frame);
+    }
+  }
+
+  return annotated.join('\n');
+}
 
 export function extractStacktrace (e, offset) {
   offset = offset === undefined ? 4 : offset;
 
   // Support: IE9, e.stack is not supported, we will return undefined
   if (e && e.stack) {
     const stack = e.stack.split('\n');
+    // In Firefox and Safari, e.stack starts immediately with the first frame.
+    //
+    // In V8 (Chrome/Node.js), the stack starts first with a stringified error message,
+    // and the real stack starting on line 2.
     if (/^error$/i.test(stack[0])) {
       stack.shift();
     }
@@ -69,8 +136,9 @@ export function extractStacktrace (e, offset) {
 export function sourceFromStacktrace (offset) {
   let error = new Error();
 
-  // Support: Safari <=7 only, IE <=10 - 11 only
-  // Not all browsers generate the `stack` property for `new Error()`, see also #636
+  // Support: IE 9-11, iOS 7
+  // Not all browsers generate the `stack` property for `new Error()`
+  // See also https://github.com/qunitjs/qunit/issues/636
   if (!error.stack) {
     try {
       throw error;
diff --git a/src/reporters/TapReporter.js b/src/reporters/TapReporter.js
@@ -1,6 +1,7 @@
 import kleur from 'kleur';
 import { errorString } from '../core/utilities';
 import { console } from '../globals';
+import { annotateStacktrace } from '../core/stacktrace';
 
 /**
  * Format a given value into YAML.
@@ -264,7 +265,10 @@ export default class TapReporter {
     if (error.stack) {
       // Since stacks aren't user generated, take a bit of liberty by
       // adding a trailing new line to allow a straight-forward YAML Blocks.
-      out += `\n  stack: ${prettyYamlValue(error.stack + '\n')}`;
+      const fmtStack = annotateStacktrace(error.stack, kleur.grey);
+      if (fmtStack.length) {
+        out += `\n  stack: ${prettyYamlValue(fmtStack + '\n')}`;
+      }
     }
 
     out += '\n  ...';
@@ -276,7 +280,10 @@ export default class TapReporter {
     out += `\n  message: ${prettyYamlValue(errorString(error))}`;
     out += `\n  severity: ${prettyYamlValue('failed')}`;
     if (error && error.stack) {
-      out += `\n  stack: ${prettyYamlValue(error.stack + '\n')}`;
+      const fmtStack = annotateStacktrace(error.stack, kleur.grey, error.toString());
+      if (fmtStack.length) {
+        out += `\n  stack: ${prettyYamlValue(fmtStack + '\n')}`;
+      }
     }
     out += '\n  ...';
     this.log(out);
diff --git a/test/cli/cli-main.js b/test/cli/cli-main.js
@@ -38,8 +38,6 @@ not ok 2 slow
   ---
   message: Test took longer than 7ms; test timed out.
   severity: failed
-  stack: |
-        at internal
   ...
 ok 3 config
 1..3
diff --git a/test/cli/fixtures/config-noglobals-add.tap.txt b/test/cli/fixtures/config-noglobals-add.tap.txt
@@ -6,8 +6,6 @@ not ok 1 adds global var
   ---
   message: Introduced global variable(s): dummyGlobal
   severity: failed
-  stack: |
-        at qunit.js
   ...
 1..1
 # pass 0
diff --git a/test/cli/fixtures/config-noglobals-remove.tap.txt b/test/cli/fixtures/config-noglobals-remove.tap.txt
@@ -6,8 +6,6 @@ not ok 1 deletes global var
   ---
   message: Deleted global variable(s): dummyGlobal
   severity: failed
-  stack: |
-        at qunit.js
   ...
 1..1
 # pass 0
diff --git a/test/cli/fixtures/config-testTimeout.tap.txt b/test/cli/fixtures/config-testTimeout.tap.txt
@@ -6,8 +6,6 @@ not ok 1 slow
   ---
   message: Test took longer than 10ms; test timed out.
   severity: failed
-  stack: |
-        at internal
   ...
 1..1
 # pass 0
diff --git a/test/cli/fixtures/done-after-timeout.tap.txt b/test/cli/fixtures/done-after-timeout.tap.txt
@@ -6,8 +6,6 @@ not ok 1 times out before scheduled done is called
   ---
   message: Test took longer than 10ms; test timed out.
   severity: failed
-  stack: |
-        at internal
   ...
 1..1
 # pass 0
diff --git a/test/cli/fixtures/pending-async-after-timeout.tap.txt b/test/cli/fixtures/pending-async-after-timeout.tap.txt
@@ -6,8 +6,6 @@ not ok 1 example
   ---
   message: Test took longer than 10ms; test timed out.
   severity: failed
-  stack: |
-        at internal
   ...
 1..1
 # pass 0
diff --git a/test/cli/fixtures/timeout.tap.txt b/test/cli/fixtures/timeout.tap.txt
@@ -6,8 +6,6 @@ not ok 1 timeout > first
   ---
   message: Test took longer than 10ms; test timed out.
   severity: failed
-  stack: |
-        at internal
   ...
 ok 2 timeout > second
 1..2
