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

Breakpoints fail to bind in Node 16 or Chrome 90 in functions with destructured arguments #1017

Closed
richardsimko opened this issue Jun 2, 2021 · 17 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Milestone

Comments

@richardsimko
Copy link

richardsimko commented Jun 2, 2021

Describe the bug
If a file contains two nested anonymous functions with parameters in Node 16 it's impossible to set a breakpoint on the inner function. Minimal example here: https://github.com/richardsimko/vscode-debugger-issues

if you run the code with npm run start and try to use any of the debug targets to attach to the process it becomes impossible to set a breakpoint on line 5 of index.js.

To Reproduce
Steps to reproduce the behavior:

  1. git clone git@github.com:richardsimko/vscode-debugger-issues.git
  2. npm run start
  3. Launch debug target
  4. Try to set breakpoint in the inner function

Log File

I've attached a log file of me launching the debug config, trying to add the breakpoint and quitting the debug config.

VS Code Version: 1.56.2

Additional context
Possibly related to using type: module, but I'm not sure. It works on any node versions <16 and I couldn't find anything specific to neither the debugger nor ESM in the patch notes.

@richardsimko richardsimko added the bug Issue identified by VS Code Team member as probable bug label Jun 2, 2021
@connor4312
Copy link
Member

connor4312 commented Jun 2, 2021

This looks like a bug in V8 that was fixed in Chrome 91. It seems that they fail to bind breakpoints in any async function that has destructured arguments.

It should be fixed in Node.js once they pick up the latest V8 build.

@connor4312 connor4312 added the *out-of-scope Posted issue is not in scope of VS Code label Jun 2, 2021
@connor4312 connor4312 changed the title Breakpoints moving automatically in node 16 with ESM Breakpoints fail to bind in Node 16 or Chrome 90 in functions with destructured arguments Jun 2, 2021
@connor4312
Copy link
Member

connor4312 commented Jun 2, 2021

I'll add a warning if we detect usages of Node 16-16.2. Verifier: make sure the warning is shown and makes sense.

@connor4312 connor4312 reopened this Jun 2, 2021
@connor4312 connor4312 removed the *out-of-scope Posted issue is not in scope of VS Code label Jun 2, 2021
@connor4312 connor4312 added this to the May 2021 milestone Jun 2, 2021
@richardsimko
Copy link
Author

Thanks for the reply! I'll hold out for a Node update.

@richardsimko
Copy link
Author

@connor4312 I saw that you added the warning for 16.2 but the issue is still present in 16.3.

@JacksonKearl JacksonKearl reopened this Jun 4, 2021
@JacksonKearl
Copy link

^ verification-found by @richardsimko .

@JacksonKearl JacksonKearl added the verification-found Issue verification failed label Jun 4, 2021
@connor4312
Copy link
Member

Please verify the warning is present; 16.3 was just released on Thursday and I don't plan to push candidate fixes for this (or future) releases. It's a helpful hint to exist, but Node will continue releasing updates and I will patch for them on our normal release cadence.

@connor4312 connor4312 removed the verification-found Issue verification failed label Jun 4, 2021
@JacksonKearl
Copy link

Did not see any notification when running the above repro steps.
recording (23)

@connor4312
Copy link
Member

@JacksonKearl it's only shown when launching Node. When attaching we don't get the version in the same way.

@JacksonKearl
Copy link

Tried these launch configs and never saw a notification?

{
    "configurations": [
        {
            "command": "npm start",
            "name": "Run npm start",
            "request": "launch",
            "type": "node-terminal"
        },
        {
            "type": "node",
            "request": "launch",
            "name": "Launch Program",
            "program": "${workspaceFolder}/index.js",
            "skipFiles": [
                "<node_internals>/**"
            ]
        },
        {
            "name": "Launch via NPM",
            "request": "launch",
            "runtimeArgs": [
                "run-script",
                "start"
            ],
            "runtimeExecutable": "npm",
            "skipFiles": [
                "<node_internals>/**"
            ],
            "type": "pwa-node"
        },
        {
            "name": "Attach",
            "port": 9393,
            "request": "attach",
            "skipFiles": [
                "<node_internals>/**"
            ],
            "type": "pwa-node"
        },
    {
        "name": "Attach: process",
        "processId": "${command:PickProcess}",
        "request": "attach",
        "skipFiles": [
            "<node_internals>/**"
        ],
        "type": "pwa-node",
        "restart": true,
    }
    ]
}

@connor4312
Copy link
Member

Hm, those should work. Can you stick a trace: true in Launch Program and share the logs?

@JacksonKearl
Copy link

Ah, I see it. I was expecting a pop-up.
image

@JacksonKearl JacksonKearl added the verified Verification succeeded label Jun 5, 2021
@connor4312
Copy link
Member

Yea, I figured a popup might be too intrusive. If breakpoints aren't working, I think most users' first inclination will be to check the console for errors, so I figured it was a decent place for it.

@JacksonKearl
Copy link

Fair enough. Though I cant shake that it might be nicer to assume Node.js 16.X broken until confirmed fixed versus constantly having folks who install the latest release be confused until we get to bumping our known-broken limit.

@icetbr
Copy link

icetbr commented Jun 9, 2021

I think this is the MR we're waiting for.
nodejs/node#38273

@ppicom
Copy link

ppicom commented Jun 11, 2021

What options do those of us that are running Mac with M1 chips and hence cannot downgrade to node 14 have? :D wait for the fix? Is there a workaround? Thanks!

@richardsimko
Copy link
Author

It works in Node 15 if that runs on M1. Otherwise I saw that the PR to upgrade V8 has been merged so I would presume it will be released in the next minor for 16.

@ppicom
Copy link

ppicom commented Jun 11, 2021

I can't seem to run v15 on mac M1. On every post I find online they say is as simple as nvm install v15 but in my case that yields a cascade of error logs. Will ask for some help on node's repo :D thanks @richardsimko for your answer!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants