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

Introduce a debugSessionEnds event #28234

Closed
weinand opened this issue Jun 8, 2017 · 10 comments
Closed

Introduce a debugSessionEnds event #28234

weinand opened this issue Jun 8, 2017 · 10 comments
Assignees
Labels
api debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-testplan
Milestone

Comments

@weinand
Copy link
Contributor

weinand commented Jun 8, 2017

Currently it is easy to track newly created debug sessions in an extension but it is not possible to track when a session ends.

things to consider:

  • we need a session ID for this because VS Code supports concurrent sessions
  • we should not only solve the "debugSessionEnds" problem but broaden the solution for all kind of debug session related events (e.g. loadedScript events from the DA).
  • probably related: Extension API - How to determine debug state #10077
@weinand weinand added api debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality labels Jun 8, 2017
@weinand weinand added this to the June 2017 milestone Jun 8, 2017
@isidorn
Copy link
Contributor

isidorn commented Jun 8, 2017

The idea here is that vscode extensions can listen on this event? Should this then be part of the vscode api? I feel this makes sense but what is the use case?

Instead of using a session ID we can just use the name of the launch config, since we use that as a sort of an ID.
Currently we have support for commands depending on this with the debugState context key.

@isidorn
Copy link
Contributor

isidorn commented Jun 8, 2017

To conform with our current API we could go with something like the following.
I guess we should think if we want to surface additional stuff which makes sense for the initial debug namespace.
A very first step proposal:

export namespace debug {

  export const onDidEndDebugSession<DebugSessionData>;
  export const onDidChangeDebugState<string>;

}

@weinand
Copy link
Contributor Author

weinand commented Jun 8, 2017

Instead of "DebugSessionData" we need a type (interface) DebugSession (and we would promote the current "workbench.customDebugRequest" command to become a first class method customRequest on it).

Since VS Code supports concurrent debug sessions, I don't think that "debug state" changes make a lot of sense without debug sessions. So the DebugSessionStateChangeEvent combines a "DebugSession" with details about the state change.

So my proposal:

export namespace debug {
  export const onDidEndDebugSession: Event<DebugSession>;
  export const onDidChangeDebugState: Event<DebugSessionStateChangeEvent>;
}

export interface DebugSession {
   readonly name: string;
   customRequest(requestName: string, args: any): Thenable<DebugRequestResult>;
}

export interface DebugSessionStateChangeEvent {
  debugSession: DebugSession;
  newState: string;
}

@isidorn
Copy link
Contributor

isidorn commented Jun 8, 2017

Currently on the vscode implementation side just a newState being a string makes sense since everything is a slave to the focused session. Though I agree that the API is nicer without this implicit knowledge.

DebugRequestResult in this case is just empty, or should we also sketch it up?

We can always start minimal as you already proposed.

I will sync up with Joh on monday to get his thoughts on this.

@weinand
Copy link
Contributor Author

weinand commented Jun 8, 2017

Relying on an implicit "focused debug session" is neither robust nor easy to use. Extension code would have to figure out what the "focused debug session" is in order to know where the state change has happened. Due to race conditions there is no guarantee that the "focused debug session" is still the one for which the state change event was delivered. So it is better to keep them together.

BTW, we probably need an activeDebugSession variable on the debug namespace.

@isidorn isidorn mentioned this issue Jun 12, 2017
@weinand
Copy link
Contributor Author

weinand commented Jun 12, 2017

We will now continue the discussion about the general debug API in #28500.
This feature request is only for the debugSessionEnds event.

@brendandburns
Copy link

Regarding the use case. I'm writing an extension (vs-kubernetes) which does some debugger setup work (setups port-forwarding) and then makes a call to an existing debugger (e.g. the nodejs debugger).

The nodejs debugger (which I didn't write) spins up and debugs my app.

But, when the user terminates the debugger, I want to tear down the port forwarding, but I don't have an event I can listen to, to trigger this tear down.

I know that I could write my own debugger that wraps the node debugger and get this interface, but that really shouldn't be necesary (and is a support burden since I want to support so many different langauges...)

@weinand
Copy link
Contributor Author

weinand commented Jun 20, 2017

@brendandburns Is it correct that you call into an existing debugger by using the vscode.startDebug command (similar to what node-debug is using here: https://github.com/Microsoft/vscode-node-debug/blob/master/src/node/extension/extension.ts#L393)?

If we give you a onDidTerminateDebugSession event, you will have to somehow map this event to your session so that you can tear down the correct port forwarding.

Our current thinking is that the onDidTerminateDebugSession event has the debug session object which has a name and a type (both from the corresponding values from the launch config). By using those two attributes you could find the port forwarding that you need to tear down.

Alternatively you can replace the vscode.startDebug command that you might be using by debug.createDebugSession(...) that returns the debug session and you could remember that and compare it later with the debug session received as part of the onDidTerminateDebugSession event.

@weinand
Copy link
Contributor Author

weinand commented Jun 20, 2017

BTW, for May I've explored something that looks similar to the issue you are trying to solve: #26205

weinand added a commit that referenced this issue Jun 22, 2017
@weinand
Copy link
Contributor Author

weinand commented Jun 22, 2017

@brendandburns I've added proposed API for tracking termination of debug sessions. You can find the API here: https://github.com/Microsoft/vscode/blob/master/src/vs/vscode.proposed.d.ts#L472
Tomorrows insiders build should have the new API.

See my comment from above for the two possible approaches to use the API.

In order to use proposed API, you must opt into it by adding a "enableProposedApi": true, to the package.json of your extension. And you'll have to copy the "vscode.proposed.d.ts" from VS Code into your extension project.

/cc @chrisdias

@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
api debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality on-testplan
Projects
None yet
Development

No branches or pull requests

3 participants