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

Source maps not supported in eval and new Function #43047

Closed
edemaine opened this issue May 10, 2022 · 8 comments
Closed

Source maps not supported in eval and new Function #43047

edemaine opened this issue May 10, 2022 · 8 comments
Assignees
Labels
feature request Issues that request new features to be added to Node.js. source maps Issues and PRs related to source map support.

Comments

@edemaine
Copy link

edemaine commented May 10, 2022

Version

v18.0.0

Platform

Microsoft Windows NT 10.0.19044.0 x64

Subsystem

No response

What steps will reproduce the bug?

eval

node --enable-source-maps example.js where example.js the following file:

const comment = '//#';
const code = `// Generated by CoffeeScript 2.7.0
(function() {
  throw new Error('hello');

}).call(this);

${comment} sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoic2ltcGxlLmpzIiwic291cmNlUm9vdCI6IlxcIiwic291cmNlcyI6WyJzaW1wbGUuY29mZmVlIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiI7QUFBQTtFQUFBLE1BQU0sSUFBSSxLQUFKLENBQVUsT0FBVjtBQUFOIiwic291cmNlc0NvbnRlbnQiOlsidGhyb3cgbmV3IEVycm9yICdoZWxsbydcbiJdfQ==
${comment} sourceURL=simple.coffee
`;
eval(code);

new Function

node --enable-source-maps example.js where example.js the following file: (only the last line differs from the previous example)

const comment = '//#';
const code = `// Generated by CoffeeScript 2.7.0
(function() {
  throw new Error('hello');

}).call(this);

${comment} sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoic2ltcGxlLmpzIiwic291cmNlUm9vdCI6IlxcIiwic291cmNlcyI6WyJzaW1wbGUuY29mZmVlIl0sIm5hbWVzIjpbXSwibWFwcGluZ3MiOiI7QUFBQTtFQUFBLE1BQU0sSUFBSSxLQUFKLENBQVUsT0FBVjtBQUFOIiwic291cmNlc0NvbnRlbnQiOlsidGhyb3cgbmV3IEVycm9yICdoZWxsbydcbiJdfQ==
${comment} sourceURL=simple.coffee
`;
new Function(code)();

Source

This example was generated from a one-line simple.coffeethrow new Error 'hello' — via coffee -M -c simple.coffee. I replaced //# with a string interpolation so the source map isn't applied to example.js itself.

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

Always

What is the expected behavior?

eval

This produces the following stack trace:

Error: hello
    at Object.eval (simple.coffee:3:9)
    at eval (simple.coffee:5:4)
    at Object.<anonymous> (C:\...\example.js:11:1)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Module._load (node:internal/modules/cjs/loader:827:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_ma
in:77:12)
    at node:internal/main/run_main_module:17:47

The line numbers for simple.coffee (lines 3 and 5) are incorrect (both should be line 1).

new Function

This produces the following stack trace:

Error: hello
    at eval (simple.coffee:5:9)
    at eval (simple.coffee:7:4)
    at Object.<anonymous> (C:\...\example.js:11:19)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Module._load (node:internal/modules/cjs/loader:827:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_ma
in:77:12)
    at node:internal/main/run_main_module:17:47

The line numbers for simple.coffee (lines 5 and 7) are incorrect (both should be line 1), and different from the eval case.

What do you see instead?

I expect everything to refer to line 1 of simple.coffee, as that's the only line.

Running npm install coffeescript and coffee simple.coffee, where simple.coffee is the following file:

throw new Error 'hello'

produces the correct line numbers (via a specialized mapping implemented by CoffeeScript, I believe):

