Skip to content
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: Improve stack trace of errors in transformed scripts #1951

Draft
wants to merge 3 commits into
base: dev
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
"clsx": "^1.2.1",
"convert-source-map": "^2.0.0",
"date-fns": "^2.30.0",
"error-stack-parser": "^2.1.4",
"escodegen": "^2.1.0",
"jszip": "^3.10.1",
"material-ui-color": "^1.2.0",
Expand All @@ -47,6 +48,7 @@
"react-resizable": "^3.0.5",
"react-syntax-highlighter": "^15.5.0",
"remark-gfm": "^3.0.1",
"source-map-js": "^1.2.1",
"sprintf-js": "^1.1.3",
"tss-react": "^4.9.10"
},
Expand Down
55 changes: 6 additions & 49 deletions src/Netscript/ErrorMessages.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { WorkerScript } from "./WorkerScript";
import { ScriptDeath } from "./ScriptDeath";
import type { NetscriptContext } from "./APIWrapper";
import { parseStackTrace } from "../utils/StackTraceUtils";

/** Log a message to a script's logs */
export function log(ctx: NetscriptContext, message: () => string) {
Expand All @@ -25,57 +26,13 @@ export function basicErrorMessage(ws: WorkerScript | ScriptDeath, msg: string, t
* instance, then remove "unrelated" traces (code in our codebase) and leave only traces of the player's code.
*/
export function errorMessage(ctx: NetscriptContext, msg: string, type = "RUNTIME"): string {
const errstack = new Error().stack;
if (errstack === undefined) throw new Error("how did we not throw an error?");
const stack = errstack.split("\n").slice(1);
const ws = ctx.workerScript;
const caller = ctx.functionPath;
const userstack = [];
for (const stackline of stack) {
const filename = (() => {
// Check urls for dependencies
for (const [url, script] of ws.scriptRef.dependencies) if (stackline.includes(url)) return script.filename;
// Check for filenames directly if no URL found
if (stackline.includes(ws.scriptRef.filename)) return ws.scriptRef.filename;
for (const script of ws.scriptRef.dependencies.values()) {
if (stackline.includes(script.filename)) return script.filename;
}
})();
if (!filename) continue;

let call = { line: "-1", func: "unknown" };
const chromeCall = parseChromeStackline(stackline);
if (chromeCall) {
call = chromeCall;
}

const firefoxCall = parseFirefoxStackline(stackline);
if (firefoxCall) {
call = firefoxCall;
}

userstack.push(`${filename}:L${call.line}@${call.func}`);
}

const stackLines = parseStackTrace(new Error(), ws);
log(ctx, () => msg);
const caller = ctx.functionPath;
let rejectMsg = `${caller}: ${msg}`;
if (userstack.length !== 0) rejectMsg += `\n\nStack:\n${userstack.join("\n")}`;
return basicErrorMessage(ws, rejectMsg, type);

interface ILine {
line: string;
func: string;
}
function parseChromeStackline(line: string): ILine | null {
const lineMatch = line.match(/.*:(\d+):\d+.*/);
const funcMatch = line.match(/.*at (.+) \(.*/);
if (lineMatch && funcMatch) return { line: lineMatch[1], func: funcMatch[1] };
return null;
}
function parseFirefoxStackline(line: string): ILine | null {
const lineMatch = line.match(/.*:(\d+):\d+$/);
const lio = line.lastIndexOf("@");
if (lineMatch && lio !== -1) return { line: lineMatch[1], func: line.slice(0, lio) };
return null;
if (stackLines.length !== 0) {
rejectMsg += `\n\nStack: ${stackLines}`;
}
return basicErrorMessage(ws, rejectMsg, type);
}
2 changes: 1 addition & 1 deletion src/NetscriptJSEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ function generateLoadedModule(script: Script, scripts: Map<ScriptFilePath, Scrip
// directly return the module, without even attempting to fetch, due to the way
// modules work.
URL.revokeObjectURL(url);
script.mod = new LoadedModule(url, module);
script.mod = new LoadedModule(url, module, sourceURL, sourceMap);
moduleCache.set(newCode, new WeakRef(script.mod));
cleanup.register(script.mod, newCode);
}
Expand Down
2 changes: 1 addition & 1 deletion src/NetscriptWorker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ Otherwise, this can also occur if you have attempted to launch a script from a t
workerScript.log("", () =>
error instanceof ScriptDeath
? "main() terminated."
: getErrorMessageWithStackAndCause(error, "Script crashed due to an error: "),
: getErrorMessageWithStackAndCause(error, "Script crashed due to an error:\n", workerScript),
);
})
.finally(() => {
Expand Down
15 changes: 14 additions & 1 deletion src/Script/LoadedModule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,24 @@ export interface ScriptModule {
}

export class LoadedModule {
/** This is a blob URL */
url: ScriptURL;
module: Promise<ScriptModule>;
/**
* URL of the first script containing the exact code of the module. For example, let's say we have test.ts and
* test-clone.ts on "home", and they have the same code. If we run test.ts first, sourceUrl is "home/test.ts". When we
* run test-clone.ts after that, instead of generating another module, we reuse the cache module of test.ts.
*
* Each script instance (src\Script\Script.ts) has a property called "mod". That property is a LoadedModule instance.
* In the previous example, sourceUrl of mod of both test.ts and test-clone.ts are "home/test.ts".
*/
sourceUrl: string;
sourceMap?: string;

constructor(url: ScriptURL, module: Promise<ScriptModule>) {
constructor(url: ScriptURL, module: Promise<ScriptModule>, sourceUrl: string, sourceMap?: string) {
this.url = url;
this.module = module;
this.sourceUrl = sourceUrl;
this.sourceMap = sourceMap;
}
}
39 changes: 30 additions & 9 deletions src/utils/ErrorHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ export function handleUnknownError(e: unknown, ws: WorkerScript | null = null, i
}
if (ws && typeof e === "string") {
const headerText = basicErrorMessage(ws, "", "");
if (!e.includes(headerText)) e = basicErrorMessage(ws, e);
if (!e.includes(headerText)) {
e = basicErrorMessage(ws, e);
}
} else if (e instanceof SyntaxError) {
const msg = `${e.message} (sorry we can't be more helpful)`;
e = ws ? basicErrorMessage(ws, msg, "SYNTAX") : `SYNTAX ERROR:\n\n${msg}`;
Expand All @@ -21,17 +23,36 @@ export function handleUnknownError(e: unknown, ws: WorkerScript | null = null, i
if (e.name === "Canceled" && e.message === "Canceled") {
return;
}
const msg = getErrorMessageWithStackAndCause(e, "", ws);
/**
* Print the error to console. This is useful when the player wants to check the stack trace after closing the error
* dialog and relevant tail windows.
*/
if (ws) {
console.error(`An error was thrown in your script. Hostname: ${ws.hostname}, script name: ${ws.name}.`);
/**
* With scripts contain inline source map (e.g., transformed TypeScript/JSX scripts), the built-in developer tool
* of browsers uses the inline source map and show the original lines/columns in the stack trace. However, due to
* how we cache module, filenames in the stack trace may be wrong. For more information, please check
* parseStackTrace in src\utils\StackTraceUtils.ts.
*
* getErrorMessageWithStackAndCause uses parseStackTrace to recover correct debug information (file name, lines,
* columns).
*/
console.error(msg);
} else {
/**
* This happens when there is:
* - Uncaught async error in the player's script. [1]
* - Uncaught async error in our codebase or our dependencies. [2]
*
* In both cases, we don't have access to a WorkerScript instance, so printing the error as-is is fine. Doing that
* is also useful for [2]. Occasionally, our dependencies have bugs (especially monaco), so having the stack trace
* is good for debugging.
*/
console.error(e);
console.error("check this", msg);
}
/**
* If e is an instance of Error, we print it to the console. This is especially useful when debugging a TypeScript
* script. The stack trace in the error popup contains only the trace of the transpiled code. Even with a source
* map, parsing it to get the relevant info from the original TypeScript file is complicated. The built-in developer
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of this comment is no longer relevant. However, in general we shouldn't be logging things to console, so I think some explanation is still warranted - specifically the bit about how the links are still helpful with the built-in developer tools. (Without that, I don't think it would be worth doing this here.)

* tool of browsers will do that for us if we print the error to the console.
*/
console.error(e);
const msg = getErrorMessageWithStackAndCause(e);
e = ws ? basicErrorMessage(ws, msg) : `RUNTIME ERROR:\n\n${msg}`;
}
if (typeof e !== "string") {
Expand Down
24 changes: 18 additions & 6 deletions src/utils/ErrorHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type React from "react";
import type { Page } from "../ui/Router";
import { commitHash } from "./helpers/commitHash";
import { CONSTANTS } from "../Constants";
import { type LiteWorkerScript, parseStackTrace } from "./StackTraceUtils";

enum GameEnv {
Production,
Expand Down Expand Up @@ -54,7 +55,10 @@ export interface IErrorData {

export const newIssueUrl = `https://github.com/bitburner-official/bitburner-src/issues/new`;

export function parseUnknownError(error: unknown): {
export function parseUnknownError(
error: unknown,
ws: LiteWorkerScript | null,
): {
errorAsString: string;
stack?: string;
causeAsString?: string;
Expand All @@ -65,11 +69,19 @@ export function parseUnknownError(error: unknown): {
let causeAsString: string | undefined = undefined;
let causeStack: string | undefined = undefined;
if (error instanceof Error) {
stack = error.stack;
if (ws) {
stack = parseStackTrace(error, ws);
} else {
stack = error.stack;
}
if (error.cause != null) {
causeAsString = String(error.cause);
if (error.cause instanceof Error) {
causeStack = error.cause.stack;
if (ws) {
causeStack = parseStackTrace(error.cause, ws);
} else {
causeStack = error.cause.stack;
}
}
}
}
Expand All @@ -81,8 +93,8 @@ export function parseUnknownError(error: unknown): {
};
}

export function getErrorMessageWithStackAndCause(error: unknown, prefix = ""): string {
const errorData = parseUnknownError(error);
export function getErrorMessageWithStackAndCause(error: unknown, prefix = "", ws: LiteWorkerScript | null): string {
const errorData = parseUnknownError(error, ws);
let errorMessage = `${prefix}${errorData.errorAsString}`;
if (errorData.stack) {
errorMessage += `\nStack: ${errorData.stack}`;
Expand Down Expand Up @@ -127,7 +139,7 @@ export function getErrorMetadata(error: unknown, errorInfo?: React.ErrorInfo, pa

export function getErrorForDisplay(error: unknown, errorInfo?: React.ErrorInfo, page?: Page): IErrorData {
const metadata = getErrorMetadata(error, errorInfo, page);
const errorData = parseUnknownError(error);
const errorData = parseUnknownError(error, null);
const fileName = String(metadata.error.fileName);
const features =
`lang=${metadata.features.language} cookiesEnabled=${metadata.features.cookiesEnabled.toString()}` +
Expand Down
Loading