Skip to content

Conversation

@iampopovich
Copy link
Contributor

@iampopovich iampopovich commented Jan 2, 2026

User description

🔗 Related Issues

fixes #16339
fixes review for #16340

💥 What does this PR do?

This pull request introduces improvements to the BiDi network response handling in the Selenium WebDriver JavaScript bindings, focusing on enhanced header manipulation and testing. The main changes add a utility method to the Header class and introduce a new test to verify providing custom headers in responses.

Enhancements to header handling:

  • Added an asMap() method to the Header class in networkTypes.js, allowing easy conversion of header objects to a map representation for further processing.

Testing improvements:

  • Added imports for BytesValue and Header in network_commands_test.js to support new test functionality.
  • Introduced a new test case to verify that responses can be provided with custom headers using the BiDi protocol, ensuring correct integration and header handling.

🔧 Implementation Notes

💡 Additional Considerations

🔄 Types of changes

  • Bug fix (backwards compatible)
  • New feature (non-breaking change which adds functionality and tests!)

PR Type

Enhancement, Tests


Description

  • Add asMap() method to Header class for map conversion

  • Implement comprehensive tests for header handling in responses

  • Fix issue where header.asMap() was not available in BiDi network operations

  • Validate ProvideResponseParameters with custom headers integration


Diagram Walkthrough

flowchart LR
  A["Header Class"] -->|"adds asMap() method"| B["Map Representation"]
  C["ProvideResponseParameters"] -->|"uses headers with"| B
  D["Test Suite"] -->|"validates"| C
  D -->|"validates"| E["Header Conversion"]
Loading

File Walkthrough

Relevant files
Enhancement
networkTypes.js
Add asMap conversion method to Header class                           

javascript/selenium-webdriver/bidi/networkTypes.js

  • Added asMap() method to Header class that converts header name and
    value to a Map
  • Method converts the header value using Object.fromEntries() for proper
    serialization
  • Includes JSDoc documentation for the new method
+11/-0   
Tests
network_commands_test.js
Add integration test for response headers handling             

javascript/selenium-webdriver/test/bidi/network_commands_test.js

  • Added imports for BytesValue and Header from networkTypes module
  • Introduced new test case can provide response with headers that
    validates custom header handling
  • Test creates headers with content-type and custom headers, sets
    response body and status code
  • Verifies that ProvideResponseParameters correctly handles headers in
    BiDi protocol
+32/-0   
network_types_test.js
Create unit tests for Header asMap functionality                 

javascript/selenium-webdriver/test/bidi/network_types_test.js

  • New test file created to validate ProvideResponseParameters with
    headers
  • Test specifically addresses issue [🐛 Bug]: TypeError: header.asMap is not a function #16339 by verifying asMap() method
    availability
  • Creates multiple headers with different content types and validates no
    errors are thrown
  • Tests the complete flow of header creation and parameter building
+43/-0   

@selenium-ci selenium-ci added C-nodejs JavaScript Bindings B-devtools Includes everything BiDi or Chrome DevTools related labels Jan 2, 2026
@iampopovich iampopovich marked this pull request as ready for review January 2, 2026 21:20
Copilot AI review requested due to automatic review settings January 2, 2026 21:20
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 2, 2026

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: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

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: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing null checks: Header.asMap() assumes this.value is always present and has asMap(), which can throw at
runtime for null/undefined or unexpected BytesValue shapes without a clearer error or
guard.

Referred Code
asMap() {
  return new Map([
    ['name', this.name],
    ['value', Object.fromEntries(this.value.asMap())],
  ])
}

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:
No input validation: Header.asMap() emits header name/value without validating header name format or guarding
against unexpected BytesValue contents, which may be fine for trusted internal
construction but is not verifiable from the diff alone.

Referred Code
asMap() {
  return new Map([
    ['name', this.name],
    ['value', Object.fromEntries(this.value.asMap())],
  ])
}

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
Copy link
Contributor

qodo-code-review bot commented Jan 2, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Strengthen test by asserting response content

Improve the 'can provide response with headers' test by adding assertions to
verify the mocked response content (headers and body) from the browser's
perspective, ensuring the end-to-end flow works correctly.

javascript/selenium-webdriver/test/bidi/network_commands_test.js [183-190]

+const params = new ProvideResponseParameters(event.request.request)
+  .statusCode(200)
+  .body(body)
+  .headers(headers)
+  .reasonPhrase('OK')
 
+await network.provideResponse(params)
+counter = counter + 1
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that the new test only verifies that the event handler is called, not that the mocked response is correctly processed, and proposes a valuable improvement to make the test more robust.

Medium
Validate constructor parameter type

In the Header constructor, add a check to verify that the value parameter is an
instance of BytesValue to prevent runtime errors.

javascript/selenium-webdriver/bidi/networkTypes.js [99-105]

 class Header {
   /**
    * @param {string} name
    * @param {BytesValue} value
    */
   constructor(name, value) {
+    if (!(value instanceof BytesValue)) {
+      throw new Error('Header value must be an instance of BytesValue.')
+    }
     this._name = name
     this._value = value
   }

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why: The suggestion improves robustness by adding a runtime type check in the Header constructor, preventing potential errors if an incorrect type is passed for the value parameter.

Low
Learned
best practice
Ensure intercept cleanup in tests

Capture the intercept id and remove it in a finally block so the
intercept/handler is cleaned up even if navigation or assertions fail.

javascript/selenium-webdriver/test/bidi/network_commands_test.js [167-196]

 it('can provide response with headers', async function () {
-  await network.addIntercept(new AddInterceptParameters(InterceptPhase.BEFORE_REQUEST_SENT))
+  const intercept = await network.addIntercept(new AddInterceptParameters(InterceptPhase.BEFORE_REQUEST_SENT))
 
   let counter = 0
 
-  await network.beforeRequestSent(async (event) => {
-    const headers = [
-      new Header('content-type', new BytesValue(BytesValue.Type.STRING, 'application/json')),
-      new Header('x-custom-header', new BytesValue(BytesValue.Type.STRING, 'test-value')),
-    ]
+  try {
+    await network.beforeRequestSent(async (event) => {
+      const headers = [
+        new Header('content-type', new BytesValue(BytesValue.Type.STRING, 'application/json')),
+        new Header('x-custom-header', new BytesValue(BytesValue.Type.STRING, 'test-value')),
+      ]
 
-    const body = new BytesValue(
-      BytesValue.Type.BASE64,
-      Buffer.from(JSON.stringify({ status: 'ok' })).toString('base64'),
-    )
+      const body = new BytesValue(
+        BytesValue.Type.BASE64,
+        Buffer.from(JSON.stringify({ status: 'ok' })).toString('base64'),
+      )
 
-    const params = new ProvideResponseParameters(event.request.request)
-      .statusCode(200)
-      .body(body)
-      .headers(headers)
-      .reasonPhrase('OK')
+      const params = new ProvideResponseParameters(event.request.request)
+        .statusCode(200)
+        .body(body)
+        .headers(headers)
+        .reasonPhrase('OK')
 
-    await network.provideResponse(params)
-    counter = counter + 1
-  })
+      await network.provideResponse(params)
+      counter = counter + 1
+    })
 
-  await driver.get(Pages.logEntryAdded)
+    await driver.get(Pages.logEntryAdded)
 
-  assert.strictEqual(counter >= 1, true)
+    assert.strictEqual(counter >= 1, true)
+  } finally {
+    await network.removeIntercept(intercept)
+  }
 })
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - In tests that create external resources (sessions/intercepts/contexts), use try/finally to ensure cleanup even when assertions fail.

Low
  • Update

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request fixes issue #16339 by adding an asMap() method to the Header class in the BiDi network module, enabling proper serialization of Header objects when providing custom responses through the network interception API.

Key Changes:

  • Added asMap() method to the Header class that returns a Map with 'name' and 'value' entries
  • Added unit test file to verify the fix prevents the "header.asMap is not a function" error
  • Added integration test demonstrating the ability to provide responses with custom headers

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
javascript/selenium-webdriver/bidi/networkTypes.js Adds the asMap() method to the Header class, enabling conversion to Map representation consistent with BytesValue.asMap()
javascript/selenium-webdriver/test/bidi/network_types_test.js New test file verifying that ProvideResponseParameters can accept headers without throwing the asMap error
javascript/selenium-webdriver/test/bidi/network_commands_test.js Adds integration test demonstrating end-to-end functionality of providing responses with custom headers via BiDi protocol

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@diemol diemol merged commit 968d615 into SeleniumHQ:trunk Jan 6, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-devtools Includes everything BiDi or Chrome DevTools related C-nodejs JavaScript Bindings Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🐛 Bug]: TypeError: header.asMap is not a function

3 participants