Repository: mochajs/mocha (js)

Issue:
🚀 Feature: Add support for outputting Error cause
We are using VError to gather up nested traces which is something proving to be very useful with promises and generators (we can actually get complete stack traces that way). However the default reporting with Mocha only outputs the first error in the stack.

For example, with this code:

```
const VError = require('verror');

describe('Test', () => {
  it('Test', () => {
    function1();
  });
});

function function1() {
  try {
    function2();
  } catch (e) {
    throw new VError(e);
  }
}

function function2() {
  throw new Error('Something Bad');
}
```

You get the output: 
```
 VError: : Something Bad
  at function1 (test\sandbox.js:13:11)
  at Context.it (test\sandbox.js:5:5)
  at Test.Runnable.run (node_modules\co-mocha\lib\co-mocha.js:43:16)
```

This doesn't actually tell me where the root error came from (in `function2`)

If I add a try...catch and then `console.error(e)` the top level error, I got a lot more detail:

```
{ VError: : Something Bad
    at function1 (C:\Work\NPS\nps-dev\test\sandbox.js:17:11)
    at Context.it (C:\Work\NPS\nps-dev\test\sandbox.js:6:7)
    at callFn (C:\Work\NPS\nps-dev\node_modules\mocha\lib\runnable.js:343:21)
    at Test.Runnable.run (C:\Work\NPS\nps-dev\node_modules\mocha\lib\runnable.js:335:7)
    at Test.Runnable.run (C:\Work\NPS\nps-dev\node_modules\co-mocha\lib\co-mocha.js:43:16)
    at Runner.runTest (C:\Work\NPS\nps-dev\node_modules\mocha\lib\runner.js:444:10)
    at C:\Work\NPS\nps-dev\node_modules\mocha\lib\runner.js:550:12
    at next (C:\Work\NPS\nps-dev\node_modules\mocha\lib\runner.js:361:14)
    at C:\Work\NPS\nps-dev\node_modules\mocha\lib\runner.js:371:7
    at next (C:\Work\NPS\nps-dev\node_modules\mocha\lib\runner.js:295:14)
    at Immediate.<anonymous> (C:\Work\NPS\nps-dev\node_modules\mocha\lib\runner.js:339:5)
    at runCallback (timers.js:649:20)
    at tryOnImmediate (timers.js:622:5)
    at processImmediate [as _immediateCallback] (timers.js:594:5)
  jse_shortmsg: '',
  jse_cause: 
   Error: Something Bad
       at function2 (C:\Work\NPS\nps-dev\test\sandbox.js:22:9)
       at function1 (C:\Work\NPS\nps-dev\test\sandbox.js:15:5)
       at Context.it (C:\Work\NPS\nps-dev\test\sandbox.js:6:7)
       at callFn (C:\Work\NPS\nps-dev\node_modules\mocha\lib\runnable.js:343:21)
       at Test.Runnable.run (C:\Work\NPS\nps-dev\node_modules\mocha\lib\runnable.js:335:7)
       at Test.Runnable.run (C:\Work\NPS\nps-dev\node_modules\co-mocha\lib\co-mocha.js:43:16)
       at Runner.runTest (C:\Work\NPS\nps-dev\node_modules\mocha\lib\runner.js:444:10)
       at C:\Work\NPS\nps-dev\node_modules\mocha\lib\runner.js:550:12
       at next (C:\Work\NPS\nps-dev\node_modules\mocha\lib\runner.js:361:14)
       at C:\Work\NPS\nps-dev\node_modules\mocha\lib\runner.js:371:7
       at next (C:\Work\NPS\nps-dev\node_modules\mocha\lib\runner.js:295:14)
       at Immediate.<anonymous> (C:\Work\NPS\nps-dev\node_modules\mocha\lib\runner.js:339:5)
       at runCallback (timers.js:649:20)
       at tryOnImmediate (timers.js:622:5)
       at processImmediate [as _immediateCallback] (timers.js:594:5),
  jse_info: {},
  message: ': Something Bad' }
```

Of course, this is incredibly verbose but I would prefer this over less. I suspect this is because of the features that have been implemented that clean up the stack traces. I don't see why those couldn't run recursively and clean up all the nested stacks and output something more like this:

```
{ VError: : Something Bad
    at function1 (C:\Work\NPS\nps-dev\test\sandbox.js:17:11)
    at Context.it (C:\Work\NPS\nps-dev\test\sandbox.js:6:7)
    at Test.Runnable.run (C:\Work\NPS\nps-dev\node_modules\co-mocha\lib\co-mocha.js:43:16)
   Error: Something Bad
       at function2 (C:\Work\NPS\nps-dev\test\sandbox.js:22:9)
       at function1 (C:\Work\NPS\nps-dev\test\sandbox.js:15:5)
       at Context.it (C:\Work\NPS\nps-dev\test\sandbox.js:6:7)
       at Test.Runnable.run (C:\Work\NPS\nps-dev\node_modules\co-mocha\lib\co-mocha.js:43:16)
```

Now I can see the original exception in `function2`.

Pull request description:
feat: include `.cause` stacks in the error stack traces
### Requirements

* Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
* All new code requires tests to ensure against regressions.

### Description of the Change

Appends the full error stack chain for errors with causes.

### Alternate Designs

<!-- Explain what other alternates were considered and why the proposed version was selected -->

Prior to the Error cause standardization, there was `VError` which I did a PR for supporting some years ago #2381. That one as primarily closed because an aversion to supporting random module hooks, which this now being a standard changes.

So I think this is now more a matter of:

* Should this be on by default?
* How should this be formatted?

### Why should this be in core?

All current browsers supports this property and Node.js as well since `16.x`.

See also eg:

* [V8 blog post](https://v8.dev/features/error-cause)
* [My blog post describing my ponyfill + helpers for it](https://dev.to/voxpelli/pony-cause-1-0-error-causes-2l2o)
* `pino` merging support for it: https://github.com/pinojs/pino-std-serializers/pull/78

### Benefits

Error causes are a great way to enrich errors and to ensure that one can catch all relevant places across an async workflow. See also eg. this old Joyent article explaining [how they uses VError](https://www.joyent.com/node-js/production/design/errors#5-if-you-pass-a-lower-level-error-to-your-caller-consider-wrapping-it-instead).

### Possible Drawbacks

<!-- What are the possible side-effects or negative impacts of the code change? -->

* It can become a bit too verbose if a lot of unnecessary causes gets printed, but is that an issue to take into consideration here?

### Applicable issues

<!--
* Enter any applicable Issues here.

* Mocha follows semantic versioning: http://semver.org

* Is this a breaking change (major release)?
* Is it an enhancement (minor release)?
* Is it a bug fix, or does it not impact production code (patch release)?
-->

* This would be a breaking change for everyone that relies on specific stack output for current error object's containing a stack possessing `cause` property, else it would be a minor. So depends on whether the specific stack output is seen as an API that should be stable or whether it's simply a presentation detail.

Fixes #2735.

Patch:
diff --git a/lib/reporters/base.js b/lib/reporters/base.js
index 40b59964..5af6e7bd 100644
--- a/lib/reporters/base.js
+++ b/lib/reporters/base.js
@@ -221,6 +221,56 @@ var generateDiff = (exports.generateDiff = function (actual, expected) {
   }
 });
 
+/**
+ * Traverses err.cause and returns all stack traces
+ *
+ * @private
+ * @param {Error} err
+ * @param {Set<Error>} [seen]
+ * @return {{ message: string, msg: string, stack: string }}
+ */
+var getFullErrorStack = function (err, seen) {
+  if (seen && seen.has(err)) {
+    return { message: '', msg: '<circular>', stack: '' };
+  }
+
+  var message;
+
+  if (typeof err.inspect === 'function') {
+    message = err.inspect() + '';
+  } else if (err.message && typeof err.message.toString === 'function') {
+    message = err.message + '';
+  } else {
+    message = '';
+  }
+
+  var msg;
+  var stack = err.stack || message;
+  var index = message ? stack.indexOf(message) : -1;
+
+  if (index === -1) {
+    msg = message;
+  } else {
+    index += message.length;
+    msg = stack.slice(0, index);
+    // remove msg from stack
+    stack = stack.slice(index + 1);
+
+    if (err.cause) {
+      seen = seen || new Set();
+      seen.add(err);
+      const causeStack = getFullErrorStack(err.cause, seen)
+      stack += '\n   Caused by: ' + causeStack.msg + (causeStack.stack ? '\n' + causeStack.stack : '');
+    }
+  }
+
+  return {
+    message,
+    msg,
+    stack
+  };
+};
+
 /**
  * Outputs the given `failures` as a list.
  *
@@ -241,7 +291,6 @@ exports.list = function (failures) {
       color('error stack', '\n%s\n');
 
     // msg
-    var msg;
     var err;
     if (test.err && test.err.multiple) {
       if (multipleTest !== test) {
@@ -252,25 +301,8 @@ exports.list = function (failures) {
     } else {
       err = test.err;
     }
-    var message;
-    if (typeof err.inspect === 'function') {
-      message = err.inspect() + '';
-    } else if (err.message && typeof err.message.toString === 'function') {
-      message = err.message + '';
-    } else {
-      message = '';
-    }
-    var stack = err.stack || message;
-    var index = message ? stack.indexOf(message) : -1;
 
-    if (index === -1) {
-      msg = message;
-    } else {
-      index += message.length;
-      msg = stack.slice(0, index);
-      // remove msg from stack
-      stack = stack.slice(index + 1);
-    }
+    var { message, msg, stack } = getFullErrorStack(err);
 
     // uncaught
     if (err.uncaught) {
diff --git a/lib/runner.js b/lib/runner.js
index 12807725..60a19f0e 100644
--- a/lib/runner.js
+++ b/lib/runner.js
@@ -443,11 +443,22 @@ Runner.prototype.fail = function (test, err, force) {
     err = thrown2Error(err);
   }
 
-  try {
-    err.stack =
-      this.fullStackTrace || !err.stack ? err.stack : stackFilter(err.stack);
-  } catch (ignore) {
-    // some environments do not take kindly to monkeying with the stack
+  // Filter the stack traces
+  if (!this.fullStackTrace) {
+    const alreadyFiltered = new Set();
+    let currentErr = err;
+
+    while (currentErr && currentErr.stack && !alreadyFiltered.has(currentErr)) {
+      alreadyFiltered.add(currentErr);
+
+      try {
+        currentErr.stack = stackFilter(currentErr.stack);
+      } catch (ignore) {
+        // some environments do not take kindly to monkeying with the stack
+      }
+
+      currentErr = currentErr.cause;
+    }
   }
 
   this.emit(constants.EVENT_TEST_FAIL, test, err);


Test patch:
diff --git a/test/reporters/base.spec.js b/test/reporters/base.spec.js
index 6b30b2cc..84778f36 100644
--- a/test/reporters/base.spec.js
+++ b/test/reporters/base.spec.js
@@ -491,6 +491,108 @@ describe('Base reporter', function () {
     expect(errOut, 'to be', '1) test title:\n     Error\n  foo\n  bar');
   });
 
+  describe('error causes', function () {
+    it('should append any error cause trail to stack traces', function () {
+      var err = {
+        message: 'Error',
+        stack: 'Error\nfoo\nbar',
+        showDiff: false,
+        cause: {
+          message: 'Cause1',
+          stack: 'Cause1\nbar\nfoo',
+          showDiff: false,
+          cause: {
+            message: 'Cause2',
+            stack: 'Cause2\nabc\nxyz',
+            showDiff: false
+          }
+        }
+      };
+      var test = makeTest(err);
+
+      list([test]);
+
+      var errOut = stdout.join('\n').trim();
+      expect(
+        errOut,
+        'to be',
+        '1) test title:\n     Error\n  foo\n  bar\n     Caused by: Cause1\n  bar\n  foo\n     Caused by: Cause2\n  abc\n  xyz'
+      );
+    });
+
+    it('should not get stuck in a hypothetical circular error cause trail', function () {
+      var err1 = {
+        message: 'Error',
+        stack: 'Error\nfoo\nbar',
+        showDiff: false,
+      };
+      var err2 = {
+        message: 'Cause1',
+        stack: 'Cause1\nbar\nfoo',
+        showDiff: false,
+        cause: err1
+      }
+      err1.cause = err2;
+
+      var test = makeTest(err1);
+
+      list([test]);
+
+      var errOut = stdout.join('\n').trim();
+      expect(
+        errOut,
+        'to be',
+        '1) test title:\n     Error\n  foo\n  bar\n     Caused by: Cause1\n  bar\n  foo\n     Caused by: <circular>'
+      );
+    });
+
+    it("should set an empty cause if neither 'inspect' nor 'message' is set", function () {
+      var err = {
+        message: 'Error',
+        stack: 'Error\nfoo\nbar',
+        showDiff: false,
+        cause: {
+          showDiff: false,
+        }
+      };
+
+      var test = makeTest(err);
+
+      list([test]);
+
+      var errOut = stdout.join('\n').trim();
+      expect(
+        errOut,
+        'to be',
+        '1) test title:\n     Error\n  foo\n  bar\n     Caused by:'
+      );
+    });
+
+    it('should not add cause trail if error does not contain message', function () {
+      var err = {
+        message: 'Error',
+        stack: 'foo\nbar',
+        showDiff: false,
+        cause: {
+          message: 'Cause1',
+          stack: 'Cause1\nbar\nfoo',
+          showDiff: false,
+          cause: {
+            message: 'Cause2',
+            stack: 'Cause2\nabc\nxyz',
+            showDiff: false
+          }
+        }
+      };
+      var test = makeTest(err);
+
+      list([test]);
+
+      var errOut = stdout.join('\n').trim();
+      expect(errOut, 'to be', '1) test title:\n     Error\n  foo\n  bar');
+    });
+  });
+
   it('should list multiple Errors per test', function () {
     var err = new Error('First Error');
     err.multiple = [new Error('Second Error - same test')];
diff --git a/test/unit/runner.spec.js b/test/unit/runner.spec.js
index dd965580..66ca6a05 100644
--- a/test/unit/runner.spec.js
+++ b/test/unit/runner.spec.js
@@ -629,6 +629,22 @@ describe('Runner', function () {
             });
             runner.fail(hook, err);
           });
+
+          it('should prettify stack-traces in error cause trail', function (done) {
+            var hook = new Hook();
+            hook.parent = suite;
+            var causeErr = new Error();
+            // Fake stack-trace
+            causeErr.stack = stack.join('\n');
+            var err = new Error();
+            err.cause = causeErr;
+
+            runner.on(EVENT_TEST_FAIL, function (_hook, _err) {
+              expect(_err.cause.stack, 'to be', stack.slice(0, 3).join('\n'));
+              done();
+            });
+            runner.fail(hook, err);
+          });
         });
 
         describe('long', function () {
@@ -647,6 +663,24 @@ describe('Runner', function () {
             });
             runner.fail(hook, err);
           });
+
+          it('should display full stack-traces in error cause trail', function (done) {
+            var hook = new Hook();
+            hook.parent = suite;
+            var causeErr = new Error();
+            // Fake stack-trace
+            causeErr.stack = stack.join('\n');
+            var err = new Error();
+            err.cause = causeErr;
+            // Add --stack-trace option
+            runner.fullStackTrace = true;
+
+            runner.on(EVENT_TEST_FAIL, function (_hook, _err) {
+              expect(_err.cause.stack, 'to be', stack.join('\n'));
+              done();
+            });
+            runner.fail(hook, err);
+          });
         });
 
         describe('ginormous', function () {