errors: display original symbol name #36042

bcoe · 2020-11-08T17:19:56Z

If symbol names array has been populated in source map, include
original symbol name in error message.

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:

const functionA = () => {
  functionB()
}

const functionB = () => {
  functionC()
}

const functionC = () => {
  functionD()
}

const functionD = () => {
  throw new Error('Uh oh')
}

functionA()

The stack trace before this change is as follows:

Error: Uh oh
    at o (/Users/bencoe/oss/node-sourcemap-repro/dist/main.js:1:970)
        -> /Users/bencoe/oss/node-sourcemap-repro/dist/webpack:/index.js:14:9
    at n (/Users/bencoe/oss/node-sourcemap-repro/dist/main.js:1:952)
        -> /Users/bencoe/oss/node-sourcemap-repro/dist/webpack:/index.js:10:3
    at r (/Users/bencoe/oss/node-sourcemap-repro/dist/main.js:1:940)

It's stack trace after this change is:

webpack:///index.js:14
  throw new Error('Roh Ruh')
        ^

Error: Roh Ruh
    at o (/Users/bencoe/oss/node-sourcemap-repro/dist/main.js:1:970)
        -> at functionD (webpack:///index.js:14:9)
    at n (/Users/bencoe/oss/node-sourcemap-repro/dist/main.js:1:952)
        -> at functionC (webpack:///index.js:10:3)
    at r (/Users/bencoe/oss/node-sourcemap-repro/dist/main.js:1:940)
        -> at functionB (webpack:///index.js:6:3)
    at Object.<anonymous> (/Users/bencoe/oss/node-sourcemap-repro/dist/main.js:1:992)
        -> at call (webpack:///index.js:2:3)

Edit: this implementation should now match source-map-support.

CC: @schuay, @LinusU, @michael-wolfenden, @sokra

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

nodejs-github-bot · 2020-11-08T17:20:00Z

Review requested:

@nodejs/modules

aduh95 · 2020-11-08T17:42:04Z

lib/internal/source_map/prepare_stack_trace.js

@@ -64,14 +64,14 @@ const prepareStackTrace = (globalThis, error, trace) => {
        const {
          originalLine,
          originalColumn,
-          originalSource
+          originalSource,
+          name


nit: consider adding a trailing comma to make future PR diffs cleaner?

Suggested change

name

name,

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)

Agreed, to be clear I'm +1 landing this with or without the trailing commas.

lib/internal/source_map/source_map.js

bcoe · 2020-11-08T17:51:37Z

lib/internal/source_map/prepare_stack_trace.js

          const originalSourceNoScheme =
            StringPrototypeStartsWith(originalSource, 'file://') ?
              fileURLToPath(originalSource) : originalSource;
-          str += `\n        -> ${originalSourceNoScheme}:${originalLine + 1}:` +
-            `${originalColumn + 1}`;
+          str += `${prefix} (${originalSourceNoScheme}:${originalLine + 1}:` +


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.

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.

@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.

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 :-/

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 ~~transpiled~~ server 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.

@ljharb let's move this conversation to the TC39 proposal; this PR is just patching a bug, not introducing new behavior to Node.js.

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.

Did this land as-is, with the parentheticals discussed here?

aduh95 · 2020-11-08T18:18:14Z

It seems the tests are failing.

sokra

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.

bcoe · 2020-11-08T18:42:13Z

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.

robpalme · 2020-11-08T18:45:30Z

Stack traces should display the name of the enclosing function and not the symbol at the call site

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.

bcoe · 2020-11-08T21:09:03Z

@robpalme @sokra on my mind, is whether we have call site information about the enclosing function in v8 we could expose (as a starting point for an improved stack trace).

I'll open a corresponding issue on v8, and see if anyone from the project has advice.

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>

bcoe · 2020-11-27T19:44:28Z

@aduh95 @benjamingr did a tiny bit of refactoring, this could probably use another set of eyes, output is looking good now:

webpack:///index.js:14
  throw new Error('Roh Ruh')
        ^

Error: Roh Ruh
    at o (/Users/bencoe/oss/node-sourcemap-repro/dist/main.js:1:970)
        -> at functionD (webpack:///index.js:14:9)
    at n (/Users/bencoe/oss/node-sourcemap-repro/dist/main.js:1:952)
        -> at functionC (webpack:///index.js:10:3)
    at r (/Users/bencoe/oss/node-sourcemap-repro/dist/main.js:1:940)
        -> at functionB (webpack:///index.js:6:3)
    at Object.<anonymous> (/Users/bencoe/oss/node-sourcemap-repro/dist/main.js:1:992)
        -> at call (webpack:///index.js:2:3)

Ultimately, I landed on an approach similar to source-map-support, where if we don't find a symbol attached to the enclosing function, we fallback to checking the next call site; this seemed to work best for webpack.

I'm going to experiment a bit later with babel minify and terser.

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>

nodejs-github-bot · 2020-11-29T14:57:38Z

CI: https://ci.nodejs.org/job/node-test-pull-request/34616/

If symbol names array has been populated in source map, include original symbol name in error message. Fixes #35325 PR-URL: #36042 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

bcoe · 2020-12-01T15:24:56Z

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 #35325 PR-URL: #36042 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 #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 PR-URL: #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 #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 PR-URL: #36042 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

bcoe requested review from addaleax, benjamingr and aduh95 November 8, 2020 17:19

benjamingr approved these changes Nov 8, 2020

View reviewed changes

aduh95 approved these changes Nov 8, 2020

View reviewed changes

bcoe commented Nov 8, 2020

View reviewed changes

sokra suggested changes Nov 8, 2020

View reviewed changes

bcoe mentioned this pull request Nov 25, 2020

deps: V8: cherry-pick 86991d0587a1 #36254

Closed

4 tasks

bcoe added 2 commits November 26, 2020 07:25

errors: display original symbol name

56d7975

If symbol names array has been populated in source map, include original symbol name in error message. Fixes nodejs#35325

refactor: refactor approch based on research

f46eb0c

bcoe force-pushed the symbol-names branch from 6f903ab to f46eb0c Compare November 27, 2020 19:26

bcoe and others added 2 commits November 27, 2020 14:29

Update lib/internal/source_map/source_map.js

a5f883c

Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

chore: make sure symbols are correct

25c45f9

bcoe requested review from benjamingr and aduh95 November 27, 2020 19:41

bcoe added 3 commits November 27, 2020 17:07

docs: slight tweaks to comments

8b4a2b1

test: fix up tests; add test for enclosing function

e469554

test: fix up test output

2624239

benjamingr approved these changes Nov 28, 2020

View reviewed changes

This comment has been minimized.

Sign in to view

aduh95 approved these changes Nov 28, 2020

View reviewed changes

bcoe closed this Dec 1, 2020

bcoe deleted the symbol-names branch December 1, 2020 15:25

bcoe mentioned this pull request Dec 1, 2020

Future work for source-map functionality #29865

Closed

7 tasks

danielleadams mentioned this pull request Dec 7, 2020

v15.4.0 proposal #36435

Merged

targos mentioned this pull request Jun 6, 2021

v14.17.1 release proposal #38948

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

errors: display original symbol name #36042

errors: display original symbol name #36042

bcoe commented Nov 8, 2020 •

edited

nodejs-github-bot commented Nov 8, 2020

aduh95 Nov 8, 2020

ljharb Nov 8, 2020

aduh95 Nov 8, 2020

bcoe Nov 8, 2020 •

edited

ljharb Nov 28, 2020

bcoe Nov 28, 2020

ljharb Nov 28, 2020

bcoe Nov 29, 2020 •

edited

bcoe Nov 30, 2020

ljharb Dec 1, 2020

ljharb Dec 1, 2020

aduh95 commented Nov 8, 2020

sokra left a comment

bcoe commented Nov 8, 2020

robpalme commented Nov 8, 2020

bcoe commented Nov 8, 2020 •

edited

bcoe commented Nov 27, 2020

This comment has been minimized.

nodejs-github-bot commented Nov 29, 2020

bcoe commented Dec 1, 2020

errors: display original symbol name #36042

errors: display original symbol name #36042

Conversation

bcoe commented Nov 8, 2020 • edited

What is this?

Checklist

nodejs-github-bot commented Nov 8, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcoe Nov 8, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcoe Nov 29, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aduh95 commented Nov 8, 2020

sokra left a comment

Choose a reason for hiding this comment

bcoe commented Nov 8, 2020

robpalme commented Nov 8, 2020

bcoe commented Nov 8, 2020 • edited

bcoe commented Nov 27, 2020

This comment has been minimized.

nodejs-github-bot commented Nov 29, 2020

bcoe commented Dec 1, 2020

bcoe commented Nov 8, 2020 •

edited

bcoe Nov 8, 2020 •

edited

bcoe Nov 29, 2020 •

edited

bcoe commented Nov 8, 2020 •

edited