[wasm] Improved error handling in debug proxy (#65828)
authorAnkit Jain <radical@gmail.com>
Mon, 28 Mar 2022 22:49:05 +0000 (18:49 -0400)
committerGitHub <noreply@github.com>
Mon, 28 Mar 2022 22:49:05 +0000 (18:49 -0400)
* [wasm] Mark result as Error if it has 'exceptionDetails' field

* [wasm] Don't swallow errors from methods that enumerate properties

.. like for getting locals. Instead, surface the original errors which
have details of the actual failure, to the point before we send a
response.

This helps debugging a lot, so instead of `Unable to evaluate dotnet:scope:1`,
we get more detailed errors.

* [wasm] Throw an exception when a debugger agent command fails

We are checking `HasError` property on the binary reader in very few
places. Other places just try to use the reader which then fails with
errors in reading, or base64 conversion etc, and we don't get any info
about what command failed.

Instead, throw an exception on error by default. But the existing
locations that do check `HasError`, don't throw for them.

* [wasm] Fix evaluating expression calling a non-existant method

Issue: https://github.com/dotnet/runtime/issues/65744

The test doesn't fail because it is expecting an error, and it gets
that.

But the log shows an assertion:

`console.error: * Assertion at /workspaces/runtime/src/mono/mono/metadata/class-accessors.c:71, condition `<disabled>' not met`

1. This is because the debugger-agent does not check that the `klass`
argument is NULL, which is fixed by adding that check.

2. But the reason why this got called with `klass==NULL`, is because
`MemberReferenceResolver.Resolve` tries first to find the non-existant
method on the type itself. Then it tries to find the method on
`System.Linq.Enumerable`, but fails to find a typeid for that.
  - but continues to send a command to get the methods on that
  `linqTypeId==0`.

* [wasm] Add some missing exception logging in the proxy

* cleaup

* [wasm] GetMethodIdByName: throw on invalid type ids

* [wasm] Improve error handling in expression evaluation

* Cleanup

* Disable failing test - https://github.com/dotnet/runtime/issues/65881

* Add missed files

* Address @ilonatommy's feedback

13 files changed:
src/mono/mono/component/debugger-agent.c
src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs
src/mono/wasm/debugger/BrowserDebugProxy/DebuggerAgentException.cs [new file with mode: 0644]
src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs
src/mono/wasm/debugger/BrowserDebugProxy/ExpressionEvaluationFailedException.cs [new file with mode: 0644]
src/mono/wasm/debugger/BrowserDebugProxy/InternalErrorException.cs [new file with mode: 0644]
src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs
src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
src/mono/wasm/debugger/BrowserDebugProxy/ValueOrError.cs [new file with mode: 0644]
src/mono/wasm/debugger/DebuggerTestSuite/AssignmentTests.cs
src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs
src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs

index a57c7cb..05ebfe0 100644 (file)
@@ -8229,6 +8229,9 @@ type_commands_internal (int command, MonoClass *klass, MonoDomain *domain, guint
                ERROR_DECL (error);
                GPtrArray *array;
 
+               if (!klass)
+                       goto invalid_argument;
+
                error_init (error);
                array = mono_class_get_methods_by_name (klass, name, flags & ~BINDING_FLAGS_IGNORE_CASE, mlisttype, TRUE, error);
                if (!is_ok (error)) {
index a283bcc..2ee855d 100644 (file)
@@ -1229,7 +1229,7 @@ namespace Microsoft.WebAssembly.Diagnostics
             }
             catch (Exception e)
             {
-                logger.LogDebug($"Failed to load assembly: ({e.Message})");
+                logger.LogError($"Failed to load assembly: ({e.Message})");
                 yield break;
             }
 
@@ -1292,7 +1292,7 @@ namespace Microsoft.WebAssembly.Diagnostics
                 }
                 catch (Exception e)
                 {
-                    logger.LogDebug($"Failed to load {step.Url} ({e.Message})");
+                    logger.LogError($"Failed to load {step.Url} ({e.Message})");
                 }
                 if (assembly == null)
                     continue;
diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/DebuggerAgentException.cs b/src/mono/wasm/debugger/BrowserDebugProxy/DebuggerAgentException.cs
new file mode 100644 (file)
index 0000000..b4ca2bd
--- /dev/null
@@ -0,0 +1,19 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#nullable enable
+
+using System;
+
+namespace Microsoft.WebAssembly.Diagnostics;
+
+public class DebuggerAgentException : Exception
+{
+    public DebuggerAgentException(string message) : base(message)
+    {
+    }
+
+    public DebuggerAgentException(string? message, Exception? innerException) : base(message, innerException)
+    {
+    }
+}
index a348a42..f28b972 100644 (file)
@@ -129,7 +129,11 @@ namespace Microsoft.WebAssembly.Diagnostics
 
         private Result(JObject resultOrError, bool isError)
         {
-            bool resultHasError = isError || string.Equals((resultOrError?["result"] as JObject)?["subtype"]?.Value<string>(), "error");
+            if (resultOrError == null)
+                throw new ArgumentNullException(nameof(resultOrError));
+
+            bool resultHasError = isError || string.Equals((resultOrError["result"] as JObject)?["subtype"]?.Value<string>(), "error");
+            resultHasError |= resultOrError["exceptionDetails"] != null;
             if (resultHasError)
             {
                 Value = null;
@@ -146,7 +150,7 @@ namespace Microsoft.WebAssembly.Diagnostics
             var error = obj["error"] as JObject;
             if (error != null)
                 return new Result(error, true);
-            var result = obj["result"] as JObject;
+            var result = (obj["result"] as JObject) ?? new JObject();
             return new Result(result, false);
         }
 
diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/ExpressionEvaluationFailedException.cs b/src/mono/wasm/debugger/BrowserDebugProxy/ExpressionEvaluationFailedException.cs
new file mode 100644 (file)
index 0000000..8805198
--- /dev/null
@@ -0,0 +1,19 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#nullable enable
+
+using System;
+
+namespace Microsoft.WebAssembly.Diagnostics;
+
+public class ExpressionEvaluationFailedException : Exception
+{
+    public ExpressionEvaluationFailedException(string message) : base(message)
+    {
+    }
+
+    public ExpressionEvaluationFailedException(string? message, Exception? innerException) : base(message, innerException)
+    {
+    }
+}
diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/InternalErrorException.cs b/src/mono/wasm/debugger/BrowserDebugProxy/InternalErrorException.cs
new file mode 100644 (file)
index 0000000..74c2165
--- /dev/null
@@ -0,0 +1,19 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#nullable enable
+
+using System;
+
+namespace Microsoft.WebAssembly.Diagnostics;
+
+public class InternalErrorException : Exception
+{
+    public InternalErrorException(string message) : base($"Internal error: {message}")
+    {
+    }
+
+    public InternalErrorException(string message, Exception? innerException) : base($"Internal error: {message}", innerException)
+    {
+    }
+}
index ef744be..b9f5c8e 100644 (file)
@@ -199,7 +199,7 @@ namespace Microsoft.WebAssembly.Diagnostics
                 (retObject, string remaining) = await ResolveStaticMembersInStaticTypes(varName, token);
                 if (!string.IsNullOrEmpty(remaining))
                 {
-                    if (retObject?["subtype"]?.Value<string>() == "null")
+                    if (retObject.IsNullValuedObject())
                     {
                         // NRE on null.$remaining
                         retObject = null;
@@ -221,7 +221,7 @@ namespace Microsoft.WebAssembly.Diagnostics
                 {
                     Result scope_res = await proxy.GetScopeProperties(sessionId, scopeId, token);
                     if (!scope_res.IsOk)
-                        throw new Exception($"BUG: Unable to get properties for scope: {scopeId}. {scope_res}");
+                        throw new ExpressionEvaluationFailedException($"BUG: Unable to get properties for scope: {scopeId}. {scope_res}");
                     localsFetched = true;
                 }
 
@@ -234,8 +234,14 @@ namespace Microsoft.WebAssembly.Diagnostics
                 if (!DotnetObjectId.TryParse(objThis?["value"]?["objectId"]?.Value<string>(), out DotnetObjectId objectId))
                     return null;
 
-                var rootResObj = await proxy.RuntimeGetPropertiesInternal(sessionId, objectId, null, token);
-                var objRet = rootResObj.FirstOrDefault(objPropAttr => objPropAttr["name"].Value<string>() == nameTrimmed);
+                ValueOrError<JToken> valueOrError = await proxy.RuntimeGetPropertiesInternal(sessionId, objectId, null, token);
+                if (valueOrError.IsError)
+                {
+                    logger.LogDebug($"ResolveAsLocalOrThisMember failed with : {valueOrError.Error}");
+                    return null;
+                }
+
+                JToken objRet = valueOrError.Value.FirstOrDefault(objPropAttr => objPropAttr["name"].Value<string>() == nameTrimmed);
                 if (objRet != null)
                     return await GetValueFromObject(objRet, token);
 
@@ -255,8 +261,14 @@ namespace Microsoft.WebAssembly.Diagnostics
                     if (!DotnetObjectId.TryParse(resolvedObject?["objectId"]?.Value<string>(), out DotnetObjectId objectId))
                         return null;
 
-                    var resolvedResObj = await proxy.RuntimeGetPropertiesInternal(sessionId, objectId, null, token);
-                    var objRet = resolvedResObj.FirstOrDefault(objPropAttr => objPropAttr["name"]?.Value<string>() == partTrimmed);
+                    ValueOrError<JToken> valueOrError = await proxy.RuntimeGetPropertiesInternal(sessionId, objectId, null, token);
+                    if (valueOrError.IsError)
+                    {
+                        logger.LogDebug($"ResolveAsInstanceMember failed with : {valueOrError.Error}");
+                        return null;
+                    }
+
+                    JToken objRet = valueOrError.Value.FirstOrDefault(objPropAttr => objPropAttr["name"]?.Value<string>() == partTrimmed);
                     if (objRet == null)
                         return null;
 
@@ -264,7 +276,7 @@ namespace Microsoft.WebAssembly.Diagnostics
                     if (resolvedObject == null)
                         return null;
 
-                    if (resolvedObject["subtype"]?.Value<string>() == "null")
+                    if (resolvedObject.IsNullValuedObject())
                     {
                         if (i < parts.Length - 1)
                         {
@@ -360,9 +372,9 @@ namespace Microsoft.WebAssembly.Diagnostics
                 }
                 return null;
             }
-            catch (Exception)
+            catch (Exception ex) when (ex is not ExpressionEvaluationFailedException)
             {
-                throw new Exception($"Unable to evaluate method '{elementAccess}'");
+                throw new ExpressionEvaluationFailedException($"Unable to evaluate element access '{elementAccess}': {ex.Message}", ex);
             }
         }
 
@@ -379,41 +391,33 @@ namespace Microsoft.WebAssembly.Diagnostics
                     var memberAccessExpressionSyntax = expr as MemberAccessExpressionSyntax;
                     rootObject = await Resolve(memberAccessExpressionSyntax.Expression.ToString(), token);
                     methodName = memberAccessExpressionSyntax.Name.ToString();
+
+                    if (rootObject.IsNullValuedObject())
+                        throw new ExpressionEvaluationFailedException($"Expression '{memberAccessExpressionSyntax}' evaluated to null");
                 }
                 else if (expr is IdentifierNameSyntax)
-                    if (scopeCache.ObjectFields.TryGetValue("this", out JObject valueRet)) {
+                {
+                    if (scopeCache.ObjectFields.TryGetValue("this", out JObject valueRet))
+                    {
                         rootObject = await GetValueFromObject(valueRet, token);
-                    methodName = expr.ToString();
+                        methodName = expr.ToString();
+                    }
                 }
 
                 if (rootObject != null)
                 {
-                    DotnetObjectId.TryParse(rootObject?["objectId"]?.Value<string>(), out DotnetObjectId objectId);
+                    if (!DotnetObjectId.TryParse(rootObject?["objectId"]?.Value<string>(), out DotnetObjectId objectId))
+                        throw new ExpressionEvaluationFailedException($"Cannot invoke method '{methodName}' on invalid object id: {rootObject}");
+
                     var typeIds = await context.SdbAgent.GetTypeIdFromObject(objectId.Value, true, token);
                     int methodId = await context.SdbAgent.GetMethodIdByName(typeIds[0], methodName, token);
                     var className = await context.SdbAgent.GetTypeNameOriginal(typeIds[0], token);
                     if (methodId == 0) //try to search on System.Linq.Enumerable
-                    {
-                        if (linqTypeId == -1)
-                            linqTypeId = await context.SdbAgent.GetTypeByName("System.Linq.Enumerable", token);
-                        methodId = await context.SdbAgent.GetMethodIdByName(linqTypeId, methodName, token);
-                        if (methodId != 0)
-                        {
-                            foreach (var typeId in typeIds)
-                            {
-                                var genericTypeArgs = await context.SdbAgent.GetTypeParamsOrArgsForGenericType(typeId, token);
-                                if (genericTypeArgs.Count > 0)
-                                {
-                                    isExtensionMethod = true;
-                                    methodId = await context.SdbAgent.MakeGenericMethod(methodId, genericTypeArgs, token);
-                                    break;
-                                }
-                            }
-                        }
-                    }
+                        methodId = await FindMethodIdOnLinqEnumerable(typeIds, methodName);
+
                     if (methodId == 0) {
                         var typeName = await context.SdbAgent.GetTypeName(typeIds[0], token);
-                        throw new ReturnAsErrorException($"Method '{methodName}' not found in type '{typeName}'", "ArgumentError");
+                        throw new ExpressionEvaluationFailedException($"Method '{methodName}' not found in type '{typeName}'");
                     }
                     using var commandParamsObjWriter = new MonoBinaryWriter();
                     if (!isExtensionMethod)
@@ -427,20 +431,18 @@ namespace Microsoft.WebAssembly.Diagnostics
                         int passedArgsCnt = method.ArgumentList.Arguments.Count;
                         int methodParamsCnt = passedArgsCnt;
                         ParameterInfo[] methodParamsInfo = null;
-                        logger.LogInformation($"passed: {passedArgsCnt}, isExtensionMethod: {isExtensionMethod}");
                         var methodInfo = await context.SdbAgent.GetMethodInfo(methodId, token);
                         if (methodInfo != null) //FIXME: #65670
                         {
                             methodParamsInfo = methodInfo.Info.GetParametersInfo();
                             methodParamsCnt = methodParamsInfo.Length;
-                            logger.LogInformation($"got method info with {methodParamsCnt} params");
                             if (isExtensionMethod)
                             {
                                 // implicit *this* parameter
                                 methodParamsCnt--;
                             }
                             if (passedArgsCnt > methodParamsCnt)
-                                throw new ReturnAsErrorException($"Unable to evaluate method '{methodName}'. Too many arguments passed.", "ArgumentError");
+                                throw new ExpressionEvaluationFailedException($"Cannot invoke method '{method}' - too many arguments passed");
                         }
 
                         if (isExtensionMethod)
@@ -461,23 +463,23 @@ namespace Microsoft.WebAssembly.Diagnostics
                             if (arg.Expression is LiteralExpressionSyntax literal)
                             {
                                 if (!await commandParamsObjWriter.WriteConst(literal, context.SdbAgent, token))
-                                    throw new ReturnAsErrorException($"Unable to evaluate method '{methodName}'. Unable to write LiteralExpressionSyntax into binary writer.", "ArgumentError");
+                                    throw new InternalErrorException($"Unable to write LiteralExpressionSyntax ({literal}) into binary writer.");
                             }
                             else if (arg.Expression is IdentifierNameSyntax identifierName)
                             {
                                 if (!await commandParamsObjWriter.WriteJsonValue(memberAccessValues[identifierName.Identifier.Text], context.SdbAgent, token))
-                                    throw new ReturnAsErrorException($"Unable to evaluate method '{methodName}'. Unable to write IdentifierNameSyntax into binary writer.", "ArgumentError");
+                                    throw new InternalErrorException($"Unable to write IdentifierNameSyntax ({identifierName}) into binary writer.");
                             }
                             else
                             {
-                                throw new ReturnAsErrorException($"Unable to evaluate method '{methodName}'. Unable to write into binary writer, not recognized expression type: {arg.Expression.GetType().Name}", "ArgumentError");
+                                throw new InternalErrorException($"Unable to write into binary writer, not recognized expression type: {arg.Expression.GetType().Name}");
                             }
                         }
                         // optional arguments that were not overwritten
                         for (; argIndex < methodParamsCnt; argIndex++)
                         {
                             if (!await commandParamsObjWriter.WriteConst(methodParamsInfo[argIndex].TypeCode, methodParamsInfo[argIndex].Value, context.SdbAgent, token))
-                                throw new ReturnAsErrorException($"Unable to write optional parameter {methodParamsInfo[argIndex].Name} value in method '{methodName}' to the mono buffer.", "ArgumentError");
+                                throw new InternalErrorException($"Unable to write optional parameter {methodParamsInfo[argIndex].Name} value in method '{methodName}' to the mono buffer.");
                         }
                         var retMethod = await context.SdbAgent.InvokeMethod(commandParamsObjWriter.GetParameterBuffer(), methodId, "methodRet", token);
                         return await GetValueFromObject(retMethod, token);
@@ -485,9 +487,38 @@ namespace Microsoft.WebAssembly.Diagnostics
                 }
                 return null;
             }
-            catch (Exception ex) when (ex is not ReturnAsErrorException)
+            catch (Exception ex) when (ex is not (ExpressionEvaluationFailedException or ReturnAsErrorException))
             {
-                throw new Exception($"Unable to evaluate method '{methodName}'", ex);
+                throw new ExpressionEvaluationFailedException($"Unable to evaluate method '{method}': {ex.Message}", ex);
+            }
+
+            async Task<int> FindMethodIdOnLinqEnumerable(IList<int> typeIds, string methodName)
+            {
+                if (linqTypeId == -1)
+                {
+                    linqTypeId = await context.SdbAgent.GetTypeByName("System.Linq.Enumerable", token);
+                    if (linqTypeId == 0)
+                    {
+                        logger.LogDebug($"Cannot find type 'System.Linq.Enumerable'");
+                        return 0;
+                    }
+                }
+
+                int newMethodId = await context.SdbAgent.GetMethodIdByName(linqTypeId, methodName, token);
+                if (newMethodId == 0)
+                    return 0;
+
+                foreach (int typeId in typeIds)
+                {
+                    List<int> genericTypeArgs = await context.SdbAgent.GetTypeParamsOrArgsForGenericType(typeId, token);
+                    if (genericTypeArgs.Count > 0)
+                    {
+                        isExtensionMethod = true;
+                        return await context.SdbAgent.MakeGenericMethod(newMethodId, genericTypeArgs, token);
+                    }
+                }
+
+                return 0;
             }
         }
     }
index 4140202..f269430 100644 (file)
@@ -469,12 +469,16 @@ namespace Microsoft.WebAssembly.Diagnostics
                         if (!DotnetObjectId.TryParse(args?["objectId"], out DotnetObjectId objectId))
                             break;
 
-                        var ret = await RuntimeGetPropertiesInternal(id, objectId, args, token, true);
-                        if (ret == null) {
-                            SendResponse(id, Result.Err($"Unable to RuntimeGetProperties '{objectId}'"), token);
+                        var valueOrError = await RuntimeGetPropertiesInternal(id, objectId, args, token, true);
+                        if (valueOrError.IsError)
+                        {
+                            logger.LogDebug($"Runtime.getProperties: {valueOrError.Error}");
+                            SendResponse(id, valueOrError.Error.Value, token);
                         }
                         else
-                            SendResponse(id, Result.OkFromObject(ret), token);
+                        {
+                            SendResponse(id, Result.OkFromObject(valueOrError.Value), token);
+                        }
                         return true;
                     }
 
@@ -593,7 +597,8 @@ namespace Microsoft.WebAssembly.Diagnostics
                         try {
                             return await CallOnFunction(id, args, token);
                         }
-                        catch (Exception){
+                        catch (Exception ex) {
+                            logger.LogDebug($"Runtime.callFunctionOn failed for {id} with args {args}: {ex}");
                             SendResponse(id,
                                 Result.Exception(new ArgumentException(
                                     $"Runtime.callFunctionOn not supported with ({args["objectId"]}).")),
@@ -607,8 +612,9 @@ namespace Microsoft.WebAssembly.Diagnostics
                         {
                             SetJustMyCode(id, args, token);
                         }
-                        catch (Exception)
+                        catch (Exception ex)
                         {
+                            logger.LogDebug($"DotnetDebugger.justMyCode failed for {id} with args {args}: {ex}");
                             SendResponse(id,
                                 Result.Exception(new ArgumentException(
                                     $"DotnetDebugger.justMyCode got incorrect argument ({args})")),
@@ -722,7 +728,7 @@ namespace Microsoft.WebAssembly.Diagnostics
             return true;
         }
 
-        internal async Task<JToken> RuntimeGetPropertiesInternal(SessionId id, DotnetObjectId objectId, JToken args, CancellationToken token, bool sortByAccessLevel = false)
+        internal async Task<ValueOrError<JToken>> RuntimeGetPropertiesInternal(SessionId id, DotnetObjectId objectId, JToken args, CancellationToken token, bool sortByAccessLevel = false)
         {
             var context = GetContext(id);
             var accessorPropertiesOnly = false;
@@ -744,51 +750,57 @@ namespace Microsoft.WebAssembly.Diagnostics
                 {
                     case "scope":
                     {
-                        var resScope = await GetScopeProperties(id, objectId.Value, token);
-                        if (sortByAccessLevel)
-                            return resScope.Value;
-                        return resScope.Value?["result"];
+                        Result resScope = await GetScopeProperties(id, objectId.Value, token);
+                        return resScope.IsOk
+                            ? ValueOrError<JToken>.WithValue(sortByAccessLevel ? resScope.Value : resScope.Value?["result"])
+                            : ValueOrError<JToken>.WithError(resScope);
                     }
                     case "valuetype":
                     {
                         var resValType = await context.SdbAgent.GetValueTypeValues(objectId.Value, accessorPropertiesOnly, token);
-                        return sortByAccessLevel ? JObject.FromObject(new { result = resValType }) : resValType;
+                        return resValType switch
+                        {
+                            null => ValueOrError<JToken>.WithError($"Could not get properties for {objectId}"),
+                            _    => ValueOrError<JToken>.WithValue(sortByAccessLevel ? JObject.FromObject(new { result = resValType }) : resValType)
+                        };
                     }
                     case "array":
                     {
                         var resArr = await context.SdbAgent.GetArrayValues(objectId.Value, token);
-                        return sortByAccessLevel ? JObject.FromObject(new { result = resArr }) : resArr;
+                        return ValueOrError<JToken>.WithValue(sortByAccessLevel ? JObject.FromObject(new { result = resArr }) : resArr);
                     }
                     case "methodId":
                     {
                         var resMethod = await context.SdbAgent.InvokeMethodInObject(objectId.Value, objectId.SubValue, null, token);
-                        return sortByAccessLevel ? JObject.FromObject(new { result = new JArray(resMethod) }) : new JArray(resMethod);
+                        return ValueOrError<JToken>.WithValue(sortByAccessLevel ? JObject.FromObject(new { result = new JArray(resMethod) }) : new JArray(resMethod));
                     }
                     case "object":
                     {
-                        var resObj = (await context.SdbAgent.GetObjectValues(objectId.Value, objectValuesOpt, token, sortByAccessLevel));
-                        return sortByAccessLevel ? resObj[0] : resObj;
+                        var resObj = await context.SdbAgent.GetObjectValues(objectId.Value, objectValuesOpt, token, sortByAccessLevel);
+                        return ValueOrError<JToken>.WithValue(sortByAccessLevel ? resObj[0] : resObj);
                     }
                     case "pointer":
                     {
                         var resPointer = new JArray { await context.SdbAgent.GetPointerContent(objectId.Value, token) };
-                        return sortByAccessLevel ? JObject.FromObject(new { result = resPointer }) : resPointer;
+                        return ValueOrError<JToken>.WithValue(sortByAccessLevel ? JObject.FromObject(new { result = resPointer }) : resPointer);
                     }
                     case "cfo_res":
                     {
                         Result res = await SendMonoCommand(id, MonoCommands.GetDetails(RuntimeId, objectId.Value, args), token);
                         string value_json_str = res.Value["result"]?["value"]?["__value_as_json_string__"]?.Value<string>();
-                        return value_json_str != null ?
-                                (sortByAccessLevel ? JObject.FromObject(new { result = JArray.Parse(value_json_str) }) : JArray.Parse(value_json_str)) :
-                                null;
+                        if (res.IsOk && value_json_str == null)
+                            return ValueOrError<JToken>.WithError($"Internal error: Could not find expected __value_as_json_string__ field in the result: {res}");
+
+                        return value_json_str != null
+                                    ? ValueOrError<JToken>.WithValue(sortByAccessLevel ? JObject.FromObject(new { result = JArray.Parse(value_json_str) }) : JArray.Parse(value_json_str))
+                                    : ValueOrError<JToken>.WithError(res);
                     }
                     default:
-                        return null;
-
+                        return ValueOrError<JToken>.WithError($"RuntimeGetProperties: unknown object id scheme: {objectId.Scheme}");
                 }
             }
-            catch (Exception) {
-                return null;
+            catch (Exception ex) {
+                return ValueOrError<JToken>.WithError($"RuntimeGetProperties: Failed to get properties for {objectId}: {ex}");
             }
         }
 
@@ -1283,6 +1295,11 @@ namespace Microsoft.WebAssembly.Diagnostics
             {
                 SendResponse(msg_id, ree.Error, token);
             }
+            catch (ExpressionEvaluationFailedException eefe)
+            {
+                logger.LogDebug($"Error in EvaluateOnCallFrame for expression '{expression}' with '{eefe}.");
+                SendResponse(msg_id, Result.Exception(eefe), token);
+            }
             catch (Exception e)
             {
                 logger.LogDebug($"Error in EvaluateOnCallFrame for expression '{expression}' with '{e}.");
index 82d81aa..f8ad79a 100644 (file)
@@ -906,8 +906,13 @@ namespace Microsoft.WebAssembly.Diagnostics
             return true;
         }
 
-        internal async Task<MonoBinaryReader> SendDebuggerAgentCommand<T>(T command, MonoBinaryWriter arguments, CancellationToken token) =>
-            MonoBinaryReader.From (await proxy.SendMonoCommand(sessionId, MonoCommands.SendDebuggerAgentCommand(proxy.RuntimeId, GetNewId(), (int)GetCommandSetForCommand(command), (int)(object)command, arguments?.ToBase64().data ?? string.Empty), token));
+        internal async Task<MonoBinaryReader> SendDebuggerAgentCommand<T>(T command, MonoBinaryWriter arguments, CancellationToken token, bool throwOnError = true)
+        {
+            Result res = await proxy.SendMonoCommand(sessionId, MonoCommands.SendDebuggerAgentCommand(proxy.RuntimeId, GetNewId(), (int)GetCommandSetForCommand(command), (int)(object)command, arguments?.ToBase64().data ?? string.Empty), token);
+            return !res.IsOk && throwOnError
+                        ? throw new DebuggerAgentException($"SendDebuggerAgentCommand failed for {command}")
+                        : MonoBinaryReader.From(res);
+        }
 
         private static CommandSet GetCommandSetForCommand<T>(T command) =>
             command switch {
@@ -929,8 +934,13 @@ namespace Microsoft.WebAssembly.Diagnostics
                 _ => throw new Exception ("Unknown CommandSet")
             };
 
-        internal async Task<MonoBinaryReader> SendDebuggerAgentCommandWithParms<T>(T command, (string data, int length) encoded, int type, string extraParm, CancellationToken token) =>
-            MonoBinaryReader.From(await proxy.SendMonoCommand(sessionId, MonoCommands.SendDebuggerAgentCommandWithParms(proxy.RuntimeId, GetNewId(), (int)GetCommandSetForCommand(command), (int)(object)command, encoded.data, encoded.length, type, extraParm), token));
+        internal async Task<MonoBinaryReader> SendDebuggerAgentCommandWithParms<T>(T command, (string data, int length) encoded, int type, string extraParm, CancellationToken token, bool throwOnError = true)
+        {
+            Result res = await proxy.SendMonoCommand(sessionId, MonoCommands.SendDebuggerAgentCommandWithParms(proxy.RuntimeId, GetNewId(), (int)GetCommandSetForCommand(command), (int)(object)command, encoded.data, encoded.length, type, extraParm), token);
+            return !res.IsOk && throwOnError
+                        ? throw new DebuggerAgentException($"SendDebuggerAgentCommand failed for {command}: {res.Error}")
+                        : MonoBinaryReader.From(res);
+        }
 
         public async Task<int> CreateString(string value, CancellationToken token)
         {
@@ -1206,7 +1216,7 @@ namespace Microsoft.WebAssembly.Diagnostics
             commandParamsWriter.Write((int)StepSize.Line);
             commandParamsWriter.Write((int)kind);
             commandParamsWriter.Write((int)(StepFilter.StaticCtor)); //filter
-            using var retDebuggerCmdReader = await SendDebuggerAgentCommand(CmdEventRequest.Set, commandParamsWriter, token);
+            using var retDebuggerCmdReader = await SendDebuggerAgentCommand(CmdEventRequest.Set, commandParamsWriter, token, throwOnError: false);
             if (retDebuggerCmdReader.HasError)
                 return false;
             var isBPOnManagedCode = retDebuggerCmdReader.ReadInt32();
@@ -1221,7 +1231,7 @@ namespace Microsoft.WebAssembly.Diagnostics
             commandParamsWriter.Write((byte)EventKind.Step);
             commandParamsWriter.Write((int) req_id);
 
-            using var retDebuggerCmdReader = await SendDebuggerAgentCommand(CmdEventRequest.Clear, commandParamsWriter, token);
+            using var retDebuggerCmdReader = await SendDebuggerAgentCommand(CmdEventRequest.Clear, commandParamsWriter, token, throwOnError: false);
             return !retDebuggerCmdReader.HasError ? true : false;
         }
 
@@ -1419,9 +1429,9 @@ namespace Microsoft.WebAssembly.Diagnostics
 
                 return retValue?["value"]?.Value<string>();
             }
-            catch (Exception)
+            catch (Exception ex)
             {
-                logger.LogDebug($"Could not evaluate DebuggerDisplayAttribute - {expr} - {await GetTypeName(typeId, token)}");
+                logger.LogDebug($"Could not evaluate DebuggerDisplayAttribute - {expr} - {await GetTypeName(typeId, token)}: {ex}");
             }
             return null;
         }
@@ -1534,6 +1544,9 @@ namespace Microsoft.WebAssembly.Diagnostics
 
         public async Task<int> GetMethodIdByName(int type_id, string method_name, CancellationToken token)
         {
+            if (type_id <= 0)
+                throw new DebuggerAgentException($"Invalid type_id {type_id} (method_name: {method_name}");
+
             using var commandParamsWriter = new MonoBinaryWriter();
             commandParamsWriter.Write((int)type_id);
             commandParamsWriter.Write(method_name);
@@ -2250,7 +2263,7 @@ namespace Microsoft.WebAssembly.Diagnostics
             commandParamsWriter.Write(arrayId);
             commandParamsWriter.Write(0);
             commandParamsWriter.Write(dimensions.TotalLength);
-            var retDebuggerCmdReader = await SendDebuggerAgentCommand<CmdArray>(CmdArray.GetValues, commandParamsWriter, token);
+            var retDebuggerCmdReader = await SendDebuggerAgentCommand(CmdArray.GetValues, commandParamsWriter, token);
             JArray array = new JArray();
             for (int i = 0 ; i < dimensions.TotalLength; i++)
             {
@@ -2736,10 +2749,8 @@ namespace Microsoft.WebAssembly.Diagnostics
             JArray locals = new JArray();
             using var getDebuggerCmdReader = await SendDebuggerAgentCommand(CmdFrame.GetValues, commandParamsWriter, token);
             int etype = getDebuggerCmdReader.ReadByte();
-            using var setDebuggerCmdReader = await SendDebuggerAgentCommandWithParms(CmdFrame.SetValues, commandParamsWriter.ToBase64(), etype, newValue, token);
-            if (setDebuggerCmdReader.HasError)
-                return false;
-            return true;
+            using var setDebuggerCmdReader = await SendDebuggerAgentCommandWithParms(CmdFrame.SetValues, commandParamsWriter.ToBase64(), etype, newValue, token, throwOnError: false);
+            return !setDebuggerCmdReader.HasError;
         }
 
         public async Task<bool> SetNextIP(MethodInfoWithDebugInformation method, int threadId, IlLocation ilOffset, CancellationToken token)
@@ -2816,5 +2827,9 @@ namespace Microsoft.WebAssembly.Diagnostics
             }
             return result;
         }
+
+        public static bool IsNullValuedObject(this JObject obj)
+            => obj != null && obj["type"]?.Value<string>() == "object" && obj["subtype"]?.Value<string>() == "null";
+
     }
 }
diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/ValueOrError.cs b/src/mono/wasm/debugger/BrowserDebugProxy/ValueOrError.cs
new file mode 100644 (file)
index 0000000..b5d0785
--- /dev/null
@@ -0,0 +1,32 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#nullable enable
+
+using System;
+
+namespace Microsoft.WebAssembly.Diagnostics;
+
+public struct ValueOrError<TValue>
+{
+    public TValue? Value { get; init; }
+    public Result? Error { get; init; }
+
+    public bool IsError => Error != null;
+
+    private ValueOrError(TValue? value = default, Result? error = default)
+    {
+        if (value != null && error != null)
+            throw new ArgumentException($"Both {nameof(value)}, and {nameof(error)} cannot be non-null");
+
+        if (value == null && error == null)
+            throw new ArgumentException($"Both {nameof(value)}, and {nameof(error)} cannot be null");
+
+        Value = value;
+        Error = error;
+    }
+
+    public static ValueOrError<TValue> WithValue(TValue value) => new ValueOrError<TValue>(value: value);
+    public static ValueOrError<TValue> WithError(Result err) => new ValueOrError<TValue>(error: err);
+    public static ValueOrError<TValue> WithError(string msg) => new ValueOrError<TValue>(error: Result.Err(msg));
+}
index b4ce80d..9e1aec0 100644 (file)
@@ -23,7 +23,10 @@ namespace DebuggerTests
             { "MONO_TYPE_VALUETYPE",   TValueType("DebuggerTests.Point"),                       TValueType("DebuggerTests.Point") },
             { "MONO_TYPE_VALUETYPE2",  TValueType("System.Decimal","0"),                        TValueType("System.Decimal", "1.1") },
             { "MONO_TYPE_GENERICINST", TObject("System.Func<int>", is_null: true),              TDelegate("System.Func<int>", "int Prepare ()") },
-            { "MONO_TYPE_FNPTR",       TPointer("*()",  is_null: true),                         TPointer("*()") },
+
+            // Disabled due to https://github.com/dotnet/runtime/issues/65881
+            //{ "MONO_TYPE_FNPTR",       TPointer("*()",  is_null: true),                         TPointer("*()") },
+
             { "MONO_TYPE_PTR",         TPointer("int*", is_null: true),                         TPointer("int*") },
             { "MONO_TYPE_I1",          TNumber(0),                                              TNumber(-1) },
             { "MONO_TYPE_I2",          TNumber(0),                                              TNumber(-1) },
index 3e3071a..3c01201 100644 (file)
@@ -494,8 +494,6 @@ namespace DebuggerTests
 
 
         [Fact]
-        [Trait("Category", "windows-failing")] // https://github.com/dotnet/runtime/issues/65744
-        [Trait("Category", "linux-failing")] // https://github.com/dotnet/runtime/issues/65744
         public async Task EvaluateSimpleMethodCallsError() => await CheckInspectLocalsAtBreakpointSite(
             "DebuggerTests.EvaluateMethodTestsClass.TestEvaluate", "run", 9, "run",
             "window.setTimeout(function() { invoke_static_method ('[debugger-test] DebuggerTests.EvaluateMethodTestsClass:EvaluateMethods'); })",
@@ -504,19 +502,19 @@ namespace DebuggerTests
                 var id = pause_location["callFrames"][0]["callFrameId"].Value<string>();
 
                 var (_, res) = await EvaluateOnCallFrame(id, "this.objToTest.MyMethodWrong()", expect_ok: false );
-                AssertEqual("Unable to evaluate method 'MyMethodWrong'", res.Error["message"]?.Value<string>(), "wrong error message");
+                Assert.Contains($"Method 'MyMethodWrong' not found", res.Error["message"]?.Value<string>());
 
-                (_, res) = await EvaluateOnCallFrame(id, "this.objToTest.MyMethod(1)", expect_ok: false );
-                AssertEqual("Unable to evaluate method 'MyMethod'. Too many arguments passed.", res.Error["result"]["description"]?.Value<string>(), "wrong error message");
+                (_, res) = await EvaluateOnCallFrame(id, "this.objToTest.MyMethod(1)", expect_ok: false);
+                Assert.Contains("Cannot invoke method 'this.objToTest.MyMethod(1)' - too many arguments passed", res.Error["message"]?.Value<string>());
 
                 (_, res) = await EvaluateOnCallFrame(id, "this.CallMethodWithParm(\"1\")", expect_ok: false );
-                AssertEqual("Unable to evaluate method 'CallMethodWithParm'", res.Error["message"]?.Value<string>(), "wrong error message");
+                Assert.Contains("Unable to evaluate method 'this.CallMethodWithParm(\"1\")'", res.Error["message"]?.Value<string>());
 
                 (_, res) = await EvaluateOnCallFrame(id, "this.ParmToTestObjNull.MyMethod()", expect_ok: false );
-                AssertEqual("Unable to evaluate method 'MyMethod'", res.Error["message"]?.Value<string>(), "wrong error message");
+                Assert.Contains("Expression 'this.ParmToTestObjNull.MyMethod' evaluated to null", res.Error["message"]?.Value<string>());
 
                 (_, res) = await EvaluateOnCallFrame(id, "this.ParmToTestObjException.MyMethod()", expect_ok: false );
-                AssertEqual("Unable to evaluate method 'MyMethod'", res.Error["message"]?.Value<string>(), "wrong error message");
+                Assert.Contains("Cannot invoke method 'MyMethod'", res.Error["message"]?.Value<string>());
            });
 
         [Fact]
@@ -607,7 +605,7 @@ namespace DebuggerTests
                    ("f.textList[j]", TString("2")),
                    ("f.numArray[j]", TNumber(2)),
                    ("f.textArray[i]", TString("1")));
-                
+
            });
 
         [Fact]
@@ -625,7 +623,7 @@ namespace DebuggerTests
                    ("f.textList[f.idx1]", TString("2")),
                    ("f.numArray[f.idx1]", TNumber(2)),
                    ("f.textArray[f.idx0]", TString("1")));
-                
+
            });
 
         [Fact]
@@ -642,7 +640,7 @@ namespace DebuggerTests
                    ("f.textList[f.numList[f.idx0]]", TString("2")),
                    ("f.numArray[f.numArray[f.idx0]]", TNumber(2)),
                    ("f.textArray[f.numArray[f.idx0]]", TString("2")));
-                
+
            });
 
         [Fact]
@@ -668,7 +666,7 @@ namespace DebuggerTests
                    ("f.textListOfLists[1][1]", TString("2")),
                    ("f.textListOfLists[j][j]", TString("2")),
                    ("f.textListOfLists[f.idx1][f.idx1]", TString("2")));
-                
+
            });
 
         [Fact]
@@ -843,7 +841,7 @@ namespace DebuggerTests
                     ("  str", "ReferenceError")
                 );
             });
-        
+
         [Fact]
         public async Task EvaluateConstantValueUsingRuntimeEvaluate() => await CheckInspectLocalsAtBreakpointSite(
             "DebuggerTests.EvaluateTestsClass", "EvaluateLocals", 9, "EvaluateLocals",
@@ -857,7 +855,7 @@ namespace DebuggerTests
                    ("\"15\"\n//comment as vs does\n", TString("15")),
                    ("\"15\"", TString("15")));
            });
-        
+
         [Theory]
         [InlineData("EvaluateBrowsableProperties", "TestEvaluateFieldsNone", "testFieldsNone", 10)]
         [InlineData("EvaluateBrowsableProperties", "TestEvaluatePropertiesNone", "testPropertiesNone", 10)]
@@ -872,7 +870,7 @@ namespace DebuggerTests
                 var (testNone, _) = await EvaluateOnCallFrame(id, localVarName);
                 await CheckValue(testNone, TObject($"DebuggerTests.{outerClassName}.{className}"), nameof(testNone));
                 var testNoneProps = await GetProperties(testNone["objectId"]?.Value<string>());
-                
+
                 if (isCustomGetter)
                     await CheckProps(testNoneProps, new
                     {
@@ -941,7 +939,7 @@ namespace DebuggerTests
                         textCollapsed = TString("textCollapsed")
                     }, "testCollapsedProps#1");
            });
-        
+
         [Theory]
         [InlineData("EvaluateBrowsableProperties", "TestEvaluateFieldsRootHidden", "testFieldsRootHidden", 10)]
         [InlineData("EvaluateBrowsableProperties", "TestEvaluatePropertiesRootHidden", "testPropertiesRootHidden", 10)]
@@ -1008,7 +1006,7 @@ namespace DebuggerTests
                var id = pause_location["callFrames"][0]["callFrameId"].Value<string>();
                var (obj, _) = await EvaluateOnCallFrame(id, "testClass");
                var (pub, internalAndProtected, priv) = await GetPropertiesSortedByProtectionLevels(obj["objectId"]?.Value<string>());
-            
+
                Assert.True(pub[0] != null);
                Assert.True(internalAndProtected[0] != null);
                Assert.True(internalAndProtected[1] != null);
@@ -1019,7 +1017,7 @@ namespace DebuggerTests
                Assert.Equal(internalAndProtected[1]["value"]["value"], "protected");
                Assert.Equal(priv[0]["value"]["value"], "private");
            });
-        
+
         [Fact]
         public async Task StructureGetters() =>  await CheckInspectLocalsAtBreakpointSite(
             "DebuggerTests.StructureGetters", "Evaluate", 2, "Evaluate",
@@ -1027,14 +1025,14 @@ namespace DebuggerTests
             wait_for_event_fn: async (pause_location) =>
             {
                 var id = pause_location["callFrames"][0]["callFrameId"].Value<string>();
-                var (obj, _) = await EvaluateOnCallFrame(id, "s"); 
+                var (obj, _) = await EvaluateOnCallFrame(id, "s");
                 var props = await GetProperties(obj["objectId"]?.Value<string>());
-               
+
                 await CheckProps(props, new
                 {
                     Id = TGetter("Id", TNumber(123))
                 }, "s#1");
-                
+
                 var getter = props.FirstOrDefault(p => p["name"]?.Value<string>() == "Id");
                 Assert.NotNull(getter);
                 var getterId = getter["get"]["objectId"]?.Value<string>();
@@ -1083,22 +1081,22 @@ namespace DebuggerTests
                    ("test.GetDouble()", JObject.FromObject( new { type = "number", value = 1.23, description = "1.23" })),
                    ("test.GetSingleNullable()", JObject.FromObject( new { type = "number", value = 1.23, description = "1.23" })),
                    ("test.GetDoubleNullable()", JObject.FromObject( new { type = "number", value = 1.23, description = "1.23" })),
-                   
+
                    ("test.GetBool()", JObject.FromObject( new { type = "object", value = true, description = "True", className = "System.Boolean" })),
                    ("test.GetBoolNullable()", JObject.FromObject( new { type = "object", value = true, description = "True", className = "System.Boolean" })),
                    ("test.GetNull()", JObject.FromObject( new { type = "object", value = true, description = "True", className = "System.Boolean" })),
-                   
+
                    ("test.GetDefaultAndRequiredParam(2)", TNumber(5)),
                    ("test.GetDefaultAndRequiredParam(3, 2)", TNumber(5)),
                    ("test.GetDefaultAndRequiredParamMixedTypes(\"a\")", TString("a; -1; False")),
                    ("test.GetDefaultAndRequiredParamMixedTypes(\"a\", 23)", TString("a; 23; False")),
                    ("test.GetDefaultAndRequiredParamMixedTypes(\"a\", 23, true)", TString("a; 23; True"))
                    );
-                
-                var (_, res) = await EvaluateOnCallFrame(id, "test.GetDefaultAndRequiredParamMixedTypes(\"a\", 23, true, 1.23f)", expect_ok: false );
-                AssertEqual("Unable to evaluate method 'GetDefaultAndRequiredParamMixedTypes'. Too many arguments passed.", res.Error["result"]["description"]?.Value<string>(), "wrong error message");
+
+                var (_, res) = await EvaluateOnCallFrame(id, "test.GetDefaultAndRequiredParamMixedTypes(\"a\", 23, true, 1.23f)", expect_ok: false);
+                Assert.Contains("method 'test.GetDefaultAndRequiredParamMixedTypes(\"a\", 23, true, 1.23f)' - too many arguments passed", res.Error["message"]?.Value<string>());
            });
-        
+
         [Fact]
         public async Task EvaluateMethodWithLinq() => await CheckInspectLocalsAtBreakpointSite(
             $"DebuggerTests.DefaultParamMethods", "Evaluate", 2, "Evaluate",
index cde3ef3..91ae31b 100644 (file)
@@ -188,12 +188,14 @@ namespace DebuggerTests
                 {
                     string line = FormatConsoleAPICalled(args);
                     _logger.LogInformation(line);
-                    if (DetectAndFailOnAssertions && line.Contains("console.error: * Assertion at"))
+                    if (DetectAndFailOnAssertions &&
+                            (line.Contains("console.error: [MONO]") || line.Contains("console.warning: [MONO]")))
                     {
                         args["__forMethod"] = method;
-                        Client.Fail(new ArgumentException($"Assertion detected in the messages: {line}{Environment.NewLine}{args}"));
+                        Client.Fail(new ArgumentException($"Unexpected runtime error/warning message detected: {line}{Environment.NewLine}{args}"));
                         return;
                     }
+
                     break;
                 }
                 case "Inspector.detached":