Debug protocol: Support browser launching

[gregg-miskelly]

Currently the debug protocol has no way for a debug adapter to request that the IDE open up a web browser. Some debug adapters, such as the C# debug adapter, can start a web browser as part of launching a web server project. The debug adapter needs some control to be able to signal when the web server is ready.

This could either be designed as an event (ex: WebServerReady) or as a callback request like starting processes in the terminal.

Dependent issue: dotnet/vscode-csharp#2657
Related issue: microsoft/vscode-debugadapter-node#153

[weinand]

Proposal:

If a client (frontend) supports the 'openExternal' request, it must announce this by setting the supportsOpenExternalRequest in the 'initialize' request to true:

/** Arguments for 'initialize' request. */
export interface InitializeRequestArguments {

	// ...
	
	/** Client supports the openExternal request. */
	supportsOpenExternalRequest?: boolean;
}

And a debug adapter can then use the 'openExternal' request by passing the uri as the only (mandatory) argument:

/** OpenExternal request; value of command field is 'openExternal'.
	This request is sent from the debug adapter to the client to open a url in an external program, e.g. a http(s) or mailto-link, using the default application.
*/
export interface OpenExternalRequest extends Request {
	// command: 'openExternal';
	arguments: OpenExternalArguments;
}

/** Arguments for 'openExternal' request. */
export interface OpenExternalArguments {
	/** The uri that should be opened. */
	url: string;
}

/** Response to 'openExternal' request. This is just an acknowledgement, so no body field is required. */
export interface OpenExternalResponse extends Response {
}

[weinand]

@gregg-miskelly @andrewcrawley @chuckries @DonJayamanne @pieandcakes
You can find my proposal for a openExternal request in the branch;

For your convenience I've added the corresponding TypeScript definitions above.

I'd appreciate any feedback.

[DonJayamanne]

Sounds, good, however wouldn't a custom request also solve this.
Send a custom request from debug adapter, and handle that from the extension to launch the browser?
Just trying to understand use causes for custom requests.

[weinand]

Yes, a custom request picked up in the corresponding extension could do the same. However, including this new request in the DAP simplifies the case where debug adapters are hosted by a different client than VS Code.

[auchenberg]

One aspect we should think about here, is how this potentially could play together with other debug adaptors in VS Code?

A typical scenario for ASP.Net developers is
0) Press F5 / Launch

1. Launch asp.net server-side
2. Launch Chrome in debug mode
3. Attach VS code to .net
4. Attach VS Code to Chrome
   = debug both stacks at the same time.

Ideas: In the .net DA, for the OpenExternalRequest handler instead of having it's own browser logic, maybe it could try to start a browser debugging session for Chrome/Edge/Firefox?

If no debugging is wished, fallback to new vscode.openExternal api?

[weinand]

@auchenberg multi-debugger cases like your scenario are best handled in an extension since a debug adapter has no access to VS Code's extension API (and cannot assume that it runs in VS Code at all).

The openExternal request is for the scenario from @gregg-miskelly's initial comment or maybe for "The debugger for Chrome".

@roblourens and @gregg-miskelly please chime in for additional thoughts...

[gregg-miskelly]

I didn't see any changes in your branch, but the type script definitions above look good to me.

[weinand]

@gregg-miskelly sorry, I forgot to push. The commit is ac0289e

[gregg-miskelly]

LGTM

[roblourens]

The original scenario is that your C# app is running a server and you want to launch a browser to see the page. If someone wants to debug the page in vscode, this doesn't help with that. For that we would need a way for the chrome debug extension to know when the server is ready and what the url is. Or the C# extension could generate a chrome launch config and start it.

Are you also thinking about that scenario? If that's not what you're focusing on right now this is fine for just launching the system's default app for some uri.

[weinand]

@roblourens your scenario sounds more like Kenneth's use case from above: you want to debug both the client and the server. This is a useful scenario but probably out of scope for DAP because launching a second debug session from a DA (via a generated launch config) needs to make too many assumptions about the frontend (e.g. VS Code). Such a scenario is best served from an extension (which has access to VS Code's debugger API).

The scenario to be addressed by the "openExternal" request is the single debug session case where a C# app is running a server and a browser is just launched to access the server's web UI.

@gregg-miskelly Is this a correct representation of your scenario/needs?

[gregg-miskelly]

In order to address dotnet/vscode-csharp#2657 in a clean way, what I need is a way for a debug adapter to be able to request the client launch a web browser. The current proposal solves this, so it is fine with me.

That said, I certainly don't have a problem if this is a more general solution, at least if it doesn't significantly impact the amount of work. For example, an alternate solution might be some sort of 'WebServerReady' event that VS Code cold initially handle by launching a web browser, but, if we wanted to support debugging client side and server side code at the same time, VS Code could add another builtin launch.json setting (like preLaunchTask) that it could consume. For example, something like the webBrowserAction section of the below configuration. This could also solve the protocol part of microsoft/vscode-debugadapter-node#153.

        {
            "name": ".NET Core Launch (web)",
            "type": "coreclr",
            "request": "launch",
            "preLaunchTask": "build",
            // If you have changed target frameworks, make sure to update the program path.
            "program": "${workspaceFolder}/bin/Debug/netcoreapp2.2/climvc.dll",
            ...
            "webBrowserAction": {
                "type": "chrome",
                "request": "launch",
                "name": "Launch Chrome",
                "webRoot": "${workspaceFolder}"
            }
        },

[roblourens]

Is the webBrowserAction something that the .net debugger already supports? That looks like what I was imagining where that debug extension would launch the chrome debugger.

[gregg-miskelly]

No, it is something that I made up as an example.

[roblourens]

Oh right, got it.

[weinand]

I like the idea of a 'WebServerReady' DAP event but I don't like if VS Code core would have to implement that event and launch a browser or start a debug session.

But we could easily delegate this to an extension. An extension could handle the event and launch a browser based on the information found in the launch config:

"webBrowserAction": {
    "type": "chrome",
    "request": "launch",
    "name": "Launch Chrome",
    "webRoot": "${workspaceFolder}"
}

The extension can even contribute the schema for the "webBrowserAction" JSON, so that Intellisense works seamlessly.

The 'WebServerReady' DAP event could provide port information or a full URL.

The extension would create a url based on the info from the 'WebServerReady' event and use vscode.openExternal API to open it in a browser.

Based on the "webBrowserAction" configuration it could launch a Chrome debug session with the URL.

The extension could be

• a built-in extension that ships with VS Code,
• an extension from the Marketplace, or
• a part of the C# extension, or
• a part of the Chrome debugger (because it would provide the functionality to debug the client anyway).

@gregg-miskelly @andrewcrawley @chuckries @DonJayamanne @pieandcakes @roblourens
what do you think?

[auchenberg]

@weinand Would the WebServerReady event become an official part of the DAP or would it be a custom event?

One concern could be that WebServerReady is really scenario specific. I know the Live Share folks had a similar request for automatic port forwarding, and they ended up analyzing terminal output from their extension to detect when a framework starts a website, parses the url and ports and setups the tunnel.

[weinand]

@auchenberg yes, the WebServerReady event would be official part of the DAP.

Probably the event would be called serverReady.

The semantics would be:
"Announce that the server is ready to accept client connections via the specified information."

The "information" could be a port or a URI.

How the debug adapter determines when to fire the event and what port or URI to include, is an implementation detail of the DA. The implementation might involve scraping terminal output.

[weinand]

I've just created an extension server-ready that implements the "serverReady" approach based on scraping debug console output.

After installing the extension it is possible to add this to any(!) launch config:

"serverReadyAction": {
    "pattern": "listening on port ([0-9]+)",
    "debug": true
}

The extension looks for the "pattern" in the debug console output and extracts the port number.
If debug is false, a url "http://localhost:port_number" is opened in the default browser.
If debug is true, a Chrome debug session is launched for the url.

Here are all supported properties and their default values:

"serverReadyAction": {
    "pattern": "listening on port ([0-9]+)",
    "urlFormat": "http://localhost:%s",
    "webRoot": "${workspaceFolder}",
    "debug": false
}

Here is the complete code of the extension:

import * as vscode from 'vscode';
import * as util from 'util';

export function activate(context: vscode.ExtensionContext) {

  // scan debug console output for a PORT message
  vscode.debug.registerDebugAdapterTrackerFactory('*', {
    createDebugAdapterTracker(session: vscode.DebugSession) {
      const args = session.configuration.serverReadyAction;
      if (args && args.pattern) {
        const regexp = new RegExp(args.pattern);
        return {
          onDidSendMessage: m => {
            if (m.type === 'event' && m.event === 'output' && m.body.output) {
              const result = regexp.exec(m.body.output);
              if (result && result.length === 2) {
                openExternalWithUri(session, util.format(args.urlFormat || 'http://localhost:%s', result[1]));
              }
            }
          }
        };
      }
    }
  });
}

function openExternalWithUri(session: vscode.DebugSession, uri: string) {
  const args = session.configuration.serverReadyAction;
  if (args.debug) {
    vscode.debug.startDebugging(session.workspaceFolder, {
      type: 'chrome',
      name: 'Chrome Debug',
      request: 'launch',
      url: uri,
      webRoot: args.webRoot || '${workspaceFolder}'
    });
  } else {
    vscode.env.openExternal(vscode.Uri.parse(uri));
  }
}

@auchenberg @gregg-miskelly @andrewcrawley @chuckries @DonJayamanne @pieandcakes
What should we do with this now?

Do we still need a serverReady event in DAP?

@gregg-miskelly how would you implement the serverReady event in the C# debug adapter? Do you have a better way than scanning the output?

@roblourens should this extension become part of Chrome debug?

Or should we include this extension in VS Code or ship it via the marketplace?

[puremourning]

I feel that we should avoid having elaborate extension-specific stuff allowed in the protocol.

This is an error that was mace in the language-server protocol where it is not possible to have a completely generic client implementation.

Currently Vimspector has absolutely no adapter-specific knowledge and i want to keep it that way.

If the protocol requires that we support launching a web browser (something i am strongly opposed to), then so be it, but i would strongly resist requiring debug-adapter-protocol implementations to have knowledge of adapter-specific handling instructions, code, methods or behaviours.

[puremourning]

I should say that for the record, this is already somewhat of a problem, if you take for example the way the java debug adapter is a sort of parasite on jdt.ls (perhaps, sympathetically, symbiotic rather than parasitic! :)), but ultimately once even that adapter is started up, there's no problem having a generic implementation of the protocol.

[weinand]

I think the original "openExternal" is no longer an option as the discussion has shifted to an event based approach ("serverReady" event).
With my extension prototype it might even be possible to stay completely out of the debug adapter protocol.

[puremourning]

I have no problem with additional events, so long as a generic DAP client knows what to do with them without adapter-specific configuration or code.

An example is runInTerminal. This is a pretty straightforward requirement: Fire up a terminal, with some well defined properties and semantics. OK that might not be easy for some clients to implement, but it is well-defined.

An event that is like "something happened" with the expectation that clients do unique and marvellous things depending on some adapter-specific client-side code is what is irksome because it requires developers of clients to know about adapter-specifics. And adapter vendors will simply only make it work in VSCode, because why would they repeat their work for all clients?

So if the request was "runInBrowser" and the body properties were just a URL (or something like that), then sure, I could make that happen (Vim could guess what your default browser is and launch that). It wouldn't be easy or fun, but it would be generic. When coupled with a capability flag, this works nicely.

The main problem with your extension is that is explicitly only works in VSCode meaning that the adapter is useless in other clients. Or at the least crippled.

[weinand]

I don't understand what you mean by "the adapter is useless in other clients". My extension does not have an adapter. It is just a VS Code extension that contributes a schema and a bit glue code to VS Code.

The extension implements a VS Code specific feature based on the official DAP. It does not rely on any changes in the DA. If you want to support the same feature in your client, you can do it.

[puremourning]

Sorry I meant in context of the original request:

Currently the debug protocol has no way for a debug adapter to request that the IDE open up a web browser. Some debug adapters, such as the C# debug adapter, can start a web browser as part of launching a web server project

I.e. the request appeared to suggest that the adapter wanted to instruct the client to fire up a web browser, as this is typical or required of the debugging experience for the adapter (similar to the runInTerminal request).

I interpreted this requirement as generic, not specific to some particular adapter or client.

[weinand]

As said before:

I think the original "openExternal" is no longer an option as the discussion has shifted to an event based approach ("serverReady" event).

[puremourning]

I see. though, TBH I didn't follow why that was. As @gregg-miskelly said:

In order to address OmniSharp/omnisharp-vscode#2657 in a clean way, what I need is a way for a debug adapter to be able to request the client launch a web browser

[weinand]

The discussion has now moved to the approach sketched in Gregg's next paragraph:

That said, I certainly don't have a problem if this is a more general solution, at least if it doesn't significantly impact the amount of work. For example, an alternate solution might be some sort of 'WebServerReady' event that VS Code cold initially handle by launching a web browser, but, if we wanted to support debugging client side and server side code at the same time, VS Code could add another builtin launch.json setting (like preLaunchTask) that it could consume. For example, something like the webBrowserAction section of the below configuration. This could also solve the protocol part of Microsoft/vscode-debugadapter-node#153.

[puremourning]

I think what I'm saying is that this alternative approach restricts the solution to VSCode only. If that's the intention then sobeit, but it would be nice for us as a community providing tools to users to provide those tools consistently.

As I understood it, the idea of DAP is to allow multiple clients to enjoy the benefits of having the excellent debugging experience we get from the wealth of great adapters. And we, the community, are super thankful for that. By introducing protocol elements which are there to facilitate things that uniquely work in one client, this would IMO go against the ethos of the protocol.

But you're in control of the protocol, and its ethos, not me. I'm just one guy, trying to make a useful tool :). I will just never say I support this event, request, whatever and my users just won't get this feature.

They still get loads of really cool features though from the base protocol though. So I'm still pretty happy. I just would like to avoid the LSP-problem where each client has to have server-specific knowledge by default. Hence why I was suggesting steering the conversation back to a generic event.

[weinand]

Why does the "alternative approach restricts the solution to VSCode only"?

The "alternative approach" proposes that the DA sends out an event to the client that says "the debugged server is ready and here is a URI you could use to talk to the server".

Where do you see a VSCode restriction here?

[puremourning]

The "alternative approach" proposes that the DA sends out an event to the client that says "the debugged server is ready and here is a URI you could use to talk to the server".

What would a generic client be expected to do with such an event? i.e. what would the documentation for the event say the client should do in response to this event?

If it says something along the lines of "for particular languages..." or "you might wish to..." or "adapter specific hooks should ...." or something then it's not a clear and consise protocol that can be implemented independently of servers.

If the instruction is "upon receiving this event clients must launch a web browser pointing at the address supplied in URL" then that is easily implemented in a generic client and that's fine.

[puremourning]

To be clear, it's the could vs must that's the problem IMO. Protocols and APIs state what a conforming implementing must do, not what it might do.

[weinand]

@puremourning "runInTerminal" is a request because the DA relies on it to launch a target. If "runInTerminal" fails, launching a target fails. The "runInTerminal" spec is a "must".

"serverReady" is an event because it does not require anything from a client. Events don't fail and they don't return a result. Events are used for loose coupling. The "serverReady" spec would be a "might" (like all event specs).

If a headless DAP client decides to not handle "serverReady" because it does not have a browser, then that's perfectly fine and the spec should make this clear.

Eliminating "could" and "shoulds" from a spec like DAP doesn't make sense, because the protocol spec neither covers the functionality or scope of a client (frontend), nor the functionality or scope of the DA (backend).

[weinand]

