Giter VIP home page Giter VIP logo

Comments (33)

polinasok avatar polinasok commented on May 18, 2024 1

Overview

This overview is being updated as new options and details surface in the comments below.

Before

VSCODE <=dap=> FULL ADAPTER <=json-rpc=> DELVE
  • The current adaptor acts as an intermediary between VS code and Delve debugger.
  • It is registered as an adapter in package.json debuggers under type "go".
  • It is launched by the extension as a separate Node.js process at the beginning of each debug session of that debug type, using DebugAdapterExecutable.
  • It communicates with the editor using DAP over stdin/stdout and with Delve over JSON-RPC.
  • It handles initialize and shutdown sequences for the debug session and translates and routes everything in-between to delve.
  • For remote debugging, it now provides local-to-remote path mapping using delve's RPCServer methods (#45)
In response to `launch` or `attach` request, the DA launches `dlv --headless` command (that builds, executes or attaches to the debugee) or connects to an already running delve server.
  • request=“launch” mode=“auto
  • request=“launch” mode=“debug
    • dlv debug <program> --headless=true --listen=<host>:<port> --api-version=<apiVersion>
  • request=“launch” mode=“test"
    • dlv test <program> --headless=true --listen=<host>:<port> --api-version=<apiVersion>
  • request=“launch” mode=“exec
    • dlv exec <program> --headless=true --listen=<host>:<port> --api-version=<apiVersion>
  • request=“launch” mode=“remote
    • warning Request type of ‘launch’ with mode ‘remote’ is deprecated, please use request type ‘attach’ with mode ‘remote’ instead
    • connects to delve server already running at specified <host>:<port> (or default)
  • request=“attach” mode=“local
    • dlv attach <processId> --headless=true --listen=127.0.0.1:42309 --api-version=<apiVersion> --wd=<cwd>
  • request=“attach” mode=“remote
    • connects to delve server already running at specified <host>:<port>

After (Option 1)

VSCODE <=dap=> DELVE
  • Delve with DAP will act as an adaptor and debugger in one without the middleman.
  • It will be registered in package.json under a new debug type, so we can support both adaptors during the transition period.
  • It will be launched by the extension as a separate server process with dlv dap command at the beginning of each debug session of the new debug type, using DebugAdapterServer. (To be revisited if dlv-dap starts supporting more than one debug session, so we can reuse a single server instance.) This can be done similarly to how the dlv command is spawned in the existing adaptor. The server will run at port specified by debugServer, defined dynamically within the extension as part of DebugConfigurationProvider implementation. The user-specified option in launch.json can still be used for debugging.
  • It will communicate with the editor using DAP over TCP connection.
  • It will handle the initialize and shutdown sequences for the debug session and everything in-between.
  • Open Question: Are we guaranteed to always receive absolute file path from IDE via request args or does delve also need to keep track of the working directory?
  • Open Question: For remote debugging, can local-to-remote path mapping be added on the delve or extension side while local source tree is passed via attach arguments?
  • Open Question: Lack of adaptor could be an issue if we need to add any vscode-specific logic that does not belong in delve. Can it be shifted to the extension code instead?
In response to `launch` or `attach` request, delve-adapter will build, test, execute or attach to the debugee.
  • request=“launch” mode=“debug
    • run go debug, launch process
  • request=“launch” mode=“test"
    • run go test, launch process
  • request=“launch” mode=“exec
    • launch process
  • request=“launch” mode=“remote
    • will not be supported
  • request=“attach” mode=“local
    • attach to process
  • request=“attach” mode=“remote
    • already attached to the debugger server as part of adaptor launch

After (Option 2)

VSCODE <=dap=> LEAN ADAPTER <=dap=> DELVE
  • A lean TypeScript adaptor will exist between VS Code and Delve-on-DAP.
  • It will be registered in package.json under a new debug type, so we can support both adaptors during the transition period.
  • It will be launched by the extension as a separate Node.js process at the beginning of each debug session of that debug type, using DebugAdapterExecutable.
  • It will communicate with the editor using DAP over stdin/stdout and with delve using DAP over TCP connection.
  • TBD Will this adapter route the initialize and shutdown sequences for the debug session and everything in-between to delve, while intercepting the messages to adjust file paths and other settings?
    • Open Question: will delve or the thin adaptor handle initialize request? (delve supports it anyway, so it can be used by itself)
    • Open Question: should dlv dap be launched/connected to on initialize or launch/attach?
    • Open Question: which requests should be intercepted and why?
    • Open Question: will the rest of the communication happen directly between editor and delve?
  • Open Question For remote debugging will the adapter support local-to-remote path mapping? What requests need to be added to DAP to support this?
  • The lean adaptor can host vscode-specific logic that does not belong in delve.

Open Question: will this option make inline mode possible?
Open Question: will this option make RunInTerminal support easier?

from vscode-go.

polinasok avatar polinasok commented on May 18, 2024

DAP-in-delve work is tracked under go-delve/delve#1515
It relies on a new library repo for DAP support in Go: https://github.com/google/go-dap
You can reference delve's dap/server.go::handleRequest() for an account of which DAP requests are already supported in delve and which ones are still TODO.

from vscode-go.

lggomez avatar lggomez commented on May 18, 2024

I would like to add that some of the current issues due to incompatibility between the current DAP and delve that should be solved (at least partially) are the following: #118 #119 #129 #130

Tacking these would help a lot to the goal of achieving a seamless debug experience

from vscode-go.

eliben avatar eliben commented on May 18, 2024

I have an experimental branch where a new debug adapter is added alongside the existing one, and registered under a different type - godlvdap. A PR showing the diff is here.

As @polinasok's message above details, it registers a new debugger with a new entry in packages.json for the debuggers contribution point. For the purposes of this demo, the new adapter is just a copy of the existing adapter with some extra logging thrown in so we could distinguish between them. There are a couple more minor changes required, like registering a configuration provider for the new debugger in goMain.ts.

The prototype works. When debugging Go code, we can select between the two different debug adaptors by adjusting the "type" field in the project's launch.json.

from vscode-go.

eliben avatar eliben commented on May 18, 2024

After

VSCODE <=dap=> DELVE

[...]

Another alternative is to retain a thin debug adapter even when Delve's DAP functionality is complete. This debug adapter will take care of launching Delve and then will just pass-through all DAP commands back and forth.

The obvious cost of this alternative is having to maintain an additional debug adapter. That said, this adapter is expected to be very minimal.

The potential benefits of this approach:

  • It will avoid pushing too much vscode-specific logic into Delve. As of now, the LaunchRequestArguments interface contains a lot of information that's outside the spec of the DAP protocol. It's specific to how vscode invokes debug adapters. If we talk to Delve directly, we'll have to encode all this logic in Delve itself, making it vscode-specific. If folks want to use Delve with other IDEs (which we already know is the case!) they will have to implement their own logic inside Delve or masquerade as vscode.
  • It uses a familiar debug adapter invocation path. For example, the existing debug adapter has some logic for finding Delve and prints out customized error messages if Delve is not found. It's not clear how easy this is when dlv is specified directly in package.json. There's a difference between an adapter whose code ships with the extension, so the extension can safely assume it will find goDebug.ts, and between dlv which doesn't ship with the extension. It's likely that dlv won't be installed on new users' machines, and if it is, it's likely that the version will be wrong. We need a graceful way to handle this and it's not 100% clear how to do this without an adapter.

from vscode-go.

hyangah avatar hyangah commented on May 18, 2024

The debug node client supports multiple ways of running a debug adapter.

https://github.com/microsoft/vscode-mock-debug/blob/master/src/extension.ts#L57

Can we utilize that and by default run the thin wrapper in inlined mode?

from vscode-go.

eliben avatar eliben commented on May 18, 2024

The debug node client supports multiple ways of running a debug adapter.

https://github.com/microsoft/vscode-mock-debug/blob/master/src/extension.ts#L57

Can we utilize that and by default run the thin wrapper in inlined mode?

Yes, though it's still not 100% clear to me what the tradeoff is. It seems like this allows us to debug the adapter and the extension in the same process, which is nice. This seems like a fairly new option in vscode, and I imagine the current Go debug adapter could also use this since it's written in TS/JS.

[adding more details]

Re-reading the documentation carefully again, when we use the DebugAdapterExecutable option (which is what happens when the path is specified in package.json), vscode expects to talk to the adapter via stdin/stdout, and not sockets. To talk to an adapter via sockets, we have to set debugServer. Therefore, if we want vscode to talk directly to Delve without an adapter in between, we'll have to either ask users to have debugServer in each
launch.json or we'll have to adapt Delve to talk DAP over stdin/stdout as well.

from vscode-go.

quoctruong avatar quoctruong commented on May 18, 2024

@eliben When you register the debug configuration provider, there is a method called resolveDebugConfiguration that you can use to intercept the configuration before sending it to the adapter. This is where you can set the debugServer on behalf of the user.

@polinasok I also agree with @eliben's suggestion that we should have a thin layer of DAP in VSCode unless we want to move some of the existing logic that we have like path inference into Delve.

from vscode-go.

polinasok avatar polinasok commented on May 18, 2024

@quoctruong I was just going to ask you to chime in on the option of the thin adaptor, which we discussed as potentially necessary because of your path inference work. Could you please add more details/links as to what logic will end up in the thin adaptor for this?

For inspiration, python has a DAP adaptor to launch PTVSD, handling the rest of communication directly over DAP. The comment says

 * Primary purpose of this class is to perform the handshake with VS Code and launch PTVSD process.
 * I.e. it communicate with VS Code before PTVSD gets into the picture, once PTVSD is launched, PTVSD will talk directly to VS Code.
 * We're re-using DebugSession so we don't have to handle request/response ourselves.

from vscode-go.

polinasok avatar polinasok commented on May 18, 2024

I updated the overview comment to include the 2nd option with the thin adapter, so we have both at a glance.

from vscode-go.

polinasok avatar polinasok commented on May 18, 2024

@eliben

It will avoid pushing too much vscode-specific logic into Delve.

Agreed that we want to avoid pushing anything vscode-specific into Delve.
I believe so far we have been successful in avoid this, but please do let me know if you think otherwise.

As of now, the LaunchRequestArguments interface contains a lot of information that's outside the spec of the DAP protocol. It's specific to how vscode invokes debug adapters.

The current debug adaptor also gets all these extra arguments and just ignores them. That configuration passes through all the layers, so I think the idea is to just capture all attributes in one place and to let each layer handle those that of interest to it.

Since the arguments are a free-form map and not a struct, we do not have any of those vscode-specific fields coded anywhere. Are you concerned that they are polluting the request that delve receives even if it never reads those fields? Or do you think we should avoid having delve handle launch requests all-together and go back to specifying the program on the delve command line?

If folks want to use Delve with other IDEs (which we already know is the case!) they will have to implement their own logic inside Delve or masquerade as vscode.

Do you know of a specific example where the current approach might cause issues? I have not looked into other IDEs very closely yet.

My general understanding is that launch.json-like configuration is of part of the contract, even if DAP spec does not specify it in detail. Some fields are editor-specific. Others are adapter-specific. So the adapter (and in our case delve) has to make it clear, which argument fields it relies on and vscode or any other IDE have to supply them in launch/attach requests. And how they source them from the user or internally (via launch.json or something else), will be up to each client.

the existing debug adapter has some logic for finding Delve and prints out customized error messages if Delve is not found. It's not clear how easy this is when dlv is specified directly in package.json

Your concern about the dlv installation is very valid. I believe by default you get a "no debug adaptor" message, which is not very useful because the user would not know that delve is the adaptor and delve is missing and needs to be installed. However, if debugServer option is used, it takes precedence over the executable in package.json, which is the very last fallback. So whatever extension code launches delve as an adapter and sets debugServer, can in theory do the same kind of error-checking and messaging that the current adapter does.

Therefore, if we want vscode to talk directly to Delve without an adapter in between, we'll have to either ask users to have debugServer in each launch.json.

I tried to highlight in my overview comment that debugServer will be set in the extension code (goDebugConfiguration.ts). Looks like it was not clear that that we will not need any input from the user via launch.json (unless they want to debug an external adapter). Reworded a bit. Please let me know if it is still unclear.

from vscode-go.

eliben avatar eliben commented on May 18, 2024

@polinasok

Thanks for updating the overview comment with all the options side by side; this is very helpful.

from vscode-go.

eliben avatar eliben commented on May 18, 2024

Regarding "thin adapter" vs. "no adapter", it's also worth pointing out that if we want to implement the RunInTerminal request, we may need to have an adapter. This came up in discussions with the Delve folks. The RunInTerminal reverse request is described in more detail in the Launching And Attaching section of the DAP overview.

This may help eventually resolve issues like #124

from vscode-go.

hyangah avatar hyangah commented on May 18, 2024

@eliben @polinasok Thanks for keeping the overview up to date. I don't think directly communicating with DAP is an option. As @quoctruong pointed out in offline discussion, we already have vscode specific stuff. In LSP we handled it using a middleware and that gives us great flexibility (accessing information available in the extension host, translating vscode specific file paths, or dealing with bugs in the language server, etc). For DAP, I guess the thin adapter is the only option that provides such flexibility.

I wonder if we can still aim at supporting both modes - to run it as an inlined implementation, and to run it as a separate process.

Thanks for the pointer to RunInTerminal. That will be a huge improvement.

from vscode-go.

polinasok avatar polinasok commented on May 18, 2024

More on RunInTerminal. It is a reverse request from the adapter that passes the debug command to the editor to start in the integrated or external terminal. It is used to allow for things like:

  1. reading the program’s stdin
  2. redirecting its stdout (which is otherwise mixed in with the messages from the client and the adaptor in the Debug Console)
  3. giving access to a native command-line debugger interface from the terminal

The following related issues have been filed against vscode-go: microsoft/vscode-go#843 (=#124), microsoft/vscode-go#219, microsoft/vscode-go#186, microsoft/vscode-go#2204.

However, because of delve's split frontend/backend architecture, this is not straightforward, which is likely why the current architecture does not yet support RunInTerminal in spite of having that extra layer that we intend to introduce with a lean adapter. The issues listed above were closed with "based not the current state of delve, this feature is not doable". The only open issue #124 says "debug my go project at outside console" (3) and "support standard input" (1), so requesting the same things as the closed issues.

The complication is that in delve's case, the frontend is a separate client from the server backend and is not launched when the backend connects to the vscode client. Perhaps, the --accept-multiclient feature can be adapted for this, but that would require reworking dlv dap, which currently assumes only one client at a time. Accessing stdin in delve is non-trivial as well (see go-delve/delve#65, go-delve/delve#1274).

from vscode-go.

eliben avatar eliben commented on May 18, 2024

I wonder if we can still aim at supporting both modes - to run it as an inlined implementation, and to run it as a separate process.

I tried taking the existing debug adapter and making it "inline mode", executing in-process with the rest of the extension. This turned out to be pretty simple - here's a commit from my branch where it was implemented. I can verify that I can debug the extension and the DA in the same vscode session right now.

As far as I could figure out from digging in vscode's source, even in inline mode the debugServer setting in launch.json will still be respected, so we can still run the DA in a separate process if we want to.

We should probably move this discussion to a separate issue, though, since it's orthogonal to DAP.

from vscode-go.

hyangah avatar hyangah commented on May 18, 2024

@eliben Agree that we discuss it in a separate issue #232

For the DAP layer implementation, can we start merging the prototype so more users can play with it?

BTW from @polinasok's comment above - it looks like the dap mode assumes only a single client. Isn't it problematic? I saw multiple users using the multiclient mode.

@quoctruong can you also take a look if @eliben's prototype and the current delve DAP meets the requirements for the cloud debugging use cases (remote, and attach)?

from vscode-go.

polinasok avatar polinasok commented on May 18, 2024

@hyangah @quoctruong
DAP-Delve is still work in progress and does not yet have the features to support remote debugging. I can work on that next if that helps, but neither local nor remote debugging will be fully supported until a number of other requests, applicable in both cases, are implemented.

As far as I know, vscode-go does not take advantage of delve's --accept-multiclient flag when starting delve. So must apply to the case when the debugger is started remotely. This requirement has not come up with our discussions with @quoctruong & his team before, but if this is critical please let us know.

from vscode-go.

polinasok avatar polinasok commented on May 18, 2024

If I understand correctly, the new path inference mapping (#45) uses delve's RPCServer methods (ListSources and ListPackagesBuildInfo). Equivalent requests are not available under DAP, are they? It appears that we would need to upgrade the DAP protocol to support this via an adaptor in vscode-go.

If delve supports the mapping, it could likely get the local side of the mapping via the launch/attach arguments. Those are the only implementation specific fields under DAP, so we are free to put whatever we want there without global changes to the protocol.

from vscode-go.

polinasok avatar polinasok commented on May 18, 2024

@hyangah Unlike LSP case, I do not think the debug adaptor has direct access to extension host information. Having direct access to that state could be quite powerful. But I think at least the current version gets everything from the DAP requests.

from vscode-go.

polinasok avatar polinasok commented on May 18, 2024

We must also be careful about handling noDebug case right. This logic does not belong in delve - the user should not need to install a debugger to skip debugging. This is currently done in the debug adapter, but one can also argue that an IDE could have language support for editing and running code without any debugging support, so this could potentially belong in the extension code. Do we know if there is precedent in other languages for this?

Related issue: #336

from vscode-go.

hyangah avatar hyangah commented on May 18, 2024

@polinasok Isn't vscode.DebugAdapterDescriptorFactory the place where we can switch between dlv for usual debug mode and handling of noDebug mode (passing go run or the output after go build)? I think significant part of logic currently in goDebug.ts should move to the factory.

I have no good idea about the automated path inferring yet - I wonder if there is a way to teach vscode or the debug adapter to send a partial file path instead of the full path. Trim the local GOROOT, GOPATH, GOMODCACHE parts.
And in case of ambiguous file names, ask users to provide explicit mappings and apply it by using config substitute-path or something equivalent, instead of trying hard to guess the path mapping by ourselves. @quoctruong

from vscode-go.

gopherbot avatar gopherbot commented on May 18, 2024

Change https://golang.org/cl/240417 mentions this issue: debug: implement new prototype thin DA for delve

from vscode-go.

gopherbot avatar gopherbot commented on May 18, 2024

Change https://golang.org/cl/241117 mentions this issue: debug: more robust wait for server to listen to socket

from vscode-go.

gopherbot avatar gopherbot commented on May 18, 2024

Change https://golang.org/cl/241659 mentions this issue: debug: implement noDebug launches in debugAdapter2

from vscode-go.

gopherbot avatar gopherbot commented on May 18, 2024

Change https://golang.org/cl/246777 mentions this issue: src/debugAdapter2: launch as an external process, and fix config

from vscode-go.

gopherbot avatar gopherbot commented on May 18, 2024

Change https://golang.org/cl/246959 mentions this issue: [release] src/debugAdapter2: launch as an external process, and fix config

from vscode-go.

gopherbot avatar gopherbot commented on May 18, 2024

Change https://golang.org/cl/248659 mentions this issue: src/goDebugConfiguration: combine envFile and env

from vscode-go.

hyangah avatar hyangah commented on May 18, 2024

Updates:


Based on our experiments, I want to propose some adjustments in the plan:

We want to eliminate the node.js-based thin adapter, and the extension will start a local dlv dap server for a debug session.
The dlv dap can work like Option 1 for simple launch requests, but can switch to Option 2 mode depending on the launch/attach request types.

Why?

  1. One less node.js program to maintain.
  2. Directly launching dlv dap allows a cleaner initialization handshake process, and works efficiently for the most common scenario (Case 1 below).
  3. Ability to switch to Option 2 allows us to implement "Reverse Requests" more naturally. Initially, we were thinking to implement the "Reverse Request" feature from the node.js thin adapter, but by implementing it inside dlv dap, the feature will be accessible by other editors easily, and help dlv dap be more feature complete.
  4. We cannot reuse most of the current debug adapter's code for automated path mapping as it is now because it heavily depends on Delve's JSON-RPC. So, lacking equivalent DAP protocol support, we have to reimplement it on the delve side anyway. Some of the tasks can be done more efficiently from dlv dap side. And this feature can be beneficial to other editors.
  5. We cannot think of vscode-specific features that require implementation inside the thin adapter yet. VS Code specific features the current DAP is implementing can be moved to resolveDebugConfiguration as @quoctruong commented. The current DAP predates most of the VSCode Debug APIs, that's why some of VS Code specific features leaked to the adapter. We already moved some to the extension side utilizing the resolveDebugConfiguration. If we ever need vscode-specific modification after the launch configuration is sent, that's a failure of the protocol spec. :-)

Disadvantages:

  1. Inlining the adapter is no longer an option.
  2. dlv dap has to support different modes of operations, and becomes more complex - but I hope switching to go makes it more accessible to a wider group of go developers.
  3. dlv dap need to be able to communicate over stdin/stdout in addition to the network socket for better user experience.

How?

  • Initialization

    • Extension will configure to launch a new dlv dap using DebugAdapterExecutable mode adaptor factory. VSCode and DelveDAP will communicate over stdin/stdout.
             VSCode   <=(stdin/out)=>  DelveDAP (main)
    
    • VSCode and DelveDAP complete the initial handshakes including the initialize request, and VSCode sends the launch request.
  • Case 1: Launch (local, debug/test/exec mode) without need for RunInTerminal

            VSCode   <=(stdin/out)=> DelveDAP === Debugee
    
    • If noDebug is false, DelveDAP will build (debug/test) and start Debugee for normal debug setup. If noDebug is true, DelveDAP will build (debug/test) and start the program (no ptracing). Debugee uses DelveDAP's stdin/stdout/stderr, so its output will appear in the standard DEBUG CONSOLE along with other Delve DAP's logging/error messages.
  • Case 2: Launch (local, debug/test mode) with need for RunInTerminal

           VSCode  <=(stdin/out)=> DelveDAP   <=(network socket)=> DelveDAP'  === Debugee 
    
    • Upon receiving a launch request, DelveDAP will issue a RunInTerminal request to the editor with the command to start DelveDAP' (e.g. -listen=localhost:<port> etc). VSCode will open an integrated or external terminal, start DelveDAP' from the terminal, and send a response.
    • DelveDAP' listens on the specified port, and the terminal's stdin/stdout/stderr will be accessible.
    • DelveDAP connects to DelveDAP' using the port, performs the handshake based on the information it learned from the prior handshake with VSCode. Then sends a modified launch request (to disable RunInTerminal).
    • DelveDAP' starts* the Debugee. Debugee will use the terminal's stdin/stdout/stderr.
    • DelveDAP's logging/error messages will print in the standard DEBUG CONSOLE.
    • DelveDAP' terminates upon receiving a Terminate/Disconnect request.
    • Question: Who is responsible for building the Debugee binary? DelveDAP or DelveDAP'?
    • Note: if DelveDAP fails to initiate a proper teardown process for some reasons (e.g. crash), it's possible that DelveDAP' is not cleaned up. In that case, the user can manually terminate it with Ctrl+C, etc)
  • Case 3: Attach, Local: DelveDAP will attach to the existing process, so Debugee will have its own stdin/stdout/stderr.

            VSCode   <=(stdin/out)=> DelveDAP ---- Debugee
    
    • Whether to terminate Debugee or not follows the DAP protocol.
  • Case 4: Attach, Remote: DelveDAP connects to the remote dlv dap process (DelveDAP'').

           VSCode <=(stdin/out)=> DelveDAP <=(network socket)=> DelveDAP'' === Debugee
    
    • Similar to case 2 except that DelveDAP does not launch DelveDAP''.
    • DelveDAP'' may have different capabilities. During the initialization, if the difference is detected, DelveDAP may issue the updated capabilities events back to VSCode, or we may give up with warnings if it is too complicated (version mismatch?)
    • Whether to terminate Debugee or not follows the DAP protocol.
    • Question: How will the Debugee and the DelveDAP'' start? In the current workflow, users have an option to launch the debugee when starting the headless dlv server using dlv exec/debug/test command. On the other hand, dlv dap does not launch the debugee process until it receives the Launch request.

Note: We also found --accept-multiclient use case. Many users favor the full-featured dlv CLI to the limited DEBUG CONSOLE that depends on DAP's evaluate requests. For those users, we can make DelveDAP launch DelveDAP' in multiclient mode, and launch dlv connect using RunInTerminal. One caveat is that dlv connect needs Delve's JSON-RPC interface but the delve dap server does not support it. So, we need a way to make a delve server speak in both interfaces.


Q. the existing debug adapter has some logic for finding Delve and prints out customized error messages if Delve is not found. It's not clear how easy this is when dlv is specified directly in package.json

In this new setup, VSCode will warn users properly if the dlv binary is not found. And, the extension will try to ensure the dlv binary exists and supply the right executable path through VSCode's DebugAdapterDescriptorFactory.

/cc @polinasok @eliben @quoctruong @suzmue @aarzilli @derekparker

from vscode-go.

polinasok avatar polinasok commented on May 18, 2024

@hyangah could you please comment as to why you think we need to have a DA connection over stdin/out? Since delve is designed to talk over a socket, that would require some major changes to delve and/or implementation of an extra layer.

Currently vscode offers us the following debug adapter options:

  1. an executable, communicating over stdin/out, launched and discarded with every debug session
  2. a server, communicating over a socket
    A. single-use: server is terminated and restarted with each debug session
    B. multi-use: client connection is terminated and restarted with each debug session
  3. inlined within the extension
  4. [NEW] a server, communicating over named pipe/domain socket

We cannot think of vscode-specific features that require implementation inside the thin adapter yet. VS Code specific features the current DAP is implementing can be moved to resolveDebugConfiguration

If we do not know of anything that must be in the thin (TS or Go) adapter, then I propose that we, at least initially, consider option 2 above and run dlv-dap as a server, communicating with the editor over a socket and handling the full sequence of the debug session requests without any intermediaries. We can use our version of DebugAdapterDescriptorFactory configuring the adapter as DebugAdapterServer that would create the server (unless we were already given host/port by the remote attach mode) and connect to it in createDebugAdapterDescriptor from the extension. As the need for another layer evolves (e.g. status quo dlv-dap is released and we work on RunInTerminal), we can always insert it as needed, can't we?

The current dlv-dap server terminates itself with every disconnect request, so we would need to redo this for every debug session. But as we upgrade dlv-dap with more features, we can let the server survive beyond a single client connection and accept more connections. This ties nicely into supporting --accept-multiclient that we need to support --continue for remote mode and to support simultaneous connections from multiple clients, e.g. to take advantage of alternative UIs (like @hyangah suggestion to use dlv connect if we upgrade the server to speak both APIs).

In the single-use server case, the server will terminate itself on disconnect. We should use random ports to avoid any conflicts (see discussion in microsoft/debug-adapter-protocol#126). In the multi-use server case, we should be able to terminate in dispose as shown in the mock example. And we could use the same mechanism to clean up any rogue single-use servers that did not properly clean themselves up or check that the previous server is gone when we launch a new one.

I have not prototyped this yet, so this is all theoretical. Plus my TypeScript familiarity is very superficial, so take my next suggestions with a grain of salt. I will revise them as I learn more. But for now I was thinking we could use the same spawn mechanism to launch the dlv-dap server process as we currently do in debugAdatper to start headless dlv, which allows us to register callbacks on close, error, etc. There is also the net module that gives us an asynchronous network wrapper. Seems like we need a way to detect and clean-up unresponsive servers. Are there additional lifetime management concerns here we need to think through?

Also, here is a pointer to a Java server set-up although in this case the server is both LSP and DAP server and starts up as soon as the project is open:
https://github.com/microsoft/vscode-java-debug/blob/9be9832e4775201f5c01618ebde4df692e000138/src/javaDebugAdapterDescriptorFactory.ts#L10-L34

from vscode-go.

polinasok avatar polinasok commented on May 18, 2024

I have confirmed with @weinand that to implement a single-use local version of:

VSCode <=(dap over socket)=> DelveDap

we can:

  • use DebugAdapterDescriptorFactory to start and connect to dlv-dap server (example) whenever a debug session is triggered
  • spawn dlv-dap process (example)
  • use random port and pass host+port to the vscode.DebugAdapterServer (api)
  • register the factory (example)

If we make dlv-dap multi-use (able to accept multiple sequential client connections, each launching/attach to new process), we can change the factory to create it only for the first debug session, then just keep reconnecting to it for every new debug session trigger, and then clean it up in dispose(), which is called when the extension is deactivated (example). See go-delve/delve#2329 for more details.

In the remote case, the user would launch dlv-dap on the command line on whatever machine of the process, then specify the following:

  • Connect to server and launch: host=... port=... request='launch' mode='debug'/'test'/'exec' program=...
  • Connect to server and attach: host=... port=... request='attach' mode='local' processId=...
    We can add these configurations to goDebugConfigurations.
    The host and the port can be obtained from vscode.DebugConfiguration before the debug adapter is triggered, but I need to figure out how to access it from the factory that will need to connect to this server. Request and mode will drive launch/attach requests as usual. We will need to be clear if the program path is a local or remote one and how it relates to any of the mappings specified by the user's launch config.

I also have an idea to add support for just "Connect to server", by adding an option to dlv-dap to specify processId or program on the command-line and then making the launch/attach request a no-op or a sanity check. See go-delve/delve#2328 for more details.

from vscode-go.

gopherbot avatar gopherbot commented on May 18, 2024

Change https://golang.org/cl/288954 mentions this issue: src/goDebugFactory.ts: run dlv-dap as a server and connect as server

from vscode-go.

hyangah avatar hyangah commented on May 18, 2024

dlv-dap is available from the stable extension for try.
https://github.com/golang/vscode-go/blob/master/docs/dlv-dap.md

Many details had changed this year. Since we passed the initial brainstorming and design stage, and most discussion happens outside this specific issue, I will close this.

There are still a few missing features and caveats, and we still have many tasks planned for polishing.
We will keep tracking of the remaining tasks for v0 in the project dashboard https://github.com/golang/vscode-go/projects/3

from vscode-go.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.