From 819b40953a3ba5e4958ec61d1bf44ac284f1c803 Mon Sep 17 00:00:00 2001 From: Thays Grazia Date: Mon, 6 Feb 2023 17:52:52 -0300 Subject: [PATCH] [wasm][debugger]Adding stack info to the exception details when evaluating expressions (#81440) * Adding stack info to the exception details when evaluating expressions. * passing only one parameter. * Fixing CI. * Fixing format. * Added a new test: Evaluate on a callframe different of the first and check if the callstack is correct. And fix it. * Addressing @radical comments. * addressing @radical comments. --- .../BrowserDebugProxy/EvaluateExpression.cs | 4 +- .../ExpressionEvaluationFailedException.cs | 19 -------- .../BrowserDebugProxy/MemberReferenceResolver.cs | 14 +++--- .../wasm/debugger/BrowserDebugProxy/MonoProxy.cs | 42 +++++++++++++---- .../DebuggerTestSuite/EvaluateOnCallFrameTests.cs | 52 +++++++++++++--------- .../DebuggerTestSuite/EvaluateOnCallFrameTests2.cs | 12 ++++- 6 files changed, 84 insertions(+), 59 deletions(-) delete mode 100644 src/mono/wasm/debugger/BrowserDebugProxy/ExpressionEvaluationFailedException.cs diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs b/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs index fffe806..455517f 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/EvaluateExpression.cs @@ -482,7 +482,6 @@ namespace Microsoft.WebAssembly.Diagnostics { get { - _error.Value["exceptionDetails"]["stackTrace"] = StackTrace; return _error; } set { } @@ -506,8 +505,7 @@ namespace Microsoft.WebAssembly.Diagnostics result = result, exceptionDetails = new { - exception = result, - stackTrace = StackTrace + exception = result } })); } diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/ExpressionEvaluationFailedException.cs b/src/mono/wasm/debugger/BrowserDebugProxy/ExpressionEvaluationFailedException.cs deleted file mode 100644 index 8805198..0000000 --- a/src/mono/wasm/debugger/BrowserDebugProxy/ExpressionEvaluationFailedException.cs +++ /dev/null @@ -1,19 +0,0 @@ -// 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/MemberReferenceResolver.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs index 4ec4b0b..781413f 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MemberReferenceResolver.cs @@ -270,7 +270,7 @@ namespace Microsoft.WebAssembly.Diagnostics } catch (Exception ex) { - throw new ExpressionEvaluationFailedException($"BUG: Unable to get properties for scope: {scopeId}. {ex}"); + throw new ReturnAsErrorException($"BUG: Unable to get properties for scope: {scopeId}. {ex}", ex.GetType().Name); } localsFetched = true; } @@ -454,9 +454,9 @@ namespace Microsoft.WebAssembly.Diagnostics throw new InvalidOperationException($"Cannot apply indexing with [] to an expression of scheme '{objectId.Scheme}'"); } } - catch (Exception ex) when (ex is not ExpressionEvaluationFailedException) + catch (Exception ex) { - throw new ExpressionEvaluationFailedException($"Unable to evaluate element access '{elementAccess}': {ex.Message}", ex); + throw new ReturnAsErrorException($"Unable to evaluate element access '{elementAccess}': {ex.Message}", ex.GetType().Name); } async Task GetElementIndexInfo() @@ -549,7 +549,7 @@ namespace Microsoft.WebAssembly.Diagnostics methodName = memberAccessExpressionSyntax.Name.ToString(); if (rootObject.IsNullValuedObject()) - throw new ExpressionEvaluationFailedException($"Expression '{memberAccessExpressionSyntax}' evaluated to null"); + throw new ReturnAsErrorException($"Expression '{memberAccessExpressionSyntax}' evaluated to null", "NullReferenceException"); } else if (expr is IdentifierNameSyntax && scopeCache.ObjectFields.TryGetValue("this", out JObject thisValue)) { @@ -558,7 +558,7 @@ namespace Microsoft.WebAssembly.Diagnostics } return (rootObject, methodName); } - catch (Exception ex) when (ex is not (ExpressionEvaluationFailedException or ReturnAsErrorException)) + catch (Exception ex) when (ex is not ReturnAsErrorException) { throw new Exception($"Unable to evaluate method '{methodName}'", ex); } @@ -683,9 +683,9 @@ namespace Microsoft.WebAssembly.Diagnostics } throw new ReturnAsErrorException($"No implementation of method '{methodName}' matching '{method}' found in type {rootObject["className"]}.", "ArgumentError"); } - catch (Exception ex) when (ex is not (ExpressionEvaluationFailedException or ReturnAsErrorException)) + catch (Exception ex) when (ex is not ReturnAsErrorException) { - throw new ExpressionEvaluationFailedException($"Unable to evaluate method '{method}': {ex.Message}", ex); + throw new ReturnAsErrorException($"Unable to evaluate method '{method}': {ex.Message}", ex.GetType().Name); } async Task FindMethodIdOnLinqEnumerable(IList typeIds, string methodName) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 2ebf69b..e910a74 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -1409,12 +1409,40 @@ namespace Microsoft.WebAssembly.Diagnostics await SendResume(sessionId, token); } } + private Result AddCallStackInfoToException(Result _error, ExecutionContext context, int scopeId) + { + try { + var retStackTrace = new JArray(); + foreach(var call in context.CallStack) + { + if (call.Id < scopeId) + continue; + retStackTrace.Add(JObject.FromObject(new + { + functionName = call.Method.Name, + scriptId = call.Location.Id.ToString(), + url = context.Store.ToUrl(call.Location), + lineNumber = call.Location.Line, + columnNumber = call.Location.Column + })); + } + if (!_error.Value.ContainsKey("exceptionDetails")) + _error.Value["exceptionDetails"] = new JObject(); + _error.Value["exceptionDetails"]["stackTrace"] = JObject.FromObject(new {callFrames = retStackTrace}); + return _error; + } + catch (Exception e) + { + logger.LogDebug($"Unable to add stackTrace information to exception. {e}"); + } + return _error; + } private async Task OnEvaluateOnCallFrame(MessageId msg_id, int scopeId, string expression, CancellationToken token) { + ExecutionContext context = GetContext(msg_id); try { - ExecutionContext context = GetContext(msg_id); if (context.CallStack == null) return false; @@ -1432,22 +1460,18 @@ namespace Microsoft.WebAssembly.Diagnostics } else { - SendResponse(msg_id, Result.Err($"Unable to evaluate '{expression}'"), token); + SendResponse(msg_id, AddCallStackInfoToException(Result.Err($"Unable to evaluate '{expression}'"), context, scopeId), token); } } catch (ReturnAsErrorException ree) { - 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); + SendResponse(msg_id, AddCallStackInfoToException(ree.Error, context, scopeId), token); } catch (Exception e) { logger.LogDebug($"Error in EvaluateOnCallFrame for expression '{expression}' with '{e}."); - SendResponse(msg_id, Result.Exception(e), token); + var exc = new ReturnAsErrorException(e.Message, e.GetType().Name); + SendResponse(msg_id, AddCallStackInfoToException(exc.Error, context, scopeId), token); } return true; diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs index efdf807..f9db1ca 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests.cs @@ -487,28 +487,32 @@ namespace DebuggerTests "DebuggerTests.EvaluateMethodTestsClass.TestEvaluate", "run", 9, "DebuggerTests.EvaluateMethodTestsClass.TestEvaluate.run", "window.setTimeout(function() { invoke_static_method ('[debugger-test] DebuggerTests.EvaluateMethodTestsClass:EvaluateMethods'); })", wait_for_event_fn: async (pause_location) => - { - var id = pause_location["callFrames"][0]["callFrameId"].Value(); + { + var id = pause_location["callFrames"][0]["callFrameId"].Value(); - var (_, res) = await EvaluateOnCallFrame(id, "this.objToTest.MyMethodWrong()", expect_ok: false ); - Assert.Equal( - $"Method 'MyMethodWrong' not found in type 'DebuggerTests.EvaluateMethodTestsClass.ParmToTest'", - res.Error["result"]?["description"]?.Value()); + var (_, res) = await EvaluateOnCallFrame(id, "this.objToTest.MyMethodWrong()", expect_ok: false ); + Assert.Equal( + $"Method 'MyMethodWrong' not found in type 'DebuggerTests.EvaluateMethodTestsClass.ParmToTest'", + res.Error["result"]?["description"]?.Value()); - (_, res) = await EvaluateOnCallFrame(id, "this.objToTest.MyMethod(1)", expect_ok: false); - Assert.Equal( - "Unable to evaluate method 'MyMethod'. Too many arguments passed.", - res.Error["result"]?["description"]?.Value()); + (_, res) = await EvaluateOnCallFrame(id, "this.objToTest.MyMethod(1)", expect_ok: false); + Assert.Equal( + "Unable to evaluate method 'MyMethod'. Too many arguments passed.", + res.Error["result"]?["description"]?.Value()); - (_, res) = await EvaluateOnCallFrame(id, "this.CallMethodWithParm(\"1\")", expect_ok: false ); - Assert.Contains("No implementation of method 'CallMethodWithParm' matching 'this.CallMethodWithParm(\"1\")' found in type DebuggerTests.EvaluateMethodTestsClass.TestEvaluate.", res.Error["result"]?["description"]?.Value()); + (_, res) = await EvaluateOnCallFrame(id, "this.CallMethodWithParm(\"1\")", expect_ok: false ); + Assert.Contains("No implementation of method 'CallMethodWithParm' matching 'this.CallMethodWithParm(\"1\")' found in type DebuggerTests.EvaluateMethodTestsClass.TestEvaluate.", res.Error["result"]?["description"]?.Value()); - (_, res) = await EvaluateOnCallFrame(id, "this.ParmToTestObjNull.MyMethod()", expect_ok: false ); - Assert.Equal("Expression 'this.ParmToTestObjNull.MyMethod' evaluated to null", res.Error["message"]?.Value()); + (_, res) = await EvaluateOnCallFrame(id, "this.ParmToTestObjNull.MyMethod()", expect_ok: false ); + Assert.Equal("Expression 'this.ParmToTestObjNull.MyMethod' evaluated to null", res.Error["result"]?["description"]?.Value()); + var exceptionDetailsStack = res.Error["exceptionDetails"]?["stackTrace"]?["callFrames"]?[0]; + Assert.Equal("DebuggerTests.EvaluateMethodTestsClass.TestEvaluate.run", exceptionDetailsStack?["functionName"]?.Value()); + Assert.Equal(358, exceptionDetailsStack?["lineNumber"]?.Value()); + Assert.Equal(16, exceptionDetailsStack?["columnNumber"]?.Value());; - (_, res) = await EvaluateOnCallFrame(id, "this.ParmToTestObjException.MyMethod()", expect_ok: false ); - Assert.Equal("Method 'MyMethod' not found in type 'string'", res.Error["result"]?["description"]?.Value()); - }); + (_, res) = await EvaluateOnCallFrame(id, "this.ParmToTestObjException.MyMethod()", expect_ok: false ); + Assert.Equal("Method 'MyMethod' not found in type 'string'", res.Error["result"]?["description"]?.Value()); + }); [Fact] public async Task EvaluateSimpleMethodCallsWithoutParms() => await CheckInspectLocalsAtBreakpointSite( @@ -578,9 +582,13 @@ namespace DebuggerTests { var id = pause_location["callFrames"][0]["callFrameId"].Value(); var (_, res) = await EvaluateOnCallFrame(id, "f.idx0[2]", expect_ok: false ); - Assert.Equal("Unable to evaluate element access 'f.idx0[2]': Cannot apply indexing with [] to a primitive object of type 'number'", res.Error["message"]?.Value()); + Assert.Equal("Unable to evaluate element access 'f.idx0[2]': Cannot apply indexing with [] to a primitive object of type 'number'", res.Error["result"]?["description"]?.Value()); + var exceptionDetailsStack = res.Error["exceptionDetails"]?["stackTrace"]?["callFrames"]?[0]; + Assert.Equal("DebuggerTests.EvaluateLocalsWithIndexingTests.EvaluateLocals", exceptionDetailsStack?["functionName"]?.Value()); + Assert.Equal(556, exceptionDetailsStack?["lineNumber"]?.Value()); + Assert.Equal(12, exceptionDetailsStack?["columnNumber"]?.Value()); (_, res) = await EvaluateOnCallFrame(id, "f[1]", expect_ok: false ); - Assert.Equal( "Unable to evaluate element access 'f[1]': Cannot apply indexing with [] to an object of type 'DebuggerTests.EvaluateLocalsWithIndexingTests.TestEvaluate'", res.Error["message"]?.Value()); + Assert.Equal( "Unable to evaluate element access 'f[1]': Cannot apply indexing with [] to an object of type 'DebuggerTests.EvaluateLocalsWithIndexingTests.TestEvaluate'", res.Error["result"]?["description"]?.Value()); }); [Fact] @@ -711,7 +719,11 @@ namespace DebuggerTests // indexing with expression of a wrong type var id = pause_location["callFrames"][0]["callFrameId"].Value(); var (_, res) = await EvaluateOnCallFrame(id, "f.numList[\"a\" + 1]", expect_ok: false ); - Assert.Equal("Unable to evaluate element access 'f.numList[\"a\" + 1]': Cannot index with an object of type 'string'", res.Error["message"]?.Value()); + Assert.Equal("Unable to evaluate element access 'f.numList[\"a\" + 1]': Cannot index with an object of type 'string'", res.Error["result"]?["description"]?.Value()); + var exceptionDetailsStack = res.Error["exceptionDetails"]?["stackTrace"]?["callFrames"]?[0]; + Assert.Equal("DebuggerTests.EvaluateLocalsWithIndexingTests.EvaluateLocals", exceptionDetailsStack?["functionName"]?.Value()); + Assert.Equal(556, exceptionDetailsStack?["lineNumber"]?.Value()); + Assert.Equal(12, exceptionDetailsStack?["columnNumber"]?.Value()); }); [ConditionalFact(nameof(RunningOnChrome))] diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests2.cs b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests2.cs index e43a9f3..0453341 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests2.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/EvaluateOnCallFrameTests2.cs @@ -197,10 +197,20 @@ namespace DebuggerTests wait_for_event_fn: async (pause_location) => { var id = pause_location["callFrames"][0]["callFrameId"].Value(); + var id_prev = pause_location["callFrames"][1]["callFrameId"].Value(); var (_, res) = await EvaluateOnCallFrame(id, "DebuggerTests.EvaluateStaticFieldsInStaticClass.StaticProperty2", expect_ok: false); AssertEqual("Failed to resolve member access for DebuggerTests.EvaluateStaticFieldsInStaticClass.StaticProperty2", res.Error["result"]?["description"]?.Value(), "wrong error message"); - + var exceptionDetailsStack = res.Error["exceptionDetails"]?["stackTrace"]?["callFrames"]?[0]; + Assert.Equal("DebuggerTests.EvaluateMethodTestsClass.TestEvaluate.run", exceptionDetailsStack?["functionName"]?.Value()); + Assert.Equal(358, exceptionDetailsStack?["lineNumber"]?.Value()); + Assert.Equal(16, exceptionDetailsStack?["columnNumber"]?.Value()); + (_, res) = await EvaluateOnCallFrame(id_prev, "DebuggerTests.EvaluateStaticFieldsInStaticClass.StaticProperty2", expect_ok: false); + exceptionDetailsStack = res.Error["exceptionDetails"]?["stackTrace"]?["callFrames"]?[0]; + AssertEqual("Failed to resolve member access for DebuggerTests.EvaluateStaticFieldsInStaticClass.StaticProperty2", res.Error["result"]?["description"]?.Value(), "wrong error message"); + Assert.Equal("DebuggerTests.EvaluateMethodTestsClass.EvaluateMethods", exceptionDetailsStack?["functionName"]?.Value()); + Assert.Equal(422, exceptionDetailsStack?["lineNumber"]?.Value()); + Assert.Equal(12, exceptionDetailsStack?["columnNumber"]?.Value()); (_, res) = await EvaluateOnCallFrame(id, "DebuggerTests.InvalidEvaluateStaticClass.StaticProperty2", expect_ok: false); AssertEqual("Failed to resolve member access for DebuggerTests.InvalidEvaluateStaticClass.StaticProperty2", res.Error["result"]?["description"]?.Value(), "wrong error message"); }); -- 2.7.4