[grid] Retry session only when capabilities are found. Add test to co…

…nfirm session retry due to unexpected error. (#8864) Co-authored-by: Diego Molina <[email protected]>
SeleniumHQ · Nov 15, 2020 · 04ec49d · 04ec49d
1 parent a9a4371
commit 04ec49d
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 3 deletions.
diff --git a/java/server/src/org/openqa/selenium/grid/distributor/Distributor.java b/java/server/src/org/openqa/selenium/grid/distributor/Distributor.java
@@ -198,9 +198,11 @@ public Either<SessionNotCreatedException, CreateSessionResponse> newSession(
           .anyMatch(nodeStatus -> nodeStatus.hasCapability(firstRequest.getCapabilities()));
 
         if (!hostsWithCaps) {
-          throw new SessionNotCreatedException(
-            "No host supports the capabilities required: " + payload.stream()
-              .map(Capabilities::toString).collect(Collectors.joining(", ")));
+          String errorMessage = String.format(
+            "No host supports the capabilities required: %s",
+            payload.stream().map(Capabilities::toString).collect(Collectors.joining(", ")));
+          SessionNotCreatedException exception = new SessionNotCreatedException(errorMessage);
+          return Either.left(exception);
         }
 
         // Find a host that supports the capabilities present in the new session

diff --git a/java/server/test/org/openqa/selenium/grid/router/NewSessionCreationTest.java b/java/server/test/org/openqa/selenium/grid/router/NewSessionCreationTest.java
@@ -22,6 +22,9 @@
 import com.google.common.collect.ImmutableSet;
 import org.junit.Before;
 import org.junit.Test;
+import org.openqa.selenium.Capabilities;
+import org.openqa.selenium.ImmutableCapabilities;
+import org.openqa.selenium.SessionNotCreatedException;
 import org.openqa.selenium.WebDriverInfo;
 import org.openqa.selenium.chrome.ChromeDriverInfo;
 import org.openqa.selenium.events.EventBus;
@@ -39,10 +42,12 @@
 import org.openqa.selenium.grid.server.Server;
 import org.openqa.selenium.grid.sessionmap.SessionMap;
 import org.openqa.selenium.grid.sessionmap.local.LocalSessionMap;
+import org.openqa.selenium.grid.sessionqueue.NewSessionQueue;
 import org.openqa.selenium.grid.sessionqueue.NewSessionQueuer;
 import org.openqa.selenium.grid.sessionqueue.local.LocalNewSessionQueue;
 import org.openqa.selenium.grid.sessionqueue.local.LocalNewSessionQueuer;
 import org.openqa.selenium.grid.testing.TestSessionFactory;
+import org.openqa.selenium.grid.web.CombinedHandler;
 import org.openqa.selenium.grid.web.EnsureSpecCompliantHeaders;
 import org.openqa.selenium.netty.server.NettyServer;
 import org.openqa.selenium.remote.http.Contents;
@@ -55,15 +60,19 @@
 import org.openqa.selenium.remote.tracing.Tracer;
 import org.openqa.selenium.testing.drivers.Browser;
 
+import java.net.MalformedURLException;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.time.Duration;
 import java.time.Instant;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import static java.net.HttpURLConnection.HTTP_INTERNAL_ERROR;
+import static java.net.HttpURLConnection.HTTP_OK;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assumptions.assumeThat;
 import static org.openqa.selenium.json.Json.JSON_UTF_8;
+import static org.openqa.selenium.remote.http.Contents.asJson;
 import static org.openqa.selenium.remote.http.HttpMethod.POST;
 
 public class NewSessionCreationTest {
@@ -157,6 +166,76 @@ public void ensureJsCannotCreateANewSession() throws URISyntaxException {
     assertThat(res.isSuccessful()).isTrue();
   }
 
+  @Test
+  public void shouldRetryNewSessionRequestOnUnexpectedError() throws
+    URISyntaxException, MalformedURLException {
+    Capabilities capabilities = new ImmutableCapabilities("browserName", "cheese");
+    URI nodeUri = new URI("http://localhost:4444");
+    CombinedHandler handler = new CombinedHandler();
+
+    SessionMap sessions = new LocalSessionMap(tracer, events);
+    handler.addHandler(sessions);
+    NewSessionQueue localNewSessionQueue = new LocalNewSessionQueue(
+      tracer,
+      events,
+      Duration.ofSeconds(2),
+      Duration.ofSeconds(10));
+    NewSessionQueuer queuer = new LocalNewSessionQueuer(tracer, events, localNewSessionQueue);
+    handler.addHandler(queuer);
+
+    Distributor distributor = new LocalDistributor(
+      tracer,
+      events,
+      clientFactory,
+      sessions,
+      queuer,
+      registrationSecret);
+    handler.addHandler(distributor);
+
+    AtomicInteger count = new AtomicInteger();
+
+    // First session creation attempt throws an error.
+    // Second attempt creates a session.
+    TestSessionFactory sessionFactory = new TestSessionFactory((id, caps) -> {
+      if (count.get() == 0) {
+        count.incrementAndGet();
+        throw new SessionNotCreatedException("Expected the exception");
+      } else {
+        return new Session(
+          id,
+          nodeUri,
+          new ImmutableCapabilities(),
+          caps,
+          Instant.now());
+      }
+    });
+
+    LocalNode localNode = LocalNode.builder(tracer, events, nodeUri, nodeUri, registrationSecret)
+      .add(capabilities, sessionFactory).build();
+    handler.addHandler(localNode);
+    distributor.add(localNode);
+
+    Router router = new Router(tracer, clientFactory, sessions, queuer, distributor);
+    handler.addHandler(router);
+
+    Server<?> server = new NettyServer(
+      new BaseServerOptions(
+        new MapConfig(ImmutableMap.of())),
+      handler);
+
+    server.start();
+
+    HttpRequest request = new HttpRequest(POST, "/session");
+    request.setContent(asJson(
+      ImmutableMap.of(
+        "capabilities", ImmutableMap.of(
+          "alwaysMatch", capabilities))));
+
+    HttpClient client = clientFactory.createClient(server.getUrl());
+    HttpResponse httpResponse = client.execute(request);
+    assertThat(httpResponse.getStatus()).isEqualTo(HTTP_OK);
+  }
+
   private LocalNode.Builder addDriverFactory(
     LocalNode.Builder builder,
     WebDriverInfo info,