Skip to content

[dotnet] [bidi] Don't propagate cancellation token for websocket - #17220

Merged
nvborisenko merged 1 commit into
SeleniumHQ:trunkfrom
nvborisenko:bidi-socket-cancellation
Mar 13, 2026
Merged

nvborisenko merged 1 commit into
SeleniumHQ:trunkfrom
nvborisenko:bidi-socket-cancellation

Conversation

@nvborisenko

Copy link
Copy Markdown
Member

To avoid transition websocket to Aborted state.

💥 What does this PR do?

This pull request makes a small change to how cancellation tokens are handled when sending data over a WebSocket in the WebSocketTransport.cs file. The cancellation token has been replaced with CancellationToken.None to ensure that send operations are not cancelled prematurely.

🔄 Types of changes

  • Bug fix (backwards compatible)

Copilot AI review requested due to automatic review settings March 13, 2026 12:36
@qodo-code-review

Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Prevent WebSocket cancellation in BiDi transport send operations

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Replace cancellation token with CancellationToken.None in WebSocket send operations
• Prevents WebSocket transition to Aborted state during cancellation
• Applies to both NET8_0_OR_GREATER and earlier framework versions

Grey Divider

File Changes

1. dotnet/src/webdriver/BiDi/WebSocketTransport.cs 🐞 Bug fix +2/-2

Replace cancellation token with None in WebSocket sends

• Replaced cancellationToken parameter with CancellationToken.None in two SendAsync calls
• Changes apply to both .NET 8.0+ and earlier framework conditional compilation branches
• Prevents premature cancellation of WebSocket send operations

dotnet/src/webdriver/BiDi/WebSocketTransport.cs


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Mar 13, 2026 •

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (3) 📎 Requirement gaps (0)

Grey Divider


Action required

1. SendAsync ignores cancellationToken 📘 Rule violation ⛯ Reliability
Description
WebSocketTransport.SendAsync no longer passes the caller-provided cancellationToken to the
underlying _webSocket.SendAsync, changing cancellation behavior for a public method. Callers that
relied on cancellation to stop an in-flight send may now observe different (potentially hanging)
behavior.
Code

dotnet/src/webdriver/BiDi/WebSocketTransport.cs[R110-117]

+            await _webSocket.SendAsync(data, WebSocketMessageType.Text, true, CancellationToken.None).ConfigureAwait(false);
#else
            if (!System.Runtime.InteropServices.MemoryMarshal.TryGetArray(data, out ArraySegment<byte> segment))
            {
                segment = new ArraySegment<byte>(data.ToArray());
            }

-            await _webSocket.SendAsync(segment, WebSocketMessageType.Text, true, cancellationToken).ConfigureAwait(false);
+            await _webSocket.SendAsync(segment, WebSocketMessageType.Text, true, CancellationToken.None).ConfigureAwait(false);
Evidence
PR Compliance ID 1 requires preserving user-visible behavioral contracts of public interfaces unless
explicitly handled; the changed lines show _webSocket.SendAsync(...) now uses
CancellationToken.None rather than the provided cancellationToken, altering observable
cancellation semantics.

AGENTS.md
dotnet/src/webdriver/BiDi/WebSocketTransport.cs[103-117]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`WebSocketTransport.SendAsync(..., cancellationToken)` no longer propagates the caller token to `_webSocket.SendAsync`, changing the public method’s cancellation behavior.

## Issue Context
The PR intentionally switches to `CancellationToken.None` for send operations; however, the public API still accepts a `CancellationToken`, so the behavioral contract should be explicitly defined and (where reasonable) preserved.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/WebSocketTransport.cs[103-117]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. No test for cancellation 📘 Rule violation ⛯ Reliability
Description
This change modifies send cancellation behavior but no corresponding test coverage is evident for
the new semantics. Without a test, future changes may reintroduce premature
cancellation/aborted-state regressions or unintentionally break expected cancellation behavior.
Code

dotnet/src/webdriver/BiDi/WebSocketTransport.cs[R110-117]

+            await _webSocket.SendAsync(data, WebSocketMessageType.Text, true, CancellationToken.None).ConfigureAwait(false);
#else
            if (!System.Runtime.InteropServices.MemoryMarshal.TryGetArray(data, out ArraySegment<byte> segment))
            {
                segment = new ArraySegment<byte>(data.ToArray());
            }

-            await _webSocket.SendAsync(segment, WebSocketMessageType.Text, true, cancellationToken).ConfigureAwait(false);
+            await _webSocket.SendAsync(segment, WebSocketMessageType.Text, true, CancellationToken.None).ConfigureAwait(false);
Evidence
PR Compliance ID 4 requires adding/updating tests when behavior changes are made; the PR description
states the behavior change (replacing the token with CancellationToken.None), and the modified
code confirms the new behavior path.

AGENTS.md
dotnet/src/webdriver/BiDi/WebSocketTransport.cs[110-117]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR changes cancellation behavior of WebSocket sends but does not add a test validating the new contract.

## Issue Context
BiDi behavior can be sensitive to transport edge cases; a regression test would protect against reintroducing premature cancellation/aborted socket behavior.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/WebSocketTransport.cs[103-124]
- dotnet/test/common/BiDi/BiDiFixture.cs[1-61]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. No cross-binding parity note 📘 Rule violation ⛯ Reliability
Description
The PR changes user-visible cancellation behavior in the .NET BiDi transport but does not indicate
any cross-binding parity verification or follow-up. This may lead to inconsistent BiDi behavior
across Selenium language bindings.
Code

dotnet/src/webdriver/BiDi/WebSocketTransport.cs[R110-117]

+            await _webSocket.SendAsync(data, WebSocketMessageType.Text, true, CancellationToken.None).ConfigureAwait(false);
#else
            if (!System.Runtime.InteropServices.MemoryMarshal.TryGetArray(data, out ArraySegment<byte> segment))
            {
                segment = new ArraySegment<byte>(data.ToArray());
            }

-            await _webSocket.SendAsync(segment, WebSocketMessageType.Text, true, cancellationToken).ConfigureAwait(false);
+            await _webSocket.SendAsync(segment, WebSocketMessageType.Text, true, CancellationToken.None).ConfigureAwait(false);
Evidence
PR Compliance ID 5 requires considering parity for user-visible behavior changes; the PR description
and diff show a .NET-only change to WebSocket send cancellation handling with no indication of
parallel changes elsewhere.

AGENTS.md
dotnet/src/webdriver/BiDi/WebSocketTransport.cs[110-117]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A .NET-only change alters BiDi transport cancellation semantics; parity across bindings is not documented.

## Issue Context
Low-level transport behavior differences can surprise users switching bindings or using multi-language test infrastructure.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/WebSocketTransport.cs[110-117]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts the .NET BiDi WebSocket transport send behavior to avoid transitioning the underlying ClientWebSocket into the Aborted state when a cancellation token is triggered during sends.

Changes:

  • Passes CancellationToken.None to ClientWebSocket.SendAsync on NET8_0_OR_GREATER.
  • Passes CancellationToken.None to ClientWebSocket.SendAsync on pre-.NET 8 (array-segment) path.

@nvborisenko
nvborisenko merged commit 491207a into SeleniumHQ:trunk Mar 13, 2026
20 of 21 checks passed
@nvborisenko
nvborisenko deleted the bidi-socket-cancellation branch March 13, 2026 13:07
krishnamohan-kothapalli pushed a commit to krishnamohan-kothapalli/selenium that referenced this pull request Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C-dotnet .NET Bindings

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants