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

Run to Cursor doesn't actually run #104385

Closed
Spown opened this issue Aug 9, 2020 · 22 comments
Closed

Run to Cursor doesn't actually run #104385

Spown opened this issue Aug 9, 2020 · 22 comments
Assignees
Labels
author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Milestone

Comments

@Spown
Copy link

Spown commented Aug 9, 2020

"Run to Cursor" just sets a breakpoint (obviously not a desired behavior) at the selected line and stays on the line it is currently on (even less so desired).

If I switch back to the default debugger (debug.javascript.usePreview=false) - it works as intended.

VS Code Version: 1.47.3

Additional context
I am debugging a plain NodeJS, not TS or Browser JS. Also tried Insiders.

@connor4312
Copy link
Member

Setting a breakpoint at the cursor position is actually how VS Code implements "run to cursor". However, staying on the line it's on is not a desired behavior. I can't reproduce this using a simple Node script -- can you collect a trace log using the instructions in the issue template?

@connor4312
Copy link
Member

connor4312 commented Aug 10, 2020

Thanks for the emailed log! In looking for the second issue you mentioned, I'm seeing:

  1. You stop on a breakpoint around L94
  2. Presumably running to cursor, we get a new breakpoint at L107
  3. There a 5 second pause (is this where you expected VS Code to continue automatically?)
  4. We get a continue request, and then pause at the requested location at L107 (did you hit "continue" manually here?)
  5. The breakpoint at L107 is not cleared.

@Spown
Copy link
Author

Spown commented Aug 10, 2020

There a 5 second pause (is this where you expected VS Code to continue automatically?)
did you hit "continue" manually here?

I may as well have had... it was yesterday. I mean it has never menaged to run on it's own, so if there was a continue - it should've been me. do you need a trace log where I just do "run to cursor" and leave it for a couple seconds before terminating, without doing anything?

Setting a breakpoint at the cursor position is actually how VS Code implements "run to cursor".

yeah, I know. this is actually a slight problem in the default debuggers (node, chrome) as well: when the debug session is able to reach the "run to cursor" point - it will delete it right away, which is fine, but if it is not reachable - the breakpoint stays. forever. and this is not optimal, since when you are jumping between files - you can forget to clear it manually. and then it may break there during an unrelated dev cycle and you'll wonder "why is this breakpoint here? what did I try to resolve with it?" the optimal behavior would be to clear the run to cursor bp on debug session termination and/or restart, regardless of whether it was reached or not.

I can make it a separate request issue, but since you are going to be fixing it regardless and it may as well be a part of the problem - maybe you can tackle this as well...

@connor4312
Copy link
Member

do you need a trace log where I just do "run to cursor" and leave it for a couple seconds before terminating, without doing anything?

Yea that would be helpful to make sure where the issue lies and avoid barking up the wrong tree. Thanks!

@Spown
Copy link
Author

Spown commented Aug 10, 2020

I let it sit fo ~20 secs before terminating.

if that helps, both the true breakpoint and the RtC one are in the same scope, but the scope itself is an async callback (.then(callb))

@Spown
Copy link
Author

Spown commented Aug 10, 2020

ret = userPromise.then(function (user) {
    var render // true breakpoint here
    ;
    if (user) {
        if (req.body.ajax) {
            locals.user = user;
        } else {
            render = {
                'redirect':  '/admin'
            };
        }
    } else {
        render = toRender;
    }
    return render; //run to cursor here, at the line start
});

userPromise is the end of a variable async chain, mainly composed of Mongoose queries.

@connor4312 connor4312 transferred this issue from microsoft/vscode-js-debug Aug 10, 2020
@connor4312
Copy link
Member

Thank you for the log, that's helpful. I can't repro locally, but this seems like a bug in vscode rather than the debugger.

The sequence above is accurate -- VS Code sets a breakpoint on line 107, which is verified by the child session (not the wrapper parent session, but that doesn't cause an issue locally...) but never sends a continue.

{
  "command": "setBreakpoints",
  "arguments": {
    "source": {
      "name": "page.js",
      "path": "<snip>",
      "sourceReference": 0
    },
    "lines": [
      94,
      107
    ],
    "breakpoints": [
      {
        "line": 94
      },
      {
        "line": 107,
        "column": 17
      }
    ],
    "sourceModified": false
  },
  "type": "request",
  "seq": 24
}
{
  "seq": 1939,
  "type": "response",
  "request_seq": 24,
  "command": "setBreakpoints",
  "success": true,
  "body": {
    "breakpoints": [
      {
        "id": 2,
        "verified": true,
        "source": {
          "name": "page.js",
          "path": "<snip>",
          "sourceReference": 0
        },
        "line": 96,
        "column": 17
      },
      {
        "id": 5,
        "verified": true,
        "source": {
          "name": "page.js",
          "path": "<snip>",
          "sourceReference": 0
        },
        "line": 107,
        "column": 17
      }
    ]
  }
}

cc @isidorn. I can take a look later this iteration if you don't get to it.

@connor4312 connor4312 added bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues labels Aug 10, 2020
@connor4312 connor4312 added this to the August 2020 milestone Aug 10, 2020
@isidorn
Copy link
Contributor

isidorn commented Aug 11, 2020

@Spown since you have repro steps maybe you just put a breakpoint here and try to figure out what is going on

const debugService = accessor.get(IDebugService);

The only way for vscode to not send a continue is if the focusedThread is somehow undefined which should not be the case. But then you would see an error in the Developer Console. @Spown do you see an null pointer exception if you F1 > toggle developer console

@Spown
Copy link
Author

Spown commented Aug 11, 2020

So, at first I had a couple problems with extensions microsoft/vscode-js-debug-companion and JavaScript Debugger (Nightly). I removed vscode-js-debug-companion and disabled JavaScript Debugger (Nightly) - not sure whether I installed it myself, but probably myself, since disabling it and having debug.javascript.usePreview=true still works and produces the faulty behavior.

Anyway after that the console is basically completely clean. No null pointer exceptions.

[Extension Host] [vscode-icons] v10.2.0 activated!
4log.ts:185  INFO Setting search error: XHR timeout: 5000ms

Debugging the debugEditorActions.ts:125 shows that debugService.state is a ref error with a msg "debugService is not defined". I'm not sure how to trace it back, though...

Thank you.

@isidorn
Copy link
Contributor

isidorn commented Aug 12, 2020

@Spown thanks for trying it out, however I believe that error is only there during debugging. It is not an actual issue.

@connor4312 if there is no null pointer exception or other excpetions then I am not sure how is it possible that VS Code adds a breakoint and does not send a continue. I am open for ideas
Code pointer

await debugService.getViewModel().focusedThread!.continue();

@isidorn
Copy link
Contributor

isidorn commented Aug 28, 2020

I am not sure what is to be done here.
I leave this up to you @connor4312
I would personally leave it open and push it out of August to see if more users hit this.

@isidorn isidorn removed their assignment Aug 28, 2020
@connor4312
Copy link
Member

I was able to replicate this with some contrivance, but maybe not an especially hard-to-hit scenario: if the run-to-cursor was issued for a file in a different debug session than the one I currently had focused in VS Code.

Example: https://memes.peet.io/img/20-08-4bd8aaad-7962-420c-a3eb-16e9ac77bd34.webm

  1. parent.js spawns child.js
  2. I pause in child.js and parent.js
  3. I select parent.js in the call stack view
  4. I issue a run-to-cursor in child.js
  5. The 🐛 as described

Maybe instead of issuing the continued in the focused thread, we issue it to any or the first session who verifies the breakpoint and is paused?

@Spown does this sound like it might have been the scenario you were running?

@isidorn
Copy link
Contributor

isidorn commented Aug 28, 2020

Aaaa. So this only happens if you have two sessions and you do the Run to Cursor for the file which the focused session is unaware of. Sounds like a corner case and even though your idea makes sense, I feel like it might break some other corner case.

So I would simply not fix this. Sounds very rare and nothing gets broken.
Alternative: use your idea with a simple update: if mulitple sessions verify the breakpoint, send it to the focused one.

@connor4312
Copy link
Member

connor4312 commented Aug 28, 2020

Yea, this is a single report and very much an edge case. I'm fine backlogging unless we get more reports.

if mulitple sessions verify the breakpoint, send it to the focused one.

This can be a halting problem since breakpoint verification can be asynchronous. Verification used to be 100% async in js-debug until earlier this year

@connor4312 connor4312 modified the milestones: August 2020, Backlog Aug 28, 2020
@Spown
Copy link
Author

Spown commented Aug 28, 2020

does this sound like it might have been the scenario you were running?

well, I'm not using child_process anywhere. but there are some modules that seem to use it. my first thought was open-in-editor-connect, but disabling it doesn't seem to affect the bug. other modules I don't even use directly - they are deep down the tree so it would take time to figure out what to switch off to test. and it is probably not needed since I've found the problem.

I have only 1 node-debug session and 1 chrome-debug session. And believe it or not, not using chrome-debug (only debugging node, and hitting the braekpoint via just browser) - eliminates the bug. Needless to say, these debuggers watch over very different environments and files and should not interfere with each other.

Also I wouldn't call it an edge case - since debugging both back- and front end simultaneously belongs to a typical full stack developer dev loop.

thank you.

@connor4312
Copy link
Member

Thanks for the info, it sounds like you're hitting the same scenario, in one way or another.

@Spown
Copy link
Author

Spown commented Aug 28, 2020

Concerning the rarity of the report... I don't think many people use RTC and even when they do and the problem occurs - hitting run will resume the session so it may look like a normal (albeit suboptimal) behavior. And of course full stack dev is less common than pure back or pure front dev... combining this all makes it rare, but rarity alone doesn't constitute an edge case.

@Spown
Copy link
Author

Spown commented Aug 28, 2020

So this only happens if you have two sessions and you do the Run to Cursor for the file which the focused session is unaware of

hitting the same scenario, in one way or another.

the focused session is always the node-debug one if under "focused" you mean what is selected in the widget drop down select. But I guess internally the focus shifts back and forth...

@connor4312
Copy link
Member

connor4312 commented Aug 28, 2020

I mean focus in the "call stack" view.

The fact that this problem doesn't repro if you only run a single session makes me strongly suspect that this is what you're running into.

@Spown
Copy link
Author

Spown commented Aug 28, 2020

I mean focus in the "call stack" view.

isn't it the same/proxy? switching in "call stack" - switches the in the widget and vice versa...

@connor4312
Copy link
Member

I've implemented the following heuristic:

  • Set the breakpoint. If it doesn't verify immediately, wait up to 2s to be verified.
  • In order of preference, picked the paused thread that:
    1. Is the focused thread and from a session that verified the bp, or
    2. Is a thread from a session that verified the breakpoint and is paused in this file, or
    3. Is a thread from a session that verified the breakpoint, or
    4. Is the focused thread

@connor4312 connor4312 modified the milestones: Backlog, November 2020 Nov 4, 2020
@connor4312 connor4312 added the author-verification-requested Issues potentially verifiable by issue author label Nov 6, 2020
@roblourens roblourens added the verified Verification succeeded label Dec 3, 2020
@github-actions github-actions bot locked and limited conversation to collaborators Dec 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

5 participants
@roblourens @isidorn @connor4312 @Spown and others