-
Notifications
You must be signed in to change notification settings - Fork 30k
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
errors: display original symbol name #36042
Conversation
Review requested:
|
@@ -64,14 +64,14 @@ const prepareStackTrace = (globalThis, error, trace) => { | |||
const { | |||
originalLine, | |||
originalColumn, | |||
originalSource | |||
originalSource, | |||
name |
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.
nit: consider adding a trailing comma to make future PR diffs cleaner?
name | |
name, |
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.
I think this is a style that a separate PR should use linting to enforce, and also apply everywhere. (iow i think it’s a great change, but i don’t think node’s followed it consistently thus far)
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.
Agreed, to be clear I'm +1 landing this with or without the trailing commas.
const originalSourceNoScheme = | ||
StringPrototypeStartsWith(originalSource, 'file://') ? | ||
fileURLToPath(originalSource) : originalSource; | ||
str += `\n -> ${originalSourceNoScheme}:${originalLine + 1}:` + | ||
`${originalColumn + 1}`; | ||
str += `${prefix} (${originalSourceNoScheme}:${originalLine + 1}:` + |
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.
Worth noting that this puts a (
, )
around the file paths in supplemental stack traces now. I felt that this looked better when we have an alternate symbol name, and it seemed more consistent to always wrap in parenthesis:
Error: an exception
at branch (/Users/bencoe/oss/node-1/test/fixtures/source-map/typescript-throw.js:20:15)
-> (/Users/bencoe/oss/node-1/test/fixtures/source-map/typescript-throw.ts:18:11)
Edit: figured while we're calling the feature experimental, it's the time to make these tweaks.
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.
as a heads up, this might make it incompatible with the tc39 stacks proposal, if it ever lands - it’ll only specify the union of what browsers do.
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.
@ljharb what would you suggest we do to provide appropriate context about the transpiled call site and original call site, in the spirit of the spirit of the tc39 stacks proposal?
I'm hoping to make the case for removing the experimental language from --enable-source-maps
soon -- so now would be a good time to make significant changes to output if needed.
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.
The challenge is specifying what browsers all already do, without permitting anything beyond that.
What do browsers do here on source mapped exception stacks? If the answer is “nothing special”, then that’s probably what node should do too :-/
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.
What do browsers do here on source mapped exception stacks? If the answer is “nothing special”, then that’s probably what node should do too :-/
TypeScript and other transpiled flavors or JavaScript have exploded in popularity over the past several years, I'd argue that stack traces just haven't caught up with this need:
- 6.1m projects rely
source-map-support
, to provide better debugging information in stack traces are used. - A healthy number of folks are finding
--enable-source-maps
(my team is using it ourselves because it speeds up our debugging during test). - Threads like this indicate that folks are more concerned about
transpiledserver code, when it comes to using contextual information about the stack.
I would have originally gone with the approach source-map-support
, of simply replacing the transpiled Call Site with the original Call Site. I believe @hashseed had the idea of displaying both the source and original Call Site, and I believe there's value in this, for a variety of reasons: when looking at an error, it can be good to see how the source was transpiled; source maps, despite best efforts, are sometimes off.
It would be nice if the stacks proposal gave us enough flexibility that we could insert this extra contextual information into the stack trace, without breaking upstream parser.
Couldn't we just consider this to be one line of the stack trace:
at o (/Users/bencoe/oss/node-sourcemap-repro/dist/main.js:1:970)
-> at functionD (webpack:///index.js:14:9)
☝️ the tabbed in line without an at
is fairly distinct.
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.
@ljharb let's move this conversation to the TC39 proposal; this PR is just patching a bug, not introducing new behavior to Node.js.
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.
per our offline discussion, it might be simpler for now to leave .stack
as having only one of the results (source, or transpiled), and maybe providing a separate API (Error.getOriginalStack()
, Error.getUnmappedStack()
) to access the other one? That would ensure no conflicts with the stacks proposal.
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.
Did this land as-is, with the parentheticals discussed here?
It seems the tests are failing. |
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.
Even though the implementation proposed in this PR returns different symbols from source-map-support, and from the non-transpiled code, I'm pretty sure it's following the spirit of the Source Map v3 spec:
I don't agree with that. Stack traces should look as similar as possible to untranspiled stack traces and they do not do that here.
Instead a different logic is applied which I would say is incorrect. Stack traces should display the name of the enclosing function and not the symbol at the call site.
The challenge is that we don't have the call site of the enclosing function. Reading through the V8 codebase, it looked like V8::JSFunction doesn't track its source column and source line. I was hoping that someone from V8 might be able to chime in, the ideal would be that we can get this additional information from the stack. |
This is the problem that the pasta-sourcemaps extension solves. I would love to see that integrated into Node and webpack but have not yet had the time to progress it. Anyone else would be welcome to pick that up. |
Adds methods for fetching stack trace information about enclosing function. Refs nodejs#36042 Original commit message: Reland "stack-trace-api: implement getEnclosingLine/Column" This reverts commit 5557a63beb5a53c93e9b590eaf2933e21bcb3768. Reason for revert: Sheriff's mistake, failing test was previously flaking. Original change's description: > Revert "stack-trace-api: implement getEnclosingLine/Column" > > This reverts commit c48ae2d96cbfdc2216706a5e9a79ae1dce5a638b. > > Reason for revert: Breaks a profiling test: > https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/30010 > > Original change's description: > > stack-trace-api: implement getEnclosingLine/Column > > > > Introduces getEnclosingColumn and getEnclosingLine on CallSite > > so that the position can be used to lookup the original symbol > > for function when source maps are used. > > > > BUG=v8:11157 > > > > Change-Id: I06c4c374d172d206579abb170c7b7a2bd3bb159f > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2547218 > > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > > Commit-Queue: Benjamin Coe <bencoe@google.com> > > Cr-Commit-Position: refs/heads/master@{#71343} > > TBR=jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com > > Change-Id: Iab5c250c1c4fbdab86971f4a7e40abc8f87cf79c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: v8:11157 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555384 > Reviewed-by: Bill Budge <bbudge@chromium.org> > Commit-Queue: Bill Budge <bbudge@chromium.org> > Cr-Commit-Position: refs/heads/master@{#71345} TBR=bbudge@chromium.org,jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com # Not skipping CQ checks because this is a reland. Bug: v8:11157 Change-Id: I8dba19ceb29a24594469d2cf79626f741dc4cad3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555499 Reviewed-by: Bill Budge <bbudge@chromium.org> Commit-Queue: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#71348} Refs: v8/v8@86991d0
Adds methods for fetching stack trace information about enclosing function. Refs #36042 Original commit message: Reland "stack-trace-api: implement getEnclosingLine/Column" This reverts commit 5557a63beb5a53c93e9b590eaf2933e21bcb3768. Reason for revert: Sheriff's mistake, failing test was previously flaking. Original change's description: > Revert "stack-trace-api: implement getEnclosingLine/Column" > > This reverts commit c48ae2d96cbfdc2216706a5e9a79ae1dce5a638b. > > Reason for revert: Breaks a profiling test: > https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/30010 > > Original change's description: > > stack-trace-api: implement getEnclosingLine/Column > > > > Introduces getEnclosingColumn and getEnclosingLine on CallSite > > so that the position can be used to lookup the original symbol > > for function when source maps are used. > > > > BUG=v8:11157 > > > > Change-Id: I06c4c374d172d206579abb170c7b7a2bd3bb159f > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2547218 > > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > > Commit-Queue: Benjamin Coe <bencoe@google.com> > > Cr-Commit-Position: refs/heads/master@{#71343} > > TBR=jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com > > Change-Id: Iab5c250c1c4fbdab86971f4a7e40abc8f87cf79c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: v8:11157 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555384 > Reviewed-by: Bill Budge <bbudge@chromium.org> > Commit-Queue: Bill Budge <bbudge@chromium.org> > Cr-Commit-Position: refs/heads/master@{#71345} TBR=bbudge@chromium.org,jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com # Not skipping CQ checks because this is a reland. Bug: v8:11157 Change-Id: I8dba19ceb29a24594469d2cf79626f741dc4cad3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555499 Reviewed-by: Bill Budge <bbudge@chromium.org> Commit-Queue: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#71348} Refs: v8/v8@86991d0 PR-URL: #36254 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
If symbol names array has been populated in source map, include original symbol name in error message. Fixes nodejs#35325
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@aduh95 @benjamingr did a tiny bit of refactoring, this could probably use another set of eyes, output is looking good now:
Ultimately, I landed on an approach similar to I'm going to experiment a bit later with babel minify and terser. |
This comment has been minimized.
This comment has been minimized.
Adds methods for fetching stack trace information about enclosing function. Refs nodejs#36042 Original commit message: Reland "stack-trace-api: implement getEnclosingLine/Column" This reverts commit 5557a63beb5a53c93e9b590eaf2933e21bcb3768. Reason for revert: Sheriff's mistake, failing test was previously flaking. Original change's description: > Revert "stack-trace-api: implement getEnclosingLine/Column" > > This reverts commit c48ae2d96cbfdc2216706a5e9a79ae1dce5a638b. > > Reason for revert: Breaks a profiling test: > https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/30010 > > Original change's description: > > stack-trace-api: implement getEnclosingLine/Column > > > > Introduces getEnclosingColumn and getEnclosingLine on CallSite > > so that the position can be used to lookup the original symbol > > for function when source maps are used. > > > > BUG=v8:11157 > > > > Change-Id: I06c4c374d172d206579abb170c7b7a2bd3bb159f > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2547218 > > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > > Commit-Queue: Benjamin Coe <bencoe@google.com> > > Cr-Commit-Position: refs/heads/master@{#71343} > > TBR=jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com > > Change-Id: Iab5c250c1c4fbdab86971f4a7e40abc8f87cf79c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: v8:11157 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555384 > Reviewed-by: Bill Budge <bbudge@chromium.org> > Commit-Queue: Bill Budge <bbudge@chromium.org> > Cr-Commit-Position: refs/heads/master@{#71345} TBR=bbudge@chromium.org,jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com # Not skipping CQ checks because this is a reland. Bug: v8:11157 Change-Id: I8dba19ceb29a24594469d2cf79626f741dc4cad3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555499 Reviewed-by: Bill Budge <bbudge@chromium.org> Commit-Queue: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#71348} Refs: v8/v8@86991d0 PR-URL: nodejs#36254 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Landed in 09fd8f1 |
Adds methods for fetching stack trace information about enclosing function. Refs nodejs#36042 Original commit message: Reland "stack-trace-api: implement getEnclosingLine/Column" This reverts commit 5557a63beb5a53c93e9b590eaf2933e21bcb3768. Reason for revert: Sheriff's mistake, failing test was previously flaking. Original change's description: > Revert "stack-trace-api: implement getEnclosingLine/Column" > > This reverts commit c48ae2d96cbfdc2216706a5e9a79ae1dce5a638b. > > Reason for revert: Breaks a profiling test: > https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/30010 > > Original change's description: > > stack-trace-api: implement getEnclosingLine/Column > > > > Introduces getEnclosingColumn and getEnclosingLine on CallSite > > so that the position can be used to lookup the original symbol > > for function when source maps are used. > > > > BUG=v8:11157 > > > > Change-Id: I06c4c374d172d206579abb170c7b7a2bd3bb159f > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2547218 > > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > > Commit-Queue: Benjamin Coe <bencoe@google.com> > > Cr-Commit-Position: refs/heads/master@{#71343} > > TBR=jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com > > Change-Id: Iab5c250c1c4fbdab86971f4a7e40abc8f87cf79c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: v8:11157 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555384 > Reviewed-by: Bill Budge <bbudge@chromium.org> > Commit-Queue: Bill Budge <bbudge@chromium.org> > Cr-Commit-Position: refs/heads/master@{#71345} TBR=bbudge@chromium.org,jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com # Not skipping CQ checks because this is a reland. Bug: v8:11157 Change-Id: I8dba19ceb29a24594469d2cf79626f741dc4cad3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555499 Reviewed-by: Bill Budge <bbudge@chromium.org> Commit-Queue: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#71348} Refs: v8/v8@86991d0 PR-URL: nodejs#36254 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Adds methods for fetching stack trace information about enclosing function. Refs #36042 Original commit message: Reland "stack-trace-api: implement getEnclosingLine/Column" This reverts commit 5557a63beb5a53c93e9b590eaf2933e21bcb3768. Reason for revert: Sheriff's mistake, failing test was previously flaking. Original change's description: > Revert "stack-trace-api: implement getEnclosingLine/Column" > > This reverts commit c48ae2d96cbfdc2216706a5e9a79ae1dce5a638b. > > Reason for revert: Breaks a profiling test: > https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/30010 > > Original change's description: > > stack-trace-api: implement getEnclosingLine/Column > > > > Introduces getEnclosingColumn and getEnclosingLine on CallSite > > so that the position can be used to lookup the original symbol > > for function when source maps are used. > > > > BUG=v8:11157 > > > > Change-Id: I06c4c374d172d206579abb170c7b7a2bd3bb159f > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2547218 > > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > > Commit-Queue: Benjamin Coe <bencoe@google.com> > > Cr-Commit-Position: refs/heads/master@{#71343} > > TBR=jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com > > Change-Id: Iab5c250c1c4fbdab86971f4a7e40abc8f87cf79c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: v8:11157 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555384 > Reviewed-by: Bill Budge <bbudge@chromium.org> > Commit-Queue: Bill Budge <bbudge@chromium.org> > Cr-Commit-Position: refs/heads/master@{#71345} TBR=bbudge@chromium.org,jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com # Not skipping CQ checks because this is a reland. Bug: v8:11157 Change-Id: I8dba19ceb29a24594469d2cf79626f741dc4cad3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555499 Reviewed-by: Bill Budge <bbudge@chromium.org> Commit-Queue: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#71348} Refs: v8/v8@86991d0 PR-URL: #36254 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
If symbol names array has been populated in source map, include original symbol name in error message. Fixes nodejs#35325 PR-URL: nodejs#36042 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Adds methods for fetching stack trace information about enclosing function. Refs nodejs#36042 Original commit message: Reland "stack-trace-api: implement getEnclosingLine/Column" This reverts commit 5557a63beb5a53c93e9b590eaf2933e21bcb3768. Reason for revert: Sheriff's mistake, failing test was previously flaking. Original change's description: > Revert "stack-trace-api: implement getEnclosingLine/Column" > > This reverts commit c48ae2d96cbfdc2216706a5e9a79ae1dce5a638b. > > Reason for revert: Breaks a profiling test: > https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/30010 > > Original change's description: > > stack-trace-api: implement getEnclosingLine/Column > > > > Introduces getEnclosingColumn and getEnclosingLine on CallSite > > so that the position can be used to lookup the original symbol > > for function when source maps are used. > > > > BUG=v8:11157 > > > > Change-Id: I06c4c374d172d206579abb170c7b7a2bd3bb159f > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2547218 > > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > > Commit-Queue: Benjamin Coe <bencoe@google.com> > > Cr-Commit-Position: refs/heads/master@{#71343} > > TBR=jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com > > Change-Id: Iab5c250c1c4fbdab86971f4a7e40abc8f87cf79c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: v8:11157 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555384 > Reviewed-by: Bill Budge <bbudge@chromium.org> > Commit-Queue: Bill Budge <bbudge@chromium.org> > Cr-Commit-Position: refs/heads/master@{#71345} TBR=bbudge@chromium.org,jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com # Not skipping CQ checks because this is a reland. Bug: v8:11157 Change-Id: I8dba19ceb29a24594469d2cf79626f741dc4cad3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555499 Reviewed-by: Bill Budge <bbudge@chromium.org> Commit-Queue: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#71348} Refs: v8/v8@86991d0 PR-URL: nodejs#36254 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Adds methods for fetching stack trace information about enclosing function. Refs nodejs#36042 Original commit message: Reland "stack-trace-api: implement getEnclosingLine/Column" This reverts commit 5557a63beb5a53c93e9b590eaf2933e21bcb3768. Reason for revert: Sheriff's mistake, failing test was previously flaking. Original change's description: > Revert "stack-trace-api: implement getEnclosingLine/Column" > > This reverts commit c48ae2d96cbfdc2216706a5e9a79ae1dce5a638b. > > Reason for revert: Breaks a profiling test: > https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/30010 > > Original change's description: > > stack-trace-api: implement getEnclosingLine/Column > > > > Introduces getEnclosingColumn and getEnclosingLine on CallSite > > so that the position can be used to lookup the original symbol > > for function when source maps are used. > > > > BUG=v8:11157 > > > > Change-Id: I06c4c374d172d206579abb170c7b7a2bd3bb159f > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2547218 > > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > > Commit-Queue: Benjamin Coe <bencoe@google.com> > > Cr-Commit-Position: refs/heads/master@{#71343} > > TBR=jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com > > Change-Id: Iab5c250c1c4fbdab86971f4a7e40abc8f87cf79c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: v8:11157 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555384 > Reviewed-by: Bill Budge <bbudge@chromium.org> > Commit-Queue: Bill Budge <bbudge@chromium.org> > Cr-Commit-Position: refs/heads/master@{#71345} TBR=bbudge@chromium.org,jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com # Not skipping CQ checks because this is a reland. Bug: v8:11157 Change-Id: I8dba19ceb29a24594469d2cf79626f741dc4cad3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555499 Reviewed-by: Bill Budge <bbudge@chromium.org> Commit-Queue: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#71348} Refs: v8/v8@86991d0 PR-URL: nodejs#36254 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Adds methods for fetching stack trace information about enclosing function. Refs nodejs#36042 Original commit message: Reland "stack-trace-api: implement getEnclosingLine/Column" This reverts commit 5557a63beb5a53c93e9b590eaf2933e21bcb3768. Reason for revert: Sheriff's mistake, failing test was previously flaking. Original change's description: > Revert "stack-trace-api: implement getEnclosingLine/Column" > > This reverts commit c48ae2d96cbfdc2216706a5e9a79ae1dce5a638b. > > Reason for revert: Breaks a profiling test: > https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/30010 > > Original change's description: > > stack-trace-api: implement getEnclosingLine/Column > > > > Introduces getEnclosingColumn and getEnclosingLine on CallSite > > so that the position can be used to lookup the original symbol > > for function when source maps are used. > > > > BUG=v8:11157 > > > > Change-Id: I06c4c374d172d206579abb170c7b7a2bd3bb159f > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2547218 > > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > > Commit-Queue: Benjamin Coe <bencoe@google.com> > > Cr-Commit-Position: refs/heads/master@{#71343} > > TBR=jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com > > Change-Id: Iab5c250c1c4fbdab86971f4a7e40abc8f87cf79c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: v8:11157 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555384 > Reviewed-by: Bill Budge <bbudge@chromium.org> > Commit-Queue: Bill Budge <bbudge@chromium.org> > Cr-Commit-Position: refs/heads/master@{#71345} TBR=bbudge@chromium.org,jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com # Not skipping CQ checks because this is a reland. Bug: v8:11157 Change-Id: I8dba19ceb29a24594469d2cf79626f741dc4cad3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555499 Reviewed-by: Bill Budge <bbudge@chromium.org> Commit-Queue: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#71348} Refs: v8/v8@86991d0 PR-URL: nodejs#36254 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Adds methods for fetching stack trace information about enclosing function. Refs nodejs#36042 Original commit message: Reland "stack-trace-api: implement getEnclosingLine/Column" This reverts commit 5557a63beb5a53c93e9b590eaf2933e21bcb3768. Reason for revert: Sheriff's mistake, failing test was previously flaking. Original change's description: > Revert "stack-trace-api: implement getEnclosingLine/Column" > > This reverts commit c48ae2d96cbfdc2216706a5e9a79ae1dce5a638b. > > Reason for revert: Breaks a profiling test: > https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/30010 > > Original change's description: > > stack-trace-api: implement getEnclosingLine/Column > > > > Introduces getEnclosingColumn and getEnclosingLine on CallSite > > so that the position can be used to lookup the original symbol > > for function when source maps are used. > > > > BUG=v8:11157 > > > > Change-Id: I06c4c374d172d206579abb170c7b7a2bd3bb159f > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2547218 > > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > > Commit-Queue: Benjamin Coe <bencoe@google.com> > > Cr-Commit-Position: refs/heads/master@{#71343} > > TBR=jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com > > Change-Id: Iab5c250c1c4fbdab86971f4a7e40abc8f87cf79c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: v8:11157 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555384 > Reviewed-by: Bill Budge <bbudge@chromium.org> > Commit-Queue: Bill Budge <bbudge@chromium.org> > Cr-Commit-Position: refs/heads/master@{#71345} TBR=bbudge@chromium.org,jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com # Not skipping CQ checks because this is a reland. Bug: v8:11157 Change-Id: I8dba19ceb29a24594469d2cf79626f741dc4cad3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555499 Reviewed-by: Bill Budge <bbudge@chromium.org> Commit-Queue: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#71348} Refs: v8/v8@86991d0 PR-URL: nodejs#36254 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Adds methods for fetching stack trace information about enclosing function. Refs nodejs#36042 Original commit message: Reland "stack-trace-api: implement getEnclosingLine/Column" This reverts commit 5557a63beb5a53c93e9b590eaf2933e21bcb3768. Reason for revert: Sheriff's mistake, failing test was previously flaking. Original change's description: > Revert "stack-trace-api: implement getEnclosingLine/Column" > > This reverts commit c48ae2d96cbfdc2216706a5e9a79ae1dce5a638b. > > Reason for revert: Breaks a profiling test: > https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/30010 > > Original change's description: > > stack-trace-api: implement getEnclosingLine/Column > > > > Introduces getEnclosingColumn and getEnclosingLine on CallSite > > so that the position can be used to lookup the original symbol > > for function when source maps are used. > > > > BUG=v8:11157 > > > > Change-Id: I06c4c374d172d206579abb170c7b7a2bd3bb159f > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2547218 > > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > > Commit-Queue: Benjamin Coe <bencoe@google.com> > > Cr-Commit-Position: refs/heads/master@{#71343} > > TBR=jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com > > Change-Id: Iab5c250c1c4fbdab86971f4a7e40abc8f87cf79c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: v8:11157 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555384 > Reviewed-by: Bill Budge <bbudge@chromium.org> > Commit-Queue: Bill Budge <bbudge@chromium.org> > Cr-Commit-Position: refs/heads/master@{#71345} TBR=bbudge@chromium.org,jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com # Not skipping CQ checks because this is a reland. Bug: v8:11157 Change-Id: I8dba19ceb29a24594469d2cf79626f741dc4cad3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555499 Reviewed-by: Bill Budge <bbudge@chromium.org> Commit-Queue: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#71348} Refs: v8/v8@86991d0 PR-URL: nodejs#36254 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Adds methods for fetching stack trace information about enclosing function. Refs #36042 Original commit message: Reland "stack-trace-api: implement getEnclosingLine/Column" This reverts commit 5557a63beb5a53c93e9b590eaf2933e21bcb3768. Reason for revert: Sheriff's mistake, failing test was previously flaking. Original change's description: > Revert "stack-trace-api: implement getEnclosingLine/Column" > > This reverts commit c48ae2d96cbfdc2216706a5e9a79ae1dce5a638b. > > Reason for revert: Breaks a profiling test: > https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/30010 > > Original change's description: > > stack-trace-api: implement getEnclosingLine/Column > > > > Introduces getEnclosingColumn and getEnclosingLine on CallSite > > so that the position can be used to lookup the original symbol > > for function when source maps are used. > > > > BUG=v8:11157 > > > > Change-Id: I06c4c374d172d206579abb170c7b7a2bd3bb159f > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2547218 > > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > > Commit-Queue: Benjamin Coe <bencoe@google.com> > > Cr-Commit-Position: refs/heads/master@{#71343} > > TBR=jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com > > Change-Id: Iab5c250c1c4fbdab86971f4a7e40abc8f87cf79c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: v8:11157 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555384 > Reviewed-by: Bill Budge <bbudge@chromium.org> > Commit-Queue: Bill Budge <bbudge@chromium.org> > Cr-Commit-Position: refs/heads/master@{#71345} TBR=bbudge@chromium.org,jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com # Not skipping CQ checks because this is a reland. Bug: v8:11157 Change-Id: I8dba19ceb29a24594469d2cf79626f741dc4cad3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555499 Reviewed-by: Bill Budge <bbudge@chromium.org> Commit-Queue: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#71348} Refs: v8/v8@86991d0 PR-URL: #36254 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Adds methods for fetching stack trace information about enclosing function. Refs #36042 Original commit message: Reland "stack-trace-api: implement getEnclosingLine/Column" This reverts commit 5557a63beb5a53c93e9b590eaf2933e21bcb3768. Reason for revert: Sheriff's mistake, failing test was previously flaking. Original change's description: > Revert "stack-trace-api: implement getEnclosingLine/Column" > > This reverts commit c48ae2d96cbfdc2216706a5e9a79ae1dce5a638b. > > Reason for revert: Breaks a profiling test: > https://ci.chromium.org/p/v8/builders/ci/V8%20Win32/30010 > > Original change's description: > > stack-trace-api: implement getEnclosingLine/Column > > > > Introduces getEnclosingColumn and getEnclosingLine on CallSite > > so that the position can be used to lookup the original symbol > > for function when source maps are used. > > > > BUG=v8:11157 > > > > Change-Id: I06c4c374d172d206579abb170c7b7a2bd3bb159f > > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2547218 > > Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> > > Commit-Queue: Benjamin Coe <bencoe@google.com> > > Cr-Commit-Position: refs/heads/master@{#71343} > > TBR=jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com > > Change-Id: Iab5c250c1c4fbdab86971f4a7e40abc8f87cf79c > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Bug: v8:11157 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555384 > Reviewed-by: Bill Budge <bbudge@chromium.org> > Commit-Queue: Bill Budge <bbudge@chromium.org> > Cr-Commit-Position: refs/heads/master@{#71345} TBR=bbudge@chromium.org,jkummerow@chromium.org,yangguo@chromium.org,bencoe@google.com # Not skipping CQ checks because this is a reland. Bug: v8:11157 Change-Id: I8dba19ceb29a24594469d2cf79626f741dc4cad3 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2555499 Reviewed-by: Bill Budge <bbudge@chromium.org> Commit-Queue: Bill Budge <bbudge@chromium.org> Cr-Commit-Position: refs/heads/master@{#71348} Refs: v8/v8@86991d0 PR-URL: #36254 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
If symbol names array has been populated in source map, include
original symbol name in error message.
Fixes #35325
What is this?
This PR uses the optional symbol names array described in the V3 source map spec to provide additional context about an error's call site.
Take this example from #35325:
The stack trace before this change is as follows:
It's stack trace after this change is:
Edit: this implementation should now match source-map-support.
CC: @schuay, @LinusU, @michael-wolfenden, @sokra
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes