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

--enable-source-maps does not appear to show function names with webpack sourcemaps #35325

Closed
michael-wolfenden opened this issue Sep 24, 2020 · 5 comments
Assignees
Labels
source maps Issues and PRs related to source map support.

Comments

@michael-wolfenden
Copy link

  • Version: v12.13.0
  • Platform: Windows 10 64-bit
  • Subsystem: sourcemaps

What steps will reproduce the bug?

Here is a complete, minimal reproduction
https://github.com/michael-wolfenden/node-sourcemap-repro

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior?

I would expect to see the real function names in the output.

Using the source-map-support package, the stack trace looks like:

> node ./dist/main.js

D:\github\node-sourcemap-repro\dist\webpack:\index-source-map-support.js:16
  throw new Error('Roh Ruh')
        ^
Error: Roh Ruh
    at functionD (D:\github\node-sourcemap-repro\dist\webpack:\index-source-map-support.js:16:9)
    at functionC (D:\github\node-sourcemap-repro\dist\webpack:\index-source-map-support.js:12:3)
    at functionB (D:\github\node-sourcemap-repro\dist\webpack:\index-source-map-support.js:8:3)
    at Object.call (D:\github\node-sourcemap-repro\dist\webpack:\index-source-map-support.js:4:3)
    at __webpack_require__ (D:\github\node-sourcemap-repro\dist\webpack:\webpack\bootstrap:19:22)
    at D:\github\node-sourcemap-repro\dist\webpack:\webpack\bootstrap:83:10
    at Object.<anonymous> (D:\github\node-sourcemap-repro\dist\main.js:1:911)
    at Module._compile (internal/modules/cjs/loader.js:956:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:973:10)
    at Module.load (internal/modules/cjs/loader.js:812:32)

What do you see instead?

Using the --enable-source-maps flag, the stack trace looks like:

> node --enable-source-maps ./dist/main.js

Error: Roh Ruh
    at o (D:\github\node-sourcemap-repro\dist\main.js:1:970)
        -> D:\github\node-sourcemap-repro\dist\webpack:\index.js:14:9
    at n (D:\github\node-sourcemap-repro\dist\main.js:1:952)
        -> D:\github\node-sourcemap-repro\dist\webpack:\index.js:10:3
    at r (D:\github\node-sourcemap-repro\dist\main.js:1:940)
        -> D:\github\node-sourcemap-repro\dist\webpack:\index.js:6:3
    at Object.<anonymous> (D:\github\node-sourcemap-repro\dist\main.js:1:992)
        -> D:\github\node-sourcemap-repro\dist\webpack:\index.js:2:3
    at r (D:\github\node-sourcemap-repro\dist\main.js:1:110)
        -> D:\github\node-sourcemap-repro\dist\webpack:\webpack\bootstrap:19:22
    at D:\github\node-sourcemap-repro\dist\main.js:1:902
        -> D:\github\node-sourcemap-repro\dist\webpack:\webpack\bootstrap:83:10
    at Object.<anonymous> (D:\github\node-sourcemap-repro\dist\main.js:1:911)
    at Module._compile (internal/modules/cjs/loader.js:956:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:973:10)        
    at Module.load (internal/modules/cjs/loader.js:812:32)
@watilde watilde added the source maps Issues and PRs related to source map support. label Sep 26, 2020
@bcoe bcoe self-assigned this Oct 29, 2020
@bcoe
Copy link
Contributor

bcoe commented Oct 30, 2020

@michael-wolfenden I'm wondering if you have any recommendations on how we should display the original function name?

The approach we've taken is to provide information about bot the call site with the error and the original call site, I'm wondering how we'd represent an alternate function name.

@bcoe
Copy link
Contributor

bcoe commented Oct 31, 2020

@michael-wolfenden what do you think of this?

Error: oh no!
    at o (/Users/bencoe/oss/node-1/test/fixtures/source-map/webpack.js:1:970)
        -> [functionA] webpack:///./webpack.js:14:9
    at n (/Users/bencoe/oss/node-1/test/fixtures/source-map/webpack.js:1:952)
        -> [functionB] webpack:///./webpack.js:10:3

@michael-wolfenden
Copy link
Author

I would expect the output to look something like the output that source-map-support generates

> node ./dist/main.js

D:\github\node-sourcemap-repro\dist\webpack:\index-source-map-support.js:16
  throw new Error('Roh Ruh')
        ^
Error: Roh Ruh
    at functionD (D:\github\node-sourcemap-repro\dist\webpack:\index-source-map-support.js:16:9)
    at functionC (D:\github\node-sourcemap-repro\dist\webpack:\index-source-map-support.js:12:3)
    at functionB (D:\github\node-sourcemap-repro\dist\webpack:\index-source-map-support.js:8:3)
    at Object.call (D:\github\node-sourcemap-repro\dist\webpack:\index-source-map-support.js:4:3)
    at __webpack_require__ (D:\github\node-sourcemap-repro\dist\webpack:\webpack\bootstrap:19:22)
    at D:\github\node-sourcemap-repro\dist\webpack:\webpack\bootstrap:83:10
    at Object.<anonymous> (D:\github\node-sourcemap-repro\dist\main.js:1:911)
    at Module._compile (internal/modules/cjs/loader.js:956:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:973:10)
    at Module.load (internal/modules/cjs/loader.js:812:32)

@bcoe
Copy link
Contributor

bcoe commented Oct 31, 2020

@michael-wolfenden we've opted to include both the call site where the error happened, and the original call site in our implementation of applying source maps to stack traces in Node.js.

This is useful because sometimes you do want to look at the actual location where the error was thrown, i.e.,

at o (/Users/bencoe/oss/node-1/test/fixtures/source-map/webpack.js:1:970)

☝️ especially when using a minifier there can be bugs that only crop up in the minified code, e.g., a collision on symbol names.


I'd like to figure out a way to provide the original function name for you, without hiding the actual function name that executed.

bcoe added a commit to bcoe/node-1 that referenced this issue Oct 31, 2020
Based on advice from webpack project, this PR stops trying to resolve
sources with a webpack:// scheme (or other schemes, as they represent
absolute URLs). Source content is now loaded from the sourcesContent
lookup, if it is populated (allowing for non file:// schemes).

Refs: nodejs#35325
@michael-wolfenden
Copy link
Author

Ah gotcha ... In that case, your suggestion looks good 👍

bcoe added a commit that referenced this issue Nov 3, 2020
We were incorrectly trying to run path.resolve on absolute
sources URLs. This was breaking webpack:// URLs in stack
trace output.

Refs: #35325

PR-URL: #35903
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
targos pushed a commit that referenced this issue Nov 3, 2020
We were incorrectly trying to run path.resolve on absolute
sources URLs. This was breaking webpack:// URLs in stack
trace output.

Refs: #35325

PR-URL: #35903
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
bcoe added a commit to bcoe/node-1 that referenced this issue Nov 27, 2020
If symbol names array has been populated in source map, include
original symbol name in error message.

Fixes nodejs#35325
@bcoe bcoe closed this as completed in 09fd8f1 Dec 1, 2020
danielleadams pushed a commit that referenced this issue Dec 7, 2020
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>
cjihrig pushed a commit to cjihrig/node that referenced this issue Dec 8, 2020
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>
bcoe added a commit to bcoe/node-1 that referenced this issue Mar 11, 2021
We were incorrectly trying to run path.resolve on absolute
sources URLs. This was breaking webpack:// URLs in stack
trace output.

Refs: nodejs#35325

PR-URL: nodejs#35903
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
bcoe added a commit to bcoe/node-1 that referenced this issue Mar 11, 2021
We were incorrectly trying to run path.resolve on absolute
sources URLs. This was breaking webpack:// URLs in stack
trace output.

Refs: nodejs#35325

Backport-PR-URL: nodejs#37717
PR-URL: nodejs#35903
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
bcoe added a commit to bcoe/node-1 that referenced this issue Mar 16, 2021
We were incorrectly trying to run path.resolve on absolute
sources URLs. This was breaking webpack:// URLs in stack
trace output.

Refs: nodejs#35325

Backport-PR-URL: nodejs#37717
PR-URL: nodejs#35903
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
bcoe added a commit to bcoe/node-1 that referenced this issue Apr 4, 2021
We were incorrectly trying to run path.resolve on absolute
sources URLs. This was breaking webpack:// URLs in stack
trace output.

Refs: nodejs#35325

Backport-PR-URL: nodejs#37717
PR-URL: nodejs#35903
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
targos pushed a commit to bcoe/node-1 that referenced this issue Apr 24, 2021
We were incorrectly trying to run path.resolve on absolute
sources URLs. This was breaking webpack:// URLs in stack
trace output.

Refs: nodejs#35325

PR-URL: nodejs#35903
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
targos pushed a commit that referenced this issue Apr 25, 2021
We were incorrectly trying to run path.resolve on absolute
sources URLs. This was breaking webpack:// URLs in stack
trace output.

Refs: #35325

PR-URL: #35903
Backport-PR-URL: #37717
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
targos pushed a commit that referenced this issue May 16, 2021
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>
targos pushed a commit that referenced this issue Jun 11, 2021
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
source maps Issues and PRs related to source map support.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants