[grid] stop a stale session more graceful

SeleniumHQ · Nov 15, 2024 · b0464e1 · b0464e1
1 parent fc03c5e
commit b0464e1
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 12 deletions.
diff --git a/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java b/java/src/org/openqa/selenium/grid/distributor/local/LocalDistributor.java
@@ -27,6 +27,7 @@
 import static org.openqa.selenium.remote.RemoteTags.CAPABILITIES_EVENT;
 import static org.openqa.selenium.remote.RemoteTags.SESSION_ID;
 import static org.openqa.selenium.remote.RemoteTags.SESSION_ID_EVENT;
+import static org.openqa.selenium.remote.http.HttpMethod.DELETE;
 import static org.openqa.selenium.remote.tracing.AttributeKey.SESSION_URI;
 import static org.openqa.selenium.remote.tracing.Tags.EXCEPTION;
 
@@ -79,6 +80,7 @@
 import org.openqa.selenium.grid.data.NodeStatus;
 import org.openqa.selenium.grid.data.NodeStatusEvent;
 import org.openqa.selenium.grid.data.RequestId;
+import org.openqa.selenium.grid.data.Session;
 import org.openqa.selenium.grid.data.SessionRequest;
 import org.openqa.selenium.grid.data.SessionRequestCapability;
 import org.openqa.selenium.grid.data.Slot;
@@ -109,6 +111,7 @@
 import org.openqa.selenium.internal.Require;
 import org.openqa.selenium.remote.SessionId;
 import org.openqa.selenium.remote.http.HttpClient;
+import org.openqa.selenium.remote.http.HttpRequest;
 import org.openqa.selenium.remote.tracing.AttributeKey;
 import org.openqa.selenium.remote.tracing.AttributeMap;
 import org.openqa.selenium.remote.tracing.Span;
@@ -858,13 +861,29 @@ private void handleNewSessionRequest(SessionRequest sessionRequest) {
         if (!isSessionValid && response.isRight()) {
           LOG.log(
               Level.INFO,
-              "Session for request {0} has been created but it has timed out, stopping it to avoid"
-                  + " stalled browser",
+              "Session for request {0} has been created but it has timed out or the connection"
+                  + " dropped, stopping it to avoid stalled browser",
               reqId.toString());
-          URI nodeURI = response.right().getSession().getUri();
-          Node node = getNodeFromURI(nodeURI);
+          Session session = response.right().getSession();
+          Node node = getNodeFromURI(session.getUri());
           if (node != null) {
-            node.stop(response.right().getSession().getId());
+            boolean deleted;
+            try {
+              // Attempt to stop the session
+              deleted =
+                  node.execute(new HttpRequest(DELETE, "/session/" + session.getId())).getStatus()
+                      == 200;
+            } catch (Exception e) {
+              LOG.log(
+                  Level.WARNING,
+                  String.format("Exception while trying to delete session %s", session.getId()),
+                  e);
+              deleted = false;
+            }
+            if (!deleted) {
+              // Kill the session
+              node.stop(session.getId());
+            }
           }
         }
       }

diff --git a/java/test/org/openqa/selenium/grid/router/DistributedTest.java b/java/test/org/openqa/selenium/grid/router/DistributedTest.java
@@ -138,12 +138,8 @@ void clientTimeoutDoesNotLeakARunningBrowser() throws Exception {
 
       assertThat(nce.getMessage()).contains("TimeoutException");
 
-      Thread.sleep(
-          // ensure the grid has some time to start the browser
-          Duration.ofNanos((end - start) * 2)
-              // and shutdown the browser
-              .plusSeconds(20)
-              .toMillis());
+      // ensure the grid has some time to start the browser and shutdown the browser
+      Thread.sleep(Duration.ofNanos((end - start) * 3).toMillis());
 
       HttpClient client = HttpClient.Factory.createDefault().createClient(server.getUrl());
       try {
@@ -193,7 +189,7 @@ void clientTimeoutDoesNotLeakARunningBrowser() throws Exception {
         Safely.safelyCall(client::close);
       }
     } finally {
-      Safely.safelyCall(healthy::close);
+      Safely.safelyCall(healthy::quit);
     }
   }
 }