[wasm][debugger] Detect initial status of pause on exceptions. #54040
Conversation
|
Tagging subscribers to this area: @thaystg Issue DetailsWhen the debugger is already "debugging" a page and the page is refreshed, the browser does not send the status of "Pause on Exceptions", with this implementation is possible to detect if the "pause on exceptions" is set to all or to uncaught.
|
|
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsWhen the debugger is already "debugging" a page and the page is refreshed, the browser does not send the status of "Pause on Exceptions", with this implementation is possible to detect if the "pause on exceptions" is set to all or to uncaught.
|
| 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; {checkUncaughtExceptions} {checkCaughtExceptions}" }), |
There was a problem hiding this comment.
doing it here is only going to work on new documents, would we be out of sync in the attach case still? does it work if you add it to the SendMonoCommand part above?
There was a problem hiding this comment.
I tested two cases: The page is already loaded and then I connect debugger, it passes here and I can get the information that is expected.
The page is reloaded with the debugger already connected, it also passes here and I can get the information.
I'll test adding it to the SendMonoCommand.
There was a problem hiding this comment.
It does not work if I add it to SendMonoCommand because it uses Runtime.Evaluate and Runtime.Evaluate does not call Debugger.Pause when an exception is threw.
| @@ -120,8 +120,40 @@ protected override async Task<bool> AcceptEvent(SessionId sessionId, string meth | |||
| return true; | |||
| } | |||
|
|
|||
| case "Runtime.exceptionThrown": | |||
| { | |||
| if (!GetContext(sessionId).IsRuntimeReady) | |||
There was a problem hiding this comment.
might need to remove this check if you were trying the on attach version
There was a problem hiding this comment.
Attach is the case when the page is already loaded and then we connect the debugger? I tested it and it's working.
There was a problem hiding this comment.
I only tested debugging on Chrome, I didn't test with VS or VSCode.
| if (!GetContext(sessionId).IsRuntimeReady) | ||
| { | ||
| string exceptionError = args?["exceptionDetails"]?["exception"]?["value"]?.Value<string>(); | ||
| if (exceptionError == "pause_on_uncaught") |
There was a problem hiding this comment.
Can you add const fields for the two strings?
| if (context.PauseOnCaught && context.PauseOnUncaught) | ||
| await SendMonoCommand(sessionId, MonoCommands.SetPauseOnExceptions("all"), token); | ||
| if (context.PauseOnUncaught) | ||
| await SendMonoCommand(sessionId, MonoCommands.SetPauseOnExceptions("uncaught"), token); |
There was a problem hiding this comment.
Won't this send both commands when context.PauseOnCaught && context.PauseOnUncaught is true?
| 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; {checkUncaughtExceptions} {checkCaughtExceptions}" }), |
There was a problem hiding this comment.
I'm not sure I understand the flow here.
- on
attach, we throw two exceptions, so we can infer the pause-on-exceptions setting of the debugger; - then I see two code paths:
a.Runtime.exceptionThrown- where we do the inference, and set the value accordingly, in the proxy, and the app, and only when runtime is not ready
b.Debugger.paused- where we resume, if it paused because of one of the above exceptions
- so, what are the actual scenarios here?
- And what does the flow of events look like, for them?
Also, it would be very useful to express that in tests too, but that might not be straight forward, so it can be in a follow up PR.
There was a problem hiding this comment.
If we attach to an already loaded page it will only pass on Runtime.exceptionThrown and this would print a message on console if I don't return true.
If we refresh a page with the debugger attached it will pass on Debugger.paused, and we run the resume, and it will also pass on Runtime.exceptionThrown that would print a message if I don't return true.
In both cases before send the runtime ready we have already got the configuration so we can set it to mono debugger.
There was a problem hiding this comment.
Oh no, I tested again, when the page is already loaded and we attach the debugger we receive the "Debugger.setPauseOnExceptions" with the correct information.
When we refresh the page that we are debugging, then we will receive the Debugger.paused and the Runtime.exceptionThrown.
There was a problem hiding this comment.
If we attach to an already loaded page it will only pass on Runtime.exceptionThrown and this would print a message on console if I don't return true.
By "pass on foo", do you mean that foo's code path will be followed?
Question for scenario #1:
- attach to already loaded page
- we send a
Page.addScriptToEvaluateOnNewDocument, with.. {checkUncaughtExceptions} {checkCaughtExceptions} - and based on your last comment, we get
Debugger.setPauseOnExceptions - And then .. do we get
Runtime.exceptionThrown? If yes, then for which case(caught/uncaught)?
If we refresh a page with the debugger attached it will pass on Debugger.paused, and we run the resume, and it will also pass on Runtime.exceptionThrown that would print a message if I don't return true.
-
Also, what happens for the
{checkUncaughtExceptions}in the command that we send.- if the setting is
uncaught, then debugger is paused, we getexceptionThrown, resume the debugger. Does the code then resume to the next line, and we executecheckCaughtExceptions`?
- if the setting is
-
Is there any case where we might want/need to get the "current" setting from the app, and use that in the proxy?
There was a problem hiding this comment.
I changed somethings in the PR.
For scenario #1:
We get Runtime.exceptionThrown in the case of the page already loaded for uncaught and we don't receive Debugger.Pause. Then we receive the Debugger.setPauseOnExceptions with the correct information.
For scenario #2:
If the setting is uncaught and we refresh the page we get:
Debugger.pause where we set the context status that will be send later to mono runtime. And the we receive a Runtime.exceptionThrown that will just return true to avoid printing the exception on the console.
If the setting is all and we refresh the page we get:
Debugger.pause where we set the context status PauseOnCaught to true then we resume, then we receive another Debugger.pause where we set the context status PauseOnUncaught to true then we resume. Then we receive Runtime.exceptionThrown for uncaught.
If while loading the page before the runtime is ready we change the status, we will receive a Debugger.setPauseOnExceptions with the right information, so I override the information that is in context for PauseOnCaught and PauseOnUncaught and when the runtime is ready I send the correct status.
We don't need the current setting from the app, the current setting should always be correct, because during the use of the debugger we receive the Debugger.setPauseOnExceptions with the correct information.
|
One other issue to check is what happens if you disconnect the debug proxy |
|
attempt to resolve #42326 |
If I disconnect the debug proxy and reload the page I don't see any message and it doesn't stop in our "forced" throw. |
I could not test the pause on "all" exceptions because if I enable the pause on caught exceptions and reload the page it will stop in a lot of exceptions other then the one that I inserted in AttachToTarget.
|
I could not test the pause on "all" exceptions because if I enable the pause on caught exceptions and reload the page it will stop in a lot of exceptions other then the one that I inserted in AttachToTarget. |
|
I added a test to "all" option reloading the page. I don't like it too much but it really test the new functionality. |
…nitial_config * origin/main: (362 commits) [wasm][debugger] Reuse debugger-agent on wasm debugger (dotnet#52300) Put Crossgen2 in sync with dotnet#54235 (dotnet#54438) Move System.Object serialization to ObjectConverter (dotnet#54436) Move setting fHasVirtualStaticMethods out of sanity check section (dotnet#54574) [wasm] Compile .bc->.o in parallel, before passing to the linker (dotnet#54053) Change PathInternal.IsCaseSensitive to a constant (dotnet#54340) Make mono_polling_required a public symbol (dotnet#54592) Respect EventSource::IsSupported setting in more codepaths (dotnet#51977) Root ComActivator for hosting (dotnet#54524) Add ILLink annotations to S.D.Common related to DbConnectionStringBuilder (dotnet#54280) Fix finalizer issue with regions (dotnet#54550) Add support for multi-arch install locations (dotnet#53763) Update library testing docs page to reduce confusion (dotnet#54324) [FileStream] handle UNC and device paths (dotnet#54483) Update NetAnalyzers version (dotnet#54511) Added runtime dependency to fix the intermittent test failures (dotnet#54587) Disable failing System.Reflection.Tests.ModuleTests.GetMethods (dotnet#54564) [wasm] Move AOT builds from `runtime-staging` to `runtime` (dotnet#54577) Keep obj node for ArrayIndex. (dotnet#54584) Disable another failing MemoryCache test (dotnet#54578) ...
ghost
locked as resolved and limited conversation to collaborators
When the debugger is already "debugging" a page and the page is refreshed, the browser does not send the status of "Pause on Exceptions", with this implementation is possible to detect if the "pause on exceptions" is set to all or to uncaught.