[dotnet] [bidi] Fix events subscription possibilities

[nvborisenko]

🔗 Related Issues

I am not able to scope global events to context. Fixes #16582

bidi.Network.OnBeforeRequestAsync(new SubscriptionOptions { Contexts = [ ...??? ]});

It is design issue. When I am in bidi global context, then I should be able to specify Contexts or UserContexts.

💥 What does this PR do?

Now it is clear. When I use bidi global object, then I can do everything as specification describes. But if I am in browsingContext, then events are automatically scoped to this BrowsingContext (implicitly provided when subscribing).

🔄 Types of changes

• New feature (non-breaking change which adds functionality and tests!)

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Missing auditing: New subscription actions are performed without any added logging or audit trail to record who initiated the subscription, the event name, or scoping details. Referred Code var subscribeResult = await _bidi.SessionModule.SubscribeAsync([eventName], new() { Contexts = options?.Contexts, UserContexts = options?.UserContexts }).ConfigureAwait(false); var eventHandler = new SyncEventHandler<TEventArgs>(eventName, action); handlers.Add(eventHandler); return new Subscription(subscribeResult.Subscription, this, eventHandler); } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: No null checks: Methods rely on options?.Contexts and options?.UserContexts without validating empty collections or handling subscription failures, and no try/catch around SubscribeAsync. Referred Code var subscribeResult = await _bidi.SessionModule.SubscribeAsync([eventName], new() { Contexts = options?.Contexts, UserContexts = options?.UserContexts }).ConfigureAwait(false); var eventHandler = new SyncEventHandler<TEventArgs>(eventName, action); handlers.Add(eventHandler); return new Subscription(subscribeResult.Subscription, this, eventHandler); } Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Options validation: Newly added UserContexts and Contexts are passed through without validation (e.g., duplicates, invalid IDs), which may require input validation depending on upstream contracts. Referred Code public IEnumerable<BrowsingContext.BrowsingContext>? Contexts { get; set; } public IEnumerable<Browser.UserContext>? UserContexts { get; set; } } Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Propagate timeout to event subscription Propagate the Timeout value from the options parameter to the OnBeforeRequestSentAsync subscription in InterceptRequestAsync. Currently, any user-provided timeout is ignored. dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextNetworkModule.cs [28-42] public async Task<Intercept> InterceptRequestAsync(Func<InterceptedRequest, Task> handler, InterceptRequestOptions? options = null) { AddInterceptOptions addInterceptOptions = new(options) { Contexts = [context] }; var intercept = await networkModule.AddInterceptAsync([InterceptPhase.BeforeRequestSent], addInterceptOptions).ConfigureAwait(false); await intercept.OnBeforeRequestSentAsync( async req => await handler(new(req.BiDi, req.Context, req.IsBlocked, req.Navigation, req.RedirectCount, req.Request, req.Timestamp, req.Initiator, req.Intercepts)), - new() { Contexts = [context] }).ConfigureAwait(false); + new() { Contexts = [context], Timeout = options?.Timeout }).ConfigureAwait(false); return intercept; } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a minor bug where the Timeout from options is not propagated to the underlying event subscription, leading to incorrect behavior.	Medium
General	✅ Remove redundant client-side context filtering Suggestion Impact: The commit modified the subscription call, removing the options.WithContext(context) scoping and not wrapping the handler for context checks. Instead, it passed a new SubscriptionOptions with only Timeout, effectively removing the client-side context filtering as suggested (though for a special-case reason). code diff: - }, options.WithContext(context)); + }, new SubscriptionOptions() { Timeout = options?.Timeout }); // special case, don't scopoe to context, awaiting https://github.com/w3c/webdriver-bidi/issues/1032 } public Task<Subscription> OnEntryAddedAsync(Action<Log.LogEntry> handler, ContextSubscriptionOptions? options = null) @@ -44,6 +44,6 @@ { handler(args); } - }, options.WithContext(context)); + }, new SubscriptionOptions() { Timeout = options?.Timeout }); // special case, don't scopoe to context, awaiting https://github.com/w3c/webdriver-bidi/issues/1032 Remove the redundant client-side context check in OnEntryAddedAsync methods. The event subscription is already filtered by context at the protocol level, making the if statement unnecessary. dotnet/src/webdriver/BiDi/BrowsingContext/BrowsingContextLogModule.cs [28-48] public Task<Subscription> OnEntryAddedAsync(Func<Log.LogEntry, Task> handler, ContextSubscriptionOptions? options = null) { - return logModule.OnEntryAddedAsync(async args => - { - if (args.Source.Context?.Equals(context) is true) - { - await handler(args).ConfigureAwait(false); - } - }, options.WithContext(context)); + return logModule.OnEntryAddedAsync(handler, options.WithContext(context)); } public Task<Subscription> OnEntryAddedAsync(Action<Log.LogEntry> handler, ContextSubscriptionOptions? options = null) { - return logModule.OnEntryAddedAsync(args => - { - if (args.Source.Context?.Equals(context) is true) - { - handler(args); - } - }, options.WithContext(context)); + return logModule.OnEntryAddedAsync(handler, options.WithContext(context)); } [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies redundant client-side filtering logic that is now handled by the protocol-level subscription, simplifying the code and improving maintainability.	Low
Learned best practice	Clarify options usage with docs Document that ContextSubscriptionOptions only carries Timeout and must be transformed via WithContext to include scopes, clarifying intended use. dotnet/src/webdriver/BiDi/Subscription.cs [61-64] +/// <summary> +/// Options for context-scoped subscription calls. Note: this type carries only timeout; +/// call WithContext(...) to produce a SubscriptionOptions that includes the target context. +/// </summary> public class ContextSubscriptionOptions { + /// <summary> + /// Optional timeout applied to the subscription operation. + /// </summary> public TimeSpan? Timeout { get; set; } } Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Enforce accurate and consistent documentation and naming to match actual behavior for maintainability.	Low
Update

[nvborisenko]

@RenderMichael @YevgeniyShunevych now public API for end-user looks clear, and provides all required capabilities.

But internally the implementation blows my mind. Can we optimize it somehow to avoid types messing?

[RenderMichael]

I like this approach.