Skip to content

Commit

Permalink
feat: include .cause stacks in the error stack traces (#4829)
Browse files Browse the repository at this point in the history 
* Append the cause stacks to the main stack trace

It would be great to get the full error stack chain for errors with causes, especially as all current browsers and Node.js >=16 supports it, see eg https://v8.dev/features/error-cause and https://dev.to/voxpelli/pony-cause-1-0-error-causes-2l2o

Eg. `pino` merged support for this as well: pinojs/pino-std-serializers#78

* Fix tests

* Skip some string concatenation

* Don't export needlessly + improve docs

* Improved recursive filtering

* Added loop protection

* Same logic for "message" in cause trail as in top

* Apply suggestions from code review

Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>

* Revert "Apply suggestions from code review"

This reverts commit 04f7008.

---------

Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
  • Loading branch information
@voxpelli @JoshuaKGoldberg
voxpelli and JoshuaKGoldberg committed Mar 4, 2024
1 parent b88978d commit 3735873
Show file tree
Hide file tree
Showing 4 changed files with 203 additions and 24 deletions.
70 changes: 51 additions & 19 deletions lib/reporters/base.js
View file
Expand Up @@ -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.
*
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
21 changes: 16 additions & 5 deletions lib/runner.js
View file
Expand Up @@ -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);
Expand Down
102 changes: 102 additions & 0 deletions test/reporters/base.spec.js
View file
Expand Up @@ -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')];
Expand Down
34 changes: 34 additions & 0 deletions test/unit/runner.spec.js
View file
Expand Up @@ -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 () {
Expand All @@ -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 () {
Expand Down

0 comments on commit 3735873

Please sign in to comment.