Evaluation errors now show toast notification while debugging

[DanTup]

When an evaluation fails, we now get the VS Code toast in the corner:

This can be quite annoying if you have things in your watch window. I wonder if it's related to https://dart-review.googlesource.com/c/sdk/+/339741 ?

[DanTup]

Happens on latest, but not in stable 3.2.

Removing showUser: true prevents it, so it does seem related to https://dart-review.googlesource.com/c/sdk/+/339741.

(@helin24 @chingjun I'm looking into this, but FYI..)

[DanTup]

Looks like there may be two uses of showUser in VS Code, the one linked in the CL above:

https://github.com/microsoft/vscode/blob/314c9891c3b676cf476a3f95b01492c1931b4ca7/src/vs/workbench/contrib/debug/browser/debugService.ts#L635

But also another one here, where the field not being specified does not default to true:

https://github.com/microsoft/vscode/blob/314c9891c3b676cf476a3f95b01492c1931b4ca7/src/vs/workbench/contrib/debug/browser/rawDebugSession.ts#L805

I think this error is happening because of the second one, so setting showError: true I think is not a no-op/compatible change. Based on the surrounding code and discussion in microsoft/vscode#128484 I think the top code is only related to launching of debug sessions and not general errors from responses.

I think we should revert this change in DAP. @chingjun do you have more details on what the other editor/tool was that motivated this change? My interpretation now is that "showError" means to show the user the error as if it's something they care about generally (eg. the debug session is broken) whereas most of our errors should only be shown in the context where the request was sent (eg. if an evaluation/watch fails, show the error inline, but don't show an additional popup/UI for the error).

(fyi @bkonyi @christopherfujino - we might get reports about this but I don't know that it's severe enough to cherry-pick)

[DanTup]

A related issue about some of this confusion at microsoft/vscode#180488 too

[chingjun]

In the other tool, the DAP messages are parsed by go-dap, which defaults the field to false, and causing the messages to be silently dropped. Bug was filed in google/go-dap#87. The messages don't have a showUser field will be treated as showUser: false. Unfortunately this will be a breaking change for go-dap and can't be updated quickly even if they agreed to the change.

This was problematic when it failed to launch a debugging session, the error was silently dropped without any (obvious) visual indicator to the user.

Since the tools are being inconsistent, what do you think if we have two different subclass for DebugAdapterException, and always explicitly set showUser: true in one case and showUser: false in another? If that could solve the issue, it would make DAP to behave consistently in vscode and the internal tool, what do you think?

[DanTup]

It would be nice to try and get the tools to align (to be honest, right now I don't know what values we want.. in some cases I suspect we might want showUser: true for VS Code too, but for things like evaluate requests where the error is shown inline, we do not).

I suspect in the short term we won't have any answers (and any changes to either tool will be even further out) so I think having another class (or a flag on the adapter that can be set) probably makes sense. I'm not sure which works best because I'm not certain how the adapter is created internally.

[DanTup]

As a quick fix to stop this, I'm going to force showUser back to undefined in some DAP middleware in Dart-Code. Changing in the SDK will have a long route to release and it's still not entirely clear what the right behaviour is (so handling VS Code's behaviour in VS Code seems reasonable).