Use Microsoft.VisualStudio.Shared.VsCodeDebugProtocol for protocol

[DavidKarlas]

This NuGet is protocol library generated from protocol.json file.

https://www.nuget.org/packages/Microsoft.VisualStudio.Shared.VsCodeDebugProtocol/

This issue is just to raise awareness and no need for immediate switching.

[weinand]

@DavidKarlas
Hi David, I've just started to use the nuget package in mono-debug. Do you have a small example that shows a minimal debug adapter (e.g. a "mock-debug for C#"). Thanks a lot!

[DavidKarlas]

@andrewcrawley do you have some example of adapter?

[andrewcrawley]

We haven't published any samples. The short version is:

1. Derive a class from DebugAdapterBase
2. Call InitializeProtocolClient with the streams to use
3. Override the virtual methods for the requests you want to handle.
4. Call Run on your derived class.

You can also use the DebugProtocolClient class directly - the RequestReceived event would be the most important one for a debug adapter.

[weinand]

@andrewcrawley great, that is helpful, thanks a lot!

[weinand]

@andrewcrawley @DavidKarlas @gregg-miskelly

I did a first path of adopting the nuget package in mono-debug. I could remove a ton of obsolete code (which is great) and all of mono-debug's functionality is already working (see https://github.com/Microsoft/vscode-mono-debug/tree/aweinand/dap_nuget)

Two things I noticed:

Since the request handlers are expected to return the request response, it is difficult (impossible?) to create the correct sequence of responses and events: VS Code expects that a request has returned its respond before any events enabled or triggered by the request arrive.

In the JavaScript/TypeScript debug adapter library handlers do not return the response but send them explicitly back with "SendResponse" (and the handlers return type is 'void'). This makes it possible to freely control the order of "SendEvent" and "SendResponse" calls in the handler.

An example is HandleInitializeRequest. The nuget package API only allows to send the event before the response:

protected override InitializeResponse HandleInitializeRequest(InitializeArguments args)
{
	// ...

	// Mono Debug is ready to accept breakpoints immediately
	Protocol.SendEvent(new InitializedEvent());

	return new InitializeResponse(
		// This debug adapter does not need the configurationDoneRequest.
		supportsConfigurationDoneRequest: false,
		// ...
	);
}

The better order would be:

protected override void HandleInitializeRequest(InitializeResponse response, InitializeArguments args)
{
	// ...
        response.capabilities.supportsConfigurationDoneRequest = true;
	Protocol.SendResponse(response);

	// Mono Debug is ready to accept breakpoints immediately
	Protocol.SendEvent(new InitializedEvent());
}

Another nice consequence of a 'void' return type is that handlers can be marked 'async' which makes it easy to use 'await' in the handler's body.

I ran into this when trying to use 'await' to get the "runInTerminal" reverse request working.
In the HandleLaunchRequest I need to call back into VS Code by means of Protocol.SendClientRequest(...) or Protocol.SendClientRequestSync(...). With both APIs I was always blocking forever when waiting for the response. Only when ignoring the response, "runInTerminal" works fine (see https://github.com/Microsoft/vscode-mono-debug/blob/aweinand/dap_nuget/src/MonoDebugSession.cs#L340)

Are you using "runInTerminal" in any nuget based debug adapter? Do you have a code snippet that shows how to wait for the response of the "runInTerminal" request?

[andrewcrawley]

@weinand - The behavior of the library should be that any calls to SendEvent from a request handler result in the event being queued and sent after the response (except in the case of the DisconnectRequest, which sends all events immediately). I figured it was better to have the library enforce proper ordering of events and responses rather than requiring adapter authors to do it.

At the moment, the only consumer of the DebugProtocolClient is our internal SampleDebugAdapter that we use for testing the VS debug adapter host, and our host doesn't support the RunInTerminalRequest. The library uses a single thread for handling debug adapter IO, which is already busy processing the "launch" request, so blocking on that thread to wait for a response to the RunInTerminalRequest is going to deadlock (In fact, calling SendClientRequestSync in that scenario should throw an exception informing you that blocking calls are not allowed on the dispatcher thread). I hadn't considered the scenario where you might want to have one request block on another, and I can't think of any good way to make it work with the current model. It might work to send the request and have the failure handler tear down the debuggee and send a TerminatedEvent or something?

Also, rather than returning an ErrorResponse directly, you can just throw a ProtocolException with the right data set on it and we'll do the conversion. Right now, the library ignores the error id and some of the other fields while doing the conversion, but that's an easy fix.

[weinand]

@andrewcrawley great to hear that the client library takes care of the ordering problem.
My JavaScript-based client library used the same approach initially, but I changed this later because I wanted to simplify my implementation and make the code ordering match the event/response ordering. But this is just my personal preference. Your approach makes perfect sense and is probably less error prone.

Yes, I saw the ProtocolException but I tried to minimise the number of changes to my code. I will revisit when everything works again.

RunInTerminalRequest: ok, then I will have to try a bit harder... :-)

[andrewcrawley]

FYI, the code you have now for the error handling will crash - ErrorResponse can't be cast to any other response type.