The "Server Ready" extension mentioned above is now available for Insiders here: https://marketplace.visualstudio.com/items?itemName=andreweinand.server-ready

It does not require any new DAP features and works with all existing debugger extensions (as long as they use DAP "output" events for sending output to the debug console).

[puremourning]

Ok thanks for the explanation. :)

[gregg-miskelly]

Detecting when ASP.NET is ready

@gregg-miskelly how would you implement the serverReady event in the C# debug adapter? Do you have a better way than scanning the output?

The C# debugger currently is also just scrapping stdout. We look for lines that start with 'Now listening on '. If we want to avoid adding the new serverReady event, here are the special things that ASP.NET Core does (that I am aware of at least) --

1. [::] can be the host name for the any IP address. When we see this we convert it to 'localhost'. We actually currently convert anything which isn't an IP Address to localhost. So people may be relying on some other conversions as well.
2. There are normally multiple 'Now listening on:' lines as the default is configured to listen on both https and http. We only pay attention to the first one.

Server-ready extension

I would be fine with replacing our current browser launch code either with something that raised the serverReady event, or updating our launch.json templates to output the right json to light up link detection in the server-ready extension. However, we would need either the extension to ship in-box, or for there to be a good way to advertise to users that they should install the server-ready extension. Example 'advertise' solutions: the C# extension could call an API to add the recommendation, or VS Code could add the recommendation if a workspace had a launch.json file which used serverReadyAction.

For reference, here is the output from starting a 'hello world' ASP.NET Core app --

> dotnet run
info: Microsoft.AspNetCore.DataProtection.KeyManagement.XmlKeyManager[0]
      User profile is available. Using 'C:\Users\greggm\AppData\Local\ASP.NET\DataProtection-Keys' as key repository and Windows DPAPI to encrypt keys at rest.
Hosting environment: Development
Content root path: C:\proj\climvc
Now listening on: https://localhost:5001
Now listening on: http://localhost:5000
Application started. Press Ctrl+C to shut down.

[auchenberg]

Playing devils advocate here @gregg-miskelly: If you already are listening to the process output, why can't your VS Code extension that wraps the Debug Adaptor, just call vscode.env.openExternal(vscode.Uri.parse(uri)); directly? It would be an implementation detail in between your Debug Adaptor and VS Code extension.

[gregg-miskelly]

@auchenberg personally that feels like a hack to me. There are lots of programming languages that are popular for creating web servers. If we expect folks will want to use a VS Code API to start a web browser then we shouldn't make every language tie their debug adapter to VS Code to do so.

[weinand]

We propose the following:

• we ship the "server-ready" functionality as a built-in extension (most likely as a part of the existing "debug-auto-launch" extension which I will rename to "debug-features"). This makes the functionality appear as built-into VS Code.
• We will obey Gregg's implementation details from above Debug protocol: Support browser launching #23 (comment)
• In February we will ship this feature as a "Preview". This makes it possible to tweak naming and functionality in the March release.

[DonJayamanne]

Looking forward to this

[roblourens]

This is a really interesting use of the debug adapter tracker API! It could technically be used for other things besides waiting for a server to be ready so I wonder if we would want to generalize the naming of it beyond "server ready". Could call it a "launch config trigger" or something like that.

For example, when I debug vscode, I sometimes want to attach to the search process, but it doesn't start until some time after vscode starts, so a normal compound config doesn't work. I could write to the console when it starts, and set up a node attach config to trigger when that output is detected.

[weinand]

@roblourens glad you liked it ;-)
Your use cases make perfect sense and we can start to support them (and rename things) next milestone. For the February release we want to ship a "preview" of the feature to gather feedback.

The next Insider build (Monday) will have the serverReadyAction support (and I've unpublished the "Server Ready" extension from the Marketplace to avoid confusion...)

[weinand]

I've create a new feature microsoft/vscode#69311 for February to cover the built-in extension created for this.

[weinand]

I'm closing this request for a DAP addition because we could address the need by a client/frontend implementation (microsoft/vscode#69311).