fixing the error codes situation... but having to do so with exposing a deprecated method on WebDriverException which i don't like one bit. Better ideas appreciated :)

lukeis · lukeis · commit d75a86d7b191 · 2016-10-12T22:05:40.000-07:00
diff --git a/java/client/src/org/openqa/selenium/WebDriverException.java b/java/client/src/org/openqa/selenium/WebDriverException.java
@@ -29,6 +29,7 @@ public class WebDriverException extends RuntimeException {
   public static final String SESSION_ID = "Session ID";
   public static final String DRIVER_INFO = "Driver info";
   protected static final String BASE_SUPPORT_URL = "http://seleniumhq.org/exceptions/";
+  protected Integer statusCode;
 
   private Map<String, String> extraInfo = new HashMap<>();
 
@@ -48,6 +49,23 @@ public WebDriverException(String message, Throwable cause) {
     super(message, cause);
   }
 
+  public WebDriverException(String message, Throwable cause, Integer statusCode) {
+    super(message, cause);
+    this.statusCode = statusCode;
+  }
+
+  /**
+   * DO NOT USE -
+   * This is to help the standalone server map exceptions from remote ends back to clients without
+   * losing the original status code provide (if one is). This will be going away in 4.0 when status
+   * codes are only string messages
+   * @return status code from the response of the remote end driver.
+   */
+  @Deprecated
+  public Integer getStatusCode() {
+    return statusCode;
+  }
+
   @Override
   public String getMessage() {
     return createMessage(super.getMessage());
diff --git a/java/client/src/org/openqa/selenium/remote/ErrorCodes.java b/java/client/src/org/openqa/selenium/remote/ErrorCodes.java
@@ -58,6 +58,7 @@
 import java.util.Set;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  * Defines common error codes for the wire protocol.
@@ -97,6 +98,9 @@ public class ErrorCodes {
   // json wire protocol doesn't have analogous status codes for
   // these new W3C status repsonse 'codes', so making some up!
   public static final int ELEMENT_NOT_INTERACTABLE = 60;
+  public static final int INVALID_ARGUMENT = 61;
+  public static final int NO_SUCH_COOKIE = 62;
+  public static final int UNABLE_TO_CAPTURE_SCREEN = 63;
 
   // The following error codes are derived straight from HTTP return codes.
   public static final int METHOD_NOT_ALLOWED = 405;
@@ -113,9 +117,9 @@ public class ErrorCodes {
       .put(400,
            ImmutableSortedSet.<StatusTuple>naturalOrder()
              .add(new StatusTuple("element not selectable", ELEMENT_NOT_SELECTABLE, ElementNotSelectableException.class))
-             .add(new StatusTuple("element not interactable", ELEMENT_NOT_INTERACTABLE, ElementNotInteractableException.class))
+             .add(new StatusTuple("element not interactable", ELEMENT_NOT_INTERACTABLE, ElementNotInteractableException.class, UNHANDLED_ERROR))
              .add(new StatusTuple("element not visible", ELEMENT_NOT_VISIBLE, ElementNotVisibleException.class))
-             .add(new StatusTuple("invalid argument", UNHANDLED_ERROR, InvalidArgumentException.class))
+             .add(new StatusTuple("invalid argument", INVALID_ARGUMENT, InvalidArgumentException.class, UNHANDLED_ERROR))
              .add(new StatusTuple("invalid cookie domain", INVALID_COOKIE_DOMAIN, InvalidCookieDomainException.class))
              .add(new StatusTuple("invalid element coordinates", INVALID_ELEMENT_COORDINATES, InvalidCoordinatesException.class))
              .add(new StatusTuple("invalid element state", INVALID_ELEMENT_STATE, InvalidElementStateException.class))
@@ -131,9 +135,9 @@ public class ErrorCodes {
       .put(404,
            ImmutableSortedSet.<StatusTuple>naturalOrder()
              .add(new StatusTuple("invalid session id", NO_SUCH_SESSION, NoSuchSessionException.class))
-             .add(new StatusTuple("no such cookie", UNHANDLED_ERROR, NoSuchCookieException.class))
+             .add(new StatusTuple("no such cookie", NO_SUCH_COOKIE, NoSuchCookieException.class, UNHANDLED_ERROR))
              .add(new StatusTuple("no such element", NO_SUCH_ELEMENT, NoSuchElementException.class))
-             .add(new StatusTuple("unknown command", UNKNOWN_COMMAND, UnsupportedCommandException.class, UNHANDLED_ERROR))
+             .add(new StatusTuple("unknown command", UNKNOWN_COMMAND, UnsupportedCommandException.class))
              .build())
       .put(405,
            ImmutableSortedSet.<StatusTuple>naturalOrder()
@@ -146,13 +150,13 @@ public class ErrorCodes {
              .build())
   .put(500,
        ImmutableSortedSet.<StatusTuple>naturalOrder()
-         .add(new StatusTuple("javascript error", JAVASCRIPT_ERROR, WebDriverException.class, UNHANDLED_ERROR))
+         .add(new StatusTuple("javascript error", JAVASCRIPT_ERROR, WebDriverException.class))
          .add(new StatusTuple("move target out of bounds", MOVE_TARGET_OUT_OF_BOUNDS, MoveTargetOutOfBoundsException.class))
          .add(new StatusTuple("session not created", SESSION_NOT_CREATED, SessionNotCreatedException.class))
          .add(new StatusTuple("unable to set cookie", UNABLE_TO_SET_COOKIE, UnableToSetCookieException.class))
-         .add(new StatusTuple("unable to capture screen", UNHANDLED_ERROR, ScreenshotException.class))
+         .add(new StatusTuple("unable to capture screen", UNABLE_TO_CAPTURE_SCREEN, ScreenshotException.class, UNHANDLED_ERROR))
          .add(new StatusTuple("unexpected alert open", UNEXPECTED_ALERT_PRESENT, UnhandledAlertException.class))
-         .add(new StatusTuple("unknown error", UNHANDLED_ERROR, WebDriverException.class, UNHANDLED_ERROR))
+         .add(new StatusTuple("unknown error", UNHANDLED_ERROR, WebDriverException.class))
          .add(new StatusTuple("unsupported operation", METHOD_NOT_ALLOWED, UnsupportedCommandException.class, UNHANDLED_ERROR))
          .add(new StatusTuple("unsupported operation", IME_NOT_AVAILABLE, ImeNotAvailableException.class))
          .add(new StatusTuple("unsupported operation", IME_ENGINE_ACTIVATION_FAILED, ImeActivationFailedException.class))
@@ -165,6 +169,11 @@ public class ErrorCodes {
     .flatMap(Collection::stream)
     .collect(Collectors.toMap(StatusTuple::asStatus, StatusTuple::asState, (key1, key2) -> key1));
 
+  private static final Map<Integer, Class> JSON_TO_EXCEPTION = ALL_CODES.values().stream()
+    .flatMap(Collection::stream)
+    .filter(tuple -> tuple.getException() != null)
+    .collect(Collectors.toMap(StatusTuple::asStatus, StatusTuple::getException, (key1, key2) -> key1));
+
   private static final Map<String, Integer> W3C_TO_JSON = ALL_CODES.values().stream()
     .flatMap(Collection::stream)
     .collect(Collectors.toMap(StatusTuple::asState, StatusTuple::asStatus, (key1, key2) -> key1));
@@ -221,7 +230,7 @@ public int toStatus(String webdriverState, Optional<Integer> httpStatus) {
    *         {@code statusCode == 0}.
    */
   public Class<? extends WebDriverException> getExceptionType(int statusCode) {
-    return getExceptionType(toState(statusCode));
+    return JSON_TO_EXCEPTION.getOrDefault(statusCode, WebDriverException.class);
   }
 
   public Class<? extends WebDriverException> getExceptionType(String webdriverState) {
@@ -242,13 +251,34 @@ public int toStatusCode(Throwable e) {
     }
 
     // And then handle the other cases
-    Set<Integer> possibleMatches = ALL_CODES.values().stream()
+    Stream<StatusTuple> allCodesStream = ALL_CODES.values().stream()
       .flatMap(Collection::stream)
       .filter(tuple -> tuple.getException() != null)
-      .filter(tuple -> tuple.associatedException.isAssignableFrom(e.getClass()))
+      .filter(tuple -> tuple.associatedException.isAssignableFrom(e.getClass()));
+
+    if (e instanceof WebDriverException && ((WebDriverException)e).getStatusCode() != null) {
+      allCodesStream = allCodesStream.filter(tuple -> ((WebDriverException)e).getStatusCode() == tuple.jsonStatus );
+    }
+
+    Set<Integer> possibleMatches = allCodesStream
       .map(StatusTuple::getStatusFromException)
       .collect(Collectors.toSet());
 
+    if (possibleMatches.size() > 1) {
+      // if there's multiple matches, let's try filtering on the exact exception class to see if we
+      // can reduce the possibilities
+      Set<Integer> reducedPossibleMatches = ALL_CODES.values().stream()
+        .flatMap(Collection::stream)
+        .filter(tuple -> tuple.getException() != null)
+        .filter(tuple -> tuple.associatedException.equals(e.getClass()))
+        .map(StatusTuple::getStatusFromException)
+        .collect(Collectors.toSet());
+
+      if (reducedPossibleMatches.size() > 0) {
+        possibleMatches = reducedPossibleMatches;
+      }
+    }
+
     return Preconditions.checkNotNull(Iterables.getFirst(possibleMatches, UNHANDLED_ERROR));
   }
 
diff --git a/java/client/src/org/openqa/selenium/remote/ErrorHandler.java b/java/client/src/org/openqa/selenium/remote/ErrorHandler.java
@@ -159,6 +159,12 @@ public Response throwIfResponseFailed(Response response, long duration) throws R
       toThrow = createUnhandledAlertException(value);
     }
 
+    if (toThrow == null) {
+      toThrow = createThrowable(outerErrorType,
+                                new Class<?>[] {String.class, Throwable.class, Integer.class},
+                                new Object[] {message, cause, response.getStatus()});
+    }
+
     if (toThrow == null) {
       toThrow = createThrowable(outerErrorType,
           new Class<?>[] {String.class, Throwable.class},
diff --git a/java/client/test/org/openqa/selenium/remote/ErrorHandlerTest.java b/java/client/test/org/openqa/selenium/remote/ErrorHandlerTest.java
@@ -53,14 +53,12 @@
 import org.openqa.selenium.UnableToSetCookieException;
 import org.openqa.selenium.UnhandledAlertException;
 import org.openqa.selenium.UnsupportedCommandException;
-import org.openqa.selenium.WebDriver;
 import org.openqa.selenium.WebDriverException;
 import org.openqa.selenium.interactions.InvalidCoordinatesException;
 import org.openqa.selenium.interactions.MoveTargetOutOfBoundsException;
 
 import javafx.util.Pair;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
@@ -473,7 +471,12 @@ public void testStatusCodesRaisedBackToStatusMatches() {
         fail("Should have thrown an Exception");
       } catch (Exception e) {
         assertEquals("Checking status code: " + exception.getKey(), exception.getValue().getSimpleName(), e.getClass().getSimpleName());
-        assertEquals(e.getClass().getSimpleName() + " ErrorCodes.toStatusCode", exception.getKey().intValue(), new ErrorCodes().toStatusCode(e));
+        int expected = exception.getKey();
+        if (e instanceof InvalidSelectorException) {
+          // all of the special invalid selector exceptions are just mapped to the generic invalid selector
+          expected = 32;
+        }
+        assertEquals(e.getClass().getSimpleName() + " ErrorCodes.toStatusCode", expected, new ErrorCodes().toStatusCode(e));
       }
     }
   }
