Fixed issue SeleniumHQ#644. ErrorHandler now tolerates non-Number lineNumber, and also attempts to safely parse a non-Number Object if it receives a non-Number.

If absent or non-numeric it now continues to build the StackTraceElement (instead of aborting that frame) and uses the conventional -1 for lineNumber.

Signed-off-by: Brett Randall <[email protected]>

Author: javabrett

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

@@ -21,9 +21,11 @@
 import static org.openqa.selenium.remote.ErrorCodes.SUCCESS;
 
 import com.google.common.base.Function;
+import com.google.common.base.Optional;
 import com.google.common.base.Predicates;
 import com.google.common.base.Throwables;
 import com.google.common.collect.Iterables;
+import com.google.common.primitives.Ints;
 
 import org.openqa.selenium.UnhandledAlertException;
 import org.openqa.selenium.WebDriverException;
@@ -293,12 +295,20 @@ public StackTraceElement apply(Map<String, Object> frameInfo) {
       if (frameInfo == null) {
         return null;
       }
-
-      Number lineNumber = (Number) frameInfo.get(LINE_NUMBER);
-      if (lineNumber == null) {
-        return null;
+      
+      Optional<Number> maybeLineNumberInteger = Optional.absent();
+      
+      final Object lineNumberObject = frameInfo.get(LINE_NUMBER);
+      if (lineNumberObject instanceof Number) {
+    	  maybeLineNumberInteger = Optional.of((Number) lineNumberObject);
+      } else if (lineNumberObject != null) {
+    	  // might be a Number as a String
+    	  maybeLineNumberInteger = Optional.fromNullable((Number) Ints.tryParse(lineNumberObject.toString()));
       }
-
+      
+      // default -1 for unknown, see StackTraceElement constructor javadoc
+      final int lineNumber = maybeLineNumberInteger.or(-1).intValue();
+      
       // Gracefully handle remote servers that don't (or can't) send back
       // complete stack trace info. At least some of this information should
       // be included...
@@ -310,7 +320,7 @@ public StackTraceElement apply(Map<String, Object> frameInfo) {
           ? toStringOrNull(frameInfo.get(FILE_NAME)) : UNKNOWN_FILE;
 
       return new StackTraceElement(className, methodName, fileName,
-          lineNumber.intValue());
+          lineNumber);
     }
 
     private static String toStringOrNull(Object o) {

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

@@ -306,6 +306,74 @@ public void testShouldStillTryToBuildWebDriverExceptionIfClassIsNotProvidedAndSt
     }
   }
 
+  @SuppressWarnings({"unchecked", "ThrowableInstanceNeverThrown"})
+  @Test
+  public void testToleratesNonNumericLineNumber() {
+    Map<String, ?> data = ImmutableMap.of(
+        "message", "some error message",
+        "stackTrace", Lists.newArrayList(
+            ImmutableMap.of("lineNumber", "some string, might be empty or 'Not avalable'",
+                "methodName", "someMethod",
+                "className", "MyClass",
+                "fileName", "Resource.m")));
+
+    try {
+      handler.throwIfResponseFailed(createResponse(ErrorCodes.UNHANDLED_ERROR, data), 123);
+      fail("Should have thrown!");
+    } catch (WebDriverException expected) {
+      assertEquals(new WebDriverException("some error message\nCommand duration or timeout: 123 milliseconds").getMessage(),
+          expected.getMessage());
+
+      StackTraceElement[] expectedTrace = {
+          new StackTraceElement("MyClass", "someMethod", "Resource.m", -1)
+      };
+      WebDriverException helper = new WebDriverException("some error message");
+      helper.setStackTrace(expectedTrace);
+
+      Throwable cause = expected.getCause();
+      assertNotNull(cause);
+      assertEquals(WebDriverException.class, cause.getClass());
+      assertEquals(helper.getMessage(),
+          cause.getMessage());
+
+      assertStackTracesEqual(expectedTrace, cause.getStackTrace());
+    }
+  }
+  
+  @SuppressWarnings({"unchecked", "ThrowableInstanceNeverThrown"})
+  @Test
+  public void testToleratesNumericLineNumberAsString() {
+    Map<String, ?> data = ImmutableMap.of(
+        "message", "some error message",
+        "stackTrace", Lists.newArrayList(
+            ImmutableMap.of("lineNumber", "1224", // number as a string
+                "methodName", "someMethod",
+                "className", "MyClass",
+                "fileName", "Resource.m")));
+
+    try {
+      handler.throwIfResponseFailed(createResponse(ErrorCodes.UNHANDLED_ERROR, data), 123);
+      fail("Should have thrown!");
+    } catch (WebDriverException expected) {
+      assertEquals(new WebDriverException("some error message\nCommand duration or timeout: 123 milliseconds").getMessage(),
+          expected.getMessage());
+
+      StackTraceElement[] expectedTrace = {
+          new StackTraceElement("MyClass", "someMethod", "Resource.m", 1224)
+      };
+      WebDriverException helper = new WebDriverException("some error message");
+      helper.setStackTrace(expectedTrace);
+
+      Throwable cause = expected.getCause();
+      assertNotNull(cause);
+      assertEquals(WebDriverException.class, cause.getClass());
+      assertEquals(helper.getMessage(),
+          cause.getMessage());
+
+      assertStackTracesEqual(expectedTrace, cause.getStackTrace());
+    }
+  }
+  
   @SuppressWarnings({"unchecked", "ThrowableInstanceNeverThrown"})
   @Test
   public void testShouldIndicateWhenTheServerReturnedAnExceptionThatWasSuppressed()