-
Notifications
You must be signed in to change notification settings - Fork 297
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
base: dev
Are you sure you want to change the base?
MISC: Improve stack trace of errors in transformed scripts #1951
Conversation
/** | ||
* 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 |
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.
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.)
src/utils/StackTraceUtils.ts
Outdated
* Filename in the stack line is actually `${hostname}/${filename}`. Check sourceURL in generateLoadedModule | ||
* (src\NetscriptJSEvaluator.ts). | ||
*/ | ||
if (!stackFrame.fileName.startsWith(workerScript.hostname)) { |
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.
Can't these mismatch due to module sharing across hosts?
@d0sboots Parsing the stack trace to get the correct filename is a real PITA. While reworking |
I'm not at all surprised that you shook loose a few edge-cases. Thanks for your continued dedication. |
With errors in JSX, TS, TSX scripts, we are showing the stack trace of the transformed scripts. This PR uses the generated source map to get the original error position. For more information, please check comments of d0sboots in #1812. I decided to parse the stack trace and recover the error position instead of only showing a custom file extension (i.e.,
.transformedJs
).Note: I choose
source-map-js
instead ofsource-map
. Please check README ofsource-map-js
for more information. Async APIs ofsource-map
's v7 are not suitable for us.Test save file:
test stack trace.gz
Before:
After: