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 2 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);
}
3 changes: 2 additions & 1 deletion src/NetscriptJSEvaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ function generateLoadedModule(script: Script, scripts: Map<ScriptFilePath, Scrip
// servers; it will be listed under the first server it was compiled for.
// We don't include this in the cache key, so that other instances of the
// script dedupe properly.
// If we change how sourceURL is constructed, we need to update parseStackTrace in src\utils\StackTraceUtils.ts.
const sourceURL = `${script.server}/${script.filename}`;
let adjustedCode = newCode + `\n//# sourceURL=${sourceURL}`;
if (sourceMap) {
Expand Down Expand Up @@ -208,7 +209,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, 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
4 changes: 3 additions & 1 deletion src/Script/LoadedModule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ export interface ScriptModule {
export class LoadedModule {
url: ScriptURL;
module: Promise<ScriptModule>;
sourceMap?: string;

constructor(url: ScriptURL, module: Promise<ScriptModule>) {
constructor(url: ScriptURL, module: Promise<ScriptModule>, sourceMap?: string) {
this.url = url;
this.module = module;
this.sourceMap = sourceMap;
}
}
12 changes: 4 additions & 8 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 @@ -24,14 +26,8 @@ export function handleUnknownError(e: unknown, ws: WorkerScript | null = null, i
if (ws) {
console.error(`An error was thrown in your script. Hostname: ${ws.hostname}, script name: ${ws.name}.`);
}
/**
* 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);
const msg = getErrorMessageWithStackAndCause(e, "", ws);
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
111 changes: 111 additions & 0 deletions src/utils/StackTraceUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
import ErrorStackParser from "error-stack-parser";
import { type RawSourceMap, SourceMapConsumer } from "source-map-js";

/**
* parseStackTrace uses some properties of workerScript, but the dependency chain of WorkerScript is long. In order to
* avoid worsening the dependency chain of parseStackTrace's callers, we create this minimal version of WorkerScript's
* type. It violates the DRY principle, but it's worth the trouble:
* - We rarely change WorkerScript.
* - The entire codebase is riddled with massive dependency chains. We should avoid worsening the situation.
*/
export type LiteWorkerScript = {
hostname: string;
scriptRef: {
dependencies: Map<
unknown,
{
filename: string;
mod: {
sourceMap?: string;
} | null;
}
>;
};
};

/**
* This function parses the stack trace of the error and returns only stack lines in the player's scripts. With
* transformed scripts, it also parses the source map to show the original lines/columns of the original scripts.
*
* For example: This stack:
*
* at errorMessage (webpack://bitburner/./src/Netscript/ErrorMessages.ts?:35:97)
* at Object.getServer (webpack://bitburner/./src/Netscript/NetscriptHelpers.tsx?:420:72)
* at eval (webpack://bitburner/./src/NetscriptFunctions.ts?:889:86)
* at Proxy.wrappedFunction (webpack://bitburner/./src/Netscript/APIWrapper.ts?:67:16)
* at test1 (home/a.ts:10:8)
* at main (home/a.ts:23:5)
* at startNetscript2Script (webpack://bitburner/./src/NetscriptWorker.ts?:91:9)
*
* Becomes:
*
* at test1 (home/a.ts:11:5)
* at main (home/a.ts:26:2)
*
* There are 2 changes:
* - All stack lines pointing to our codebase are stripped.
* - Stack lines show original lines/columns (e.g., a.ts:23:5 -> home/a.ts:26:2).
*/
export function parseStackTrace(error: Error, workerScript: LiteWorkerScript): string {
const stackFrames = ErrorStackParser.parse(error);
const stackLines = [error.message];
// Cache of found source maps.
const sourceMaps = new Map<string, string>();
for (const stackFrame of stackFrames) {
if (!stackFrame.fileName) {
continue;
}
/**
* Filename in the stack line is actually `${hostname}/${filename}`. Check sourceURL in generateLoadedModule
* (src\NetscriptJSEvaluator.ts).
*/
if (!stackFrame.fileName.startsWith(workerScript.hostname)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can't these mismatch due to module sharing across hosts?

continue;
}
const fileName = stackFrame.fileName.replace(`${workerScript.hostname}/`, "");
if (!fileName) {
continue;
}
let line = stackFrame.lineNumber;
let column = stackFrame.columnNumber;
if (line !== undefined && column !== undefined) {
let sourceMap = sourceMaps.get(fileName);
// If the source map is not in the cache, we try to find it.
if (!sourceMap) {
/**
* workerScript.scriptRef.dependencies contains directly or indirectly import, including the script itself.
* Check Script in src\Script\Script.ts.
*/
for (const script of workerScript.scriptRef.dependencies.values()) {
// Find the current script in workerScript.scriptRef.dependencies.
if (script.filename !== fileName) {
continue;
}
// Check if sourceMap exists.
if (script.mod?.sourceMap) {
sourceMap = script.mod.sourceMap;
// Put it in the cache.
sourceMaps.set(fileName, sourceMap);
}
break;
}
}
// Parse the source map if it exists.
if (sourceMap) {
/**
* SourceMap is generated by SWC, so we assume that it's valid. Validating it with ajv is unnecessary. If there
* are bugs in SWC or source-map-js, the try-catch block will ensure that the game won't crash.
*/
try {
const sourceMapConsumer = new SourceMapConsumer(JSON.parse(sourceMap) as RawSourceMap);
({ line, column } = sourceMapConsumer.originalPositionFor({ line, column }));
} catch (errorParsingSourceMap) {
console.error(errorParsingSourceMap);
console.error(`Cannot parse map of ${fileName} in ${workerScript.hostname}. Source map: ${sourceMap}`);
}
}
}
stackLines.push(` at ${stackFrame.functionName} (${workerScript.hostname}/${fileName}:${line}:${column})`);
}
return stackLines.join("\n");
}
2 changes: 1 addition & 1 deletion src/utils/helpers/exceptionAlert.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const errorSet = new Set<string>();
*/
export function exceptionAlert(error: unknown, showOnlyOnce = false): void {
console.error(error);
const errorData = parseUnknownError(error);
const errorData = parseUnknownError(error, null);
if (showOnlyOnce) {
// Calculate the "id" of the error.
const errorId = cyrb53(errorData.errorAsString + errorData.stack);
Expand Down