-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
split err.stack so embedded stacks in err message render once #5997
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -404,38 +404,58 @@ var getFullErrorStack = function (err, seen) { | |
| } | ||
|
|
||
| var message; | ||
| var usedInspect = false; | ||
|
|
||
| if (typeof err.inspect === "function") { | ||
| message = err.inspect() + ""; | ||
| usedInspect = true; | ||
| } 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; | ||
| var rawStack = err.stack || message; | ||
| var lines = rawStack.split("\n"); | ||
| var frameStart = lines.length; | ||
| for (var i = lines.length - 1; i >= 0; i--) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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 |
||
| if (/^\s+at\s/.test(lines[i])) { | ||
| frameStart = i; | ||
| } else { | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (index === -1) { | ||
| msg = message; | ||
| var msg; | ||
| var stack; | ||
| var splitSucceeded = false; | ||
| if (frameStart < lines.length) { | ||
| stack = lines.slice(frameStart).join("\n"); | ||
| msg = usedInspect ? message : lines.slice(0, frameStart).join("\n"); | ||
| splitSucceeded = true; | ||
| } 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 : ""); | ||
| var index = message ? rawStack.indexOf(message) : -1; | ||
| if (index === -1) { | ||
| msg = message; | ||
| stack = rawStack; | ||
| } else { | ||
| index += message.length; | ||
| msg = rawStack.slice(0, index); | ||
| stack = rawStack.slice(index + 1); | ||
| splitSucceeded = true; | ||
| } | ||
| } | ||
|
|
||
| if (splitSucceeded && 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, | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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 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. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -568,6 +568,31 @@ describe("Base reporter", function () { | |
| ); | ||
| }); | ||
|
|
||
| it("should not duplicate the message when it contains an embedded stack", function () { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [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: |
||
| var embeddedMessage = | ||
| "An error occured with following trace:\n\nError\n at inner (foo.js:1:1)\n at outer (foo.js:2:2)"; | ||
| var err = { | ||
| message: embeddedMessage, | ||
| stack: | ||
| "Error: " + | ||
| embeddedMessage + | ||
| "\n at outer (foo.js:2:2)\n at runTest (lib/runner.js:1:1)", | ||
| showDiff: false, | ||
| }; | ||
| var test = makeTest(err); | ||
|
|
||
| list([test]); | ||
|
|
||
| var errOut = stdout.join("\n").trim(); | ||
| expect( | ||
| errOut.match(/Error: An error occured with following trace:/g), | ||
| "to have length", | ||
| 1, | ||
| ); | ||
| expect(errOut, "to contain", "at runTest (lib/runner.js:1:1)"); | ||
| expect(errOut.match(/at inner \(foo\.js:1:1\)/g), "to have length", 1); | ||
| }); | ||
|
|
||
| it("should not add cause trail if error does not contain message", function () { | ||
| var err = { | ||
| message: "Error", | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Bug] Make sure this handles a trailing
\nwell - we don't want the last line to be"".