[dotnet] [bidi] Give only one chance to receive from remote end

[nvborisenko]

User description

BiDi Broker listens to incoming WebSocket messages and repeats even if remote end is died.

🔗 Related Issues

Fixes #16134

💥 What does this PR do?

This PR gives only one chance. As soon as it fails for any reason, we stop to listen (with writing failure to internal logs only once). It is still not resolving more interesting issue when WebSocketTransport class should behave more robust, but this is right improvement (and quick win).

🔄 Types of changes

• Bug fix (backwards compatible)

---

PR Type

Bug fix

---

Description

• Modify BiDi message receiving to fail fast on errors

• Remove retry loop for failed WebSocket connections

• Improve error handling with single attempt strategy

---

Diagram Walkthrough

flowchart LR A["Receive BiDi Messages"] --> B["Single Attempt"] B --> C["Success: Continue"] B --> D["Failure: Log & Throw"] D --> E["Stop Listening"] 

Loading

File Walkthrough

	Relevant files
Bug fix	Broker.cs Implement fail-fast BiDi message reception                              dotnet/src/webdriver/BiDi/Communication/Broker.cs Restructured error handling in ReceiveMessagesAsync method Removed infinite retry loop on WebSocket receive failures Added fail-fast behavior with single error logging Improved exception filtering to exclude OperationCanceledException +9/-7

---

[qodo-merge-pro]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 16134 - PR Code Verified Compliant requirements: Prevent repeated/infinite logging of WebSocketException(s) when the browser exits before BiDi is disposed. Ensure BiDi stops listening/receiving once the remote end dies or the socket errors. Avoid treating normal cancellations (OperationCanceledException) as errors. Requires further human verification: Validate via manual/automated runtime test that only a single error is logged when the browser process is killed. Confirm no regressions in normal shutdown/Dispose() flow across supported browsers (Firefox/Windows per ticket).

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Regression After the first non-cancellation exception, the method rethrows which may fault the receive task; verify upstream code handles this without leaving background tasks or event processing in an inconsistent state, and that reconnection (if desired elsewhere) is not required. catch (Exception ex) when (ex is not OperationCanceledException) { if (_logger.IsEnabled(LogEventLevel.Error)) { _logger.Error($"Couldn't process received BiDi remote message: {ex}"); } throw; } Logging Behavior Error is now always logged once when enabled; confirm log level and message frequency meet expectations and that OperationCanceledException is indeed the only benign case to filter. if (_logger.IsEnabled(LogEventLevel.Error)) { _logger.Error($"Couldn't process received BiDi remote message: {ex}"); }

[qodo-merge-pro]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Prevent unhandled exception from crashing application In ReceiveMessagesAsync, handle exceptions by canceling the broker's operations via _cancellationTokenSource.Cancel() instead of re-throwing the exception, which would cause an unhandled exception and crash the application. dotnet/src/webdriver/BiDi/Communication/Broker.cs [115-144] public async Task ConnectAsync(CancellationToken cancellationToken) { await _transport.ConnectAsync(_url, cancellationToken).ConfigureAwait(false); _ = ReceiveMessagesAsync(cancellationToken); _ = ProcessEventsAwaiterAsync(); } private async Task ReceiveMessagesAsync(CancellationToken cancellationToken) { try { while (!cancellationToken.IsCancellationRequested) { var data = await _transport.ReceiveAsync(cancellationToken).ConfigureAwait(false); ProcessReceivedMessage(data); } } catch (Exception ex) when (ex is not OperationCanceledException) { if (_logger.IsEnabled(LogEventLevel.Error)) { _logger.Error($"Couldn't process received BiDi remote message: {ex}"); } - throw; + // A reception error is fatal for the connection. + // Trigger cancellation for all broker operations to shut down gracefully. + _cancellationTokenSource.Cancel(); } } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 10 __ Why: The suggestion correctly identifies a critical issue introduced by the PR where an unobserved exception from a "fire-and-forget" task would crash the application, and it proposes a robust solution.	High
Update