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 Sometimes not resolving with Node 6.1.0 #934

Closed
mjbvz opened this issue May 9, 2016 · 6 comments
Closed

Breakpoints Sometimes not resolving with Node 6.1.0 #934

mjbvz opened this issue May 9, 2016 · 6 comments
Milestone

Comments

@mjbvz
Copy link
Contributor

mjbvz commented May 9, 2016

Repo

  1. Install Node 6.1.0
  2. Create new app.
  3. Create new file (seems to repo more in larger projects and files besides the main file).
  4. Set some breakpoints this new file.
  5. require the file from main.
  6. Try debugging.

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

@mjbvz mjbvz changed the title Breakpoints not resolving with Node 6.1.0 Breakpoints not resolving with Node 6.1.0 in other files May 9, 2016
@mjbvz mjbvz changed the title Breakpoints not resolving with Node 6.1.0 in other files Breakpoints not resolving with Node 6.1.0 in files besides main May 9, 2016
@NoelAbrahams
Copy link

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.

@mjbvz
Copy link
Contributor Author

mjbvz commented May 16, 2016

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.

@mjbvz
Copy link
Contributor Author

mjbvz commented May 23, 2016

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 routes\index.js:

  1. Launch node with --nolazy --debug-brk

  2. Attach and request scripts from debugger.

  3. We hit hit initial breakpoint on first line of main file since debug-brk is passed in.

  4. Request backtrace from debugger

  5. We set an initial breakpoint using the script name:

    DebuggerConnection: Request: {"command":"setbreakpoint","seq":5,"type":"request","arguments":{"line":5,"column":4,"type":"script","target":"C:\\Users\\matb.REDMOND\\Documents\\Visual Studio 2015\\Projects\\ExpressApp6\\ExpressApp6\\routes\\index.js"}}
    
    DebuggerConnection: Response: {"seq":6,"request_seq":5,"type":"response","command":"setbreakpoint","success":true,"body":{"type":"scriptName","breakpoint":2,"script_name":"C:\\Users\\matb.REDMOND\\Documents\\Visual Studio 2015\\Projects\\ExpressApp6\\ExpressApp6\\routes\\index.js","line":5,"column":4,"actual_locations":[]},"refs":[],"running":false}
    [21:50:45.6560253] AD7Engine Event: Microsoft.NodejsTools.Debugger.DebugEngine.AD7BreakpointErrorEvent (abb0ca42-f82b-4622-84e4-6903ae90f210)
    
  6. This breakpoint does not resolve properly according to our logic (actual_locations is empty.) At this point, the script in question has not been fully loaded yet (We have not seen an afterCompile event emitted for it.) This happens in both 5.9 and 6.x

  7. We then request continue. This resumes from the main file on line zero .

  8. A bunch of afterCompile events and other stuff happens. Pretty much identical between node versions.

  9. Things diverge here. On 5.9, we hit the breakpoint set in the index.js file. On 6.x, we never hit anything.

  10. On 5.9, when node hits the breakpoint, we process the event. The breakpoint we set in step 5 is not understood to be fully bound by our code (even though it is fully bound in node). In ProcessBreakpointBreakAsync, we therefore destroy the existing breakpoint and recreate it based on script_id instead (176 is the id of routes/index.js):

    DebuggerConnection: Request: {"command":"setbreakpoint","seq":9,"type":"request","arguments":{"line":5,"column":4,"type":"scriptId","target":176}}
    
    DebuggerConnection: Response: {"seq":210,"request_seq":9,"type":"response","command":"setbreakpoint","success":true,"body":{"type":"scriptId","breakpoint":3,"script_id":176,"line":5,"column":4,"actual_locations":[{"line":5,"column":4,"script_id":176}]},"refs":[],"running":false}
    

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:

6.1: C:\\Users\\matb.REDMOND\\Documents\\Visual Studio 2015\\Projects\\node6debugtest\\node6debugtest\\app.js

5.9: c:\\users\\matb.redmond\\documents\\visual studio 2015\\Projects\\node6debugtest\\node6debugtest\\app.js

I'll investigate to see if this is the root cause.

This was referenced May 24, 2016
@mousetraps mousetraps added this to the June milestone May 24, 2016
@mjbvz mjbvz changed the title Breakpoints not resolving with Node 6.1.0 in files besides main Breakpoints Somtimes not resolving with Node 6.1.0 May 24, 2016
@mjbvz mjbvz changed the title Breakpoints Somtimes not resolving with Node 6.1.0 Breakpoints Sometimes not resolving with Node 6.1.0 May 24, 2016
@mjbvz
Copy link
Contributor Author

mjbvz commented May 24, 2016

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 Internal error: illegal access both when breakpoints are hit and when they are not. Setting the breakpoint inside of the express router seems to be a reliable 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

mjbvz added a commit to mjbvz/nodejstools that referenced this issue Jun 14, 2016
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.
@mousetraps
Copy link
Contributor

This fix is available in v1.2 beta. Please try it out and let us know what you think! https://aka.ms/ntvslatest

@NoelAbrahams
Copy link

We've been trying out 1.2 beta with Node 6.2 and the breakpoints seem to be working so far. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants