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

Compound debugging sessions doesn't seem to allow jumping between sessions #34920

Closed
auchenberg opened this issue Sep 25, 2017 · 10 comments
Closed
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues ux User experience issues verified Verification succeeded

Comments

@auchenberg
Copy link
Contributor

auchenberg commented Sep 25, 2017

When using Compound debugging sessions we no longer allow users to seamlessly jump between the two debugging sessions, which limits the experience, as it's not clear that you manually have to switch to the other debugging session to continue debugging.

See this video https://youtu.be/zdKFecmI8Ko

The issue is that when breaking on a Chrome breakpoint after execution is resumed, then a breakpoint on the Node debug session is hit, but this isn't clear in the UI, as you manually have to switch to the Node debug session to see this.

It didn't use to be this way, as we automatically jumped between debugging sessions to create a seamless experience in earlier builds.

  • VSCode Version: Version 1.16.1 (1.16.1) + insiders

Steps to Reproduce:

  1. Clone https://github.com/glimpse/stickerapp
  2. Checkout chrisdias/ignite2017 branch
  3. Start compoun debug session
@vscodebot vscodebot bot added the insiders label Sep 25, 2017
@vscodebot vscodebot bot added the debug Debug viewlet, configurations, breakpoints, adapter issues label Sep 25, 2017
@isidorn isidorn added bug Issue identified by VS Code Team member as probable bug and removed insiders labels Sep 25, 2017
@isidorn isidorn added this to the September 2017 milestone Sep 25, 2017
@isidorn
Copy link
Contributor

isidorn commented Sep 25, 2017

@auchenberg good catch, thanks

@auchenberg
Copy link
Contributor Author

💪

@weinand
Copy link
Contributor

weinand commented Sep 26, 2017

In the test pass I've immediately noticed that with this "fix", debugging two sessions is completely broken. See this video:

2017-09-26_11-32-32

After stepping in one session, the other session is activated automatically. So the next step action now goes to the other session which is very confusing because it does not become obvious why.

How this should work:
Whenever one session stops (for whatever reason), it should become the active session.
For client/server debugging this results in this (good) behavior:

  • stepping over a server request in the client does not make the server active automatically
  • but if there is a breakpoint set in the server for the request, the server stops and the server becomes the active session
  • now the user can step in the server until the request is finished and the server returns a result and resumes
  • if there is another breakpoint in the client for receiving the server response, the client will stop and the active session is moved back to the client.

From @auchenberg's initial issue "we no longer allow users to seamlessly jump between the two debugging sessions" it sounds that there was a regression of the original behaviour which I wasn't aware of.

We should investigate what that regression is (instead of fixing it with a new mis-feature).

@weinand weinand reopened this Sep 26, 2017
@weinand weinand added the important Issue identified as high-priority label Sep 26, 2017
@isidorn
Copy link
Contributor

isidorn commented Sep 27, 2017

@weinand thanks for clarification.
I do not understand what is the regression here. @auchenberg please clarify how it was working before and what is the difference between the current behavior and previous. And if what previous version was it working.

@isidorn isidorn added the info-needed Issue requires more information from poster label Sep 27, 2017
@isidorn
Copy link
Contributor

isidorn commented Sep 28, 2017

There was a following regression:

  1. Program A is stopped
  2. Program B gets stopped -> does not get auto focussed (it should)

This was introduced by a user PR which is good in general but is looking at the wrong threadId (the one given by the adapter, not the real id). By accident node always gives same ids to thread starting from 1 which would happen that the wrong threads are gettings focused.

The original issue posted by @auchenberg is not fixed (it is invalid), that is the following scenario

  1. Program A is stopped and focused
  2. Program B is stopped
  3. Program A continues -> B does not get auto focused and that is the correct behavior as explained above by @weinand

@auchenberg
Copy link
Contributor Author

auchenberg commented Sep 29, 2017

@isidorn

I just grabbed the latest insiders build, and compared it to older releases, and @weinand you are right that this isn't an regression.

That said, the problem is the following:

  1. Program A encounters a breakpoints, stops and get's focus
  2. Program A is resumed
  3. Program B encounters a breakpoints, stops and get's focus
  4. Program A encounters a breakpoints and stops
  5. Program B is resumed. Still has focus
  6. Program A is still paused, and doesn't have focus.
  7. It's not obvious that Program A is paused.

This is visualized in this recording with Node + Chrome debugging, where the Chrome is left paused, and I would expect VS Code to switch to the Chrome process after Node is resumed.

https://youtu.be/ZRWFW_U3qcg

@isidorn
Copy link
Contributor

isidorn commented Sep 29, 2017

@auchenberg thanks for clarifying, however this is expected behavior. If in step 5 we would switch focus to program A then following that logic (implementation) we would need to always switch focus whenever B is resumed - please note that step and continue are treated the same in VSCode (since both can take optionally long until they are hit again). So if we would in step 5 switch to B we would come to the issue @weinand outlined - that the focus always switches when a user is simply stepping through his one program.

A workaround for this would be to focus program A with a timeout -> however this seems too smart, but feel free to open a feature request for that and we can discuss it.
Thanks

@weinand
Copy link
Contributor

weinand commented Sep 29, 2017

IMO trying to "guess" which session a user will interact with next, most likely will fail.
So I don't think we should try to be smart and change the active session.

But we should investigate how other debugger behave in this case.

@stevencl do you know more about this UX issue?

@weinand weinand added ux User experience issues and removed info-needed Issue requires more information from poster important Issue identified as high-priority labels Sep 29, 2017
@weinand
Copy link
Contributor

weinand commented Sep 29, 2017

I've verified that the "fix" (changing the activate session automatically) has been removed.

@weinand weinand added the verified Verification succeeded label Sep 29, 2017
@auchenberg
Copy link
Contributor Author

Your right @isidorn @weinand that we can't make a "guess" on this, but it's a UX issue and if confused me (that knows how our debugger architecture works), then I'm sure it will confuse normal developers too.

Oh, the wonders of multi target debugging!

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug debug Debug viewlet, configurations, breakpoints, adapter issues ux User experience issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants