debug: don't show "Unable to eval expression" error message already under WATCH

[ramya-rao-a]

From microsoft/vscode-go#3227 by @kzhui125

In other extensions such as python, if you eval an expression which produces error, the error is only shown in "WATCH".

But in VSCode go extension, the message box is shown, which is not needed.

And the message box pop up again and again, which has very bad experience.

There are cases when you put a variable in watch, but not available in current debug point.

[ramya-rao-a]

I don't see how we can communicate the error to VS Code if we don't send the error response as shown in https://github.com/microsoft/vscode-go/blob/12fd44a26788fb285207a3482dbb21d486b9d8b9/src/debugAdapter/goDebug.ts#L1332

If we remove that, then the watch pane would simply say "not available".
But another place to evaluate symbols is the debug console which would not say anything if we don't send an error response.

[ramya-rao-a]

From microsoft/vscode-go#3227 (comment) by @hyangah:

My proposal is not to drop the error response, but to find another way (is it possible to formulate it as a regular response, but with some indication on error?). Does anyone know how python extension handles this?

[polinasok]

My first thought was the same as @hyangah's: instead of sending ErrorResponse, send back successful EvaluateResponse, setting the result of the expression to the error message. Quick test of this alternative code snippet (which could be used in case of args.context=='watch')

response.body = {result: err.toString(), variablesReference: 0};
this.sendResponse(response);

yielded expected results: the error appeared in the Watch section without the pop-up.

Request:

{"command":"evaluate","arguments":{"expression":"bbbb==1","frameId":1000,"context":"watch"},"type":"request","seq":9}

Response before:

{"seq":0,"type":"response","request_seq":9,"command":"evaluate","success":false,"message":"Unable to eval expression: \"{e}\"","body":{"error":{"id":2009,"format":"Unable to eval expression: \"{e}\"","variables":{"e":"could not find symbol value for bbbb"},"showUser":true}}}

Response after:

{"seq":0,"type":"response","request_seq":9,"command":"evaluate","success":true,"body":{"result":"could not find symbol value for bbbb","variablesReference":0}}

But this seems like the wrong way to go, given the spec.

[polinasok]

Now lets look into what vscode-python does, given that it has the desired behavior without the pop-up. Its debug adaptor appears to forward things over to PTVSD, which also uses DAP. And PTVSD sends back neither a successful EvaluateResponse nor a full-fledged ErrorResponse, but something in-between - an EvaluateResponse with the body set to EvaluteResponseBody and not ErrorResponseBody while success and message fields are set as they would be in an ErrorResponse.

Turning on logging with "logToFile": true in launch.json, I get logging from the newly released debugpy, which is consistent with the same approach:

Server[1] --> {
                 "pydevd_cmd_id": 502, 
                 "seq": 40, 
                 "type": "response", 
                 "request_seq": 15, 
                 "success": false, 
                 "command": "evaluate", 
                 "message": "NameError: name 'abcdef' is not defined", 
                 "body": {
                     "variablesReference": 0, 
                     "presentationHint": {}, 
                     "result": "NameError: name 'abcdef' is not defined"
                 }
             }

This is then forwarded up the adaptor chain, stripping the EvaluateResponseBody and turning this into a proper ErrorResponse (since success==false):

Client <-- Adapter:
{
    "seq": 19,
    "type": "response",
    "request_seq": 11,
    "success": false,
    "command": "evaluate",
    "message": "NameError: name 'abcdef' is not defined"
}

But the big difference with Go is that the optional error Message in the body is not set at all.
Upon further inspection, I noticed that the body has a field showUser, which is set to true in the vscode-go response. I think that's the key here and we just need to set it to false in case of 'watch' context.

I will put together a quick PR tomorrow.