[dotnet] Refactor WebSocket communication for BiDi #12614
Conversation
Replaces the existing WebSocket communication mechanism with one more robust. Rather than immediately relying on event handlers to react to events and command reponses, it writes the incoming data to a queue which is read from a different thread. This eliminates the issue where the user might have multiple simultaneous sends or receives to the WebSocket while their event handler is running. It also dispatches incoming events on different threads for the same reason. This should eliminate at least some of the issues surrounding socket communication with bidirectional features, whether implemented using CDP or the WebDriver BiDi protocol.
titusfortner
force-pushed
the
dotnet-websocket-stability
branch
from
47ae07a to
fb27554
Compare
Codecov ReportPatch and project coverage have no change.
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## trunk #12614 +/- ##
=======================================
Coverage 57.02% 57.02%
=======================================
Files 86 86
Lines 5322 5322
Branches 192 192
=======================================
Hits 3035 3035
Misses 2095 2095
Partials 192 192 ☔ View full report in Codecov by Sentry. |
…iumHQ/selenium into dotnet-websocket-stability
There was a problem hiding this comment.
Seems we don't need to put binaries to third_party/dotnet/nuget, please double check.
Looks good to me according current level of communication understanding. Thanks @jimevans for refactoring it.
|
@nvborisenko One was a new addition, which I forgot to add after removing the Channel-based implementation. The other was an update of a version of one already added. I've reverted that version upgrade. I believe we are ready to go with this one. |
* [dotnet] Refactor WebSocket communication for BiDi Replaces the existing WebSocket communication mechanism with one more robust. Rather than immediately relying on event handlers to react to events and command reponses, it writes the incoming data to a queue which is read from a different thread. This eliminates the issue where the user might have multiple simultaneous sends or receives to the WebSocket while their event handler is running. It also dispatches incoming events on different threads for the same reason. This should eliminate at least some of the issues surrounding socket communication with bidirectional features, whether implemented using CDP or the WebDriver BiDi protocol. * Address review comments * Removing use of System.Threading.Channels * nitpick: fix XML doc comment * Simplify WebSocket message queue processing code * Omit added test from Firefox * revert add of now unused nuget packages
Description
Replaces the existing WebSocket communication mechanism with one more robust. Rather than immediately relying on event handlers to react to events and command reponses, it writes the incoming data to a queue which is read from a different thread. This eliminates the issue where the user might have multiple simultaneous sends or receives to the WebSocket while their event handler is running. It also dispatches incoming events on different threads for the same reason. This should eliminate at least some of the issues surrounding socket communication with bidirectional features, whether implemented using CDP or the WebDriver BiDi protocol.
Motivation and Context
In the .NET WebSocket client implementation, one can have simultaneous sends and receives, but only one of each type. This can lead to unexpected errors for users attempting to use BiDI-enabled features like network interception, where it's common to respond to an event using an event handler, and issuing another command from within the event handler body. Specific instance of a reported issue this should solve is #11536 and #10054.
Types of changes
Checklist