Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign uplib: fix inspector links for stack traces #35725
Open
+52
−15
Conversation
When the chrome inspector (chrome://inspect) display stack traces, it makes the filename clickable, and developer can easily locate the relevant source with a single click. This only works when filenames are valid URLs. The CJS loader seems to compile sources with absolute path instead of URL, breaking the inspector functionality. This is especially apparent when node_modules contain "at" symbol (@). The loader already presents module to the inspector using file URL, the same URL should be used for compiling the source.
|
Review requested: |
It seems getErrMessage() function uses filename from a CallSite to open a file and extract source code location of the statement. The CallSite now returns file URL, so it must be converted back to path.
Source maps seems to use CallSite from the prepareStackTrace to the path to the original sources on the stack. Finding the source map should convert file URL to path and attached information should be file URL instead of path, so the inspector can show a proper link. Additionally, it seems URL to path conversion was made by stripping "file://" prefix, which might cause problem on windows. The url module has function to deal with that.
Various tests explore the stack trace or CallSite objects' filename. They are modified now to match file URL instead.
Dragiyski
changed the title
Codecov Report
@@ Coverage Diff @@
## master #35725 +/- ##
==========================================
- Coverage 96.40% 96.40% -0.01%
==========================================
Files 222 223 +1
Lines 73682 73713 +31
==========================================
+ Hits 71034 71061 +27
- Misses 2648 2652 +4
Continue to review full report at Codecov.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
When the chrome inspector (chrome://inspect) display stack traces, it
makes the filename clickable, and developer can easily locate the
relevant source with a single click. This only works when filenames are
valid URLs. The CJS loader seems to compile sources with absolute path
instead of URL, breaking the inspector functionality. This is especially
apparent when node_modules contain "at" symbol (@). The loader already
presents module to the inspector using file URL, the same URL should be
used for compiling the source.
For example the current stack trace looks like this:
and as shown the link covers only the part after the first slash after the
@directory. The link leads to URL starting with slash (/), which opens an empty (about:blank) tab within the browser. This is an expected behavior, because in URLs@representsuser@hostdelimiter and the path is not valid absolute URL as it does not have protocol. A properly fixed URL looks like this:and clicking on the link provided in inspector opens the actual location of the source for all call sites, including node_modules containing
@symbol in their path. For the console, the path is printed withfile://in front and clickable in some terminals.Note: (...) replaces a valid path omitted for security reasons.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes