[release/6.0] [wasm][debugger] Fix pause on exception behavior (#58895)

Co-authored-by: Ankit Jain <radical@gmail.com>
Co-authored-by: Thays <thaystg@gmail.com>

diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs
index bc0f4faeb456b5e00e47301434421bb1310fc318..950acbb2dcf915fc548746ad5ff25de7d1389a2d 100644 (file)
--- a/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs
+++ b/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs
@@ -267,6 +267,14 @@ namespace Microsoft.WebAssembly.Diagnostics
         Out
     }
 
+    internal enum PauseOnExceptionsKind
+    {
+        Unset,
+        None,
+        Uncaught,
+        All
+    }
+
     internal class ExecutionContext
     {
         public string DebugId { get; set; }
@@ -279,8 +287,7 @@ namespace Microsoft.WebAssembly.Diagnostics
         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<Frame> CallStack { get; set; }
 

diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
index f6a73852d839f25f331a2b15fc96a9c2206317ea..6c1ad7cf757d9f09930e56f026f5aa9666c3d490 100644 (file)
--- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
+++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
@@ -134,9 +134,7 @@ namespace Microsoft.WebAssembly.Diagnostics
                         {
                             string exceptionError = args?["exceptionDetails"]?["exception"]?["value"]?.Value<string>();
                             if (exceptionError == sPauseOnUncaught || exceptionError == sPauseOnCaught)
-                            {
                                 return true;
-                            }
                         }
                         break;
                     }
@@ -156,17 +154,19 @@ namespace Microsoft.WebAssembly.Diagnostics
                                 if (exceptionError == sPauseOnUncaught)
                                 {
                                     await SendCommand(sessionId, "Debugger.resume", new JObject(), token);
-                                    context.PauseOnUncaught = true;
+                                    if (context.PauseOnExceptions == PauseOnExceptionsKind.Unset)
+                                        context.PauseOnExceptions = PauseOnExceptionsKind.Uncaught;
                                     return true;
                                 }
                                 if (exceptionError == sPauseOnCaught)
                                 {
                                     await SendCommand(sessionId, "Debugger.resume", new JObject(), token);
-                                    context.PauseOnCaught = true;
+                                    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<string>();
                         switch (top_func) {
@@ -442,23 +442,16 @@ namespace Microsoft.WebAssembly.Diagnostics
                 case "Debugger.setPauseOnExceptions":
                     {
                         string state = args["state"].Value<string>();
-                        if (!context.IsRuntimeReady)
+                        context.PauseOnExceptions = state switch
                         {
-                            context.PauseOnCaught = false;
-                            context.PauseOnUncaught = false;
-                            switch (state)
-                            {
-                                case "all":
-                                    context.PauseOnCaught = true;
-                                    context.PauseOnUncaught = true;
-                                    break;
-                                case "uncaught":
-                                    context.PauseOnUncaught = true;
-                                    break;
-                            }
-                        }
-                        else
-                            await SdbHelper.EnableExceptions(id, state, token);
+                            "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
                         return false;
                     }
@@ -875,7 +868,7 @@ namespace Microsoft.WebAssembly.Diagnostics
                         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<string>().Equals("message"));
+                        var exceptionObjectMessage = exceptionObject.FirstOrDefault(attr => attr["name"].Value<string>().Equals("_message"));
                         var data = JObject.FromObject(new
                         {
                             type = "object",
@@ -965,6 +958,7 @@ namespace Microsoft.WebAssembly.Diagnostics
                 {
                     context.BreakpointRequests[kvp.Key] = kvp.Value.Clone();
                 }
+                context.PauseOnExceptions = previousContext.PauseOnExceptions;
             }
 
             if (await IsRuntimeAlreadyReadyAlready(sessionId, token))
@@ -1228,10 +1222,8 @@ namespace Microsoft.WebAssembly.Diagnostics
                 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);
@@ -1393,8 +1385,19 @@ namespace Microsoft.WebAssembly.Diagnostics
             // see https://github.com/mono/mono/issues/19549 for background
             if (sessions.Add(sessionId))
             {
-                string checkUncaughtExceptions = $"throw \"{sPauseOnUncaught}\";";
-                string checkCaughtExceptions = $"try {{throw \"{sPauseOnCaught}\";}} catch {{}}";
+                string checkUncaughtExceptions = string.Empty;
+                string checkCaughtExceptions = string.Empty;
+
+                //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",

diff --git a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
index eb7d078829ac8ea727ab5242eb7cb55001535385..44eccc2d7d55af7c0c113d7b74e1c07ae94087f5 100644 (file)
--- a/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
+++ b/src/mono/wasm/debugger/BrowserDebugProxy/MonoSDBHelper.cs
@@ -2210,8 +2210,14 @@ namespace Microsoft.WebAssembly.Diagnostics
             }
             return array;
         }
-        public async Task<bool> EnableExceptions(SessionId sessionId, string state, CancellationToken token)
+        public async Task<bool> 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);
@@ -2219,14 +2225,16 @@ namespace Microsoft.WebAssembly.Diagnostics
             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

diff --git a/src/mono/wasm/debugger/DebuggerTestSuite/ExceptionTests.cs b/src/mono/wasm/debugger/DebuggerTestSuite/ExceptionTests.cs
index eb0ebe0cc0d5406fb51ef71b07211662f77b6674..33dbd2b6db64f857360874ce561cc274ba59dd6a 100644 (file)
--- a/src/mono/wasm/debugger/DebuggerTestSuite/ExceptionTests.cs
+++ b/src/mono/wasm/debugger/DebuggerTestSuite/ExceptionTests.cs
@@ -232,10 +232,11 @@ namespace DebuggerTests
             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 @@ namespace DebuggerTests
                 i++;
             }
 
-            
+
             var eval_expr = "window.setTimeout(function() { invoke_static_method (" +
                 $"'{entry_method_name}'" +
                 "); }, 1);";
@@ -270,29 +271,31 @@ namespace DebuggerTests
             {
                 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<string>());
-            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<string>(), "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<string>());
-            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 86904ca27ea879536cd677a79402f118d3f43101..ebf82dc490920001cc4988846a7c26f6d8dd3df3 100644 (file)
--- 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 @@ namespace DebuggerTests
             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();
+        }
+
+    }
 }