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

Add debugging attach support to VS Code #6569

Closed
AndrewBrianHall opened this issue May 20, 2016 · 28 comments
Closed

Add debugging attach support to VS Code #6569

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

Comments

@AndrewBrianHall
Copy link

VS Code should add platform level support that provides a structured way for debug adapters to implement and provide attach to process functionality that doesn't rely on updating the launch.json file every time the developer needs to attach to the process.

Key requirements are:

  • Standardized entry point for all debug adapters
  • Ability for user to select desired debug adapter
  • Debug adapter is able to provide its concept of what the user intends to attach to (e.g. process id, url)
  • Debug adapter does not need to generate an on disk .json file to achieve the attach
@egamma egamma added this to the June 2016 milestone May 20, 2016
@egamma egamma added the debug Debug viewlet, configurations, breakpoints, adapter issues label May 20, 2016
@weinand
Copy link
Contributor

weinand commented May 20, 2016

What we plan to do:

We will extend the concept of system variables with a mechanism to trigger commands.
This makes it possible to add something like ${action.PickProcess} to a launch configuration, e.g.:

       {
            "name": ".NET Core Attach",
            "type": "coreclr",
            "request": "attach",
            "processName": "${action.PickProcess}"
        }

Starting a debug session with this launch config will run the 'PickProcess' command where the user can select a process. The result is a string that becomes the value of the variable. Cancelling the command will cancel the launch of the debug session.

The implementation of the PickProcess command will live in the debugger extension and most likely is based on the QuickPick UI. But an extension is free to use whatever UI is available for an extension.

The package.json of the debugger extension will contribute a mapping between the system variable (‘PickProcess’) and the command implemented by the extension, e.g. something like:

"variables": {
    "PickProcess": "extension.pickProcess"
}

At a later point we can decide to provide a default picker action in VS Core and the debugger extensions are free to remove their implementations.

/cc @isidorn @chrisdias

@edumunoz
Copy link
Contributor

Thanks Andre for the details on the proposal. I have a few questions:

I don't think this is possible, but is there a way of passing arguments to the action being launched by VSCode? Something that would be useful might be to pass the launch.json entry that is being launched.
It would be useful if we want to implement multiple configurations for attaching, e.g., remote machine and locally. If passing arguments to the action is not possible, two options are:

  1. Have multiple Quick Pick dialogs every single time the action is triggered, e.g., the first level would be the type of attach (local vs remote). This seems inconvenient given that some users will always do local and some will always do remote.
  2. Have multiple actions that serve as entry points for the different types of attach, and offer multiple entries in the launch.json file, each with a different action. This has the disadvantage of having multiple entry points, and depending on the attach permutations, may not scale nicely.

Do you have any other suggestions? I think option 2 is more user friendly.

Some questions about the behavior of this feature:

  1. What happens if there is more than one action in the active launch.json entry? Will each action be called sequentially? In this case, it seems to me that passing the launch.json entry to each action might be useful.
  2. What happens if the provided action is launched via the command palette?
  3. How is the result string communicated from the debug adapter to VSCode?

@weinand
Copy link
Contributor

weinand commented May 25, 2016

@edumunoz here are my answers to the questions from above:

  • it is possible to pass the selected launch config section to the pick action. I had planned this anyway.
  • local vs. remote: why make this decision in the pick process? The launch.json could have two launch configs, one for the local and one for remote case. For this it would use a ${action.PickProcess} and a ${action.PickRemoteProcess}. Your extension would implement two pick commands for this.
  • having more than one action in the active launch.json entry: yes, we will pass the launch config.
  • launching the pick action via the command palette: only if we cannot make these actions invisible will the user be able to run the action, but the result will go to /dev/null anyway.
  • How is the result string communicated from the debug adapter to VSCode? The string is not communicated back from the debug adapter, but from the picker command running in the extension (host). These are different processes. The string (pid etc.) becomes the command's result. Since VS Code is running the command as part of the start debug session action, the result can be assigned to the system variable and then the substitution mechanism will place it in the launch config.

@edumunoz
Copy link
Contributor

Thanks for the responses @weinand.

The string is not communicated back from the debug adapter, but from the picker command running in the extension (host).

Yes, sorry for mixing both.

The string (pid etc.) becomes the command's result

I assume this can be a Promise<string> that the callback of the command would return.

@weinand weinand added the feature-request Request for new features or functionality label May 31, 2016
@weinand
Copy link
Contributor

weinand commented May 31, 2016

@edumunoz, @Andrew-MSFT, @isidorn, @egamma
As an example for the process picker work I've added "Attach to Process" support to Node Debug. Please see this video.

Please note: I'm launching a simple express application from the command line without running node.js in debug mode. With the picker I select the node process by filtering on the arguments "bin/www". The variable ${action.pickProcess} in the launch config is then replaced by the process id of the node process.

@edumunoz
Copy link
Contributor

@weinand very cool, thanks for showing us.

@weinand
Copy link
Contributor

weinand commented May 31, 2016

@edumunoz
Copy link
Contributor

I integrated what I was working on with your prototype and it works great.

Let us know once the implementation makes it to master.

@isidorn
Copy link
Contributor

isidorn commented Jun 1, 2016

The implementation is on master now.

@weinand
Copy link
Contributor

weinand commented Jun 1, 2016

@edumunoz since node.js supports 'Attach to Process' too, I've added the full process picker flow to node-debug. This includes:

  • node-debug's 'attach' request accepts a new optional 'processId' attribute
  • extension.ts implements an extension.pickNodeProcess command
  • package.json makes this command available as an action variable action.PickProcess (Please note: since there is no command contribution for the extension.pickNodeProcess command, it is not available through the command palette),
  • the initial node-js launch.json contains an 'Attach to Process' launch config

I've already modified the process picker in extension.ts slightly to better understand the OS X application bundle structure.
I plan to add code that filters all processes that are not node or electron (because it doesn't make sense - or it is even dangerous - to try to attach to them).

@gregg-miskelly, @roblourens, @daviwil, @rkeithhill, @felixfbecker, @rebornix, @MSLaguana, @lukehoban: this feature is useful if you want to add process, url, or port pickers to your launch configs. Here is a video.

Please be aware that this is just a first cut and things might change until the end of this sprint. But we'd appreciate any feedback.

@weinand weinand closed this as completed Jun 1, 2016
@gregg-miskelly
Copy link
Member

@weinand does the extension that implements the process picker need to be the same extension that has the debug adapter, or can the two extensions be different? In one of our attach scenarios, we would need them to be different.

@weinand
Copy link
Contributor

weinand commented Jun 1, 2016

@gregg-miskelly I did not yet verify it, but it is an explicit design goal that a process picker can live in a different extension than a debug adapter. So if it does not work, it would be a bug.

@gregg-miskelly
Copy link
Member

@weinand Excellent.

One other question: looking at the node implementation at least, it looks like this facility is pretty generic, and not necessarily specific to picking processes. For example, if we wanted, could we set the launch configuration program to be '${action.getCurrentCSharpOutputProgram}' which would run code in the C# extension to get the output path to the current project? Is or this mechanism really specific to attach to process. (NOTE: I am not sure if we would really want to do this. But I am curious if we could).

@weinand
Copy link
Contributor

weinand commented Jun 1, 2016

@gregg-miskelly yes, your use-case is perfectly valid. The action-variable concept is basically a mechanism to call a function implemented in an extension (aka 'command') with the launch config as a parameter and return a string. So it does not have to involve any UI at all (and of course it is in no way specific for attaching to a process).

(And because we are really running a 'command' and not necessarily a UI 'action' we are already discussing to rename the variables to '${command.xxxxx}').

@roblourens
Copy link
Member

This is great, I've wanted something like this. FYI @auchenberg. We could use this to pick tabs/targets to attach to.

Can the debug adapter call those actions from its own code? It doesn't require a launch.json variable, right?

@felixfbecker
Copy link
Contributor

@weinand I don't get why this requires a launch.json variable. Why can't the debug adapter simply ask VS Code to display a quick pick UI with some values and VS Code returns the user pick?

@MSLaguana
Copy link
Member

The debug adapter is a separate black-box process and has no access to VS Code APIs.

@felixfbecker
Copy link
Contributor

The debug protocol could easily introduce a request/event for this.

@daviwil
Copy link
Contributor

daviwil commented Jun 1, 2016

This looks awesome so far! One question:

this feature is useful if you want to add process, url, or port pickers to your launch configs

Does this mean the action syntax can be used in other variables aside from processId? Also, is action. a special keyword in the variable syntax? I didn't see any place where it's registered other than the variables section.

@roblourens
Copy link
Member

@felixfbecker Assuming there is a way to do this from debug adapter code, this is better because the debug adapter can call arbitrary code using any of the normal Extension APIs. It's not just for quick pickers. Assuming I'm reading this right, if so, I think @weinand is underselling it :)

