-
Notifications
You must be signed in to change notification settings - Fork 270
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
MISC: Add error cause to exceptionAlert and Recovery mode #1772
MISC: Add error cause to exceptionAlert and Recovery mode #1772
Conversation
Before you decide on this, test the result of error.toString(), which IIRC is different than String(error). You'll also want to look at the results on both Chrome and Firefox, since they have distinctly different error logging. |
Test code: /** @param {NS} ns */
export async function main(ns) {
ns.clearLog();
const error1 = new Error("error1");
const error2 = new Error("error2", { cause: error1 });
const error3 = "error3";
ns.print(`error1: String constructor: ${String(error1)}`);
ns.print(`error2: String constructor: ${String(error2)}`);
ns.print(`error3: String constructor: ${String(error3)}`);
ns.print(`error1: toString(): ${error1.toString()}`);
ns.print(`error2: toString(): ${error2.toString()}`);
ns.print(`error3: toString(): ${error3.toString()}`);
} Chrome:
Firefox:
The difference between Chrome and Firefox shows up when we use /** @param {NS} ns */
export async function main(ns) {
ns.clearLog();
const error1 = new Error("error1");
const error2 = new Error("error2", { cause: error1 });
ns.print(`error1.stack: ${error1.stack}`);
ns.print(`error2.stack: ${error2.stack}`);
} Chrome:
Firefox:
Some non-standard properties are implemented by only Firefox (e.g., columnNumber, fileName, lineNumber). |
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.
We have a complicated bit of code somewhere that attempts to parse and partially rewrite stack traces. It feels like this should potentially be unified with that, so we don't have so many different error handling functions bumping around.
You get a nicely formatted error with both the message, stack, and cause, if you directly |
I guess that other function turned out not to be relevant? |
I checked that code (
|
Let's check this code:
Example output:
Without
e.cause
, we cannot know the root cause (was the error thrown bythrow1
orthrow2
?). This PR adds theerror.cause
to relevant error reporting UI.Screenshots: