Skip to content

Conversation

nvborisenko
Copy link
Member

@nvborisenko nvborisenko commented Oct 5, 2025

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   

@selenium-ci selenium-ci added the C-dotnet .NET Bindings label Oct 5, 2025
Copy link
Contributor

qodo-merge-pro bot commented Oct 5, 2025

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

Copy link
Contributor

qodo-merge-pro bot commented Oct 5, 2025

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

@nvborisenko nvborisenko merged commit 6c76af1 into SeleniumHQ:trunk Oct 5, 2025
13 checks passed
@nvborisenko nvborisenko deleted the bidi-set-extra-headers branch October 5, 2025 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants