split err.stack so embedded stacks in err message render once#5997
split err.stack so embedded stacks in err message render once#5997mostafaNazari702 wants to merge 1 commit into
Conversation
….message render once
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5997 +/- ##
==========================================
+ Coverage 80.89% 80.96% +0.07%
==========================================
Files 64 64
Lines 4602 4619 +17
Branches 976 981 +5
==========================================
+ Hits 3723 3740 +17
Misses 879 879 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
[Testing] This is really tricky stuff. Stack frames get surprisingly weirdly crafted in practice. A couple more test cases to add...
When the stack only contains frames:
var err = {
message: "Error",
stack: " at foo (foo.js:1:1)\n at bar (bar.js:2:2)",
showDiff: false,
};
var test = makeTest(err);Error output should be something like 1) test title:\n Error\n at foo (foo.js:1:1)\n at bar (bar.js:2:2).
When the embedded stack has a trailing newline, like:
var embeddedMessage =
"An error occured with following trace:\n\nError\n at inner (/original/foo.js:1:1)";
var stackMessage = embeddedMessage.replace(/\/original\//g, "/filtered/");
var err = {
message: embeddedMessage,
stack:
"Error: " + stackMessage + "\n at runTest (lib/runner.js:1:1)\n",
showDiff: false,
};The output still should smartly parse & not duplicate.
| var rawStack = err.stack || message; | ||
| var lines = rawStack.split("\n"); | ||
| var frameStart = lines.length; | ||
| for (var i = lines.length - 1; i >= 0; i--) { |
There was a problem hiding this comment.
[Bug] For an Error-like object with a stack containing only V8 frame lines:
{
message: "Boom",
stack: " at fn (foo.js:1:1)\n at run (foo.js:2:2)"
}This'll make msg "".
| ); | ||
| }); | ||
|
|
||
| it("should not duplicate the message when it contains an embedded stack", function () { |
There was a problem hiding this comment.
[Testing] This passes on the original implementation:
git checkout main -- test/reporters/base.spec.js
npx mocha test/reporters/base.spec.jsThe test should simulate the actual mismatch from the issue: err.message contains an embedded stack, but err.stack has the embedded stack slightly rewritten.
| var stack = err.stack || message; | ||
| var index = message ? stack.indexOf(message) : -1; | ||
| var rawStack = err.stack || message; | ||
| var lines = rawStack.split("\n"); |
There was a problem hiding this comment.
[Bug] Make sure this handles a trailing \n well - we don't want the last line to be "".
JoshuaKGoldberg
added
the
status: waiting for author
2 participants
PR Checklist
status: accepting prsOverview
getFullErrorStack used to split err.stack by searching for err.message in it. that breaks when mocha filters stack traces and rewrites file paths, message stays unchanged but the stack changes so indexOf fails and the error gets printed twice (message + full stack).
What we do now is instead of matching the message string we now parse the stack the V8 way: treat the trailing "at ......." lines as the stack frames and everything above as the message. If the stack doesn't match V8 format we fall back to the old behavior.
We also keep err.inspect() overrides and err.cause chaining working as before.