[java] Improve error message for died grid

java/src/org/openqa/selenium/bidi/BiDiProvider.java

@@ -93,7 +93,7 @@ private Optional<URI> getBiDiUrl(Capabilities caps) {
           try {
             return new URI(uri);
           } catch (URISyntaxException e) {
-            return null;
+            throw new BiDiException("Invalid BiDi URL: " + uri, e);
           }
         });
   }

java/src/org/openqa/selenium/bidi/Connection.java

@@ -276,14 +276,18 @@ public void onText(CharSequence data) {
             try {
               handle(data);
             } catch (Exception e) {
-              throw new BiDiException("Unable to process: " + data, e);
+              throw new BiDiException("Unable to process BiDi response: " + data, e);
             }
           });
     }
 
     @Override
     public void onClose(int code, String reason) {
-      LOG.fine("BiDi connection websocket closed");
+      LOG.log(
+          Level.FINE,
+          () ->
+              String.format(
+                  "BiDi connection websocket closed (code: %s, reason: \"%s\")", code, reason));
       underlyingSocketClosed.set(true);
     }
   }
@@ -303,7 +307,7 @@ private void handle(CharSequence data) {
     } else if (raw.get("method") instanceof String && raw.get("params") instanceof Map) {
       handleEventResponse(raw);
     } else {
-      LOG.warning(() -> "Unhandled type:" + data);
+      LOG.warning(() -> "Unhandled type BiDi response type: " + data);
     }
   }
 
@@ -340,11 +344,9 @@ private void handleEventResponse(Map<String, Object> rawDataMap) {
     LOG.log(
         getDebugLogLevel(),
         () ->
-            "Method"
-                + rawDataMap.get("method")
-                + "called with"
-                + eventCallbacks.size()
-                + "callbacks available");
+            String.format(
+                "Method %s called with %s callbacks available",
+                rawDataMap.get("method"), eventCallbacks.size()));
     Lock lock = callbacksLock.readLock();
     // A waiting writer will block a reader to enter the lock, even if there are currently other
     // readers holding the lock. TryLock will bypass the waiting writers and acquire the read lock.

java/src/org/openqa/selenium/netty/server/WebSocketMessageHandler.java

@@ -21,11 +21,14 @@
 import io.netty.channel.SimpleChannelInboundHandler;
 import io.netty.util.AttributeKey;
 import java.util.function.Consumer;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import org.openqa.selenium.internal.Require;
+import org.openqa.selenium.remote.http.CloseMessage;
 import org.openqa.selenium.remote.http.Message;
 
 class WebSocketMessageHandler extends SimpleChannelInboundHandler<Message> {
-
+  private static final Logger LOG = Logger.getLogger(WebSocketMessageHandler.class.getName());
   private final AttributeKey<Consumer<Message>> key;
 
   public WebSocketMessageHandler(AttributeKey<Consumer<Message>> key) {
@@ -39,6 +42,10 @@ protected void channelRead0(ChannelHandlerContext ctx, Message msg) {
     }
 
     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);
+      return;
+    }
 
     ctx.executor()
         .execute(

java/src/org/openqa/selenium/netty/server/WebSocketUpgradeHandler.java

@@ -233,10 +233,11 @@ public void channelInactive(ChannelHandlerContext ctx) throws Exception {
       Consumer<Message> consumer = ctx.channel().attr(key).getAndSet(null);
 
       if (consumer != null) {
+        CloseMessage channelGotInactive = new CloseMessage(1001, "channel got inactive");
         try {
-          consumer.accept(new CloseMessage(1001, "channel got inactive"));
-        } catch (Exception ex) {
-          LOG.log(Level.FINE, "failed to send the close message, code: 1001", ex);
+          consumer.accept(channelGotInactive);
+        } catch (RuntimeException ex) {
+          LOG.log(Level.FINE, ex, () -> "failed to send " + channelGotInactive);
         }
       }
     }

java/src/org/openqa/selenium/remote/RemoteWebDriver.java

@@ -93,6 +93,7 @@
 import org.openqa.selenium.remote.http.ConnectionFailedException;
 import org.openqa.selenium.remote.http.Contents;
 import org.openqa.selenium.remote.http.HttpClient;
+import org.openqa.selenium.remote.http.jdk.ConnectionException;
 import org.openqa.selenium.remote.service.DriverCommandExecutor;
 import org.openqa.selenium.remote.tracing.TracedHttpClient;
 import org.openqa.selenium.remote.tracing.Tracer;
@@ -628,6 +629,11 @@ protected Response execute(CommandPayload payload) {
         }
       } else if (e instanceof WebDriverException) {
         toThrow = (WebDriverException) e;
+      } else if (e instanceof ConnectionException) {
+        ConnectionException cause = (ConnectionException) e;
+        toThrow =
+            new UnreachableBrowserException(
+                "Error communicating with the remote browser at " + cause.uri(), cause);
       } else {
         toThrow =
             new UnreachableBrowserException(

java/src/org/openqa/selenium/remote/http/CloseMessage.java

@@ -38,4 +38,9 @@ public int code() {
   public String reason() {
     return reason;
   }
+
+  @Override
+  public String toString() {
+    return String.format("%s{code=%d, reason=%s}", getClass().getSimpleName(), code, reason);
+  }
 }

java/src/org/openqa/selenium/remote/http/jdk/ConnectionException.java

@@ -0,0 +1,34 @@
+// Licensed to the Software Freedom Conservancy (SFC) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The SFC licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.openqa.selenium.remote.http.jdk;
+
+import java.io.UncheckedIOException;
+import java.net.ConnectException;
+
+public class ConnectionException extends UncheckedIOException {
+  private final String uri;
+
+  public ConnectionException(String message, String uri, ConnectException cause) {
+    super(message, cause);
+    this.uri = uri;
+  }
+
+  public String uri() {
+    return uri;
+  }
+}

java/src/org/openqa/selenium/remote/http/jdk/JdkHttpClient.java

@@ -23,6 +23,7 @@
 import java.io.InputStream;
 import java.io.UncheckedIOException;
 import java.net.Authenticator;
+import java.net.ConnectException;
 import java.net.PasswordAuthentication;
 import java.net.ProtocolException;
 import java.net.Proxy;
@@ -443,18 +444,18 @@ private HttpResponse execute0(HttpRequest req) throws UncheckedIOException {
     long start = System.currentTimeMillis();
 
     BodyHandler<InputStream> responseBodyHandler = BodyHandlers.ofInputStream();
+    URI rawUri = messages.getRawUri(req);
+    HttpMethod method = req.getMethod();
 
     try {
-      HttpMethod method = req.getMethod();
-      URI rawUri = messages.getRawUri(req);
-
       // We need a custom handling of redirects to:
       // - increase the maximum number of retries to 100
       // - avoid a downgrade of POST requests, see the javadoc of j.n.h.HttpClient.Redirect
       // - not run into https://bugs.openjdk.org/browse/JDK-8304701
       for (int i = 0; i < 100; i++) {
         if (Thread.interrupted()) {
-          throw new InterruptedException("http request has been interrupted");
+          throw new InterruptedException(
+              String.format("http request has been interrupted: %s", describe(req)));
         }
 
         java.net.http.HttpRequest request = messages.createRequest(req, method, rawUri);
@@ -469,41 +470,29 @@ private HttpResponse execute0(HttpRequest req) throws UncheckedIOException {
           case 302:
           case 307:
           case 308:
-            URI location =
-                rawUri.resolve(
-                    response
-                        .headers()
-                        .firstValue("location")
-                        .orElseThrow(
-                            () ->
-                                new ProtocolException(
-                                    "HTTP "
-                                        + response.statusCode()
-                                        + " without 'location' header set")));
-
-            if ("https".equalsIgnoreCase(rawUri.getScheme())
-                && !"https".equalsIgnoreCase(location.getScheme())) {
-              throw new SecurityException("Downgrade from secure to insecure connection.");
-            } else if ("wss".equalsIgnoreCase(rawUri.getScheme())
-                && !"wss".equalsIgnoreCase(location.getScheme())) {
-              throw new SecurityException("Downgrade from secure to insecure connection.");
-            }
-
+            URI location = rawUri.resolve(getLocationHeader(response, method, rawUri));
+            checkNotDowngrade(rawUri, location);
             rawUri = location;
             continue;
           default:
             return messages.createResponse(response);
         }
       }
 
-      throw new ProtocolException("Too many redirects: 101");
+      throw new ProtocolException(String.format("Too many redirects: 101 (%s)", describe(req)));
     } catch (HttpTimeoutException e) {
-      throw new TimeoutException(e);
+      throw new TimeoutException(
+          String.format("Timeout when executing request (%s)", describe(req)), e);
+    } catch (ConnectException e) {
+      throw new ConnectionException(
+          String.format("Connection error (%s)", describe(req)), maskUrlCredentials(rawUri), e);
     } catch (IOException e) {
-      throw new UncheckedIOException(e);
+      throw new UncheckedIOException(
+          String.format("Failed to execute request (%s)", describe(req)), e);
     } catch (InterruptedException e) {
       Thread.currentThread().interrupt();
-      throw new WebDriverException(e.getMessage(), e);
+      throw new WebDriverException(
+          String.format("%s when executing request (%s)", e.getMessage(), describe(req)), e);
     } finally {
       LOG.log(
           Level.FINE,
@@ -512,6 +501,67 @@ private HttpResponse execute0(HttpRequest req) throws UncheckedIOException {
     }
   }
 
+  private void checkNotDowngrade(URI from, URI to) {
+    if (isDowngradeFrom("https", from, to) || isDowngradeFrom("wss", from, to)) {
+      throw new SecurityException(
+          String.format("Downgrade from secure to insecure connection (%s -> %s)", from, to));
+    }
+  }
+
+  private boolean isDowngradeFrom(String protocol, URI from, URI to) {
+    return protocol.equalsIgnoreCase(from.getScheme())
+        && !protocol.equalsIgnoreCase(to.getScheme());
+  }
+
+  private String describe(HttpRequest req) {
+    String uri = maskUrlCredentials(messages.getRawUri(req));
+    HttpMethod method = req.getMethod();
+    return String.format("%s %s", method, uri);
+  }
+
+  static String maskUrlCredentials(String uri) {
+    try {
+      return maskUrlCredentials(new URI(uri));
+    } catch (URISyntaxException invalidUri) {
+      return uri;
+    }
+  }
+
+  private static String maskUrlCredentials(URI u) {
+    if (u.getUserInfo() == null) {
+      return u.toString();
+    }
+    try {
+      return new URI(
+              u.getScheme(),
+              "***",
+              u.getHost(),
+              u.getPort(),
+              u.getPath(),
+              u.getQuery(),
+              u.getFragment())
+          .toString();
+    } catch (URISyntaxException e) {
+      return u.toString();
+    }
+  }
+
+  private String getLocationHeader(
+      java.net.http.HttpResponse<InputStream> response, HttpMethod method, URI uri)
+      throws ProtocolException {
+    return response
+        .headers()
+        .firstValue("location")
+        .orElseThrow(
+            () -> {
+              String message =
+                  String.format(
+                      "HTTP response with status %d but without \"location\" header (%s %s)",
+                      response.statusCode(), method, maskUrlCredentials(uri));
+              return new ProtocolException(message);
+            });
+  }
+
   @Override
   public <T> CompletableFuture<java.net.http.HttpResponse<T>> sendAsyncNative(
       java.net.http.HttpRequest request, java.net.http.HttpResponse.BodyHandler<T> handler) {

java/test/logging.properties

@@ -0,0 +1,6 @@
+handlers=java.util.logging.ConsoleHandler
+java.util.logging.ConsoleHandler.level = FINE
+.level=INFO
+
+#Uncomment to enable DEBUG logs for some specific package:
+#org.openqa.selenium.bidi.level=FINE