Skip to content

[wasm][debugger] Never send messages from our internal protocol extensions to the browser #77616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 31, 2022

Conversation

thaystg
Copy link
Member

@thaystg thaystg commented Oct 28, 2022

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1644970

BrowserDebugProxy was receiving from IDE messages like DotnetDebugger.SetProperty from context that didn't exist, then it was returning false, then the message was sent to browser, and browser doesn't know this message and return an error, then it was printing an error like this:

B7CCCEF5694F2EF11713AE7F86743A45:::1018 -> [Result: IsOk: False, IsErr: True, Value: , Error: { "code": -32601, "message": "'DotnetDebugger.setDebuggerProperty' wasn't found" } ]

In this PR we avoid sending DotnetDebugger.* commands that are our own extension, and unknown to the browser.

@thaystg thaystg requested a review from radical as a code owner October 28, 2022 20:48
@ghost ghost added the area-Debugger-mono label Oct 28, 2022
@ghost ghost assigned thaystg Oct 28, 2022
@ghost
Copy link

ghost commented Oct 28, 2022

Tagging subscribers to this area: @thaystg
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1644970

Author: thaystg
Assignees: -
Labels:

area-Debugger-mono

Milestone: -

@thaystg thaystg changed the title [wasm][debugger] Ignore messages from protocol extensions even for unknown context [wasm][debugger] Ignore messages from protocol extensions even if it's from an unknown context Oct 28, 2022
@thaystg
Copy link
Member Author

thaystg commented Oct 28, 2022

/backport to release/7.0

@github-actions
Copy link
Contributor

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3348521274

@thaystg
Copy link
Member Author

thaystg commented Oct 28, 2022

/backport to release/6.0

@github-actions
Copy link
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/3348522633

@github-actions
Copy link
Contributor

@thaystg backporting to release/6.0 failed, the patch most likely resulted in conflicts:

$ git am --3way --ignore-whitespace --keep-non-patch changes.patch

Applying: ignore messages from protocol extensions even for unknown context
Using index info to reconstruct a base tree...
M	src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
Falling back to patching base and 3-way merge...
Auto-merging src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
CONFLICT (content): Merge conflict in src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 ignore messages from protocol extensions even for unknown context
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128

Please backport manually!

@radical radical changed the title [wasm][debugger] Ignore messages from protocol extensions even if it's from an unknown context [wasm][debugger] Never send messages from our internal protocol extensions to the browser Oct 28, 2022
thaystg added a commit to dotnet/core that referenced this pull request Oct 31, 2022
Adding the warning message fixed in this PR as a known issue for .net7.0:
dotnet/runtime#77616
@thaystg thaystg merged commit a640b08 into dotnet:main Oct 31, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Nov 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants