vscode: Clarify when a DAP should respond to requests like terminateRequest... immediately, or when the request has been handled?
I’ve always assumed that if the DAP client issues a request like terminate, that the DAP server should try to perform the termination before responding to this request (since this allows it to communicate a failure, and if the response was just always sent immediately before any work is done, it seems like it’s essentially an event and doesn’t need request/response).
However, I’ve noticed that if I don’t respond to a terminate request within 2s, the VS Code client shows an error message that the termination timed out (even though it didn’t, and the DAP server is still working on it). This happens a lot in test runs, because the Dart test runner will by default wait to allow the current test to complete when first trying to stop (to provide more consistency for teardowns, not having to have them deal with tests that were aborted mid-running).
I can change how this works to respond immediately, but I can’t find anything in the docs that describe that this is how servers should behave, and it seems like something that should be (since if clients and servers interpret this differently - as has happened here - users may see confusing timeout messages).
About this issue
- Original URL
- State: closed
- Created a year ago
- Comments: 18 (12 by maintainers)
Commits related to this issue
- Remove timeout on 'terminate' request See #172284 — committed to microsoft/vscode by roblourens a year ago
- Remove timeout on 'terminate' request (#178714) See #172284 — committed to microsoft/vscode by roblourens a year ago
I opened https://github.com/microsoft/vscode/issues/178735 for the UI, and I will close this for removing the timeout.
If the button is disabled then I don’t think waiting a little longer is too bad.
I don’t know how useful configuring the delay before showing progress is though, I don’t think it taking more or less time changes that it’s useful to show the user that the termination is in progress. In our old debug adapter, we showed a progress immediately when you clicked the button and if termination was fast, it’d just disappear quickly.
Sorry, have been distracted by other tasks. I think we just want to remove the timeout on terminate for now, right? Pushed a PR for that. I think the scenario could use some UX thinking though. Seems that there should be some kind of feedback that something is happening when you wait- a form of the stop button that shows progress, disabling the stop button for a time, a progress notification that pops up after a few seconds?
@roblourens gentle ping on this - we want to contribute to this fix, but want to make sure you are open in principle to such a fix before starting the work.
I think we agree, I’ll get rid of the timeout on terminate. But if the user keeps clicking the shutdown button, and we force a shutdown, then orphaned processes are a risk.