[java] Throw SessionNotCreatedException instead of UnreachableBrowserException if a session cannot be started

Author: barancev

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

@@ -59,6 +59,7 @@
 import org.openqa.selenium.Platform;
 import org.openqa.selenium.Point;
 import org.openqa.selenium.SearchContext;
+import org.openqa.selenium.SessionNotCreatedException;
 import org.openqa.selenium.TakesScreenshot;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.WebDriverException;
@@ -498,7 +499,7 @@ protected Response execute(CommandPayload payload) {
     long start = System.currentTimeMillis();
     String currentName = Thread.currentThread().getName();
     Thread.currentThread().setName(
-        String.format("Forwarding %s on session %s to remote", command.getName(), sessionId));
+      String.format("Forwarding %s on session %s to remote", command.getName(), sessionId));
     try {
       log(sessionId, command.getName(), command, When.BEFORE);
       response = executor.execute(command);
@@ -518,22 +519,24 @@ protected Response execute(CommandPayload payload) {
     } catch (WebDriverException e) {
       e.addInfo("Command", command.toString());
       throw e;
-    } catch (Exception e) {
+    } catch (Throwable e) {
       log(sessionId, command.getName(), command, When.EXCEPTION);
-      String errorMessage = "Error communicating with the remote browser. " +
-          "It may have died.";
       if (command.getName().equals(DriverCommand.NEW_SESSION)) {
-        errorMessage = "Could not start a new session. Possible causes are " +
-            "invalid address of the remote server or browser start-up failure.";
-      }
-      UnreachableBrowserException ube = new UnreachableBrowserException(errorMessage, e);
-      if (getSessionId() != null) {
-        ube.addInfo(WebDriverException.SESSION_ID, getSessionId().toString());
-      }
-      if (getCapabilities() != null) {
-        ube.addInfo("Capabilities", getCapabilities().toString());
+        throw new SessionNotCreatedException(
+          "Could not start a new session. Possible causes are invalid address of the remote server or browser start-up failure.",
+          e);
+      } else {
+        WebDriverException toThrow = new UnreachableBrowserException(
+          "Error communicating with the remote browser. It may have died.",
+          e);
+        if (getSessionId() != null) {
+          toThrow.addInfo(WebDriverException.SESSION_ID, getSessionId().toString());
+        }
+        if (getCapabilities() != null) {
+          toThrow.addInfo("Capabilities", getCapabilities().toString());
+        }
+        throw toThrow;
       }
-      throw ube;
     } finally {
       Thread.currentThread().setName(currentName);
     }

java/client/test/org/openqa/selenium/remote/RemoteWebDriverUnitTest.java

@@ -47,6 +47,7 @@
 import org.openqa.selenium.Platform;
 import org.openqa.selenium.Point;
 import org.openqa.selenium.Rectangle;
+import org.openqa.selenium.SessionNotCreatedException;
 import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.WebElement;
 import org.openqa.selenium.WindowType;
@@ -78,9 +79,9 @@ public void constructorShouldThrowIfExecutorIsNull() {
   }
 
   @Test
-  public void constructorShouldThrowIfExecutorCannotStartASession() throws IOException {
-    CommandExecutor executor = prepareExecutorMock(nullResponder, nullResponder);
-    assertThatExceptionOfType(UnreachableBrowserException.class)
+  public void constructorShouldThrowIfExecutorThrowsOnAnAttemptToStartASession() throws IOException {
+    CommandExecutor executor = prepareExecutorMock(exceptionalResponder);
+    assertThatExceptionOfType(SessionNotCreatedException.class)
       .isThrownBy(() -> new RemoteWebDriver(executor, new ImmutableCapabilities()));
 
     verify(executor).execute(argThat(
@@ -1140,28 +1141,32 @@ private boolean isWebElement(Object value) {
         .isPresent();
   }
 
-  private Function<Command, Response> echoCapabilities = cmd -> {
+  private final Function<Command, Response> echoCapabilities = cmd -> {
     Response nullResponse = new Response();
     nullResponse.setValue(
-        ((Capabilities) cmd.getParameters().get("desiredCapabilities")).asMap()
-            .entrySet().stream()
-            .collect(Collectors.toMap(e -> e.getKey(), e -> e.getValue().toString())));
+      ((Capabilities) cmd.getParameters().get("desiredCapabilities")).asMap()
+        .entrySet().stream()
+        .collect(Collectors.toMap(e -> e.getKey(), e -> e.getValue().toString())));
     nullResponse.setSessionId(UUID.randomUUID().toString());
     return nullResponse;
   };
 
-  private Function<Command, Response> nullResponder = cmd -> {
+  private final Function<Command, Response> nullResponder = cmd -> {
     Response nullResponse = new Response();
     nullResponse.setValue(null);
-    nullResponse.setSessionId(cmd.getSessionId().toString());
+    nullResponse.setSessionId(cmd.getSessionId() != null ? cmd.getSessionId().toString() : null);
     return nullResponse;
   };
 
+  private final Function<Command, Response> exceptionalResponder = cmd -> {
+    throw new InternalError("BOOM!!!");
+  };
+
   private Function<Command, Response> valueResponder(Object value) {
     return cmd -> {
       Response nullResponse = new Response();
       nullResponse.setValue(value);
-      nullResponse.setSessionId(cmd.getSessionId().toString());
+      nullResponse.setSessionId(cmd.getSessionId() != null ? cmd.getSessionId().toString() : null);
       return nullResponse;
     };
   }