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

Distinguish DA exit events from debugee exit events #48805

Closed
weinand opened this issue Apr 27, 2018 · 1 comment
Closed

Distinguish DA exit events from debugee exit events #48805

weinand opened this issue Apr 27, 2018 · 1 comment
Assignees
Labels
debt Code quality issues debug Debug viewlet, configurations, breakpoints, adapter issues
Milestone

Comments

@weinand
Copy link
Contributor

weinand commented Apr 27, 2018

In rawDebugSession.ts there is an event mix-up between exiting debug adapters and exiting debuggees.

An exiting debuggee is an expected and frequent event that might be relevant for the user (especially if the exit code of the debuggee is non-zero which indicates a problem).
We could show a non-zero exit status as a notification.

An exiting debug adapter (with exit code 0) is an expected and frequent event that is not relevant for a user because it is just the normal operation of VS Code debuggers. A crashing debug adapter should be logged or even added to telemetry because we should become aware of it.
We should alert the user about a crashing DA too, but that Alert must be distinguishable from a crashing debuggee (e.g. as a modal alert). A crashing DA is a serious program error and there should be a way to report this to the author of the extension.

The mix-up is in https://github.com/Microsoft/vscode/blob/75e88b676f868eebb4e3516347f8b0c540451216/src/vs/workbench/parts/debug/electron-browser/rawDebugSession.ts#L286 and https://github.com/Microsoft/vscode/blob/75e88b676f868eebb4e3516347f8b0c540451216/src/vs/workbench/parts/debug/electron-browser/rawDebugSession.ts#L542 and https://github.com/Microsoft/vscode/blob/75e88b676f868eebb4e3516347f8b0c540451216/src/vs/workbench/parts/debug/electron-browser/rawDebugSession.ts#L520.

In line 286 all "exited" events from the DA are picked and forwarded via _onDidExitAdapter. This is wrong because an exiting debuggee is not an exiting DA (and the event passed as an argument _onDidExitAdapter has the exit code from the debuggee and not the DA).

In line 542 and 520 you even create fake DAP events to fire again via _onDidExitAdapter.

I suggest the following changes:

  • rename private methods onDapServerError and onServerExit to onDebugAdapterError and onDebugAdapterExit (we do not want to continue to use the term "server")
  • do not create fake DAP events and fire them with this.onDapEvent({ event: 'exit', type: 'event', seq: 0 }); in lines 520 and 542. Instead fire them directly with this._onDidExitAdapter.fire(...);.
  • for the _onDidExitAdapter event do not misuse the DAP event object. Either create a new event type that is independent from DAP or use a generic event.
  • for the real DAP event 'exit' create a new event emitter onDidExitDebugee analog to onDidTerminateDebugee. It could very well be that you are not actually handling this event anywhere because VS Code provides no UI for the exit code yet. But please introduce the new event emitter to make the distinction clear.
@weinand weinand added debug Debug viewlet, configurations, breakpoints, adapter issues debt Code quality issues labels Apr 27, 2018
@weinand weinand added this to the May 2018 milestone Apr 27, 2018
@weinand weinand changed the title Distinguish DA exits from debugee exits Distinguish DA exit events from debugee exit events Apr 27, 2018
@isidorn
Copy link
Contributor

isidorn commented Apr 27, 2018

@weinand this makes sense, thank you for the anlaysis and the suggestion. I will work on this in debt week.

@isidorn isidorn closed this as completed in 0913b0c May 2, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Jun 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues debug Debug viewlet, configurations, breakpoints, adapter issues
Projects
None yet
Development

No branches or pull requests

2 participants