Error: hello
    at Object.<anonymous> (C:\...\simple.coffee:1:7)
    at Object.<anonymous> (C:\...\simple.coffee:1:1)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Object.CoffeeScript.run (C:\...\node_modules\coffeescript\lib\coffeescript\index.js:67:23)
    at compileScript (C:\...\node_modules\coffeescript\lib\coffeescript\command.js:285:29)
    at compilePath (C:\...\node_modules\coffeescript\lib\coffeescript\command.js:237:14)
    at Object.exports.run (C:\...\node_modules\coffeescript\lib\coffeescript\command.js:158:20)
    at Object.<anonymous> (C:\...\node_modules\coffeescript\bin\coffee:22:45)
    at Module._compile (node:internal/modules/cjs/loader:1105:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:827:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12)
    at node:internal/main/run_main_module:17:47

Additional information

The source map spec explicitly mentions code "being evaluated as a string with the eval() function or via new Function()" so I assume this is supposed to work.

@targos targos added the source maps Issues and PRs related to source map support. label May 11, 2022
@targos
Copy link
Member

targos commented May 11, 2022

/cc @bcoe

This is how it looks like in Edge:

image

@bcoe bcoe added feature request Issues that request new features to be added to Node.js. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. labels Jun 2, 2022
@bcoe
Copy link
Contributor

bcoe commented Jun 2, 2022

Support for eval has not been implemented. This is a good chance for someone to make a contribution, although I'm not sure how it would be approached, since the implementation of source map support today is handled during require and import, which eval and new Function circumvents.

I would be interested to know how v8 approaches this problem.

@legendecas
Copy link
Member

legendecas commented Jun 13, 2022

I would be interested to know how v8 approaches this problem.

V8 allows hooking into eval and new Function and get the source code with Isolate::SetModifyCodeGenerationFromStringsCallback. I've worked on a prototype and it works like a charm ^^. I'm going to update the code base to be more solid before I upload a patch.

@legendecas legendecas removed the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Jun 13, 2022
@legendecas legendecas self-assigned this Jun 13, 2022
@bcoe
Copy link
Contributor

bcoe commented Jun 20, 2022

I've worked on a prototype and it works like a charm ^^. I'm going to update the code base to be more solid before I upload a patch.

@legendecas awesome, will be excited to see this patch.

legendecas added a commit that referenced this issue Jul 11, 2022
Dynamic sources with FunctionConstructor is not supported yet as
V8 prepends lines to the sources which makes the stack line number
incorrect.

PR-URL: #43428
Refs: #43047
Reviewed-By: Ben Coe <bencoe@gmail.com>
@legendecas
Copy link
Member

As mentioned in #43428, the line number in new Function is broken. I'm going to submit a patch to v8 before closing this issue as done.

targos pushed a commit that referenced this issue Jul 12, 2022
Dynamic sources with FunctionConstructor is not supported yet as
V8 prepends lines to the sources which makes the stack line number
incorrect.

PR-URL: #43428
Refs: #43047
Reviewed-By: Ben Coe <bencoe@gmail.com>
targos pushed a commit that referenced this issue Jul 31, 2022
Dynamic sources with FunctionConstructor is not supported yet as
V8 prepends lines to the sources which makes the stack line number
incorrect.

PR-URL: #43428
Refs: #43047
Reviewed-By: Ben Coe <bencoe@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
Dynamic sources with FunctionConstructor is not supported yet as
V8 prepends lines to the sources which makes the stack line number
incorrect.

PR-URL: nodejs/node#43428
Refs: nodejs/node#43047
Reviewed-By: Ben Coe <bencoe@gmail.com>
@targos targos moved this to Pending Triage in Node.js feature requests Oct 22, 2022
@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Feb 7, 2023
@targos targos moved this from Pending Triage to Stale in Node.js feature requests Apr 4, 2023
@legendecas
Copy link
Member

According to step 12 of https://tc39.es/ecma262/#sec-createdynamicfunction, the Function constructor always prefixes the source with additional 2 lines. So I don't think Node.js can do anything regarding the line offset in new Function. Instead, the source map provided should be aware of the line offset with new Function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. source maps Issues and PRs related to source map support.
Projects
None yet
Development

No branches or pull requests

4 participants