@felixfbecker
Copy link
Contributor

@roblourens As I understand it the debug adapter is not calling anything, instead the user sets an action in launch.json and then VS Code asks the adapter to act on that. I would prefer if the adapter could trigger UI actions at own will.

@auchenberg
Copy link
Contributor

In a web context we would like the ability to populate the "target selector UI" with targets that isn't coming from a specific process. Think web apps running on a phone or HoloLens.

Looking at the current format for a target, it's very specific to local processes such as Node:

{
   label: basename(executable),
   description: pid,
   detail: cmd,
   pid: pid
}

I could easily imagine us wanting to add a icon next to the debug target for the favicons and another icon to show the device type. Can we expand the format, or is that what the detail property is intended for?

Could we remove "processName": "${action.PickProcess}" completely from the config and made an implementation detail in the extension? I don't see why the choice of action should be surfaced to the end-user developer.

For the web we currently have two defacto formats: One to describe devices, and another for debug tagets.

@roblourens
Copy link
Member

@felixfbecker I was thinking that the debug adapter was calling that action from its process, but actually that doesn't make sense, I guess I misunderstood. So yeah it would be nice to have support in the debug protocol for calling code that uses the vscode extension APIs.

@weinand
Copy link
Contributor

weinand commented Jun 1, 2016

Some answers/clarification:

As @MSLaguana correctly stated: a debug adapter is a separate black-box process that has no access to VS Code APIs (and has no UI on its own). Any debugger UI that uses the VS Code debugger protocol can use debug adapters. Opening up the debug adapter to all VS Code APIs would blur this clear separation of concerns and would make it hard to use debug adapters in a different context than VS Code. In addition if the debug adapter would be able to create its own UI, that would most likely clash with the generic VS Code debugger UI.
So @roblourens and @felixfbecker: a debug adapter can not call those actions from its own code.

@daviwil the ${action.xxxxx} syntax follows the already existing environment variable syntax ${env.PATH} and currently it can be used everywhere in a launch.json (and probably soon in tasks.json as well). In our current implementation action variables are contributed in the variables section of the debugger contribution:

"variables": {
   "PickProcess": "extension.pickNodeProcess"
}

@auchenberg every debug extension is free to implement whatever picker or other UI they need (as long as it uses the VS Code API). The ProcessPicker implemented for node.js is just an example or a starting point; it is not a shared component that should be used in other debug extensions.
If the Chrome Debugger extension wants to use the QuickPick UI but likes to use the attributes of a QP item in different ways, you can do so in your own picker implementation. If you need a richer UI experience for the QuickPick UI please create feature requests.

@auchenberg as mentioned earlier it will not be possible to directly call UI from the debug adapter. But it is possible to call UI from the debug extension. So if you want to present your pick UI independent from a launch config and its action variables, you can do so in the VS Code May release:
we added API for launching a debug session programatically. So in the debug extension you can create a launch config 'in memory' and start a session with it. For creating the launch config you could use whatever UI is available in the VS Code extension API.

@rkeithhill
Copy link

rkeithhill commented Jan 16, 2017

@weinand When the quick pick UI appears and the user presses Esc to cancel the attach, our debug adapter is still getting passed the attach request. How can I prevent that in the code that returns the result of the quick pick UI?

FWIW I also notice that the NET Core debugger doesn't handle this situation very well. The debug adapter gets called in this scenario and then errors with:
image
We do this now in the PowerShell debugger but our message is "Aborted debugger attach request" but I'd rather not have to return any error msg at all to abort the attach. cc @daviwil

@isidorn
Copy link
Contributor

isidorn commented Jan 16, 2017

@rkeithhill this sounds like a new issue and potentially a regression. Please try reproducing it with vscode insiders and if you still see it please file a new issue and ping me on that issue. Thanks!

@weinand
Copy link
Contributor

weinand commented Jan 16, 2017

I've created #18600 and #18601 for this issue.

@rkeithhill
Copy link

@weinand Thanks.

@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