[grid] Update session slot to return Either for error propagation. Fix http response. (#9270)

* [grid] Update http response content to a format recognised by the client end

* [grid] Update session slot to return Either for error propagation

Author: pujagani

java/server/src/org/openqa/selenium/grid/node/local/LocalNode.java

@@ -303,7 +303,7 @@ public Either<WebDriverException, CreateSessionResponse> newSession(CreateSessio
 
       // Identify possible slots to use as quickly as possible to enable concurrent session starting
       SessionSlot slotToUse = null;
-      synchronized(factories) {
+      synchronized (factories) {
         for (SessionSlot factory : factories) {
           if (!factory.isAvailable() || !factory.test(sessionRequest.getCapabilities())) {
             continue;
@@ -323,36 +323,35 @@ public Either<WebDriverException, CreateSessionResponse> newSession(CreateSessio
           new RetrySessionRequestException("No slot matched the requested capabilities. All slots are busy."));
       }
 
-      Optional<ActiveSession> possibleSession = slotToUse.apply(sessionRequest);
-
-      if (!possibleSession.isPresent()) {
+      Either<WebDriverException, ActiveSession> possibleSession = slotToUse.apply(sessionRequest);
+
+      if (possibleSession.isRight()) {
+        ActiveSession session = possibleSession.right();
+        currentSessions.put(session.getId(), slotToUse);
+
+        SessionId sessionId = session.getId();
+        Capabilities caps = session.getCapabilities();
+        SESSION_ID.accept(span, sessionId);
+        CAPABILITIES.accept(span, caps);
+        String downstream = session.getDownstreamDialect().toString();
+        String upstream = session.getUpstreamDialect().toString();
+        String sessionUri = session.getUri().toString();
+        span.setAttribute(AttributeKey.DOWNSTREAM_DIALECT.getKey(), downstream);
+        span.setAttribute(AttributeKey.UPSTREAM_DIALECT.getKey(), upstream);
+        span.setAttribute(AttributeKey.SESSION_URI.getKey(), sessionUri);
+
+        // The session we return has to look like it came from the node, since we might be dealing
+        // with a webdriver implementation that only accepts connections from localhost
+        Session externalSession = createExternalSession(session, externalUri, slotToUse.isSupportingCdp());
+        return Either.right(new CreateSessionResponse(
+          externalSession,
+          getEncoder(session.getDownstreamDialect()).apply(externalSession)));
+      } else {
         slotToUse.release();
         span.setAttribute("error", true);
-        span.setStatus(Status.NOT_FOUND);
         span.addEvent("Unable to create session with the driver", attributeMap);
-        return Either.left(new SessionNotCreatedException("Unable to create session with the driver"));
+        return Either.left(possibleSession.left());
       }
-
-      ActiveSession session = possibleSession.get();
-      currentSessions.put(session.getId(), slotToUse);
-
-      SessionId sessionId = session.getId();
-      Capabilities caps = session.getCapabilities();
-      SESSION_ID.accept(span, sessionId);
-      CAPABILITIES.accept(span, caps);
-      String downstream = session.getDownstreamDialect().toString();
-      String upstream = session.getUpstreamDialect().toString();
-      String sessionUri = session.getUri().toString();
-      span.setAttribute(AttributeKey.DOWNSTREAM_DIALECT.getKey(), downstream);
-      span.setAttribute(AttributeKey.UPSTREAM_DIALECT.getKey(), upstream);
-      span.setAttribute(AttributeKey.SESSION_URI.getKey(), sessionUri);
-
-      // The session we return has to look like it came from the node, since we might be dealing
-      // with a webdriver implementation that only accepts connections from localhost
-      Session externalSession = createExternalSession(session, externalUri, slotToUse.isSupportingCdp());
-      return Either.right(new CreateSessionResponse(
-        externalSession,
-        getEncoder(session.getDownstreamDialect()).apply(externalSession)));
     }
   }
 

java/server/src/org/openqa/selenium/grid/node/local/SessionSlot.java

@@ -20,12 +20,16 @@
 import org.openqa.selenium.Capabilities;
 import org.openqa.selenium.ImmutableCapabilities;
 import org.openqa.selenium.NoSuchSessionException;
+import org.openqa.selenium.RetrySessionRequestException;
+import org.openqa.selenium.SessionNotCreatedException;
+import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.WebDriverInfo;
 import org.openqa.selenium.events.EventBus;
 import org.openqa.selenium.grid.data.CreateSessionRequest;
 import org.openqa.selenium.grid.data.SessionClosedEvent;
 import org.openqa.selenium.grid.node.ActiveSession;
 import org.openqa.selenium.grid.node.SessionFactory;
+import org.openqa.selenium.internal.Either;
 import org.openqa.selenium.internal.Require;
 import org.openqa.selenium.remote.SessionId;
 import org.openqa.selenium.remote.http.HttpHandler;
@@ -45,7 +49,7 @@
 
 public class SessionSlot implements
   HttpHandler,
-  Function<CreateSessionRequest, Optional<ActiveSession>>,
+  Function<CreateSessionRequest, Either<WebDriverException, ActiveSession>>,
   Predicate<Capabilities> {
 
   private static final Logger LOG = Logger.getLogger(SessionSlot.class.getName());
@@ -126,22 +130,27 @@ public boolean test(Capabilities capabilities) {
   }
 
   @Override
-  public Optional<ActiveSession> apply(CreateSessionRequest sessionRequest) {
+  public Either<WebDriverException, ActiveSession> apply(CreateSessionRequest sessionRequest) {
     if (currentSession != null) {
-      return Optional.empty();
+      return Either.left(new RetrySessionRequestException("Slot is busy. Try another slot."));
     }
 
     if (!test(sessionRequest.getCapabilities())) {
-      return Optional.empty();
+      return Either.left(new SessionNotCreatedException("New session request capabilities does match the stereotype."));
     }
 
     try {
       Optional<ActiveSession> possibleSession = factory.apply(sessionRequest);
-      possibleSession.ifPresent(session -> currentSession = session);
-      return possibleSession;
+      if(possibleSession.isPresent()) {
+        ActiveSession session = possibleSession.get();
+        currentSession = session;
+        return Either.right(session);
+      } else {
+        return Either.left(new SessionNotCreatedException("Error while creating session with the driver service."));
+      }
     } catch (Exception e) {
       LOG.log(Level.WARNING, "Unable to create session", e);
-      return Optional.empty();
+      return Either.left(new SessionNotCreatedException(e.getMessage()));
     }
   }
 

java/server/src/org/openqa/selenium/grid/sessionqueue/GetNewSessionResponse.java

@@ -143,7 +143,7 @@ public HttpResponse add(HttpRequest request) {
   private HttpResponse internalErrorResponse(String message) {
     return new HttpResponse()
       .setStatus(HTTP_INTERNAL_ERROR)
-      .setContent(asJson(singletonMap("message", message)));
+      .setContent(asJson(singletonMap("value", singletonMap("message", message))));
   }
 
   private void removeRequest(RequestId id) {