Modified logic of ErrorHandler mapping to *Exception.

Add logic to try to resolve *Exception class in case RemoteWebDriver doesn’t use a Selenium Java Error Classes. Fixes #7437.
SeleniumHQ · Jun 6, 2014 · 4959695 · 4959695
1 parent fc79624
commit 4959695
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 16 deletions.
diff --git a/java/client/src/org/openqa/selenium/remote/ErrorHandler.java b/java/client/src/org/openqa/selenium/remote/ErrorHandler.java
@@ -115,7 +115,7 @@ public Response throwIfResponseFailed(Response response, long duration) throws R
         message = String.valueOf(e);
       }
 
-      Throwable serverError = rebuildServerError(rawErrorData);
+      Throwable serverError = rebuildServerError(rawErrorData, response.getStatus());
 
       // If serverError is null, then the server did not provide a className (only expected if
       // the server is a Java process) or a stack trace. The lack of a className is OK, but
@@ -141,7 +141,7 @@ public Response throwIfResponseFailed(Response response, long duration) throws R
 
     String duration1 = duration(duration);
 
-    if (message != null && message.indexOf(duration1) == -1) {
+    if (message != null && !message.contains(duration1)) {
       message = message + duration1;
     }
 
@@ -216,7 +216,7 @@ private <T extends Throwable> T createThrowable(
     return null;
   }
 
-  private Throwable rebuildServerError(Map<String, Object> rawErrorData) {
+  private Throwable rebuildServerError(Map<String, Object> rawErrorData, int responseStatus) {
 
     if (!rawErrorData.containsKey(CLASS) && !rawErrorData.containsKey(STACK_TRACE)) {
       // Not enough information for us to try to rebuild an error.
@@ -225,24 +225,34 @@ private Throwable rebuildServerError(Map<String, Object> rawErrorData) {
 
     Throwable toReturn = null;
     String message = (String) rawErrorData.get(MESSAGE);
+    Class clazz = null;
 
+    // First: allow Remote Driver to specify the Selenium Server internal exception
     if (rawErrorData.containsKey(CLASS)) {
       String className = (String) rawErrorData.get(CLASS);
       try {
-        Class clazz = Class.forName(className);
-        if (clazz.equals(UnhandledAlertException.class)) {
-          toReturn = createUnhandledAlertException(rawErrorData);
-        } else if (Throwable.class.isAssignableFrom(clazz)) {
-          @SuppressWarnings({"unchecked"})
-          Class<? extends Throwable> throwableType = (Class<? extends Throwable>) clazz;
-          toReturn = createThrowable(throwableType, new Class<?>[] {String.class},
-              new Object[] {message});
-        }
+        clazz = Class.forName(className);
       } catch (ClassNotFoundException ignored) {
         // Ok, fall-through
       }
     }
 
+    // If the above fails, map Response Status to Exception class
+    if (null == clazz) {
+      clazz = errorCodes.getExceptionType(responseStatus);
+    }
+
+    if (clazz.equals(UnhandledAlertException.class)) {
+      toReturn = createUnhandledAlertException(rawErrorData);
+    } else if (Throwable.class.isAssignableFrom(clazz)) {
+      @SuppressWarnings({"unchecked"})
+      Class<? extends Throwable> throwableType = (Class<? extends Throwable>) clazz;
+      toReturn = createThrowable(
+          throwableType,
+          new Class<?>[] {String.class},
+          new Object[] {message});
+    }
+
     if (toReturn == null) {
       toReturn = new UnknownServerException(message);
     }

diff --git a/java/client/test/org/openqa/selenium/remote/ErrorHandlerTest.java b/java/client/test/org/openqa/selenium/remote/ErrorHandlerTest.java
@@ -249,7 +249,7 @@ public void testShouldIncludeScreenshotIfProvided() throws Exception {
 
   @SuppressWarnings({"unchecked", "ThrowableInstanceNeverThrown"})
   @Test
-  public void testShouldDefaultToUnknownServerErrorIfClassIsNotSpecified()
+  public void testShouldDefaultToWebDriverExceptionIfClassIsNotSpecified()
       throws Exception {
     RuntimeException serverError = new RuntimeException("foo bar baz!");
     Map<String, Object> data = toMap(serverError);
@@ -264,7 +264,7 @@ public void testShouldDefaultToUnknownServerErrorIfClassIsNotSpecified()
 
       Throwable cause = expected.getCause();
       assertNotNull(cause);
-      assertEquals(ErrorHandler.UnknownServerException.class, cause.getClass());
+      assertEquals(WebDriverException.class, cause.getClass());
       assertEquals(new WebDriverException(serverError.getMessage()).getMessage(),
           cause.getMessage());
       assertStackTracesEqual(serverError.getStackTrace(), cause.getStackTrace());
@@ -273,7 +273,7 @@ public void testShouldDefaultToUnknownServerErrorIfClassIsNotSpecified()
 
   @SuppressWarnings({"unchecked", "ThrowableInstanceNeverThrown"})
   @Test
-  public void testShouldStillTryToBuildServerErrorIfClassIsNotProvidedAndStackTraceIsNotForJava() {
+  public void testShouldStillTryToBuildWebDriverExceptionIfClassIsNotProvidedAndStackTraceIsNotForJava() {
     Map<String, ?> data = ImmutableMap.of(
         "message", "some error message",
         "stackTrace", Lists.newArrayList(
@@ -297,7 +297,7 @@ public void testShouldStillTryToBuildServerErrorIfClassIsNotProvidedAndStackTrac
 
       Throwable cause = expected.getCause();
       assertNotNull(cause);
-      assertEquals(ErrorHandler.UnknownServerException.class, cause.getClass());
+      assertEquals(WebDriverException.class, cause.getClass());
       assertEquals(helper.getMessage(),
           cause.getMessage());