Skip to content

Conversation

@asolntsev
Copy link
Contributor

@asolntsev asolntsev commented Jan 10, 2026

User description

🔗 Related Issues

Fixes #16270

💥 What does this PR do?

Uses ClientConfig instance provided by user (in webdriver constructor).

🔧 Implementation Notes

All webdriver constructors (RemoteWebDriver, ChromeDriver etc.) have an optional ClientConfig parameter.

But still, the ClientConfig.defaultConfig() was used in multiple places (e.g. for establishing CDP or BiDi connection).
Now the config provided by user is used (especially WebSockets timeout for establshing CDP or BiDi connection.

💡 Additional Considerations

There are still multiple usages of ClientConfig.defaultConfig() in Selenium Grid code.
I didn't touch Grid part this time.

🔄 Types of changes

  • Bug fix (backwards compatible)

PR Type

Bug fix


Description

  • Use user-provided ClientConfig instead of default in CDP/BiDi connections

  • Pass ClientConfig through webdriver constructors to connection establishment

  • Add ClientConfig parameter to Connection and related factory methods

  • Deprecate old constructors/methods without ClientConfig parameter


Diagram Walkthrough

flowchart LR
  A["WebDriver Constructor<br/>with ClientConfig"] -->|passes config| B["ChromiumDriver/<br/>FirefoxDriver"]
  B -->|provides config| C["BiDiProvider/<br/>DevToolsProvider"]
  C -->|uses config| D["Connection<br/>Establishment"]
  D -->|applies timeout| E["CDP/BiDi<br/>WebSocket"]
Loading

File Walkthrough

Relevant files
Enhancement
18 files
BiDiProvider.java
Pass ExecuteMethod to BiDi connection establishment           
+6/-3     
ChromeDriver.java
Pass ClientConfig to parent ChromiumDriver constructor     
+5/-1     
ChromiumDriver.java
Accept and use ClientConfig in constructor and BiDi/CDP setup
+20/-9   
ChromiumDriverCommandExecutor.java
Deprecate constructor without ClientConfig parameter         
+5/-0     
CdpEndpointFinder.java
Add ClientConfig parameter to getHttpClient method             
+11/-3   
Connection.java
Accept ClientConfig and use it for WebSocket configuration
+22/-11 
DevToolsProvider.java
Pass ExecuteMethod to DevTools connection establishment   
+7/-3     
SeleniumCdpConnection.java
Add ClientConfig parameter to all factory methods               
+34/-7   
EdgeDriver.java
Pass ClientConfig to parent ChromiumDriver constructor     
+5/-1     
FirefoxDriver.java
Pass ClientConfig through constructor chain and BiDi setup
+15/-25 
ProxyNodeWebsockets.java
Pass ClientConfig to CdpEndpointFinder.getHttpClient call
+4/-1     
InternetExplorerDriver.java
Refactor constructor to require non-null ClientConfig       
+7/-13   
Augmenter.java
Use LocalExecuteMethod for non-remote drivers                       
+3/-6     
LocalExecuteMethod.java
New class for local driver command execution                         
+30/-0   
RemoteExecuteMethod.java
Change getWrappedDriver return type to RemoteWebDriver     
+1/-2     
RemoteWebDriver.java
Store ClientConfig and initialize ExecuteMethod as final field
+35/-8   
RemoteWebDriverBuilder.java
Improve error message with requested capabilities details
+5/-1     
HttpRequest.java
Add constructor accepting URI parameter                                   
+5/-0     
Formatting
1 files
AddHasFullPageScreenshot.java
Remove generic type parameter from class declaration         
+1/-1     
Dependencies
1 files
BUILD.bazel
Add jspecify dependency to augmenter target                           
+1/-0     

@selenium-ci selenium-ci added B-grid Everything grid and server related C-java Java Bindings B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related labels Jan 10, 2026
@asolntsev asolntsev added this to the 4.40.0 milestone Jan 10, 2026
@asolntsev asolntsev self-assigned this Jan 10, 2026
@qodo-code-review
Copy link
Contributor

qodo-code-review bot commented Jan 10, 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
🟡
🎫 #16270
🟢 Ensure user-provided ClientConfig is used (instead of ClientConfig.defaultConfig()) when
establishing CDP/BiDi WebSocket connections in Java so configured WebSocket timeouts can
take effect.
🔴 Support customizing the WebSocket polling/check interval used to check for a response to a
sent message via ClientConfig (Java/Python).
Support customizing the WebSocket timeout used to wait for a response to a sent message
via ClientConfig (Java/Python).
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 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:
Unsafe type cast: The new code unconditionally casts executeMethod to RemoteExecuteMethod, which can throw a
ClassCastException (e.g., when Augmenter supplies LocalExecuteMethod) instead of handling
the non-remote case gracefully.

Referred Code
private BiDi establishBiDiConnection(Capabilities caps, ExecuteMethod executeMethod) {
  URI wsUri = getBiDiUrl(caps).orElseThrow(() -> new BiDiException("BiDi not supported"));
  ClientConfig clientConfig =
      ((RemoteExecuteMethod) executeMethod).getWrappedDriver().getClientConfig();

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:
Detailed user error: The new SessionNotCreatedException message includes full requestedCapabilities and infos,
which may expose internal/environment details to end users depending on how exceptions are
surfaced.

Referred Code
throw new SessionNotCreatedException(
    String.format(
        "Unable to find matching driver for capabilities%n  requestedCapabilities: %s%n "
            + " infos: %s",
        requestedCapabilities, infos));

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:
Invalid URI fallback: When wsClientConfig encounters an invalid WebSocket URI it logs and returns the unmodified
clientConfig, which may lack a valid baseUri and can lead to undefined behavior when later
calling wsConfig.baseUri() to open the socket.

Referred Code
void reopen() {
  HttpClient.Factory clientFactory = HttpClient.Factory.createDefault();
  this.client = clientFactory.createClient(wsConfig);
  this.socket = this.client.openSocket(new HttpRequest(GET, wsConfig.baseUri()), new Listener());
}

private static ClientConfig wsClientConfig(ClientConfig clientConfig, String uri) {
  Require.nonNull("Client config", clientConfig);
  try {
    return clientConfig.baseUri(new URI(uri));
  } catch (URISyntaxException e) {
    LOG.log(Level.WARNING, "Invalid WebSockets URI: " + uri, e);
    return clientConfig;
  }

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 10, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent ClassCastException

Add a guard to prevent a ClassCastException by checking if executeMethod is an
instance of RemoteExecuteMethod before casting, falling back to a default
configuration if not.

java/src/org/openqa/selenium/bidi/BiDiProvider.java [76-77]

 ClientConfig clientConfig =
-    ((RemoteExecuteMethod) executeMethod).getWrappedDriver().getClientConfig();
+    executeMethod instanceof RemoteExecuteMethod
+        ? ((RemoteExecuteMethod) executeMethod).getWrappedDriver().getClientConfig()
+        : ClientConfig.defaultConfig();
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a potential ClassCastException when executeMethod is not a RemoteExecuteMethod and provides a concise, correct fix to prevent a runtime crash.

Medium
General
Validate clientConfig argument

Add a null-check for the clientConfig parameter in the getHttpClient method to
ensure it is not null.

java/src/org/openqa/selenium/devtools/CdpEndpointFinder.java [51-59]

 public static HttpClient getHttpClient(
     HttpClient.Factory clientFactory, URI reportedUri, ClientConfig clientConfig) {
   Require.nonNull("HTTP client factory", clientFactory);
   Require.nonNull("DevTools URI", reportedUri);
+  Require.nonNull("Client config", clientConfig);
 
   ClientConfig config = clientConfig.baseUri(reportedUri);
 
   return clientFactory.createClient(config);
 }
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly points out a missing null check for the clientConfig parameter, improving robustness by ensuring the argument is not null.

Low
Learned
best practice
Prevent resource leaks on reopen

Remove the stray empty statement and ensure reopen() closes the previous
socket/client (and is safe to call repeatedly) to avoid leaking resources on
reconnect attempts.

java/src/org/openqa/selenium/devtools/Connection.java [92-108]

 public Connection(HttpClient client, String url, ClientConfig clientConfig) {
-  ;
   this.client = Require.nonNull("HTTP client", client);
   this.wsConfig = wsClientConfig(clientConfig, url);
   this.socket = this.client.openSocket(new HttpRequest(GET, wsConfig.baseUri()), new Listener());
   this.isClosed = new AtomicBoolean();
 }
 
 void reopen() {
+  try {
+    this.socket.close();
+  } catch (Exception ignored) {
+    // Best-effort cleanup before reconnecting
+  }
+  try {
+    this.client.close();
+  } catch (Exception ignored) {
+    // Best-effort cleanup before reconnecting
+  }
+
   HttpClient.Factory clientFactory = HttpClient.Factory.createDefault();
   this.client = clientFactory.createClient(wsConfig);
   this.socket = this.client.openSocket(new HttpRequest(GET, wsConfig.baseUri()), new Listener());
 }
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Make lifecycle-sensitive code robust by avoiding resource leaks during reconnect/reopen and removing hidden side effects.

Low
  • Update

…ault one

All webdriver constructors (RemoteWebDriver, ChromeDriver etc.) have an optional `ClientConfig` parameter. But still, the `ClientConfig.defaultConfig()` was used in multiple places (e.g. for establishing CDP or BiDi connection).
@asolntsev asolntsev force-pushed the feature/16270-bidi-timeout branch from 712188f to a98d478 Compare January 10, 2026 20:26
@asolntsev asolntsev requested a review from p0deje January 10, 2026 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations B-devtools Includes everything BiDi or Chrome DevTools related B-grid Everything grid and server related C-java Java Bindings Review effort 4/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[🚀 Feature]: Allow to configure WebSocket timeouts in ClientConfig

2 participants