-
Notifications
You must be signed in to change notification settings - Fork 357
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 Sometimes not resolving with Node 6.1.0 #934
Comments
Any idea when this is likely to be resolved? This issue is blocking our adoption of Node 6.1 so would appreciate a fix soon. Thanks. |
I'm looking into this, but right now it is not clear what changed on the Node side of things or what the root cause of the problem is. |
Quick update: The root cause seems to be a change on the node side of things, not in NTVS directly , but it is possible we were relying on bugged behavior previously. The issue is nondeterministic and more common on larger projects. I believe it also exists in Node 6.0. I'm seeing several different behaviors:
More specifically, here's how the flow currently works with for a new express app with the breakpoint in
To summarize, the core problem seems to be that the first breakpoint we set using the script name does not get picked up by Node. How our code deal with fully bound is a bit odd, and this even causes us to print an error message, but I think this is a red herring. One other curious note: script paths are all lowercase in node 5.9 but properly cased in Node 6.0:
I'll investigate to see if this is the root cause. |
Update on some of the earlier leads: The path differences don't seem to matter. I tried using a case insensitive regex to match the file name when setting the initial breakpoint, and using a file path with no case differences except for the drive letter. These seem unrelated to the breakpoint being hit or not. Also, I see the backtrace trigger router.get('/', function (req, res) {
res.render('index', { title: 'Express' }); // set breakpoint here
}); This suggests the illegal access problem is unrelated, which seems to be the conclusion of the Node-inspector thread too. It looks like there is a fix in V8 already that is getting pulled into node: nodejs/node#6928 |
Issue microsoft#934 **Bug** Initial breakpoints currently sometimes do not resolve when using Node 6.x. In most cases, you can remove the breakpoint and add it again later to get it to resolve. **Fix** I believe something with Node 6 changed the casing for the names of scripts in the v8-debug protocol. There were two fixes here: * Use a case insensitive regular expression to match the script names when setting a breakpoint (I believe this is similar to what VS code does) * Fix one case where we were relying on a case sensitive comparision to update a breakpoint. This is not quite tested to be get merged in just yet, but wanted to see if there were any initial concerns or feedback with the approch here.
This fix is available in v1.2 beta. Please try it out and let us know what you think! https://aka.ms/ntvslatest |
We've been trying out 1.2 beta with Node 6.2 and the breakpoints seem to be working so far. Thanks. |
Repo
require
the file from main.Expected
Debugger stops at breakpoints
Actual
Breakpoints are not resolved.
Breakpoints were already less stable with Node 6.0.0, but with 6.1.0 I'm not seeing them resolved at all. Node 5.x branches work fine.
microsoft/vscode#6029
The text was updated successfully, but these errors were encountered: