Description

SubscribeAsync registers the remote subscription before adding the local handler, and
UnsubscribeAsync removes the handler immediately without draining queued events. This can race
with event delivery during setup/teardown and cause intermittent missed console logs unless delayed.

Code

dotnet/src/webdriver/BiDi/EventDispatcher.cs[R58-70]

+        var subscribeResult = await _sessionProvider().SubscribeAsync([eventName], new() { Contexts = options?.Contexts, UserContexts = options?.UserContexts }, cancellationToken).ConfigureAwait(false);

-        try
-        {
-            var subscribeResult = await _sessionProvider().SubscribeAsync([eventName], new() { Contexts = options?.Contexts, UserContexts = options?.UserContexts }, cancellationToken).ConfigureAwait(false);
+        registration.Handlers.Add(eventHandler);

-            return new Subscription(subscribeResult.Subscription, this, eventHandler);
-        }
-        catch
-        {
-            registration.RemoveHandler(eventHandler);
-            throw;
-        }
+        return new Subscription(subscribeResult.Subscription, this, eventHandler);
    }

    public async ValueTask UnsubscribeAsync(Subscription subscription, CancellationToken cancellationToken)
    {
        if (_events.TryGetValue(subscription.EventHandler.EventName, out var registration))
        {
            await _sessionProvider().UnsubscribeAsync([subscription.SubscriptionId], null, cancellationToken).ConfigureAwait(false);
-
-            // Wait until all pending events for this method are dispatched
-            try
-            {
-                await registration.DrainAsync(cancellationToken).ConfigureAwait(false);
-            }
-            finally
-            {
-                registration.RemoveHandler(subscription.EventHandler);
-            }
+            registration.Handlers.Remove(subscription.EventHandler);

Evidence

Compliance requires deterministic delivery of console log events without time-based delays and
avoiding loss during WebSocket shutdown; adding handlers only after subscribing and removing them
immediately on unsubscribe creates a window where events can be received but not dispatched to user
handlers.

.NET BiDi log subscription reliably receives console logs without time-based delays
BiDi WebSocket shutdown does not drop pending log events in .NET
dotnet/src/webdriver/BiDi/EventDispatcher.cs[58-62]
dotnet/src/webdriver/BiDi/EventDispatcher.cs[65-70]

Agent prompt

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

## Issue description
`SubscribeAsync` adds the local handler only after awaiting the remote `SubscribeAsync`, and `UnsubscribeAsync` removes the handler immediately without draining pending events. This creates a race where events can arrive during setup/teardown but are not dispatched, causing missed log entries unless `Task.Delay` is used.

## Issue Context
This PR is a revert related to BiDi event dispatch timing. The compliance requirement is that console logs should be delivered deterministically to `OnEntryAddedAsync` handlers even when the session/connection is torn down quickly.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/EventDispatcher.cs[58-70]

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

Description

EventRegistration.Handlers is now a plain List<T> exposed publicly and is mutated/read from
different code paths without synchronization. This can cause races/exceptions and inconsistent
dispatch during concurrent subscribe/unsubscribe and event emission.

Code

dotnet/src/webdriver/BiDi/EventDispatcher.cs[135]

+        public List<EventHandler> Handlers { get; } = [];

Evidence

The compliance rule requires atomic/thread-safe tracking of async operations/state; using an
unsynchronized List for handler registration while iterating and modifying across async code paths
is not thread-safe even if callers attempt to copy via ToArray().

dotnet/src/webdriver/BiDi/EventDispatcher.cs[135-135]
dotnet/src/webdriver/BiDi/EventDispatcher.cs[60-60]
dotnet/src/webdriver/BiDi/EventDispatcher.cs[70-70]
dotnet/src/webdriver/BiDi/EventDispatcher.cs[104-107]
Best Practice: Learned patterns

Agent prompt

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

## Issue description
`EventRegistration.Handlers` is an unsynchronized `List<EventHandler>` that is mutated and enumerated from async code paths. This violates the requirement for atomic/thread-safe state updates and risks race conditions or runtime exceptions.

## Issue Context
Event dispatching and subscription changes can occur concurrently with processing pending events, especially during shutdown. The code currently relies on `ToArray()` during iteration but does not make `Add`/`Remove` operations safe.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/EventDispatcher.cs[60-70]
- dotnet/src/webdriver/BiDi/EventDispatcher.cs[104-107]
- dotnet/src/webdriver/BiDi/EventDispatcher.cs[135-135]

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

Description

If the remote UnsubscribeAsync throws or is canceled, the local handler is not removed because
removal happens after the await with no finally. This can leak handlers and keep invoking user
callbacks after unsubscribe fails.

Code

dotnet/src/webdriver/BiDi/EventDispatcher.cs[R65-71]

    public async ValueTask UnsubscribeAsync(Subscription subscription, CancellationToken cancellationToken)
    {
        if (_events.TryGetValue(subscription.EventHandler.EventName, out var registration))
        {
            await _sessionProvider().UnsubscribeAsync([subscription.SubscriptionId], null, cancellationToken).ConfigureAwait(false);
-
-            // Wait until all pending events for this method are dispatched
-            try
-            {
-                await registration.DrainAsync(cancellationToken).ConfigureAwait(false);
-            }
-            finally
-            {
-                registration.RemoveHandler(subscription.EventHandler);
-            }
+            registration.Handlers.Remove(subscription.EventHandler);
        }

Evidence

Local removal is after awaiting the remote call, so any exception prevents cleanup. Since
Subscription.DisposeAsync calls UnsubscribeAsync, this can also leak during shutdown/cleanup paths.

dotnet/src/webdriver/BiDi/EventDispatcher.cs[65-71]
dotnet/src/webdriver/BiDi/Subscription.cs[37-45]

Agent prompt

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

### Issue description
`UnsubscribeAsync` does not guarantee local handler removal when the remote unsubscribe fails.

### Issue Context
Unsubscribe is invoked directly by users and indirectly by `Subscription.DisposeAsync`, so failure paths must still clean up local state.

### Fix Focus Areas
- dotnet/src/webdriver/BiDi/EventDispatcher.cs[65-71]
- dotnet/src/webdriver/BiDi/Subscription.cs[37-46]

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

Description

TryWrite to the pending-events channel is not checked and failures are silently ignored. During
shutdown (writer completed) this can drop events without any diagnostic logging for users.

Code

dotnet/src/webdriver/BiDi/EventDispatcher.cs[R76-78]

+        if (_events.TryGetValue(method, out var registration) && registration.TypeInfo is not null)
        {
-            if (_pendingEvents.Writer.TryWrite(new EventItem(jsonUtf8Bytes, bidi, registration)))
-            {
-                registration.IncrementEnqueued();
-            }
-            else
-            {
-                if (_logger.IsEnabled(LogEventLevel.Warn))
-                {
-                    _logger.Warn($"Failed to enqueue BiDi event with method '{method}' for processing. Event will be ignored.");
-                }
-            }
+            _pendingEvents.Writer.TryWrite(new PendingEvent(method, jsonUtf8Bytes, bidi, registration.TypeInfo));

Evidence

The compliance rule asks for user-helpful logging where insight is needed; this change removes the
prior warning path and makes event loss during teardown harder to diagnose, which is especially
relevant given the requirement to not drop pending log events.

BiDi WebSocket shutdown does not drop pending log events in .NET
AGENTS.md
dotnet/src/webdriver/BiDi/EventDispatcher.cs[76-78]

Agent prompt

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

## Issue description
`_pendingEvents.Writer.TryWrite(...)` is called without checking the return value, so enqueue failures (e.g., after `Complete()` during teardown) silently drop events and provide no diagnostics.

## Issue Context
This area is user-impacting because dropped BiDi log events are the core symptom under investigation, and lack of logging makes it difficult to understand why logs are missed.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/EventDispatcher.cs[76-78]

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

Description

The error log interpolates the full exception ({ex}), which can include sensitive/nullable message
content and lacks stable identifiers (e.g., method/request id). This reduces resilience and may leak
details in user logs.

Code

dotnet/src/webdriver/BiDi/EventDispatcher.cs[R112-115]

                    if (_logger.IsEnabled(LogEventLevel.Error))
                    {
-                        _logger.Error($"Unhandled error deserializing BiDi event: {ex}");
+                        _logger.Error($"Unhandled error processing BiDi event handler: {ex}");
                    }

Evidence

The compliance rule requires resilient logging that avoids leaking raw exception messages and
prefers stable identifiers like exception type and relevant correlation data; the new log line
directly embeds the exception object in the message.

dotnet/src/webdriver/BiDi/EventDispatcher.cs[112-115]
Best Practice: Learned patterns

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 error path logs `ex` via string interpolation, which can leak exception messages and does not include stable identifiers (like exception type + event method) in a structured/resilient way.

## Issue Context
These errors can occur while processing BiDi events; logs should help diagnose issues without exposing sensitive or unstable exception message content.

## Fix Focus Areas
- dotnet/src/webdriver/BiDi/EventDispatcher.cs[112-115]

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

Description

A single handler exception aborts dispatch to remaining handlers for the same event because the
try/catch wraps the entire foreach loop. This is a regression in robustness when multiple handlers
are registered for one event name.

Code

dotnet/src/webdriver/BiDi/EventDispatcher.cs[R96-107]

                try
                {
-                    var eventArgs = (EventArgs)JsonSerializer.Deserialize(evt.JsonUtf8Bytes.Span, evt.Registration.TypeInfo)!;
-                    eventArgs.BiDi = evt.BiDi;
-
-                    foreach (var handler in evt.Registration.GetHandlersSnapshot())
+                    if (_events.TryGetValue(result.Method, out var registration))
                    {
-                        try
+                        // Deserialize on background thread instead of network thread (single parse)
+                        var eventArgs = (EventArgs)JsonSerializer.Deserialize(result.JsonUtf8Bytes.Span, result.TypeInfo)!;
+                        eventArgs.BiDi = result.BiDi;
+
+                        foreach (var handler in registration.Handlers.ToArray()) // copy handlers avoiding modified collection while iterating
                        {
                            await handler.InvokeAsync(eventArgs).ConfigureAwait(false);
                        }
-                        catch (Exception ex)
-                        {
-                            if (_logger.IsEnabled(LogEventLevel.Error))
-                            {
-                                _logger.Error($"Unhandled error processing BiDi event handler: {ex}");
-                            }
-                        }

Evidence

ProcessEventsAwaiterAsync catches exceptions outside the foreach, so a throw from one InvokeAsync
prevents subsequent handlers from running. Multiple handlers are expected because each call to
Module.SubscribeAsync creates and registers a new EventHandler instance for the same event name.

dotnet/src/webdriver/BiDi/EventDispatcher.cs[96-116]
dotnet/src/webdriver/BiDi/Module.cs[36-48]
dotnet/src/webdriver/BiDi/EventDispatcher.cs[56-57]

Agent prompt

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

### Issue description
Exceptions thrown by a single user handler currently prevent other handlers from running for the same event.

### Issue Context
Multiple independent subscriptions to the same event name are supported; a single handler should not break others.

### Fix Focus Areas
- dotnet/src/webdriver/BiDi/EventDispatcher.cs[89-116]

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