[dotnet] [bidi] Support network SetExtraHeaders command by nvborisenko · Pull Request #16384 · SeleniumHQ/selenium

Conversation

@nvborisenko

@nvborisenko nvborisenko commented

Oct 5, 2025

edited by qodo-code-review bot

Loading

User description

https://w3c.github.io/webdriver-bidi/#command-network-setExtraHeaders

💥 What does this PR do?

Implements SetExtraHeaders command in Network module.

💡 Additional Considerations

Test is ignored, but I verified locally for Chrome Dev channel.

🔄 Types of changes

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

PR Type

Enhancement


Description

  • Add SetExtraHeaders command to Network module

  • Implement BiDi network header manipulation functionality

  • Include command parameters and options classes

  • Add test coverage with browser compatibility notes


Diagram Walkthrough

flowchart LR
  A["NetworkModule"] --> B["SetExtraHeadersAsync method"]
  B --> C["SetExtraHeadersCommand"]
  C --> D["SetExtraHeadersParameters"]
  E["BiDiJsonSerializerContext"] --> C
  F["NetworkTest"] --> B
Loading

File Walkthrough

Relevant files
Configuration changes
BiDiJsonSerializerContext.cs
Register SetExtraHeadersCommand for JSON serialization     

dotnet/src/webdriver/BiDi/Communication/Json/BiDiJsonSerializerContext.cs

  • Add JSON serialization support for SetExtraHeadersCommand
+1/-0     
Enhancement
NetworkModule.cs
Implement SetExtraHeadersAsync method                                       

dotnet/src/webdriver/BiDi/Network/NetworkModule.cs

  • Add SetExtraHeadersAsync method to NetworkModule
  • Accept headers collection and optional contexts/user contexts
+7/-0     
SetExtraHeadersCommand.cs
Create SetExtraHeaders command infrastructure                       

dotnet/src/webdriver/BiDi/Network/SetExtraHeadersCommand.cs

  • Create new command class for setting extra headers
  • Define parameters record with headers and context options
  • Add options class for command configuration
+35/-0   
Tests
NetworkTest.cs
Add SetExtraHeaders test with browser ignores                       

dotnet/test/common/BiDi/Network/NetworkTest.cs

  • Add test for SetExtraHeaders functionality
  • Include browser compatibility annotations (all ignored)
+11/-0   

@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
No custom compliance provided

Follow the guide to enable custom compliance check.

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:

CategorySuggestion                                                                                                                                    Impact
General
Fix incorrect test assertion logic

In the CanSetExtraHeaders test, replace the meaningless Is.Not.Null assertion on
a struct with Assert.That(..., Throws.Nothing) to correctly verify that the
method executes without throwing an exception.

dotnet/test/common/BiDi/Network/NetworkTest.cs [259-264]

-public async Task CanSetExtraHeaders()
+public void CanSetExtraHeaders()
 {
-    var result = await bidi.Network.SetExtraHeadersAsync([new Header("x-test-header", "test-value")]);
-
-    Assert.That(result, Is.Not.Null);
+    Assert.That(async () => await bidi.Network.SetExtraHeadersAsync([new Header("x-test-header", "test-value")]), Throws.Nothing);
 }
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly points out that asserting a struct is not null is a meaningless test. It proposes a much better assertion (Throws.Nothing) that properly verifies the method's successful execution, significantly improving test quality.

Medium
Possible issue
Add null check for headers

Add a null check for the headers parameter in the SetExtraHeadersAsync method to
prevent potential NullReferenceException and throw an ArgumentNullException if
it is null.

dotnet/src/webdriver/BiDi/Network/NetworkModule.cs [68-73]

 public async Task<EmptyResult> SetExtraHeadersAsync(IEnumerable<Header> headers, SetExtraHeadersOptions? options = null)
 {
+    if (headers is null)
+    {
+        throw new ArgumentNullException(nameof(headers));
+    }
+
     var @params = new SetExtraHeadersParameters(headers, options?.Contexts, options?.UserContexts);
 
     return await Broker.ExecuteCommandAsync<SetExtraHeadersCommand, EmptyResult>(new SetExtraHeadersCommand(@params), options).ConfigureAwait(false);
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a missing null check for the headers parameter, which improves robustness by providing a clear ArgumentNullException instead of a less specific downstream error.

Medium
  • More

This was referenced

Oct 19, 2025

This was referenced

Dec 15, 2025

This was referenced

Jan 19, 2026

This was referenced

Feb 11, 2026

This was referenced

Feb 20, 2026

Labels

2 participants

@nvborisenko @selenium-ci