Merge branch 'trunk' into null-exceptions

SeleniumHQ · Nov 17, 2024 · d8fb1d3 · d8fb1d3
2 parents e2d0d7c + e4ab299
commit d8fb1d3
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 2 deletions.
diff --git a/java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java b/java/src/org/openqa/selenium/grid/node/remote/RemoteNode.java
@@ -46,6 +46,7 @@
 import org.openqa.selenium.NoSuchSessionException;
 import org.openqa.selenium.RetrySessionRequestException;
 import org.openqa.selenium.SessionNotCreatedException;
+import org.openqa.selenium.TimeoutException;
 import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.grid.data.CreateSessionRequest;
 import org.openqa.selenium.grid.data.CreateSessionResponse;
@@ -132,7 +133,16 @@ public Either<WebDriverException, CreateSessionResponse> newSession(
     HttpTracing.inject(tracer, tracer.getCurrentContext(), req);
     req.setContent(asJson(sessionRequest));
 
-    HttpResponse httpResponse = client.with(addSecret).execute(req);
+    HttpResponse httpResponse;
+    try {
+      httpResponse = client.with(addSecret).execute(req);
+    } catch (TimeoutException e) {
+      // When using a short session timeout the node might not be able to start the session in time.
+      // The client timeout might be higher so, it makes sense to retry. In case the client does
+      // timeout, the SessionRequest is marked as canceled and the session is either not added to
+      // the queue or disposed as soon as the node started it.
+      return Either.left(new RetrySessionRequestException("Timeout while starting the session", e));
+    }
 
     Optional<Map<String, Object>> maybeResponse =
         Optional.ofNullable(Values.get(httpResponse, Map.class));

diff --git a/java/src/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueue.java b/java/src/org/openqa/selenium/grid/sessionqueue/local/LocalNewSessionQueue.java
@@ -293,11 +293,18 @@ public boolean retryAddToQueue(SessionRequest request) {
         if (!requests.containsKey(request.getRequestId())) {
           return false;
         }
-        if (isTimedOut(Instant.now(), requests.get(request.getRequestId()))) {
+        Data data = requests.get(request.getRequestId());
+        if (isTimedOut(Instant.now(), data)) {
           // as we try to re-add a session request that has already expired, force session timeout
           failDueToTimeout(request.getRequestId());
           // return true to avoid handleNewSessionRequest to call 'complete' an other time
           return true;
+        } else if (data.isCanceled()) {
+          complete(
+              request.getRequestId(),
+              Either.left(new SessionNotCreatedException("Client has gone away")));
+          // return true to avoid handleNewSessionRequest to call 'complete' an other time
+          return true;
         }
 
         if (queue.contains(request)) {
@@ -472,6 +479,10 @@ public synchronized void cancel() {
       canceled = true;
     }
 
+    public synchronized boolean isCanceled() {
+      return canceled;
+    }
+
     public synchronized boolean setResult(
         Either<SessionNotCreatedException, CreateSessionResponse> result) {
       if (complete || canceled) {