Skip to content

[dotnet][bidi] fix CS0121 ambiguous ConfigureAwait on IEventStream<T>#17711

Open
Chandan25sharma wants to merge 1 commit into
SeleniumHQ:trunkfrom
Chandan25sharma:dotnet-fix-eventstream-configureawait
Open

[dotnet][bidi] fix CS0121 ambiguous ConfigureAwait on IEventStream<T>#17711
Chandan25sharma wants to merge 1 commit into
SeleniumHQ:trunkfrom
Chandan25sharma:dotnet-fix-eventstream-configureawait

Conversation

@Chandan25sharma

Copy link
Copy Markdown
Contributor

Problem

IEventStream inherits both IAsyncEnumerable and IAsyncDisposable. System.Linq.Async ships a ConfigureAwait overload for each of those interfaces in TaskAsyncEnumerableExtensions:

ConfigureAwait(IAsyncEnumerable, bool) -> ConfiguredCancelableAsyncEnumerable
ConfigureAwait(IAsyncDisposable, bool) -> ConfiguredAsyncDisposable

Both overloads match when the receiver is an IEventStream, so the compiler emits CS0121 ("The call is ambiguous between ..."). Users had to add an explicit cast to either interface to work around this, which is both surprising and hard to discover.

Fix

Add EventStreamExtensions.ConfigureAwait(this IEventStream, bool) that explicitly delegates to the IAsyncEnumerable overload. Because a more-derived receiver type takes priority over the two BCL overloads, the compiler now resolves the call unambiguously without any cast at the call site. The IAsyncEnumerable variant is the correct choice here since callers use ConfigureAwait to control context capture inside await foreach loops, not during disposal.

Testing

EventStreamExtensionsTests verifies at the type level that the return type is ConfiguredCancelableAsyncEnumerable (not ConfiguredAsyncDisposable) and at the behavioural level that events flow through a ConfigureAwait(false) enumeration correctly.

Fixes: #17696

Problem
-------
IEventStream<T> inherits both IAsyncEnumerable<T> and IAsyncDisposable.
System.Linq.Async ships a ConfigureAwait overload for each of those
interfaces in TaskAsyncEnumerableExtensions:

  ConfigureAwait<T>(IAsyncEnumerable<T>, bool)   -> ConfiguredCancelableAsyncEnumerable<T>
  ConfigureAwait(IAsyncDisposable, bool)          -> ConfiguredAsyncDisposable

Both overloads match when the receiver is an IEventStream<T>, so the
compiler emits CS0121 ("The call is ambiguous between ...").  Users had
to add an explicit cast to either interface to work around this, which
is both surprising and hard to discover.

Fix
---
Add EventStreamExtensions.ConfigureAwait<T>(this IEventStream<T>, bool)
that explicitly delegates to the IAsyncEnumerable<T> overload.  Because
a more-derived receiver type takes priority over the two BCL overloads,
the compiler now resolves the call unambiguously without any cast at the
call site.  The IAsyncEnumerable<T> variant is the correct choice here
since callers use ConfigureAwait to control context capture inside
`await foreach` loops, not during disposal.

Testing
-------
EventStreamExtensionsTests verifies at the type level that the return
type is ConfiguredCancelableAsyncEnumerable<T> (not ConfiguredAsyncDisposable)
and at the behavioural level that events flow through a
ConfigureAwait(false) enumeration correctly.

Fixes: SeleniumHQ#17696

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@selenium-ci selenium-ci added the C-dotnet .NET Bindings label Jun 23, 2026
@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Fix CS0121 ConfigureAwait ambiguity for BiDi IEventStream
🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

Description

• Add IEventStream.ConfigureAwait extension to resolve CS0121 overload ambiguity.
• Route calls to the IAsyncEnumerable ConfigureAwait used by await foreach.
• Add tests asserting return type and event delivery with ConfigureAwait(false).
Diagram

graph TD
  Caller["BiDi consumer code"] --> Stream["IEventStream<T>"] --> Ext["EventStreamExtensions.ConfigureAwait"] --> BclEnum{{"BCL ConfigureAwait (IAsyncEnumerable)"}} --> Configured["ConfiguredCancelableAsyncEnumerable<T>"]
  Stream --> BclDisp{{"BCL ConfigureAwait (IAsyncDisposable)"}}
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Require explicit casts at call sites
  • ➕ No library change; keeps API surface minimal
  • ➖ Hard to discover; brittle and noisy for users; repeats everywhere the stream is used
2. Change IEventStream inheritance shape
  • ➕ Eliminates the root cause by removing dual-interface ConfigureAwait candidates
  • ➖ Breaking change (removing IAsyncDisposable or IAsyncEnumerable); impacts existing consumers and implementation expectations
3. Introduce a differently-named helper (e.g., ConfigureAwaitEnumerable)
  • ➕ Avoids overload name collision entirely
  • ➖ Less idiomatic; users still expect .ConfigureAwait on async enumerables; migration/documentation burden

Recommendation: Keep the PR’s approach. Providing an IEventStream-specific ConfigureAwait preserves the expected .ConfigureAwait call pattern, fixes the compiler ambiguity without breaking changes, and correctly biases toward the enumerable behavior needed by await foreach. The included tests cover both the chosen return type and basic event flow through configured enumeration.

Files changed (2) +135 / -0

Bug fix (1) +51 / -0
EventStreamExtensions.csAdd IEventStream<T>.ConfigureAwait extension to disambiguate overloads +51/-0

Add IEventStream<T>.ConfigureAwait extension to disambiguate overloads

• Introduces EventStreamExtensions.ConfigureAwait<TEventArgs>(this IEventStream<TEventArgs>, bool) returning ConfiguredCancelableAsyncEnumerable<TEventArgs>. The method explicitly casts to IAsyncEnumerable<T> to route to the enumerable ConfigureAwait overload, preventing CS0121 ambiguity when the receiver also implements IAsyncDisposable.

dotnet/src/webdriver/BiDi/EventStreamExtensions.cs

Tests (1) +84 / -0
EventStreamExtensionsTests.csAdd tests for ConfigureAwait behavior and return type on event streams +84/-0

Add tests for ConfigureAwait behavior and return type on event streams

• Adds unit coverage verifying that stream.ConfigureAwait(false) compiles and returns ConfiguredCancelableAsyncEnumerable<T>. Also validates that events can be received via await foreach over the configured enumerable using the FakeTransport test harness.

dotnet/test/webdriver/BiDi/EventStreamExtensionsTests.cs

@qodo-code-review

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 15 rules

Grey Divider


Action required

1. Public EventStreamExtensions lacks <summary> 📘 Rule violation ✧ Quality
Description
The new public type EventStreamExtensions is introduced without an XML documentation comment
containing a non-empty <summary>, which violates the requirement for documenting all public API
members. Missing docs reduce API discoverability and can break documentation generation pipelines.
Code

dotnet/src/webdriver/BiDi/EventStreamExtensions.cs[R22-25]

+namespace OpenQA.Selenium.BiDi;
+
+public static class EventStreamExtensions
+{
Evidence
PR Compliance ID 389245 requires a /// XML doc block with a non-empty <summary> for all public
API members. The new public static class EventStreamExtensions is declared without any such XML
documentation immediately above it.

Rule 389245: Require XML documentation with <summary> for all public API members
dotnet/src/webdriver/BiDi/EventStreamExtensions.cs[22-25]

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 new public API type `EventStreamExtensions` is missing an XML doc comment with a non-empty `<summary>` immediately preceding the declaration.

## Issue Context
The compliance checklist requires XML documentation with `<summary>` for all public types and members introduced or modified in the diff.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/EventStreamExtensions.cs[22-25]

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


Grey Divider

Qodo Logo

Comment on lines +22 to +25
namespace OpenQA.Selenium.BiDi;

public static class EventStreamExtensions
{

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.

Action required

1. Public eventstreamextensions lacks

📘 Rule violation ✧ Quality

The new public type EventStreamExtensions is introduced without an XML documentation comment
containing a non-empty <summary>, which violates the requirement for documenting all public API
members. Missing docs reduce API discoverability and can break documentation generation pipelines.
Agent Prompt
## Issue description
A new public API type `EventStreamExtensions` is missing an XML doc comment with a non-empty `<summary>` immediately preceding the declaration.

## Issue Context
The compliance checklist requires XML documentation with `<summary>` for all public types and members introduced or modified in the diff.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/EventStreamExtensions.cs[22-25]

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

@nvborisenko

Copy link
Copy Markdown
Member

On hold. This extension method doesn't resolve the issue. Why ConfigureAwait() over iteration instead of disposal, hidden magic.

Potentially would be resolved by #17705

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.

[🚀 Feature]: [dotnet][bidi] IEventStream<>.ConfigureAwait() call is ambiguous

3 participants