fix #96 emit stoppedEvent after sendingResponse

[tolauwae]

@chscholl found the same issue. Currently the new StoppedEvent('step', this.THREAD_ID) is sent before the nextRequestResponse. For some reason vs code seems to ignore events before it receives the response request. So by changing the order, the step is sent after the response.

It seems illogical to send a response before we starting stepping. But maybe this is the intended interface of DAP? Christophe would instead of reordering just send a stopped event in the nextRequest function, after the response is returned.

The mock example is not very helpful in this regard. We suspect that it only works due to specific interleaving of the code. The example sends the response after an async step, which will trigger the StoppedEvent after the response already sent.

https://github.com/microsoft/vscode-mock-debug/blob/edbc2672c95b7861df5fea2511591547b9c1d0cc/src/mockDebug.ts#L511-L513

I also noticed that there are two places that send stopped events for steps. Through the EventEmitter, and the old listener. https://github.com/TOPLLab/WARDuino-VSCode/blob/6cf4922de78f5498f1b19e73b338948c76c25b99/src/DebugSession/DebugSession.ts#L702-L704

Is suspect the listener has been replaced fully by the emitter and can be removed?

Conclusion: we need to figure out if this change in 1.79 is a bug or a bug fix by looking at the DAP, and find what the expected behaviour is in regards with StoppedEvents. But we can fix the immediate bug right now by sending it after the response.

[tolauwae]

Just had a look at the DAP. The response is only an acknowledgement and

The debug adapter first sends the response and then a stopped event (with reason step) after the step has completed.

So maybe the most logical code is actually how it is now. First send the response (so it is always send before the stopped event) and than step. But in that case it makes more sense in my head to make it async, so no await.

Now I don't know what the response acknowledges precisely. What happens if step fails?

[carllocos]

I was also wondering the same what happens if the step operation fails or even any other operation such as run, and so on. I assume that you have to express failure as part of the response. We reached the point where we need to clearly understand the DAP (which should have been our first step when developing the plugin). I propose we do that in the near future.

I was also considering removing the await but I was not sure whether the stopped event would always be emitted after the sending the response back. If I remember correctly the callback system of node runs callbacks until completion so then it should not be a problem to remove the await.

Regarding

Is suspect the listener has been replaced fully by the emitter and can be removed?

yes indeed. My plan was to also add another class for all the GUI changes instead of putting all that code within the debugsession.ts as it is now.

[tolauwae]

@carllocos I needed to revert. This broke the branches I am using for the demo. You can open a new PR, because I can't reopen this once since it has been merged.

[carllocos]

I'll fix the issue in PR https://github.com/TOPLLab/WARDuino-VSCode/pull/105