Node: Line numbers reported from code in `new Function` are incorrect

[connor4312]

• Version: 13.12.0
• Platform: Windows
• Subsystem: unsure

What steps will reproduce the bug?

Copied from https://github.com/microsoft/vscode/issues/94437

1. Create a file containing

const code = `    console.log(arg1);
    const a = arg1 + 1;
    console.log(a);
    return a;`;

const fn = new Function('arg1', code);
console.log(process.version);
const result = fn(1);
console.log('result', result);

2 Either in Chrome devtools attached to the process, or a debugger like VS Code, set a breakpoint on const result = fn(1);

1. Step into and through the evaluated function

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

Every time

What is the expected behavior?

The debugger should show the line being evaluated currently

What do you see instead?

The debugger is two lines off (notice '1' is logged even though I haven't gotten there yet)

This does not happen when running the code directly Chrome or Chrome Canary; it seems to be Node-specific behavior.

Additional information

Looking in CDP, this is the data I see:

{
  "method": "Debugger.paused",
  "params": {
    "callFrames": [
      {
        "callFrameId": "{\"ordinal\":0,\"injectedScriptId\":1}",
        "functionName": "",
        "functionLocation": {
          "scriptId": "130",
          "lineNumber": -2,   // <- this part is weird!
          "columnNumber": 19
        },
        {
          "scriptId": "130",
          "lineNumber": 0, // <- should be 2, not 0
          "columnNumber": 4
        }
        "url": "",

        // ...

It seems like Node.js offsets the evaluated code so that the generated function head is before the 'start' of the file, I assume so that stacktraces show the right line. However, this messes up debuggers. Some suggestions:

• Make the function header a single line prefixing the user's function, like the module wrapper, so that line numbers are identical.
• In the Debugger.scriptParsed event, set the startLine to compensate for the header. I _think_ this should make all debuggers 'just work' provided they deal with the lineNumber as a signed integer.
• Don't send the wrapper code in Debugger.getScriptSource

[devsnek]

the generated function here is expected and spec compliant, we should not change that.

[connor4312]

To clarify on the first suggestion, that would be changing the prefix from (function anonymous(arg1\n) {\n to (function anonymous(arg1) {. It shouldn't have any negative runtime side-effects, but I'm not versed enough to know whether doing so would violate spec 🙂

Thinking about it more the second approach--to send the range in scriptParsed--is probably better, that would let the debugger also be smart about what code is shown. I could hide or grey out the generated wrapper, whereas changing the prefix would cause it to still be shown to the user.

[devsnek]

To clarify on the first suggestion, that would be changing the prefix from (function anonymous(arg1\n) {\n to (function anonymous(arg1) {. It shouldn't have any negative runtime side-effects, but I'm not versed enough to know whether doing so would violate spec :slightly_smiling_face:

It would violate the spec, as I said above.

It looks like CDP is expecting VSC to perform certain offset calculations that it is not. This doesn't seem to be a bug in node.

[connor4312]

Chrome (incl Canary) do not exhibit this behavior, and it causes a 'bug' in the Chrome devtools as well if you inspect a Node.js process via chrome://inspect. If it is a matter of not following CDP, it would be quite surprising to me that the Chrome devtools would have the same behavior.

If I were to correct this on VS Code side, I'm not sure what code I would need to write other than special casing the Node version and eval'd script, because there's nothing I can see in the protocol coming from Node that would indicate that this specific script starts at a negative line number. This also increases my doubt that it's a bug on the VS Code side, but I could be missing something...

When running the code in Chrome and using its protocol monitor, it puts the functionLocation at line 0 and the paused location is correctly at lineNumber 2.

[targos]

I can reproduce this and the reason it works in Chrome may be that it was fixed in a recent version of V8.

We should check if it works with a nightly build of Node.js master. I'm trying to download the latest one but not sure I'll be able to with the current infra issues.

[targos]

Okay, I confirm it works correctly with v14.0.0-nightly2020040504b71848af.
I'll try to bisect v8-canary later this week.

[targos]

So... I managed to reduce the range of V8 commits to https://github.com/v8/v8/compare/6987ee4537a9637bf2e1fd2ec89d7bace691c531...63da8397bc967232f91fea07b1861a50d20ca717

This one looks like a really good candidate: https://github.com/v8/v8/commit/dc3a90be6ca7f7160e1865f289e76c7bc32f78c4

I do not have time to try and backport this to v13.x. Anyone feel free to do it!