[java] Improve error message for died grid

[asolntsev]

User description

💥 What does this PR do?

1. Fixes RemoteWebDriverBiDiTest - closes 3 browsers that left open after this test
2. Improves error message of few exceptions, especially UnreachableBrowserException.

🔄 Types of changes

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

---

PR Type

Bug fix, Enhancement

---

Description

• Improves error messages in BiDi and HTTP client exceptions with contextual details

• Adds new ConnectionException and ProtocolException classes for better error reporting

• Fixes RemoteWebDriverBiDiTest resource leak by explicitly closing browser before grid shutdown

• Enhances logging with formatted messages including HTTP method, URI, and WebSocket close codes

---

Old stack trace

It creates a misleading impression: feels like browser is died, while in reality Selenium Grid is stopped.

UnreachableBrowserException: Error communicating with the remote browser. It may have died.
Build info: version: '4.40.0-SNAPSHOT', revision: 'Unknown'
System info: os.name: 'Mac OS X', os.arch: 'aarch64', os.version: '26.2', java.version: '17.0.17'
Driver info: org.openqa.selenium.remote.RemoteWebDriver$ByteBuddy$UVh4JDv5
Command: [45602644-5eaf-45cc-ba2b-624dfc6d373a, quit []]
	at org.openqa.selenium.remote.RemoteWebDriver.execute(RemoteWebDriver.java:632)
	at org.openqa.selenium.remote.RemoteWebDriver.quit(RemoteWebDriver.java:502)
	at org.openqa.selenium.testing.JupiterTestBase.quitLocalDriver(JupiterTestBase.java:102)
    ...
Caused by: java.io.UncheckedIOException: java.net.ConnectException
	at org.openqa.selenium.remote.http.jdk.JdkHttpClient.execute0(JdkHttpClient.java:503)
    ...
Caused by: java.net.ConnectException
	at java.net.http/jdk.internal.net.http.HttpClientImpl.send(HttpClientImpl.java:900)
	at java.net.http/jdk.internal.net.http.HttpClientFacade.send(HttpClientFacade.java:133)
	at org.openqa.selenium.remote.http.jdk.JdkHttpClient.execute0(JdkHttpClient.java:462)
	... 105 more
Caused by: java.net.ConnectException
	at java.net.http/jdk.internal.net.http.common.Utils.toConnectException(Utils.java:1107)
	...
Caused by: java.nio.channels.ClosedChannelException
	at java.base/sun.nio.ch.SocketChannelImpl.ensureOpen(SocketChannelImpl.java:195)
	...

---

New stack trace

Still not perfect, but at least it gives a hint that this specific URL is not connectable.

UnreachableBrowserException: Error communicating with the remote browser at http://192.168.0.38:2301/session/c42c9ea7-b4ce-4bee-91d2-097e401c1754
Build info: version: '4.40.0-SNAPSHOT', revision: 'Unknown'
System info: os.name: 'Mac OS X', os.arch: 'aarch64', os.version: '26.2', java.version: '17.0.17'
Driver info: org.openqa.selenium.remote.RemoteWebDriver$ByteBuddy$vGAxmTAV
Command: [c42c9ea7-b4ce-4bee-91d2-097e401c1754, quit []]
	at org.openqa.selenium.remote.RemoteWebDriver.execute(RemoteWebDriver.java:636)
	at org.openqa.selenium.remote.RemoteWebDriver.quit(RemoteWebDriver.java:503)
	at org.openqa.selenium.testing.JupiterTestBase.quitLocalDriver(JupiterTestBase.java:102)
    ...
Caused by: org.openqa.selenium.remote.http.jdk.ConnectionException: Connection error (DELETE http://192.168.0.38:2301/session/c42c9ea7-b4ce-4bee-91d2-097e401c1754)
	at org.openqa.selenium.remote.http.jdk.JdkHttpClient.execute0(JdkHttpClient.java:486)
	...
Caused by: java.net.ConnectException
	...
Caused by: java.net.ConnectException
	...
Caused by: java.nio.channels.ClosedChannelException
	...

---

Diagram Walkthrough

flowchart LR
  A["BiDi & HTTP Errors"] -->|"Add context details"| B["Enhanced Exception Messages"]
  C["Test Resource Leak"] -->|"Close browser first"| D["Proper Cleanup Order"]
  E["Generic Exceptions"] -->|"Create custom types"| F["ConnectionException & ProtocolException"]
  B --> G["Better Debugging"]
  D --> G
  F --> G

Loading

File Walkthrough

	Relevant files
Error handling	3 files BiDiProvider.java Throw exception for invalid BiDi URL                                          +1/-1      WebSocketMessageHandler.java Add logging and null handler check                                              +8/-1      RemoteWebDriver.java Handle ConnectionException with URI details                            +6/-0
Enhancement	6 files Connection.java Improve BiDi logging with detailed context                              +10/-8    WebSocketUpgradeHandler.java Improve close message error logging                                            +4/-3      CloseMessage.java Add toString method for debugging                                                +5/-0      ConnectionException.java New exception class for connection errors                                +36/-0    JdkHttpClient.java Enhance HTTP error messages with method and URI                    +40/-29  ProtocolException.java New exception class for protocol errors                                    +27/-0
Bug fix	1 files RemoteWebDriverBiDiTest.java Fix resource leak by closing driver explicitly                      +17/-4
Configuration changes	1 files logging.properties Add logging configuration for tests                                            +6/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Sensitive data exposure Description: New logging/exception messages may expose sensitive runtime data by embedding full BiDi message payloads and close reasons (e.g., throwing new BiDiException("Unable to process BiDi response: " + data, e) and logging reason/data), which could include session identifiers, URLs, or other secrets if logs or exception messages are collected centrally. Connection.java [279-311] Referred Code throw new BiDiException("Unable to process BiDi response: " + data, e); } }); } @Override public void onClose(int code, String reason) { LOG.log( Level.FINE, () -> String.format( "BiDi connection websocket closed (code: %s, reason: \"%s\")", code, reason)); underlyingSocketClosed.set(true); } } private void handle(CharSequence data) { // It's kind of gross to decode the data twice, but this lets us get started on something // that feels nice to users. // TODO: decode once, and once only ... (clipped 12 lines)
	Information disclosure via URI Description: The new UnreachableBrowserException message includes cause.uri() ("Error communicating with the remote browser at ..."), which may leak internal network topology/hostnames or credentials embedded in URIs into logs/error reports depending on how these exceptions are surfaced. RemoteWebDriver.java [632-637] Referred Code } else if (e instanceof ConnectionException) { ConnectionException cause = (ConnectionException) e; toThrow = new UnreachableBrowserException( "Error communicating with the remote browser at " + cause.uri(), cause); } else {
	Sensitive data in logs Description: Debug logging now records the entire websocket message object (msg) when the handler is null, which could include sensitive content in its toString() and be exposed in logs if FINE logging is enabled. WebSocketMessageHandler.java [45-47] Referred Code if (handler == null && msg instanceof CloseMessage && ((CloseMessage) msg).code() == -1) { LOG.log(Level.FINE, "Failed to handle websocket message \"{0}\" - handler is null", msg); return;
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: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities 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: Null teardown risk: The new teardown unconditionally calls localDriver.quit() which can throw NullPointerException if localDriver was never created due to an earlier setup/test failure. Referred Code @AfterEach void tearDownDeployment() { localDriver.quit(); LOG.log(FINE, () -> String.format("Stopping grid server %s ...", remoteUrl)); Safely.safelyCall(() -> deployment.tearDown()); LOG.log(INFO, () -> String.format("Stopped grid server %s", remoteUrl)); } 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: Logs raw payload: New log statements may output raw BiDi message contents (data) and websocket close reasons, which can include user/session data and are not redacted or structured. Referred Code public void onClose(int code, String reason) { LOG.log( Level.FINE, () -> String.format( "BiDi connection websocket closed (code: %s, reason: \"%s\")", code, reason)); underlyingSocketClosed.set(true); } } private void handle(CharSequence data) { // It's kind of gross to decode the data twice, but this lets us get started on something // that feels nice to users. // TODO: decode once, and once only String asString = String.valueOf(data); LOG.log(getDebugLogLevel(), "<- {0}", asString); Map<String, Object> raw = JSON.toType(asString, MAP_TYPE); if (raw.get("id") instanceof Number && (raw.get("result") != null || raw.get("error") != null)) { ... (clipped 6 lines) 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: Error exposes payload: The thrown BiDiException includes the full BiDi response data, which may contain sensitive content and could be surfaced to end users depending on exception propagation/handling outside this diff. Referred Code public void onText(CharSequence data) { EXECUTOR.execute( () -> { try { handle(data); } catch (Exception e) { throw new BiDiException("Unable to process BiDi response: " + data, e); } }); 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	Prevent NullPointerException for any message Add a null check for the handler to prevent a potential NullPointerException for any message type, not just a specific CloseMessage. java/src/org/openqa/selenium/netty/server/WebSocketMessageHandler.java [44-59] Consumer<Message> handler = ctx.channel().attr(key).get(); -if (handler == null && msg instanceof CloseMessage && ((CloseMessage) msg).code() == -1) { - LOG.log(Level.FINE, "Failed to handle websocket message \"{0}\" - handler is null", msg); +if (handler == null) { + if (msg instanceof CloseMessage && ((CloseMessage) msg).code() == -1) { + LOG.log(Level.FINE, "Failed to handle websocket message \"{0}\" - handler is null", msg); + } else { + LOG.log( + Level.WARNING, + "No handler for message, but message was received: {0}", + msg); + } return; } ctx.executor() .execute( () -> { try { handler.accept(msg); ctx.flush(); } catch (Throwable t) { ctx.fireExceptionCaught(t); } }); Apply / Chat Suggestion importance[1-10]: 8 __ Why: The suggestion correctly identifies a potential NullPointerException if the handler is null for messages other than the specific CloseMessage checked. The proposed fix improves the code's robustness by handling this edge case properly.	Medium
Learned best practice	Make teardown null-safe and final Guard localDriver/deployment against null and use try/finally so grid teardown always happens even if quit() throws (or if setup failed and objects are unset). java/test/org/openqa/selenium/grid/router/RemoteWebDriverBiDiTest.java [85-92] @AfterEach void tearDownDeployment() { - localDriver.quit(); - - LOG.log(FINE, () -> String.format("Stopping grid server %s ...", remoteUrl)); - Safely.safelyCall(() -> deployment.tearDown()); - LOG.log(INFO, () -> String.format("Stopped grid server %s", remoteUrl)); + try { + if (localDriver != null) { + localDriver.quit(); + } + } finally { + LOG.log(FINE, () -> String.format("Stopping grid server %s ...", remoteUrl)); + Safely.safelyCall(() -> { + if (deployment != null) { + deployment.tearDown(); + } + }); + LOG.log(INFO, () -> String.format("Stopped grid server %s", remoteUrl)); + } } Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - In tests that create external resources, ensure cleanup is guaranteed (use try/finally and null-guards) so failures don't leak resources or cause teardown errors.	Low
Update