Skip to content

Conversation

@ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Jan 27, 2022

Fixes #61665.

  • Added a mechanism that allows us to show some exceptions (of ReturnAsErrorException type) to users.
  • Divided exceptions on script evaluation into 2 categories: thrown by user's error inside the script (compilation error) and all the other exceptions that should be hidden and are for our info only.
  • Removes IsErr property from result. Now only IsOk can be used to determine if Result has error or not.

@ilonatommy ilonatommy added arch-wasm WebAssembly architecture area-Debugger-mono labels Jan 27, 2022
@ilonatommy ilonatommy self-assigned this Jan 27, 2022
@ghost
Copy link

ghost commented Jan 27, 2022

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details
  • Added a mechanism that allows us to show some exceptions (of ReturnAsErrorException type) to users.
  • Divided exceptions on script evaluation into 2 categories: thrown by user's error inside the script (compilation error) and all the other exceptions that should be hidden and are for our info only.
Author: ilonatommy
Assignees: ilonatommy
Labels:

arch-wasm, area-Debugger-mono

Milestone: -

Copy link
Member

@lewing lewing left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of issues. I think we might want to see if we can clean up the error result handling a bit too.

@ilonatommy ilonatommy requested a review from lewing February 3, 2022 14:09
Co-authored-by: Larry Ewing <lewing@microsoft.com>
@radical
Copy link
Member

radical commented Feb 8, 2022

Can you share how it looks different in practice?

  1. how do the errors show up now, compared to earlier? For the user, and in the log
  2. How does the stack trace show up for the user?

@ilonatommy
Copy link
Member Author

ilonatommy commented Feb 9, 2022

Can you share how it looks different in practice?

  1. how do the errors show up now, compared to earlier?
    a) For the user:

Sure. So before there was no exception information shown, here we break and evaluate non-existing-variable variable that does not exist and we got nothing after hovering over the text.
image

How it looks like now - evaluation of non existing variable + hovering over results in showing a window with exception details:
image

This PR has no effect on user defined exceptions.

b) and in the log:

all exceptions that are shown for the user are not logged anymore, so if we evaluate non exisiting variable on main we get

[11:08:51] fail: Microsoft.WebAssembly.Diagnostics.DevToolsProxy[0] sending error response for id: msg-:::105 -> [Result: IsOk: False, IsErr: True, Value: , Error: {   "result": {     "type": "object",     "subtype": "error",     "description": "The name non does not exist in the current context",     "className": "ReferenceError"   } } ]

but on this PR we don't get any log entry.

  1. How does the stack trace show up for the user?

No change as well,
image

Internally thrown errors, e.g. evaluation of currentCount ~= 0 on the breakpoint from screenshot above is not shown to the user as it is recognized as AssignmentExpressionSyntax and throws Exception("Assignment not implemented") and logged as:

[11:40:43] fail: Microsoft.WebAssembly.Diagnostics.DevToolsProxy[0] sending error response for id: msg-:::123 -> [Result: IsOk: False, IsErr: True, Value: , Error: {   "message": "Assignment is not implemented yet" } ]

However, when we evaluate currentCount ~^ 0 that is not treated as assignment and the debugger is trying to compile it we are getting a detailed info why the compilation failed:
image
with nothing new in the log. Before we only got not available info that did not tell much to the user.

public bool IsErr => Error != null;

private Result(JObject result, JObject error)
private Result(JObject resultOrError, bool isError, JObject error = null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't look right. We now have a resultOrError, and error, but also isError. Why do we need all 3?

Copy link
Member Author

@ilonatommy ilonatommy Feb 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I left it only because of the FromJson function:

        public static Result FromJson(JObject obj)
        {
            //Log ("protocol", $"from result: {obj}");
            return new Result(obj["result"] as JObject, false, obj["error"] as JObject);
        }

that I did not want to change but now I did it and the version with 2 arguments works equally good. Thanks!

@ilonatommy ilonatommy merged commit aa4b8ee into dotnet:main Feb 17, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Mar 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasm WebAssembly architecture area-Debugger-mono

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[wasm][debugger] Handle exceptions while evaluating expressions

3 participants