[dotnet] [bidi] Simplify event streaming to implicitly unsubscribe

[nvborisenko]

Instead of explicitly unsubscribing on the wire, we can catch the intention implicitly via ending of enumeration.

🔗 Related Issues

Implicitly resolves #17696

💥 What does this PR do?

This pull request refactors the event streaming interface in the BiDi (Bidirectional) WebDriver .NET codebase to simplify usage and align with modern .NET asynchronous patterns. The main change is replacing the custom IEventStream<TEventArgs> interface with the standard IAsyncEnumerable<TEventArgs>, and updating all related code, implementations, and tests accordingly. Additionally, resource management and cancellation handling are improved for event streams.

API and Interface Updates:

• Replaced all usages of IEventStream<TEventArgs> with IAsyncEnumerable<TEventArgs> in public interfaces such as IBiDi and IEventSource<TEventArgs>, and updated all method signatures and implementations to reflect this change. (dotnet/src/webdriver/BiDi/IBiDi.cs, dotnet/src/webdriver/BiDi/IEventSource.cs, dotnet/src/webdriver/BiDi/BiDi.cs, dotnet/src/webdriver/BiDi/ContextEventSource.cs, dotnet/src/webdriver/BiDi/EventSource.cs) [1] [2] [3] [4] [5]

• Removed the IEventStream<TEventArgs> interface and migrated its responsibilities to IAsyncEnumerable<TEventArgs>, eliminating the need for a custom event stream interface. (dotnet/src/webdriver/BiDi/IEventStream.cs)

Implementation Improvements:

• Updated the internal EventStream<TEventArgs> class to implement IAsyncEnumerable<TEventArgs> and improved cancellation token handling by introducing a custom enumerator (UnsubscribingAsyncEnumerator) that ensures proper disposal of linked cancellation tokens and the event stream itself. (dotnet/src/webdriver/BiDi/EventStream.cs) [1] [2] [3]

Test Adjustments:

• Updated all tests to use IAsyncEnumerable<TEventArgs> instead of IEventStream<TEventArgs>, removing unnecessary await using statements for event streams and adjusting disposal patterns as needed. (dotnet/test/webdriver/BiDi/Network/NetworkTests.cs, dotnet/test/webdriver/BiDi/Session/SessionTests.cs, dotnet/test/webdriver/BiDi/SessionUnitTests.cs) [1] [2] [3] [4] [5] [6] [7]

These changes modernize the event streaming API, simplify implementation and usage, and improve resource management for consumers of the BiDi WebDriver .NET library.

🔧 Implementation Notes

Good to simplify user-facing API.

🤖 AI assistance

• No substantial AI assistance used
• AI assisted (complete below)
  • Tool(s):
  • What was generated:
  • I reviewed all AI output and can explain the change

💡 Additional Considerations

Low level API, amazing simplification.

🔄 Types of changes

• Cleanup (formatting, renaming)
• Breaking change (fix or feature that would cause existing functionality to change)

[qodo-code-review]

PR Summary by Qodo

[dotnet][bidi] Use IAsyncEnumerable for event streams with implicit unsubscribe
✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

Description

• Replace custom BiDi event stream interface with standard IAsyncEnumerable for consumption.
• Implicitly unsubscribe when enumeration ends by disposing the async enumerator.
• Update BiDi event source APIs and adjust tests for the new streaming pattern.

Diagram

graph TD
  A([Consumer test/app]) --> B[["IBiDi / IEventSource" API]] --> C([EventDispatcher]) --> D([EventStream<T>]) --> E[("Channel<T>")]
  C --> F{{"BiDi wire subscribe/unsubscribe"}}
  F --> G{{"Remote end events"}} --> C
  subgraph Legend
    direction LR
    _c([Consumer/code]) ~~~ _i[[Interface/API]] ~~~ _e{{External/wire}} ~~~ _q[(Queue/channel)]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Keep IEventStream as an alias type

• ➕ Avoids breaking changes for existing consumers
• ➕ Retains explicit IAsyncDisposable on the returned type
• ➖ Continues a custom abstraction instead of idiomatic IAsyncEnumerable
• ➖ Still encourages explicit disposal rather than natural end-of-enumeration semantics

2. Return (IAsyncEnumerable, IAsyncDisposable) wrapper object

• ➕ Preserves an obvious explicit-disposal path
• ➕ Can add future metadata (subscription id, diagnostics) without API churn
• ➖ More complex API surface than returning IAsyncEnumerable directly
• ➖ Consumers still need to learn a custom wrapper type

3. Expose explicit UnsubscribeAsync token instead of disposal-driven unsubscribe

• ➕ Makes wire unsubscribe explicit and deterministic from the API perspective
• ➕ Avoids relying on enumerator disposal behavior
• ➖ More cumbersome for typical foreach/linq consumption
• ➖ Easy to leak subscriptions if callers forget to unsubscribe

Recommendation: The PR’s approach (return IAsyncEnumerable and unsubscribe on enumerator disposal) is the most idiomatic .NET streaming model and aligns with how await foreach naturally scopes lifetimes. Keeping a custom interface or wrapper would reduce breakage but would also preserve unnecessary abstraction and complexity; the current design is a good tradeoff for a low-level API that aims to be modern and easy to use.

Files changed (9) +53 / -21

Bug fix (1) +39 / -3

EventStream.csImplement IAsyncEnumerable with implicit unsubscribe on enumeration end +39/-3

Implement IAsyncEnumerable with implicit unsubscribe on enumeration end

• Makes EventStream implement IAsyncEnumerable directly and ensures disposal/unsubscribe happens when the async enumerator is disposed. Adds an internal enumerator wrapper to dispose linked cancellation token sources and then dispose the owning EventStream.

dotnet/src/webdriver/BiDi/EventStream.cs

Refactor (5) +7 / -7

BiDi.csReturn IAsyncEnumerable from BiDi StreamAsync APIs +2/-2

Return IAsyncEnumerable from BiDi StreamAsync APIs

• Changes StreamAsync overloads to return IAsyncEnumerable<TEventArgs> rather than a custom stream interface. Keeps existing subscription behavior via EventDispatcher.SubscribeReaderAsync.

dotnet/src/webdriver/BiDi/BiDi.cs

ContextEventSource.csUpdate context-scoped event source streaming signature +1/-1

Update context-scoped event source streaming signature

• Updates IEventSource implementation for context-bound event sources to return IAsyncEnumerable<TEventArgs>. Continues to delegate stream creation to the dispatcher with context and filter.

dotnet/src/webdriver/BiDi/ContextEventSource.cs

EventSource.csUpdate global event source streaming signature +1/-1

Update global event source streaming signature

• Updates non-context event source streaming to return IAsyncEnumerable<TEventArgs>. Delegates stream creation to the dispatcher with the existing filter behavior.

dotnet/src/webdriver/BiDi/EventSource.cs

IBiDi.csChange IBiDi StreamAsync return types to IAsyncEnumerable +2/-2

Change IBiDi StreamAsync return types to IAsyncEnumerable

• Updates the public IBiDi interface so StreamAsync returns IAsyncEnumerable<TEventArgs> for both single and multi-descriptor overloads. Removes the custom stream interface dependency from the public surface.

dotnet/src/webdriver/BiDi/IBiDi.cs

IEventSource.csChange IEventSource StreamAsync to return IAsyncEnumerable +1/-1

Change IEventSource StreamAsync to return IAsyncEnumerable

• Updates the event source interface to return IAsyncEnumerable<TEventArgs> for streaming consumption. Aligns event source contracts with standard async enumeration patterns.

dotnet/src/webdriver/BiDi/IEventSource.cs

Tests (3) +7 / -11

NetworkTests.csStop relying on await using for event stream lifetime +1/-1

Stop relying on await using for event stream lifetime

• Updates network tests to treat StreamAsync results as IAsyncEnumerable without explicit await using. Relies on enumeration completion/disposal semantics to end subscriptions.

dotnet/test/webdriver/BiDi/Network/NetworkTests.cs

SessionTests.csUpdate session streaming tests to use IAsyncEnumerable patterns +4/-4

Update session streaming tests to use IAsyncEnumerable patterns

• Replaces await using stream variables with plain variables and continues enumerating via GetAsyncEnumerator. Keeps cancellation-focused assertions while matching the new implicit-unsubscribe behavior.

dotnet/test/webdriver/BiDi/Session/SessionTests.cs

SessionUnitTests.csAdjust unit tests to avoid explicit stream disposal and ensure unsubscribe response +2/-6

Adjust unit tests to avoid explicit stream disposal and ensure unsubscribe response

• Removes explicit DisposeAsync calls and instead ensures unsubscribe is observed via WithResponse during enumeration operations. Updates FirstAsync flows so the unsubscribe wire response is exercised under the new disposal model.

dotnet/test/webdriver/BiDi/SessionUnitTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (2) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used

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

✅ Compliance rules (platform): 15 rules

1. IBiDi.StreamAsync return type changed 📘 Rule violation ≡ Correctness

Description

The public StreamAsync APIs now return Task<IAsyncEnumerable<TEventArgs>> instead of
Task<IEventStream<TEventArgs>>, which is a source/binary breaking change for existing consumers.
The prior event-stream abstraction is removed/invalidated without a deprecation phase and
replacement guidance.

Code

dotnet/src/webdriver/BiDi/IBiDi.cs[R68-70]

+    Task<IAsyncEnumerable<TEventArgs>> StreamAsync<TEventArgs>(EventDescriptor<TEventArgs> descriptor, CancellationToken cancellationToken = default) where TEventArgs : EventArgs;

-    Task<IEventStream<TEventArgs>> StreamAsync<TEventArgs>(ImmutableArray<EventDescriptor> descriptors, CancellationToken cancellationToken = default) where TEventArgs : EventArgs;
+    Task<IAsyncEnumerable<TEventArgs>> StreamAsync<TEventArgs>(ImmutableArray<EventDescriptor> descriptors, CancellationToken cancellationToken = default) where TEventArgs : EventArgs;

Evidence

PR Compliance ID 389266 forbids incompatible changes to existing public API signatures;
StreamAsync changed its return type in IBiDi, and the implementation in BiDi was updated
accordingly. PR Compliance ID 389271 requires a deprecation-with-guidance phase before
removing/breaking public APIs, but there is no deprecated compatibility API present for the previous
IEventStream<TEventArgs> return type.

Rule 389266: Maintain backward-compatible public API and ABI
Rule 389271: Deprecate public APIs with guidance before removal
dotnet/src/webdriver/BiDi/IBiDi.cs[68-70]
dotnet/src/webdriver/BiDi/BiDi.cs[120-130]

Agent prompt

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

## Issue description
Public BiDi streaming APIs were changed incompatibly (`IEventStream<TEventArgs>` -> `IAsyncEnumerable<TEventArgs>`), and the old surface is not preserved with a deprecation phase.

## Issue Context
This is a user-visible .NET API change that will break existing callers at compile-time and can break binaries. The compliance policy requires backward-compatible public API/ABI, and separately requires deprecating public APIs with guidance before removal.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/IBiDi.cs[68-70]
- dotnet/src/webdriver/BiDi/BiDi.cs[120-130]

## Implementation guidance (high level)
- Reintroduce a compatibility surface for existing callers, e.g. keep `StreamAsync` returning `Task<IEventStream<TEventArgs>>` (or add a differently-named method for the new `IAsyncEnumerable<TEventArgs>` return type, since return-type-only overloads are not possible in C#).
- Mark the legacy API with `[Obsolete("Use <replacement API> instead.")]` and keep it for at least one release cycle per project policy.
- Ensure the replacement API is clearly discoverable (docs + IntelliSense) and tests cover both paths during the deprecation period.

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

---

2. BiDi.StreamAsync missing XML docs 📘 Rule violation ✧ Quality

Description

The modified public StreamAsync members on BiDi, IBiDi, and IEventSource<TEventArgs> have no
/// XML documentation block with a non-empty <summary>. This violates the PR’s public API XML
documentation requirement and reduces API discoverability for changed members.

Code

dotnet/src/webdriver/BiDi/BiDi.cs[R120-130]

+    public Task<IAsyncEnumerable<TEventArgs>> StreamAsync<TEventArgs>(EventDescriptor<TEventArgs> descriptor, CancellationToken cancellationToken = default) where TEventArgs : EventArgs
    {
        ArgumentNullException.ThrowIfNull(descriptor);

        return StreamAsync<TEventArgs>([descriptor], cancellationToken);
    }

-    public async Task<IEventStream<TEventArgs>> StreamAsync<TEventArgs>(ImmutableArray<EventDescriptor> descriptors, CancellationToken cancellationToken = default) where TEventArgs : EventArgs
+    public async Task<IAsyncEnumerable<TEventArgs>> StreamAsync<TEventArgs>(ImmutableArray<EventDescriptor> descriptors, CancellationToken cancellationToken = default) where TEventArgs : EventArgs
    {
        return await EventDispatcher.SubscribeReaderAsync<TEventArgs>(descriptors, cancellationToken: cancellationToken).ConfigureAwait(false);
    }

Evidence

PR Compliance ID 389245 requires XML documentation with a <summary> for all public API members
included in the diff. The cited BiDi.StreamAsync methods, the IBiDi.StreamAsync declarations,
and IEventSource<TEventArgs>.StreamAsync are public members whose signatures were changed in this
PR, and each appears without any preceding /// XML documentation comment block containing a
non-empty <summary>, demonstrating non-compliance with the requirement.

Rule 389245: Require XML documentation with <summary> for all public API members
dotnet/src/webdriver/BiDi/BiDi.cs[120-130]
dotnet/src/webdriver/BiDi/IBiDi.cs[68-70]
dotnet/src/webdriver/BiDi/IEventSource.cs[22-29]

Agent prompt

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

## Issue description
Modified public API members in the diff must include XML documentation with a non-empty `<summary>`.

## Issue Context
The `StreamAsync` signatures were changed in this PR across `BiDi`, `IBiDi`, and `IEventSource<TEventArgs>`, but the affected public method/member declarations do not have XML doc comments immediately preceding them. Where behavior/semantics are relevant (for example, implicit unsubscribe on end of enumeration, what the async stream yields, and how cancellation/disposal affects subscription/unsubscription), the documentation should reflect those semantics.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/BiDi.cs[120-130]
- dotnet/src/webdriver/BiDi/IBiDi.cs[68-70]
- dotnet/src/webdriver/BiDi/IEventSource.cs[28-28]

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

---

3. Undisposable stream leaks subscription 🐞 Bug ☼ Reliability

Description

StreamAsync now returns IAsyncEnumerable<TEventArgs>, so callers cannot DisposeAsync the underlying
EventStream even though SubscribeReaderAsync subscribes on the wire immediately. If a stream is
created but never enumerated, the remote subscription remains registered (and keeps dispatching)
until BiDi shutdown, potentially accumulating unused subscriptions.

Code

dotnet/src/webdriver/BiDi/IBiDi.cs[R68-70]

+    Task<IAsyncEnumerable<TEventArgs>> StreamAsync<TEventArgs>(EventDescriptor<TEventArgs> descriptor, CancellationToken cancellationToken = default) where TEventArgs : EventArgs;

-    Task<IEventStream<TEventArgs>> StreamAsync<TEventArgs>(ImmutableArray<EventDescriptor> descriptors, CancellationToken cancellationToken = default) where TEventArgs : EventArgs;
+    Task<IAsyncEnumerable<TEventArgs>> StreamAsync<TEventArgs>(ImmutableArray<EventDescriptor> descriptors, CancellationToken cancellationToken = default) where TEventArgs : EventArgs;

Evidence

The dispatcher performs wire subscription and registers the stream immediately, and the only code
path that removes the subscription is EventStream.DisposeAsync (wire unsubscribe). After the API
change, callers only receive an IAsyncEnumerable<T> and thus cannot call DisposeAsync unless
they enumerate (which triggers enumerator disposal) or downcast.

dotnet/src/webdriver/BiDi/IBiDi.cs[68-70]
dotnet/src/webdriver/BiDi/BiDi.cs[120-130]
dotnet/src/webdriver/BiDi/EventDispatcher.cs[92-113]
dotnet/src/webdriver/BiDi/EventDispatcher.cs[220-233]
dotnet/src/webdriver/BiDi/EventStream.cs[89-107]

Agent prompt

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

## Issue description
`StreamAsync` returns `IAsyncEnumerable<T>` (not disposable), but `SubscribeReaderAsync` performs an eager wire subscribe and registers the subscription in dispatcher slots. If the returned stream is never enumerated, there's no way for callers to trigger unsubscription, so the subscription stays active until the whole `IBiDi` instance is disposed.

## Issue Context
- `EventDispatcher.SubscribeReaderAsync` eagerly subscribes and adds the `EventStream` to slots.
- `EventStream` only unsubscribes in `DisposeAsync`, which is no longer reachable via the public return type.

## Fix Focus Areas
- Implement a lazy `IAsyncEnumerable<T>` wrapper so *no wire subscribe happens until the first enumeration* (and dispose/unsubscribe happens when the enumerator is disposed).
- Ensure the wrapper disposes the underlying `EventStream` if enumeration starts, and does nothing if it never starts.

### Suggested approach
- Change `BiDi.StreamAsync(...)` (and `EventSource/ContextEventSource.StreamAsync`) to return an `IAsyncEnumerable<T>` that performs the `SubscribeReaderAsync(...)` call inside the enumerator (e.g., lazy-init on first `MoveNextAsync`).
- Keep `EventDispatcher.SubscribeReaderAsync` and `EventStream` mostly as-is; the key is to avoid calling them before enumeration.

## Fix Focus Areas (files/lines)
- dotnet/src/webdriver/BiDi/BiDi.cs[120-130]
- dotnet/src/webdriver/BiDi/EventSource.cs[47-50]
- dotnet/src/webdriver/BiDi/ContextEventSource.cs[51-54]
- dotnet/src/webdriver/BiDi/EventDispatcher.cs[92-113]
- dotnet/src/webdriver/BiDi/EventStream.cs[89-107]

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

---

4. Stream silently completes on reuse 🐞 Bug ≡ Correctness

Description

UnsubscribingAsyncEnumerator.DisposeAsync always disposes the owning EventStream (wire unsubscribe +
channel completion), so the returned IAsyncEnumerable becomes single-use. Subsequent calls to
GetAsyncEnumerator will read from a completed channel and terminate immediately, which can hide
consumer mistakes and contradicts the Channel’s SingleReader optimization.

Code

dotnet/src/webdriver/BiDi/EventStream.cs[R110-141]

+    private sealed class UnsubscribingAsyncEnumerator : IAsyncEnumerator<TEventArgs>
+    {
+        private readonly EventStream<TEventArgs> _owner;
+        private readonly IAsyncEnumerator<TEventArgs> _inner;
+        private readonly CancellationTokenSource? _linkedTokenSource;
+
+        internal UnsubscribingAsyncEnumerator(EventStream<TEventArgs> owner, IAsyncEnumerator<TEventArgs> inner, CancellationTokenSource? linkedTokenSource)
+        {
+            _owner = owner;
+            _inner = inner;
+            _linkedTokenSource = linkedTokenSource;
+        }
+
+        public TEventArgs Current => _inner.Current;
+
+        public ValueTask<bool> MoveNextAsync()
+        {
+            return _inner.MoveNextAsync();
+        }
+
+        public async ValueTask DisposeAsync()
+        {
+            try
+            {
+                await _inner.DisposeAsync().ConfigureAwait(false);
+            }
+            finally
+            {
+                _linkedTokenSource?.Dispose();
+                await _owner.DisposeAsync().ConfigureAwait(false);
+            }
+        }

Evidence

The wrapper enumerator disposes the owner on enumerator disposal, and owner disposal completes the
channel. Since GetAsyncEnumerator has no disposed/started guard, later enumerations will iterate
over a completed channel and end without producing events.

dotnet/src/webdriver/BiDi/EventStream.cs[63-76]
dotnet/src/webdriver/BiDi/EventStream.cs[78-87]
dotnet/src/webdriver/BiDi/EventStream.cs[89-107]
dotnet/src/webdriver/BiDi/EventStream.cs[110-141]

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 new enumerator wrapper disposes the owning `EventStream` whenever an enumerator is disposed. This makes the returned `IAsyncEnumerable<T>` single-use; re-enumeration completes immediately (because the channel is completed) rather than failing fast, which can silently mask bugs.

## Issue Context
- `UnsubscribingAsyncEnumerator.DisposeAsync()` calls `_owner.DisposeAsync()`.
- `EventStream.DisposeAsync()` completes the channel.
- `GetAsyncEnumerator()` does not check `_disposed` or enforce single enumeration.

## Fix Focus Areas
- Fail fast on misuse (e.g., throw `ObjectDisposedException` if `_disposed != 0`).
- Optionally enforce "single active enumerator" (since the channel is configured `SingleReader = true`), e.g., track an `_enumeratorCreated` flag and throw if called twice.

## Fix Focus Areas (files/lines)
- dotnet/src/webdriver/BiDi/EventStream.cs[34-36]
- dotnet/src/webdriver/BiDi/EventStream.cs[63-76]
- dotnet/src/webdriver/BiDi/EventStream.cs[89-107]
- dotnet/src/webdriver/BiDi/EventStream.cs[110-141]

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

---

[kevinoid]

I really like this simplification too! It makes things more understandable for consumers by using only a well-known interface and reduces casting by avoiding the ConfigureAwait ambiguity, which are both big wins!

The main drawback I see is that if the IAsyncEnumerable<> returned by CreateStream() isn't used (i.e. GetAsyncEnumerator isn't called), the subscription won't be cleaned up until its finalizer runs. Is that correct? Is there much harm in delaying subscription cleanup? I'd hope that would be rare (why call CreateStream() if you aren't going to use it?) and could be worked around by casting to IAsyncDisposable for using in those rare cases. Just confirming it isn't a sharp edge which users may hurt themselves on.

Another small drawback is that if enumeration is attempted after the first is disposed (i.e. .GetAsyncEnumerator() is called after .GetAsyncEnumerator().DisposeAsync()) it will fail. It's not clear to me whether it would fail gracefully. (Probably by throwing from ChannelReader? It doesn't look like .GetAsyncEnumerator() checks whether it has been disposed?) I don't think this is a big problem since, like IEnumerable, the contract only guarantees it can be enumerated once. (Hopefully an analyzer like CA1851 will be added for IAsyncEnumerable in the future to catch this.) Perhaps it would make sense to throw ObjectDisposedException explicitly? Or, perhaps it would make sense to throw InvalidOperationException if .GetAsyncEnumerator() is called a second time (since multiple enumerators would compete to read from the same channel if neither is disposed?)?

Thanks for exploring it @nvborisenko, I really like it!

[nvborisenko]

the IAsyncEnumerable<> returned by CreateStream() isn't used

This is the same issue as before. All data, got from remote end, is buffered. This is "intentional". Users might forgot to call Dispose(). Should we resolve it?.. - Yes. How? I guess via transport backpressure mechanism: kind of "hold on buffering until already 1024 buffered messages/events are not processed". But this is different story.

.GetAsyncEnumerator() is called after .GetAsyncEnumerator().DisposeAsync()) it will fail.

Good call, we should throw with self-explainable exception.

Thanks for your review, appreciate earlier feedback.

[nvborisenko]

guad for already disposed

[nvborisenko]

double check flakiness