From efd877bb1466f898e2a8d8bdde187bae1ba45c83 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Thu, 24 Feb 2022 01:21:31 +0000 Subject: [PATCH 01/12] [wasm] Mark result as Error if it has 'exceptionDetails' field --- .../wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs index a348a42534c312..f28b972d28bb39 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs @@ -129,7 +129,11 @@ public struct Result private Result(JObject resultOrError, bool isError) { - bool resultHasError = isError || string.Equals((resultOrError?["result"] as JObject)?["subtype"]?.Value(), "error"); + if (resultOrError == null) + throw new ArgumentNullException(nameof(resultOrError)); + + bool resultHasError = isError || string.Equals((resultOrError["result"] as JObject)?["subtype"]?.Value(), "error"); + resultHasError |= resultOrError["exceptionDetails"] != null; if (resultHasError) { Value = null; @@ -146,7 +150,7 @@ public static Result FromJson(JObject obj) 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); } From fd40892db9cda9d1c30c4cad58b09c42556a4cc9 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Thu, 24 Feb 2022 01:22:33 +0000 Subject: [PATCH 02/12] [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. --- .../MemberReferenceResolver.cs | 20 +++++-- .../debugger/BrowserDebugProxy/MonoProxy.cs | 54 +++++++++++-------- .../BrowserDebugProxy/ValueOrError.cs | 33 ++++++++++++ 3 files changed, 81 insertions(+), 26 deletions(-) create mode 100644 src/mono/wasm/debugger/BrowserDebugProxy/ValueOrError.cs diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs index ef744beddfdca5..38b02d9c1791b7 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs @@ -234,8 +234,14 @@ async Task ResolveAsLocalOrThisMember(string name) if (!DotnetObjectId.TryParse(objThis?["value"]?["objectId"]?.Value(), out DotnetObjectId objectId)) return null; - var rootResObj = await proxy.RuntimeGetPropertiesInternal(sessionId, objectId, null, token); - var objRet = rootResObj.FirstOrDefault(objPropAttr => objPropAttr["name"].Value() == nameTrimmed); + ValueOrError valueOrError = await proxy.RuntimeGetPropertiesInternal(sessionId, objectId, null, token); + if (valueOrError.HasError) + { + logger.LogDebug($"ResolveAsLocalOrThisMember failed with : {valueOrError.Error}"); + return null; + } + + JToken objRet = valueOrError.Value.FirstOrDefault(objPropAttr => objPropAttr["name"].Value() == nameTrimmed); if (objRet != null) return await GetValueFromObject(objRet, token); @@ -255,8 +261,14 @@ async Task ResolveAsInstanceMember(string expr, JObject baseObject) if (!DotnetObjectId.TryParse(resolvedObject?["objectId"]?.Value(), out DotnetObjectId objectId)) return null; - var resolvedResObj = await proxy.RuntimeGetPropertiesInternal(sessionId, objectId, null, token); - var objRet = resolvedResObj.FirstOrDefault(objPropAttr => objPropAttr["name"]?.Value() == partTrimmed); + ValueOrError valueOrError = await proxy.RuntimeGetPropertiesInternal(sessionId, objectId, null, token); + if (valueOrError.HasError) + { + logger.LogDebug($"ResolveAsInstanceMember failed with : {valueOrError.Error}"); + return null; + } + + JToken objRet = valueOrError.Value.FirstOrDefault(objPropAttr => objPropAttr["name"]?.Value() == partTrimmed); if (objRet == null) return null; diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 581f7ef5cf6ba8..1e8bf2487c96c3 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -462,12 +462,16 @@ protected override async Task AcceptCommand(MessageId id, string method, J 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.HasError) + { + 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; } @@ -715,7 +719,7 @@ private async Task OnSetVariableValue(MessageId id, int scopeId, string va return true; } - internal async Task RuntimeGetPropertiesInternal(SessionId id, DotnetObjectId objectId, JToken args, CancellationToken token, bool sortByAccessLevel = false) + internal async Task> RuntimeGetPropertiesInternal(SessionId id, DotnetObjectId objectId, JToken args, CancellationToken token, bool sortByAccessLevel = false) { var context = GetContext(id); var accessorPropertiesOnly = false; @@ -737,51 +741,57 @@ internal async Task RuntimeGetPropertiesInternal(SessionId id, DotnetObj { 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.WithValue(sortByAccessLevel ? resScope.Value : resScope.Value?["result"]) + : ValueOrError.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.WithError($"Could not get properties for {objectId}"), + _ => ValueOrError.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.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.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.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.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(); - 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.WithError($"Internal error: Could not find expected __value_as_json_string__ field in the result: {res}"); + + return value_json_str != null + ? ValueOrError.WithValue(sortByAccessLevel ? JObject.FromObject(new { result = JArray.Parse(value_json_str) }) : JArray.Parse(value_json_str)) + : ValueOrError.WithError(res); } default: - return null; - + return ValueOrError.WithError($"RuntimeGetProperties: unknown object id scheme: {objectId.Scheme}"); } } - catch (Exception) { - return null; + catch (Exception ex) { + return ValueOrError.WithError($"RuntimeGetProperties: Failed to get properties for {objectId}: {ex}"); } } diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/ValueOrError.cs b/src/mono/wasm/debugger/BrowserDebugProxy/ValueOrError.cs new file mode 100644 index 00000000000000..9819ad72be6d76 --- /dev/null +++ b/src/mono/wasm/debugger/BrowserDebugProxy/ValueOrError.cs @@ -0,0 +1,33 @@ +// 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 +{ + public TValue? Value { get; init; } + public Result? Error { get; init; } + + public bool HasValue => Value != null; + public bool HasError => !HasValue; + + 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 WithValue(TValue value) => new ValueOrError(value: value); + public static ValueOrError WithError(Result err) => new ValueOrError(error: err); + public static ValueOrError WithError(string msg) => new ValueOrError(error: Result.Err(msg)); +} From dda08664fe5fd763de7095448d230c9569925dff Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Thu, 24 Feb 2022 01:24:55 +0000 Subject: [PATCH 03/12] [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. --- .../DebuggerAgentException.cs | 19 ++++++++++++ .../BrowserDebugProxy/MonoSDBHelper.cs | 30 ++++++++++++------- 2 files changed, 38 insertions(+), 11 deletions(-) create mode 100644 src/mono/wasm/debugger/BrowserDebugProxy/DebuggerAgentException.cs diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/DebuggerAgentException.cs b/src/mono/wasm/debugger/BrowserDebugProxy/DebuggerAgentException.cs new file mode 100644 index 00000000000000..b4ca2bde98927f --- /dev/null +++ b/src/mono/wasm/debugger/BrowserDebugProxy/DebuggerAgentException.cs @@ -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) + { + } +} diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index ed46b03d598e5e..b136d717357eea 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -906,8 +906,13 @@ public async Task EnableReceiveRequests(EventKind eventKind, CancellationT return true; } - internal async Task SendDebuggerAgentCommand(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 SendDebuggerAgentCommand(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}: {res.Error}") + : MonoBinaryReader.From(res); + } internal CommandSet GetCommandSetForCommand(T command) => command switch { @@ -929,8 +934,13 @@ internal CommandSet GetCommandSetForCommand(T command) => _ => throw new Exception ("Unknown CommandSet") }; - internal async Task SendDebuggerAgentCommandWithParms(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 SendDebuggerAgentCommandWithParms(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 CreateString(string value, CancellationToken token) { @@ -1206,7 +1216,7 @@ public async Task Step(int thread_id, StepKind kind, CancellationToken tok 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 @@ public async Task ClearSingleStep(int req_id, CancellationToken token) 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; } @@ -2248,7 +2258,7 @@ public async Task GetArrayValues(int arrayId, CancellationToken token) commandParamsWriter.Write(arrayId); commandParamsWriter.Write(0); commandParamsWriter.Write(dimensions.TotalLength); - var retDebuggerCmdReader = await SendDebuggerAgentCommand(CmdArray.GetValues, commandParamsWriter, token); + var retDebuggerCmdReader = await SendDebuggerAgentCommand(CmdArray.GetValues, commandParamsWriter, token); JArray array = new JArray(); for (int i = 0 ; i < dimensions.TotalLength; i++) { @@ -2734,10 +2744,8 @@ public async Task SetVariableValue(int thread_id, int frame_id, int varId, 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 SetNextIP(MethodInfoWithDebugInformation method, int threadId, IlLocation ilOffset, CancellationToken token) From 6d010f55e169a786711f0b94ed78f189a51cbd85 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Thu, 24 Feb 2022 03:51:25 +0000 Subject: [PATCH 04/12] [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 `' 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`. --- src/mono/mono/component/debugger-agent.c | 3 + .../MemberReferenceResolver.cs | 61 ++++++++++++------- 2 files changed, 43 insertions(+), 21 deletions(-) diff --git a/src/mono/mono/component/debugger-agent.c b/src/mono/mono/component/debugger-agent.c index 8221e93c36bf66..658c03d411ec05 100644 --- a/src/mono/mono/component/debugger-agent.c +++ b/src/mono/mono/component/debugger-agent.c @@ -8221,6 +8221,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)) { diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs index 38b02d9c1791b7..bf1c60e8520c0c 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs @@ -393,36 +393,25 @@ public async Task Resolve(InvocationExpressionSyntax method, Dictionary methodName = memberAccessExpressionSyntax.Name.ToString(); } 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(), out DotnetObjectId objectId); + if (!DotnetObjectId.TryParse(rootObject?["objectId"]?.Value(), out DotnetObjectId objectId)) + throw new Exception($"Cannot invoke method '{methodName}' on invalid object id: {rootObject["objectId"]?.Value()}"); + 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"); @@ -498,9 +487,39 @@ public async Task Resolve(InvocationExpressionSyntax method, Dictionary return null; } catch (Exception ex) when (ex is not ReturnAsErrorException) + { { throw new Exception($"Unable to evaluate method '{methodName}'", ex); } + + async Task FindMethodIdOnLinqEnumerable(IList 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 genericTypeArgs = await context.SdbAgent.GetTypeParamsOrArgsForGenericType(typeId, token); + if (genericTypeArgs.Count > 0) + { + isTryingLinq = 1; + return await context.SdbAgent.MakeGenericMethod(newMethodId, genericTypeArgs, token); + } + } + + return 0; + } } } } From 752136a87c5327fc82163f97cf2cc675a0e6f7de Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Thu, 24 Feb 2022 03:56:58 +0000 Subject: [PATCH 05/12] [wasm] Add some missing exception logging in the proxy --- .../debugger/BrowserDebugProxy/MemberReferenceResolver.cs | 4 ++-- src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs | 6 ++++-- src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs | 4 ++-- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs index bf1c60e8520c0c..58a65116c778f4 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs @@ -372,9 +372,9 @@ public async Task Resolve(ElementAccessExpressionSyntax elementAccess, } return null; } - catch (Exception) + catch (Exception ex) { - throw new Exception($"Unable to evaluate method '{elementAccess}'"); + throw new Exception($"Unable to evaluate method '{elementAccess}'", ex); } } diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 1e8bf2487c96c3..c686923cd6b86b 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -590,7 +590,8 @@ protected override async Task AcceptCommand(MessageId id, string method, J 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"]}).")), @@ -604,8 +605,9 @@ protected override async Task AcceptCommand(MessageId id, string method, J { 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})")), diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index b136d717357eea..9b92c552561ea9 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -1429,9 +1429,9 @@ public async Task GetValueFromDebuggerDisplayAttribute(int objectId, int return retValue?["value"]?.Value(); } - 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; } From ec1a5cd7962af057ca5fcda05a7e93052c599fe9 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Thu, 24 Feb 2022 03:58:05 +0000 Subject: [PATCH 06/12] cleaup --- src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs | 6 ++++-- src/mono/wasm/runtime/debug.ts | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs b/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs index cde3ef31145a65..0135c292665495 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs @@ -188,12 +188,14 @@ async Task OnMessage(string method, JObject args, CancellationToken token) { 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($"Possibly problematic runtime message detected: {line}{Environment.NewLine}{args}")); return; } + break; } case "Inspector.detached": diff --git a/src/mono/wasm/runtime/debug.ts b/src/mono/wasm/runtime/debug.ts index ac7b035e642d1d..a2bccbbdf0970b 100644 --- a/src/mono/wasm/runtime/debug.ts +++ b/src/mono/wasm/runtime/debug.ts @@ -315,12 +315,14 @@ export function mono_wasm_debugger_log(level: number, message_ptr: CharPtr): voi } export function mono_wasm_trace_logger(log_domain_ptr: CharPtr, log_level_ptr: CharPtr, message_ptr: CharPtr, fatal: number, user_data: VoidPtr): void { - const message = Module.UTF8ToString(message_ptr); + const origMessage = Module.UTF8ToString(message_ptr); const isFatal = !!fatal; const domain = Module.UTF8ToString(log_domain_ptr); // is this always Mono? const dataPtr = user_data; const log_level = Module.UTF8ToString(log_level_ptr); + const message = `[MONO] ${origMessage}`; + if (INTERNAL["logging"] && typeof INTERNAL.logging["trace"] === "function") { INTERNAL.logging.trace(domain, log_level, message, isFatal, dataPtr); return; From 2199697476bbf3cee845e0c8c51017fc93a94d69 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Thu, 24 Feb 2022 05:30:27 +0000 Subject: [PATCH 07/12] [wasm] GetMethodIdByName: throw on invalid type ids --- src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index 9b92c552561ea9..3e47a82cfae174 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -1544,6 +1544,9 @@ public async Task GetTypeIdFromToken(int assemblyId, int typeToken, Cancell public async Task 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); From 08c898be7d2ddd0108f37025a59bfddffd528b90 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Fri, 25 Feb 2022 02:31:20 -0500 Subject: [PATCH 08/12] [wasm] Improve error handling in expression evaluation --- .../MemberReferenceResolver.cs | 36 ++++++------- .../debugger/BrowserDebugProxy/MonoProxy.cs | 5 ++ .../BrowserDebugProxy/MonoSDBHelper.cs | 6 ++- .../EvaluateOnCallFrameTests.cs | 52 +++++++++---------- 4 files changed, 53 insertions(+), 46 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs index 58a65116c778f4..093200eb44a7ee 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs @@ -199,7 +199,7 @@ public async Task Resolve(string varName, CancellationToken token) (retObject, string remaining) = await ResolveStaticMembersInStaticTypes(varName, token); if (!string.IsNullOrEmpty(remaining)) { - if (retObject?["subtype"]?.Value() == "null") + if (retObject.IsNullValuedObject()) { // NRE on null.$remaining retObject = null; @@ -221,7 +221,7 @@ async Task ResolveAsLocalOrThisMember(string name) { 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; } @@ -276,7 +276,7 @@ async Task ResolveAsInstanceMember(string expr, JObject baseObject) if (resolvedObject == null) return null; - if (resolvedObject["subtype"]?.Value() == "null") + if (resolvedObject.IsNullValuedObject()) { if (i < parts.Length - 1) { @@ -372,9 +372,9 @@ public async Task Resolve(ElementAccessExpressionSyntax elementAccess, } return null; } - catch (Exception ex) + catch (Exception ex) when (ex is not ExpressionEvaluationFailedException) { - throw new Exception($"Unable to evaluate method '{elementAccess}'", ex); + throw new ExpressionEvaluationFailedException($"Unable to evaluate element access '{elementAccess}': {ex.Message}", ex); } } @@ -391,6 +391,9 @@ public async Task Resolve(InvocationExpressionSyntax method, Dictionary 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) { @@ -404,7 +407,7 @@ public async Task Resolve(InvocationExpressionSyntax method, Dictionary if (rootObject != null) { if (!DotnetObjectId.TryParse(rootObject?["objectId"]?.Value(), out DotnetObjectId objectId)) - throw new Exception($"Cannot invoke method '{methodName}' on invalid object id: {rootObject["objectId"]?.Value()}"); + 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); @@ -414,7 +417,7 @@ public async Task Resolve(InvocationExpressionSyntax method, Dictionary 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) @@ -428,20 +431,18 @@ public async Task Resolve(InvocationExpressionSyntax method, Dictionary 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) @@ -462,23 +463,23 @@ public async Task Resolve(InvocationExpressionSyntax method, Dictionary 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); @@ -486,10 +487,9 @@ public async Task Resolve(InvocationExpressionSyntax method, Dictionary } 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 FindMethodIdOnLinqEnumerable(IList typeIds, string methodName) @@ -513,7 +513,7 @@ async Task FindMethodIdOnLinqEnumerable(IList typeIds, string methodNa List genericTypeArgs = await context.SdbAgent.GetTypeParamsOrArgsForGenericType(typeId, token); if (genericTypeArgs.Count > 0) { - isTryingLinq = 1; + isExtensionMethod = true; return await context.SdbAgent.MakeGenericMethod(newMethodId, genericTypeArgs, token); } } diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index c686923cd6b86b..7684b553087a96 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -1286,6 +1286,11 @@ private async Task OnEvaluateOnCallFrame(MessageId msg_id, int scopeId, st { 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}."); diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index 3e47a82cfae174..9cfe4d233556ca 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -910,7 +910,7 @@ internal async Task SendDebuggerAgentCommand(T command, Mon { 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}: {res.Error}") + ? throw new DebuggerAgentException($"SendDebuggerAgentCommand failed for {command}") : MonoBinaryReader.From(res); } @@ -2825,5 +2825,9 @@ public static JArray GetValuesByKeys(this Dictionary dict, List< } return result; } + + public static bool IsNullValuedObject(this JObject obj) + => obj != null && obj["type"]?.Value() == "object" && obj["subtype"]?.Value() == "null"; + } } diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs index c24449cef87b62..6d071a1af08cad 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs @@ -494,8 +494,6 @@ async Task EvaluateOnCallFrameFail(string call_frame_id, params (string expressi [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 @@ public async Task EvaluateSimpleMethodCallsError() => await CheckInspectLocalsAt var id = pause_location["callFrames"][0]["callFrameId"].Value(); var (_, res) = await EvaluateOnCallFrame(id, "this.objToTest.MyMethodWrong()", expect_ok: false ); - AssertEqual("Unable to evaluate method 'MyMethodWrong'", res.Error["message"]?.Value(), "wrong error message"); + Assert.Contains($"Method 'MyMethodWrong' not found", res.Error["message"]?.Value()); - (_, 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(), "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()); (_, res) = await EvaluateOnCallFrame(id, "this.CallMethodWithParm(\"1\")", expect_ok: false ); - AssertEqual("Unable to evaluate method 'CallMethodWithParm'", res.Error["message"]?.Value(), "wrong error message"); + Assert.Contains("Unable to evaluate method 'this.CallMethodWithParm(\"1\")'", res.Error["message"]?.Value()); (_, res) = await EvaluateOnCallFrame(id, "this.ParmToTestObjNull.MyMethod()", expect_ok: false ); - AssertEqual("Unable to evaluate method 'MyMethod'", res.Error["message"]?.Value(), "wrong error message"); + Assert.Contains("Expression 'this.ParmToTestObjNull.MyMethod' evaluated to null", res.Error["message"]?.Value()); (_, res) = await EvaluateOnCallFrame(id, "this.ParmToTestObjException.MyMethod()", expect_ok: false ); - AssertEqual("Unable to evaluate method 'MyMethod'", res.Error["message"]?.Value(), "wrong error message"); + Assert.Contains("Cannot invoke method 'MyMethod'", res.Error["message"]?.Value()); }); [Fact] @@ -606,7 +604,7 @@ await EvaluateOnCallFrameAndCheck(id, ("f.textList[j]", TString("2")), ("f.numArray[j]", TNumber(2)), ("f.textArray[i]", TString("1"))); - + }); [Fact] @@ -624,7 +622,7 @@ await EvaluateOnCallFrameAndCheck(id, ("f.textList[f.idx1]", TString("2")), ("f.numArray[f.idx1]", TNumber(2)), ("f.textArray[f.idx0]", TString("1"))); - + }); [Fact] @@ -641,7 +639,7 @@ await EvaluateOnCallFrameAndCheck(id, ("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] @@ -667,7 +665,7 @@ await EvaluateOnCallFrameAndCheck(id, ("f.textListOfLists[1][1]", TString("2")), ("f.textListOfLists[j][j]", TString("2")), ("f.textListOfLists[f.idx1][f.idx1]", TString("2"))); - + }); [Fact] @@ -842,7 +840,7 @@ await EvaluateOnCallFrameFail(id, (" str", "ReferenceError") ); }); - + [Fact] public async Task EvaluateConstantValueUsingRuntimeEvaluate() => await CheckInspectLocalsAtBreakpointSite( "DebuggerTests.EvaluateTestsClass", "EvaluateLocals", 9, "EvaluateLocals", @@ -856,7 +854,7 @@ await RuntimeEvaluateAndCheck( ("\"15\"\n//comment as vs does\n", TString("15")), ("\"15\"", TString("15"))); }); - + [Theory] [InlineData("EvaluateBrowsableProperties", "TestEvaluateFieldsNone", "testFieldsNone", 10)] [InlineData("EvaluateBrowsableProperties", "TestEvaluatePropertiesNone", "testPropertiesNone", 10)] @@ -871,7 +869,7 @@ public async Task EvaluateBrowsableNone(string outerClassName, string className, var (testNone, _) = await EvaluateOnCallFrame(id, localVarName); await CheckValue(testNone, TObject($"DebuggerTests.{outerClassName}.{className}"), nameof(testNone)); var testNoneProps = await GetProperties(testNone["objectId"]?.Value()); - + if (isCustomGetter) await CheckProps(testNoneProps, new { @@ -940,7 +938,7 @@ public async Task EvaluateBrowsableCollapsed(string outerClassName, string class textCollapsed = TString("textCollapsed") }, "testCollapsedProps#1"); }); - + [Theory] [InlineData("EvaluateBrowsableProperties", "TestEvaluateFieldsRootHidden", "testFieldsRootHidden", 10)] [InlineData("EvaluateBrowsableProperties", "TestEvaluatePropertiesRootHidden", "testPropertiesRootHidden", 10)] @@ -1007,7 +1005,7 @@ public async Task EvaluateProtectionLevels() => await CheckInspectLocalsAtBreak var id = pause_location["callFrames"][0]["callFrameId"].Value(); var (obj, _) = await EvaluateOnCallFrame(id, "testClass"); var (pub, internalAndProtected, priv) = await GetPropertiesSortedByProtectionLevels(obj["objectId"]?.Value()); - + Assert.True(pub[0] != null); Assert.True(internalAndProtected[0] != null); Assert.True(internalAndProtected[1] != null); @@ -1018,7 +1016,7 @@ public async Task EvaluateProtectionLevels() => await CheckInspectLocalsAtBreak 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", @@ -1026,14 +1024,14 @@ public async Task StructureGetters() => await CheckInspectLocalsAtBreakpointSit wait_for_event_fn: async (pause_location) => { var id = pause_location["callFrames"][0]["callFrameId"].Value(); - var (obj, _) = await EvaluateOnCallFrame(id, "s"); + var (obj, _) = await EvaluateOnCallFrame(id, "s"); var props = await GetProperties(obj["objectId"]?.Value()); - + await CheckProps(props, new { Id = TGetter("Id", TNumber(123)) }, "s#1"); - + var getter = props.FirstOrDefault(p => p["name"]?.Value() == "Id"); Assert.NotNull(getter); var getterId = getter["get"]["objectId"]?.Value(); @@ -1081,22 +1079,22 @@ await EvaluateOnCallFrameAndCheck(id, ("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(), "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()); }); - + [Fact] public async Task EvaluateMethodWithLinq() => await CheckInspectLocalsAtBreakpointSite( $"DebuggerTests.DefaultParamMethods", "Evaluate", 2, "Evaluate", From 20c63c48782e70de8019b015906b3fb909147c90 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Fri, 25 Feb 2022 02:31:28 -0500 Subject: [PATCH 09/12] Cleanup --- src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs | 4 ++-- src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs b/src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs index 0edf43669eae17..1bc38aa9a4308d 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/DebugStore.cs @@ -1229,7 +1229,7 @@ public IEnumerable Add(SessionId id, string name, byte[] assembly_da } 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 @@ public async IAsyncEnumerable Load(SessionId id, string[] loaded_fil } 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/DebuggerTestSuite/Inspector.cs b/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs index 0135c292665495..91ae31bfa5eabf 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/Inspector.cs @@ -192,7 +192,7 @@ async Task OnMessage(string method, JObject args, CancellationToken token) (line.Contains("console.error: [MONO]") || line.Contains("console.warning: [MONO]"))) { args["__forMethod"] = method; - Client.Fail(new ArgumentException($"Possibly problematic runtime message detected: {line}{Environment.NewLine}{args}")); + Client.Fail(new ArgumentException($"Unexpected runtime error/warning message detected: {line}{Environment.NewLine}{args}")); return; } From a40eb18ee15c2942c357cadbc2ec415b210bd6a4 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Fri, 25 Feb 2022 02:33:31 -0500 Subject: [PATCH 10/12] Disable failing test - https://github.com/dotnet/runtime/issues/65881 --- src/mono/wasm/debugger/DebuggerTestSuite/AssignmentTests.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/AssignmentTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/AssignmentTests.cs index 16c61e13ba4224..939b1a65c911aa 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/AssignmentTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/AssignmentTests.cs @@ -23,7 +23,10 @@ public class AssignmentTests : DebuggerTestBase { "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", is_null: true), TDelegate("System.Func", "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) }, From 79e0833f1c1a7f653ffecd81229f1c573f78a194 Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Fri, 25 Feb 2022 02:36:56 -0500 Subject: [PATCH 11/12] Add missed files --- .../ExpressionEvaluationFailedException.cs | 19 +++++++++++++++++++ .../InternalErrorException.cs | 19 +++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 src/mono/wasm/debugger/BrowserDebugProxy/ExpressionEvaluationFailedException.cs create mode 100644 src/mono/wasm/debugger/BrowserDebugProxy/InternalErrorException.cs diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/ExpressionEvaluationFailedException.cs b/src/mono/wasm/debugger/BrowserDebugProxy/ExpressionEvaluationFailedException.cs new file mode 100644 index 00000000000000..8805198e88e937 --- /dev/null +++ b/src/mono/wasm/debugger/BrowserDebugProxy/ExpressionEvaluationFailedException.cs @@ -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 index 00000000000000..74c21657732d25 --- /dev/null +++ b/src/mono/wasm/debugger/BrowserDebugProxy/InternalErrorException.cs @@ -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) + { + } +} From 2cca6c85fa834e5082244da651fcfa6b3efdc49d Mon Sep 17 00:00:00 2001 From: Ankit Jain Date: Fri, 25 Feb 2022 08:37:49 +0000 Subject: [PATCH 12/12] Address @ilonatommy's feedback --- .../debugger/BrowserDebugProxy/MemberReferenceResolver.cs | 4 ++-- src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs | 2 +- src/mono/wasm/debugger/BrowserDebugProxy/ValueOrError.cs | 3 +-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs index 093200eb44a7ee..b9f5c8e4dbb4b7 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs @@ -235,7 +235,7 @@ async Task ResolveAsLocalOrThisMember(string name) return null; ValueOrError valueOrError = await proxy.RuntimeGetPropertiesInternal(sessionId, objectId, null, token); - if (valueOrError.HasError) + if (valueOrError.IsError) { logger.LogDebug($"ResolveAsLocalOrThisMember failed with : {valueOrError.Error}"); return null; @@ -262,7 +262,7 @@ async Task ResolveAsInstanceMember(string expr, JObject baseObject) return null; ValueOrError valueOrError = await proxy.RuntimeGetPropertiesInternal(sessionId, objectId, null, token); - if (valueOrError.HasError) + if (valueOrError.IsError) { logger.LogDebug($"ResolveAsInstanceMember failed with : {valueOrError.Error}"); return null; diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 7684b553087a96..db822406ae1b3e 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -463,7 +463,7 @@ protected override async Task AcceptCommand(MessageId id, string method, J break; var valueOrError = await RuntimeGetPropertiesInternal(id, objectId, args, token, true); - if (valueOrError.HasError) + if (valueOrError.IsError) { logger.LogDebug($"Runtime.getProperties: {valueOrError.Error}"); SendResponse(id, valueOrError.Error.Value, token); diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/ValueOrError.cs b/src/mono/wasm/debugger/BrowserDebugProxy/ValueOrError.cs index 9819ad72be6d76..b5d07851206ee2 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/ValueOrError.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/ValueOrError.cs @@ -12,8 +12,7 @@ public struct ValueOrError public TValue? Value { get; init; } public Result? Error { get; init; } - public bool HasValue => Value != null; - public bool HasError => !HasValue; + public bool IsError => Error != null; private ValueOrError(TValue? value = default, Result? error = default) {