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

Debug exploration: Async Callstacks #5552

Closed
jrieken opened this issue Apr 20, 2016 · 10 comments
Closed

Debug exploration: Async Callstacks #5552

jrieken opened this issue Apr 20, 2016 · 10 comments
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality
Milestone

Comments

@jrieken
Copy link
Member

jrieken commented Apr 20, 2016

In chrome and node dev tools you enable async callstacks which means calls like setTimeout are not without parent but it shows where setTimeout was invoked when debugging its callback function. Pretty handy

@jrieken jrieken added the debug Debug viewlet, configurations, breakpoints, adapter issues label Apr 20, 2016
@weinand weinand added the feature-request Request for new features or functionality label Apr 20, 2016
@weinand weinand added this to the Backlog milestone Apr 20, 2016
@weinand
Copy link
Contributor

weinand commented Jun 13, 2016

The v8 debugger protocol does not provide enough information for doing this. So this will require more code injection.

@roblourens
Copy link
Member

roblourens commented Feb 14, 2017

Hijacking this issue for the async callstack exploration.

Async callstacks show the origin of an asynchronous call. For example, if the debugger pauses in a then handler on a promise, the async callstack will show the stack that caused the promise to be resolved. Or if the debugger is paused in a setTimeout handler, it will show the stack that lead setTimeout to be called. Chrome and Node support this through the inspector protocol, with the caveat that it's missing from Node in setTimeout calls. See nodejs/node#11370.

In F12 and Chrome Devtools, async callstacks can be enabled or disabled with a button next to the callstack. In the inspector protocol, we use Debugger.setAsyncCallStackDepth to set the number of async calls to trace back. This could be incredibly large, like if a timeout handler calls setTimeout on each call. Chrome Devtools uses 4.

UI
In Edge's F12, async calls are shown in collapsible tree sections

image

In Chrome Devtools, async sections are demarcated by a non-clickable label row with a description of the async call.
image
We will get this description too via the inspector protocol.

Visual Studio is similar - it shows an extra row that says [Async Call]
image

For VS Code, if we show the async frames and include their info in the existing callstack, it might look like this:
image

But there are some downsides to this, like you can't tell how many different Promise.resolve calls are in that stack.

Alternately, we could allow a debug adapter to add a non-clickable frame or even collapsible sections like F12.
image

We either need to

  • Make async frames a first-class citizen in the debug protocol - this would also be useful to other debug adapters and keep the experience consistent.
  • Build on the presentationHint property introduced in 1.9 to indicate a 'label' row that doesn't try to open a file when you click on it
  • Or just change the way VS Code interprets existing frames, and don't complain if a frame omits the 'source' property

Clicking an async call
Async frames are not 'live' frames - we can't get their locals or evaluate code in their context. We only have their function names and locations. So when clicking on a frame, we would show the line in the editor, but show an empty variables window. So it needs to be obvious that a frame is an async frame.

FYI @weinand @isidorn

@kieferrm kieferrm mentioned this issue Feb 14, 2017
38 tasks
@roblourens roblourens self-assigned this Feb 14, 2017
@roblourens roblourens changed the title debug - support async callstacks with nodejs Debug exploration: Async Callstacks Feb 14, 2017
@weinand
Copy link
Contributor

weinand commented Feb 14, 2017

@roblourens great analysis!

Following the "Do The Simplest Thing That Could Possibly Work" principle I suggest that we proceed along the lines of @roblourens bullets from above:

  • we use the Chrome Devtools approach with non-clickable labels as separators
  • we enable this feature by default for node2 and initially provide no nice UI to turn if off (because we believe that it will be always useful; but maybe we need a setting for it)
  • we will not promote async frames to become first-class citizen in the DAP, so VS Code does not know whether a frame is a normal frame or an async frame. As a consequence the DA has to make sure that scopes, variables, and evaluate requests (which use the async frame as a parameter) are returning empty results (or a warning).
  • we surface the label frames as stack frames that have no source. I'm pretty sure that this is already supported. Without source clicking them doesn't do anything.
  • we introduce a new presentationHint on StackFrame with a value label to request a centred, deemphasized look in the UI (see protocol request Support a presentation hint on stack frames  vscode-debugadapter-node#106)

@roblourens
Copy link
Member

Sounds good. I would add a launch config option to disable this, since it can always be enabled/disabled in other devtools (I'm not totally sure why people would disable it, maybe they just want to eliminate noise).

I was trying to return frames with no source, but was getting errors when I clicked on them. If it's supposed to work, I'll look at it again or file an issue.

I'm not sure I want to 'deemphasize' them, since it may be user code. And there may be skipped async frames, and I don't want to eliminate the contrast between async user code and skipped async code. But it needs to be obvious that they're different, so maybe I will.

@weinand
Copy link
Contributor

weinand commented Feb 14, 2017

Yep, launch config option sounds good to me.

Yes, please file a bug for the source-less frames.

I only meant to 'deemphasize' the label frames (which are basically "headers"), but not the async frames. In Chrome Devtools the label frames look 'deemphasized' as well (they are grey).

@roblourens
Copy link
Member

Someone filed #20622 which is what I was seeing.

@weinand
Copy link
Contributor

weinand commented Feb 23, 2017

There is nothing to test in this milestone because it was only an exploration.

@roblourens
Copy link
Member

Reopening for implementing this in March

@weinand
Copy link
Contributor

weinand commented Mar 14, 2017

@roblourens I've added an optional presentationHint attribute to type StackFrame.
The only supported value for now is label.
Available in [email protected]

@roblourens
Copy link
Member

Implemented

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests

4 participants