From bee5599ff4c02a9c7da9148bc185186f75722f6c Mon Sep 17 00:00:00 2001 From: Thays Date: Fri, 3 Sep 2021 18:31:00 -0300 Subject: [PATCH 1/6] We don't need this hacky anymore, now we receive the initial status. --- .../debugger/BrowserDebugProxy/MonoProxy.cs | 70 ++++--------------- 1 file changed, 14 insertions(+), 56 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 1f8c498cad9018..b012a150e3ed59 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -23,8 +23,6 @@ internal class MonoProxy : DevToolsProxy private static HttpClient client = new HttpClient(); private HashSet sessions = new HashSet(); private Dictionary contexts = new Dictionary(); - private const string sPauseOnUncaught = "pause_on_uncaught"; - private const string sPauseOnCaught = "pause_on_caught"; public MonoProxy(ILoggerFactory loggerFactory, IList urlSymbolServerList) : base(loggerFactory) { @@ -124,49 +122,12 @@ protected override async Task AcceptEvent(SessionId sessionId, string meth return true; } - case "Runtime.exceptionThrown": - { - // Don't process events from sessions we aren't tracking - if (!contexts.TryGetValue(sessionId, out ExecutionContext context)) - return false; - - if (!context.IsRuntimeReady) - { - string exceptionError = args?["exceptionDetails"]?["exception"]?["value"]?.Value(); - if (exceptionError == sPauseOnUncaught || exceptionError == sPauseOnCaught) - { - return true; - } - } - break; - } - case "Debugger.paused": { // Don't process events from sessions we aren't tracking if (!contexts.TryGetValue(sessionId, out ExecutionContext context)) return false; - if (!context.IsRuntimeReady) - { - string reason = args?["reason"]?.Value(); - if (reason == "exception") - { - string exceptionError = args?["data"]?["value"]?.Value(); - if (exceptionError == sPauseOnUncaught) - { - await SendCommand(sessionId, "Debugger.resume", new JObject(), token); - context.PauseOnUncaught = true; - return true; - } - if (exceptionError == sPauseOnCaught) - { - await SendCommand(sessionId, "Debugger.resume", new JObject(), token); - context.PauseOnCaught = true; - return true; - } - } - } //TODO figure out how to stich out more frames and, in particular what happens when real wasm is on the stack string top_func = args?["callFrames"]?[0]?["functionName"]?.Value(); switch (top_func) { @@ -442,22 +403,19 @@ protected override async Task AcceptCommand(MessageId id, string method, J case "Debugger.setPauseOnExceptions": { string state = args["state"].Value(); - if (!context.IsRuntimeReady) + context.PauseOnCaught = false; + context.PauseOnUncaught = false; + switch (state) { - context.PauseOnCaught = false; - context.PauseOnUncaught = false; - switch (state) - { - case "all": - context.PauseOnCaught = true; - context.PauseOnUncaught = true; - break; - case "uncaught": - context.PauseOnUncaught = true; - break; - } + case "all": + context.PauseOnCaught = true; + context.PauseOnUncaught = true; + break; + case "uncaught": + context.PauseOnUncaught = true; + break; } - else + if (context.IsRuntimeReady) await SdbHelper.EnableExceptions(id, state, token); // Pass this on to JS too return false; @@ -964,6 +922,8 @@ private async Task OnDefaultContext(SessionId sessionId, ExecutionContext contex { context.BreakpointRequests[kvp.Key] = kvp.Value.Clone(); } + context.PauseOnCaught = previousContext.PauseOnCaught; + context.PauseOnUncaught = previousContext.PauseOnUncaught; } if (await IsRuntimeAlreadyReadyAlready(sessionId, token)) @@ -1392,12 +1352,10 @@ private async Task AttachToTarget(SessionId sessionId, CancellationToken token) // see https://github.com/mono/mono/issues/19549 for background if (sessions.Add(sessionId)) { - string checkUncaughtExceptions = $"throw \"{sPauseOnUncaught}\";"; - string checkCaughtExceptions = $"try {{throw \"{sPauseOnCaught}\";}} catch {{}}"; await SendMonoCommand(sessionId, new MonoCommands("globalThis.dotnetDebugger = true"), token); Result res = await SendCommand(sessionId, "Page.addScriptToEvaluateOnNewDocument", - JObject.FromObject(new { source = $"globalThis.dotnetDebugger = true; delete navigator.constructor.prototype.webdriver; {checkCaughtExceptions} {checkUncaughtExceptions}" }), + JObject.FromObject(new { source = $"globalThis.dotnetDebugger = true; delete navigator.constructor.prototype.webdriver;" }), token); if (sessionId != SessionId.Null && !res.IsOk) From cd52ade4452ed336833eb86d0b2726d2bcd0c24f Mon Sep 17 00:00:00 2001 From: "DESKTOP-GEPIA6N\\Thays" Date: Mon, 6 Sep 2021 10:49:08 -0300 Subject: [PATCH 2/6] Fix for debugging with and without vs. --- .../BrowserDebugProxy/DevToolsHelper.cs | 11 ++- .../debugger/BrowserDebugProxy/MonoProxy.cs | 76 +++++++++++++++---- .../BrowserDebugProxy/MonoSDBHelper.cs | 8 +- 3 files changed, 76 insertions(+), 19 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs index bc0f4faeb456b5..950acbb2dcf915 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs @@ -267,6 +267,14 @@ internal enum StepKind Out } + internal enum PauseOnExceptionsKind + { + Unset, + None, + Uncaught, + All + } + internal class ExecutionContext { public string DebugId { get; set; } @@ -279,8 +287,7 @@ internal class ExecutionContext public int Id { get; set; } public object AuxData { get; set; } - public bool PauseOnUncaught { get; set; } - public bool PauseOnCaught { get; set; } + public PauseOnExceptionsKind PauseOnExceptions { get; set; } public List CallStack { get; set; } diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index b012a150e3ed59..49770709a6eb25 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -23,6 +23,8 @@ internal class MonoProxy : DevToolsProxy private static HttpClient client = new HttpClient(); private HashSet sessions = new HashSet(); private Dictionary contexts = new Dictionary(); + private const string sPauseOnUncaught = "pause_on_uncaught"; + private const string sPauseOnCaught = "pause_on_caught"; public MonoProxy(ILoggerFactory loggerFactory, IList urlSymbolServerList) : base(loggerFactory) { @@ -122,12 +124,48 @@ protected override async Task AcceptEvent(SessionId sessionId, string meth return true; } + case "Runtime.exceptionThrown": + { + // Don't process events from sessions we aren't tracking + if (!contexts.TryGetValue(sessionId, out ExecutionContext context)) + return false; + + if (!context.IsRuntimeReady) + { + string exceptionError = args?["exceptionDetails"]?["exception"]?["value"]?.Value(); + if (exceptionError == sPauseOnUncaught || exceptionError == sPauseOnCaught) + return true; + } + break; + } + case "Debugger.paused": { // Don't process events from sessions we aren't tracking if (!contexts.TryGetValue(sessionId, out ExecutionContext context)) return false; + if (!context.IsRuntimeReady) + { + string reason = args?["reason"]?.Value(); + if (reason == "exception") + { + string exceptionError = args?["data"]?["value"]?.Value(); + if (exceptionError == sPauseOnUncaught) + { + await SendCommand(sessionId, "Debugger.resume", new JObject(), token); + context.PauseOnExceptions = PauseOnExceptionsKind.Uncaught; + return true; + } + if (exceptionError == sPauseOnCaught) + { + await SendCommand(sessionId, "Debugger.resume", new JObject(), token); + context.PauseOnExceptions = PauseOnExceptionsKind.All; + return true; + } + } + } + //TODO figure out how to stich out more frames and, in particular what happens when real wasm is on the stack string top_func = args?["callFrames"]?[0]?["functionName"]?.Value(); switch (top_func) { @@ -403,20 +441,20 @@ protected override async Task AcceptCommand(MessageId id, string method, J case "Debugger.setPauseOnExceptions": { string state = args["state"].Value(); - context.PauseOnCaught = false; - context.PauseOnUncaught = false; switch (state) { case "all": - context.PauseOnCaught = true; - context.PauseOnUncaught = true; + context.PauseOnExceptions = PauseOnExceptionsKind.All; break; case "uncaught": - context.PauseOnUncaught = true; + context.PauseOnExceptions = PauseOnExceptionsKind.Uncaught; + break; + case "none": + context.PauseOnExceptions = PauseOnExceptionsKind.None; break; } if (context.IsRuntimeReady) - await SdbHelper.EnableExceptions(id, state, token); + await SdbHelper.EnableExceptions(id, context.PauseOnExceptions, token); // Pass this on to JS too return false; } @@ -832,7 +870,7 @@ private async Task OnReceiveDebuggerAgentEvent(SessionId sessionId, JObjec int object_id = retDebuggerCmdReader.ReadInt32(); var caught = retDebuggerCmdReader.ReadByte(); var exceptionObject = await SdbHelper.GetObjectValues(sessionId, object_id, GetObjectCommandOptions.WithProperties | GetObjectCommandOptions.OwnProperties, token); - var exceptionObjectMessage = exceptionObject.FirstOrDefault(attr => attr["name"].Value().Equals("message")); + var exceptionObjectMessage = exceptionObject.FirstOrDefault(attr => attr["name"].Value().Equals("_message")); var data = JObject.FromObject(new { type = "object", @@ -922,8 +960,7 @@ private async Task OnDefaultContext(SessionId sessionId, ExecutionContext contex { context.BreakpointRequests[kvp.Key] = kvp.Value.Clone(); } - context.PauseOnCaught = previousContext.PauseOnCaught; - context.PauseOnUncaught = previousContext.PauseOnUncaught; + context.PauseOnExceptions = previousContext.PauseOnExceptions; } if (await IsRuntimeAlreadyReadyAlready(sessionId, token)) @@ -1187,10 +1224,8 @@ private async Task RuntimeReady(SessionId sessionId, CancellationTok Log("verbose", $"Failed to clear breakpoints"); } - if (context.PauseOnCaught && context.PauseOnUncaught) - await SdbHelper.EnableExceptions(sessionId, "all", token); - else if (context.PauseOnUncaught) - await SdbHelper.EnableExceptions(sessionId, "uncaught", token); + if (context.PauseOnExceptions != PauseOnExceptionsKind.None && context.PauseOnExceptions != PauseOnExceptionsKind.Unset) + await SdbHelper.EnableExceptions(sessionId, context.PauseOnExceptions, token); await SdbHelper.SetProtocolVersion(sessionId, token); await SdbHelper.EnableReceiveRequests(sessionId, EventKind.UserBreak, token); @@ -1352,10 +1387,23 @@ private async Task AttachToTarget(SessionId sessionId, CancellationToken token) // see https://github.com/mono/mono/issues/19549 for background if (sessions.Add(sessionId)) { + string checkUncaughtExceptions = ""; + string checkCaughtExceptions = ""; + + //we only need this check if it's a non-vs debugging + if (sessionId == SessionId.Null) + { + if (!contexts.TryGetValue(sessionId, out ExecutionContext context) || context.PauseOnExceptions == PauseOnExceptionsKind.Unset) + { + checkUncaughtExceptions = $"throw \"{sPauseOnUncaught}\";"; + checkCaughtExceptions = $"try {{throw \"{sPauseOnCaught}\";}} catch {{}}"; + } + } + await SendMonoCommand(sessionId, new MonoCommands("globalThis.dotnetDebugger = true"), token); Result res = await SendCommand(sessionId, "Page.addScriptToEvaluateOnNewDocument", - JObject.FromObject(new { source = $"globalThis.dotnetDebugger = true; delete navigator.constructor.prototype.webdriver;" }), + JObject.FromObject(new { source = $"globalThis.dotnetDebugger = true; delete navigator.constructor.prototype.webdriver; {checkCaughtExceptions} {checkUncaughtExceptions}" }), token); if (sessionId != SessionId.Null && !res.IsOk) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index eb7d078829ac8e..45e7414bb9a2ff 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -2210,7 +2210,7 @@ public async Task GetArrayValues(SessionId sessionId, int arrayId, Cance } return array; } - public async Task EnableExceptions(SessionId sessionId, string state, CancellationToken token) + public async Task EnableExceptions(SessionId sessionId, PauseOnExceptionsKind state, CancellationToken token) { var commandParams = new MemoryStream(); var commandParamsWriter = new MonoBinaryWriter(commandParams); @@ -2219,14 +2219,16 @@ public async Task EnableExceptions(SessionId sessionId, string state, Canc commandParamsWriter.Write((byte)1); commandParamsWriter.Write((byte)ModifierKind.ExceptionOnly); commandParamsWriter.Write(0); //exc_class - if (state == "all") + if (state == PauseOnExceptionsKind.All) commandParamsWriter.Write((byte)1); //caught else commandParamsWriter.Write((byte)0); //caught - if (state == "uncaught" || state == "all") + + if (state == PauseOnExceptionsKind.Uncaught || state == PauseOnExceptionsKind.All) commandParamsWriter.Write((byte)1); //uncaught else commandParamsWriter.Write((byte)0); //uncaught + commandParamsWriter.Write((byte)1);//subclasses commandParamsWriter.Write((byte)0);//not_filtered_feature commandParamsWriter.Write((byte)0);//everything_else From 5e70ca74ac34fe2e5c3bdd73fc19cceb30342e6c Mon Sep 17 00:00:00 2001 From: Thays Date: Mon, 6 Sep 2021 11:03:54 -0300 Subject: [PATCH 3/6] Fix test behavior. --- src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 49770709a6eb25..3c5df37f1881d9 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -154,7 +154,8 @@ protected override async Task AcceptEvent(SessionId sessionId, string meth if (exceptionError == sPauseOnUncaught) { await SendCommand(sessionId, "Debugger.resume", new JObject(), token); - context.PauseOnExceptions = PauseOnExceptionsKind.Uncaught; + if (context.PauseOnExceptions == PauseOnExceptionsKind.Unset) + context.PauseOnExceptions = PauseOnExceptionsKind.Uncaught; return true; } if (exceptionError == sPauseOnCaught) From 9d00648bdba2f96f441a37a4822bf2d3cd0d9a0e Mon Sep 17 00:00:00 2001 From: Thays Date: Mon, 6 Sep 2021 11:16:17 -0300 Subject: [PATCH 4/6] Adding more tests. --- .../DebuggerTestSuite/ExceptionTests.cs | 25 +++++++------ .../debugger-test/debugger-exception-test.cs | 36 +++++++++++++++++++ 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/ExceptionTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/ExceptionTests.cs index eb0ebe0cc0d540..33dbd2b6db64f8 100644 --- a/src/mono/wasm/debugger/DebuggerTestSuite/ExceptionTests.cs +++ b/src/mono/wasm/debugger/DebuggerTestSuite/ExceptionTests.cs @@ -232,10 +232,11 @@ await CheckValue(pause_location["data"], JObject.FromObject(new CheckString(exception_members, "message", "not implemented uncaught"); } - [Fact] - public async Task ExceptionTestAllWithReload() + [Theory] + [InlineData("[debugger-test] DebuggerTests.ExceptionTestsClassDefault:TestExceptions", "System.Exception", 76)] + [InlineData("[debugger-test] DebuggerTests.ExceptionTestsClass:TestExceptions", "DebuggerTests.CustomException", 28)] + public async Task ExceptionTestAllWithReload(string entry_method_name, string class_name, int line_number) { - string entry_method_name = "[debugger-test] DebuggerTests.ExceptionTestsClass:TestExceptions"; var debugger_test_loc = "dotnet://debugger-test.dll/debugger-exception-test.cs"; await SetPauseOnException("all"); @@ -255,7 +256,7 @@ public async Task ExceptionTestAllWithReload() i++; } - + var eval_expr = "window.setTimeout(function() { invoke_static_method (" + $"'{entry_method_name}'" + "); }, 1);"; @@ -270,29 +271,31 @@ await CheckValue(pause_location["data"], JObject.FromObject(new { type = "object", subtype = "error", - className = "DebuggerTests.CustomException", - uncaught = false + className = class_name, + uncaught = false, + description = "not implemented caught" }), "exception0.data"); var exception_members = await GetProperties(pause_location["data"]["objectId"]?.Value()); - CheckString(exception_members, "message", "not implemented caught"); + CheckString(exception_members, "_message", "not implemented caught"); pause_location = await WaitForManagedException(null); AssertEqual("run", pause_location["callFrames"]?[0]?["functionName"]?.Value(), "pause1"); //stop in the uncaught exception - CheckLocation(debugger_test_loc, 28, 16, scripts, pause_location["callFrames"][0]["location"]); + CheckLocation(debugger_test_loc, line_number, 16, scripts, pause_location["callFrames"][0]["location"]); await CheckValue(pause_location["data"], JObject.FromObject(new { type = "object", subtype = "error", - className = "DebuggerTests.CustomException", - uncaught = true + className = class_name, + uncaught = true, + description = "not implemented uncaught" }), "exception1.data"); exception_members = await GetProperties(pause_location["data"]["objectId"]?.Value()); - CheckString(exception_members, "message", "not implemented uncaught"); + CheckString(exception_members, "_message", "not implemented uncaught"); } diff --git a/src/mono/wasm/debugger/tests/debugger-test/debugger-exception-test.cs b/src/mono/wasm/debugger/tests/debugger-test/debugger-exception-test.cs index 86904ca27ea879..ebf82dc4909200 100644 --- a/src/mono/wasm/debugger/tests/debugger-test/debugger-exception-test.cs +++ b/src/mono/wasm/debugger/tests/debugger-test/debugger-exception-test.cs @@ -52,4 +52,40 @@ public CustomException(string message) this.message = message; } } + + public class ExceptionTestsClassDefault + { + public class TestCaughtException + { + public void run() + { + try + { + throw new Exception("not implemented caught"); + } + catch + { + Console.WriteLine("caught exception"); + } + } + } + + public class TestUncaughtException + { + public void run() + { + throw new Exception("not implemented uncaught"); + } + } + + public static void TestExceptions() + { + TestCaughtException f = new TestCaughtException(); + f.run(); + + TestUncaughtException g = new TestUncaughtException(); + g.run(); + } + + } } From 8e33c74e879600881068b754eff34cd14400ea7b Mon Sep 17 00:00:00 2001 From: Thays Grazia Date: Wed, 8 Sep 2021 13:46:55 -0300 Subject: [PATCH 5/6] Update src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs Co-authored-by: Ankit Jain --- src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index 3c5df37f1881d9..edb04036c474be 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -1388,8 +1388,8 @@ private async Task AttachToTarget(SessionId sessionId, CancellationToken token) // see https://github.com/mono/mono/issues/19549 for background if (sessions.Add(sessionId)) { - string checkUncaughtExceptions = ""; - string checkCaughtExceptions = ""; + string checkUncaughtExceptions = string.Empty; + string checkCaughtExceptions = string.Empty; //we only need this check if it's a non-vs debugging if (sessionId == SessionId.Null) From a75d494e398cd3b2901013ed0cbebf5a607b0c5f Mon Sep 17 00:00:00 2001 From: Thays Date: Wed, 8 Sep 2021 14:20:05 -0300 Subject: [PATCH 6/6] Addressing @radical comments. --- .../debugger/BrowserDebugProxy/MonoProxy.cs | 18 +++++++----------- .../BrowserDebugProxy/MonoSDBHelper.cs | 6 ++++++ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs index edb04036c474be..9ac22527877056 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs @@ -442,18 +442,14 @@ protected override async Task AcceptCommand(MessageId id, string method, J case "Debugger.setPauseOnExceptions": { string state = args["state"].Value(); - switch (state) + context.PauseOnExceptions = state switch { - case "all": - context.PauseOnExceptions = PauseOnExceptionsKind.All; - break; - case "uncaught": - context.PauseOnExceptions = PauseOnExceptionsKind.Uncaught; - break; - case "none": - context.PauseOnExceptions = PauseOnExceptionsKind.None; - break; - } + "all" => PauseOnExceptionsKind.All, + "uncaught" => PauseOnExceptionsKind.Uncaught, + "none" => PauseOnExceptionsKind.None, + _ => PauseOnExceptionsKind.Unset + }; + if (context.IsRuntimeReady) await SdbHelper.EnableExceptions(id, context.PauseOnExceptions, token); // Pass this on to JS too diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs index 45e7414bb9a2ff..44eccc2d7d55af 100644 --- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs +++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs @@ -2212,6 +2212,12 @@ public async Task GetArrayValues(SessionId sessionId, int arrayId, Cance } public async Task EnableExceptions(SessionId sessionId, PauseOnExceptionsKind state, CancellationToken token) { + if (state == PauseOnExceptionsKind.Unset) + { + logger.LogDebug($"Trying to setPauseOnExceptions using status Unset"); + return false; + } + var commandParams = new MemoryStream(); var commandParamsWriter = new MonoBinaryWriter(commandParams); commandParamsWriter.Write((byte)EventKind.Exception);