JavaScriptException is now approximately 327% more uber.  In addition to really great error messages, the underlying exception is also accessible. There's new hosted mode integration to make it work the same in hosted mode, as well as a new test+suite.

Issue: #1845
Suggested by: fredsa
Review by: jat


git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@2126 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/shell/JavaScriptHost.java b/dev/core/src/com/google/gwt/dev/shell/JavaScriptHost.java
index 42eadf4..7d84f14 100644
--- a/dev/core/src/com/google/gwt/dev/shell/JavaScriptHost.java
+++ b/dev/core/src/com/google/gwt/dev/shell/JavaScriptHost.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2007 Google Inc.
+ * Copyright 2008 Google Inc.
  * 
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
@@ -37,8 +37,8 @@
     sHost.createNative(file, line, jsniSignature, paramNames, js);
   }
 
-  public static void exceptionCaught(int number, String name, String description) {
-    sHost.exceptionCaught(number, name, description);
+  public static void exceptionCaught(Object exception) {
+    sHost.exceptionCaught(exception);
   }
 
   /**
diff --git a/dev/core/src/com/google/gwt/dev/shell/JsniInjector.java b/dev/core/src/com/google/gwt/dev/shell/JsniInjector.java
index 50805f5..723eadc 100644
--- a/dev/core/src/com/google/gwt/dev/shell/JsniInjector.java
+++ b/dev/core/src/com/google/gwt/dev/shell/JsniInjector.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2007 Google Inc.
+ * Copyright 2008 Google Inc.
  * 
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
@@ -155,8 +155,8 @@
           + "  __static[\\\"@"
           + Jsni.JAVASCRIPTHOST_NAME
           + "::exceptionCaught"
-          + "(ILjava/lang/String;Ljava/lang/String;)\\\"]"
-          + "((e && e.number) || 0, (e && e.name) || null , (e && e.message) || null);\\n"
+          + "(Ljava/lang/Object;)\\\"]"
+          + "(e == null ? null : e);\\n"
           + "}\\n";
 
       /*
diff --git a/dev/core/src/com/google/gwt/dev/shell/ModuleSpace.java b/dev/core/src/com/google/gwt/dev/shell/ModuleSpace.java
index fcec6cf..e11d268 100644
--- a/dev/core/src/com/google/gwt/dev/shell/ModuleSpace.java
+++ b/dev/core/src/com/google/gwt/dev/shell/ModuleSpace.java
@@ -44,15 +44,13 @@
   }
 
   protected static RuntimeException createJavaScriptException(ClassLoader cl,
-      String name, String desc) {
+      Object exception) {
     Exception caught;
     try {
       Class<?> javaScriptExceptionClass = Class.forName(
           "com.google.gwt.core.client.JavaScriptException", true, cl);
-      Class<?> string = String.class;
-      Constructor<?> ctor = javaScriptExceptionClass.getDeclaredConstructor(new Class<?>[] {
-          string, string});
-      return (RuntimeException) ctor.newInstance(new Object[] {name, desc});
+      Constructor<?> ctor = javaScriptExceptionClass.getDeclaredConstructor(Object.class);
+      return (RuntimeException) ctor.newInstance(new Object[] {exception});
     } catch (InstantiationException e) {
       caught = e;
     } catch (IllegalAccessException e) {
@@ -134,20 +132,19 @@
     host.getClassLoader().clear();
   }
 
-  public void exceptionCaught(int number, String name, String message) {
+  public void exceptionCaught(Object exception) {
+    Throwable caught;
     Throwable thrown = sThrownJavaExceptionObject.get();
-
-    if (thrown != null) {
-      // See if the caught exception was thrown by us
-      if (isExceptionSame(thrown, number, name, message)) {
-        sCaughtJavaExceptionObject.set(thrown);
-        sThrownJavaExceptionObject.set(null);
-        return;
-      }
+    if (thrown != null && isExceptionSame(thrown, exception)) {
+      // The caught exception was thrown by us.
+      caught = thrown;
+      sThrownJavaExceptionObject.set(null);
+    } else if (exception instanceof Throwable) {
+      caught = (Throwable) exception;
+    } else {
+      caught = createJavaScriptException(getIsolatedClassLoader(), exception);
     }
-
-    sCaughtJavaExceptionObject.set(createJavaScriptException(
-        getIsolatedClassLoader(), name, message));
+    sCaughtJavaExceptionObject.set(caught);
   }
 
   /**
@@ -490,12 +487,11 @@
     throw thrown;
   }
 
-  @SuppressWarnings("unused")
-  protected boolean isExceptionSame(Throwable original, int number,
-      String name, String message) {
+  protected boolean isExceptionSame(@SuppressWarnings("unused")
+  Throwable original, Object exception) {
     // For most platforms, the null exception means we threw it.
     // IE overrides this.
-    return (name == null && message == null);
+    return exception == null;
   }
 
   protected String rebind(String sourceName) throws UnableToCompleteException {
diff --git a/dev/core/src/com/google/gwt/dev/shell/ShellJavaScriptHost.java b/dev/core/src/com/google/gwt/dev/shell/ShellJavaScriptHost.java
index f4e4821..23a8f6d 100644
--- a/dev/core/src/com/google/gwt/dev/shell/ShellJavaScriptHost.java
+++ b/dev/core/src/com/google/gwt/dev/shell/ShellJavaScriptHost.java
@@ -42,7 +42,7 @@
   /**
    * Call this when a JavaScript exception is caught.
    */
-  void exceptionCaught(int number, String name, String description);
+  void exceptionCaught(Object exception);
 
   /**
    * Invoke a native JavaScript function that returns a boolean value.
diff --git a/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Exceptions.java b/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Exceptions.java
index 63459d9..c437453 100644
--- a/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Exceptions.java
+++ b/dev/core/super/com/google/gwt/dev/jjs/intrinsic/com/google/gwt/lang/Exceptions.java
@@ -26,8 +26,7 @@
     if (e instanceof Throwable) {
       return e;
     }
-    return new JavaScriptException(javaScriptExceptionName(e),
-        javaScriptExceptionDescription(e));
+    return new JavaScriptException(e);
   }
 
   static boolean throwAssertionError() {
@@ -66,23 +65,5 @@
   static boolean throwAssertionError_Object(Object message) {
     throw new AssertionError(message);
   }
-
   // CHECKSTYLE_ON
-
-  /**
-   * Returns the description of an unexpected JavaScript exception (not a normal
-   * Java one).
-   */
-  private static native String javaScriptExceptionDescription(Object e) /*-{
-    return e.message;
-  }-*/;
-
-  /**
-   * Returns the name of an unexpected JavaScript exception (not a normal Java
-   * one).
-   */
-  private static native String javaScriptExceptionName(Object e) /*-{
-    return e.name;
-  }-*/;
-
 }
diff --git a/dev/windows/src/com/google/gwt/dev/shell/ie/ModuleSpaceIE6.java b/dev/windows/src/com/google/gwt/dev/shell/ie/ModuleSpaceIE6.java
index db77672..2ff7941 100644
--- a/dev/windows/src/com/google/gwt/dev/shell/ie/ModuleSpaceIE6.java
+++ b/dev/windows/src/com/google/gwt/dev/shell/ie/ModuleSpaceIE6.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2007 Google Inc.
+ * Copyright 2008 Google Inc.
  * 
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
@@ -25,6 +25,9 @@
 import org.eclipse.swt.ole.win32.OleAutomation;
 import org.eclipse.swt.ole.win32.Variant;
 
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
 /**
  * An implementation of {@link com.google.gwt.dev.shell.ModuleSpace} for
  * Internet Explorer 6.
@@ -74,13 +77,6 @@
     }
   }
 
-  // CHECKSTYLE_OFF
-  private static int CODE(int hresult) {
-    return hresult & 0xFFFF;
-  }
-
-  // CHECKSTYLE_ON
-
   private final OleAutomation window;
 
   /**
@@ -177,12 +173,35 @@
     return new IDispatchProxy(getIsolatedClassLoader());
   }
 
+  /**
+   * On IE6, we currently have no way of throwing arbitrary exception objects
+   * into JavaScript. What we throw in exception cases is an exception not under
+   * our exact control, so the best we can do is match descriptions to indicate
+   * a match. In practice this works well.
+   */
   @Override
-  protected boolean isExceptionSame(Throwable original, int number,
-      String name, String message) {
-    HResultException hre = new HResultException(original);
-    return CODE(hre.getHResult()) == CODE(number)
-        && hre.getMessage().equals(message);
+  protected boolean isExceptionSame(Throwable original, Object exception) {
+    Throwable caught;
+    try {
+      HResultException hre = new HResultException(original);
+      RuntimeException jse = createJavaScriptException(
+          getIsolatedClassLoader(), exception);
+      Method method = jse.getClass().getMethod("getDescription");
+      String description = (String) method.invoke(jse);
+      return hre.getMessage().equals(description);
+    } catch (SecurityException e) {
+      caught = e;
+    } catch (NoSuchMethodException e) {
+      caught = e;
+    } catch (IllegalArgumentException e) {
+      caught = e;
+    } catch (IllegalAccessException e) {
+      caught = e;
+    } catch (InvocationTargetException e) {
+      caught = e;
+    }
+    throw new RuntimeException(
+        "Failed to invoke JavaScriptException.getDescription()", caught);
   }
 
   private Variant execute(String code) {
diff --git a/user/src/com/google/gwt/core/client/JavaScriptException.java b/user/src/com/google/gwt/core/client/JavaScriptException.java
index b4b26b1..bf9a9e5 100644
--- a/user/src/com/google/gwt/core/client/JavaScriptException.java
+++ b/user/src/com/google/gwt/core/client/JavaScriptException.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2006 Google Inc.
+ * Copyright 2008 Google Inc.
  * 
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
@@ -24,11 +24,72 @@
  */
 public final class JavaScriptException extends RuntimeException {
 
+  private static String constructMessage(Object e) {
+    return "(" + getName(e) + "): " + getDescription(e) + getProperties(e);
+  }
+
+  private static String getDescription(Object e) {
+    if (e instanceof JavaScriptObject) {
+      return getDescription0((JavaScriptObject) e);
+    } else {
+      return e + "";
+    }
+  }
+
+  private static native String getDescription0(JavaScriptObject e) /*-{
+    if (e == null) {
+      return null;
+    }
+    return e.message == null ? null : e.message;
+  }-*/;
+
+  private static JavaScriptObject getException(Object e) {
+    if (e instanceof JavaScriptObject) {
+      return (JavaScriptObject) e;
+    } else {
+      return null;
+    }
+  }
+
+  private static String getName(Object e) {
+    if (e == null) {
+      return "null";
+    } else if (e instanceof JavaScriptObject) {
+      return getName0((JavaScriptObject) e);
+    } else if (e instanceof String) {
+      return "String";
+    } else {
+      return e.getClass().getName();
+    }
+  }
+
+  private static native String getName0(JavaScriptObject e) /*-{
+    if (e == null) {
+      return null;
+    }
+    return e.name == null ? null : e.name;
+  }-*/;
+
+  private static String getProperties(Object e) {
+    if (e instanceof JavaScriptObject) {
+      return getProperties0((JavaScriptObject) e);
+    } else {
+      return null;
+    }
+  }
+
   /**
-   * The original type name of the JavaScript exception this class wraps,
-   * initialized as <code>e.name</code>.
+   * Returns the list of properties of an unexpected JavaScript exception.
    */
-  private final String name;
+  private static native String getProperties0(JavaScriptObject e) /*-{
+    var result = "";
+    for (prop in e) {
+      if (prop != "name" && prop != "description") {
+        result += "\n " + prop + ": " + e[prop];
+      }
+    }
+    return result;
+  }-*/;
 
   /**
    * The original description of the JavaScript exception this class wraps,
@@ -37,35 +98,65 @@
   private final String description;
 
   /**
-   * @param name the original JavaScript type name of the exception
-   * @param description the original JavaScript message of the exception
+   * The underlying exception this class wraps.
    */
+  private final JavaScriptObject exception;
+
+  /**
+   * The original type name of the JavaScript exception this class wraps,
+   * initialized as <code>e.name</code>.
+   */
+  private final String name;
+
+  /**
+   * @param exception
+   */
+  public JavaScriptException(Object e) {
+    super(constructMessage(e));
+    this.name = getName(e);
+    this.description = getDescription(e);
+    this.exception = getException(e);
+  }
+
   public JavaScriptException(String name, String description) {
     super("JavaScript " + name + " exception: " + description);
     this.name = name;
     this.description = description;
+    this.exception = null;
   }
 
   /**
-   * Useful for server-side instantiation.
+   * Used for server-side instantiation during JUnit runs. Exceptions are
+   * manually marshaled through
+   * {@link com.google.gwt.junit.client.impl.ExceptionWrapper} objects.
    * 
-   * @param message the detail message.
+   * @param message the detail message
    */
   protected JavaScriptException(String message) {
     super(message);
     this.name = null;
     this.description = message;
+    this.exception = null;
   }
 
   /**
-   * @return the original JavaScript message of the exception
+   * Returns the original JavaScript message of the exception; may be
+   * <code>null</code>.
    */
   public String getDescription() {
     return description;
   }
 
   /**
-   * @return the original JavaScript type name of the exception
+   * Returns the original JavaScript the exception; may be <code>null</code>.
+   */
+  public JavaScriptObject getException() {
+    return exception;
+  }
+
+  /**
+   * Returns the original JavaScript type name of the exception; may be
+   * <code>null</code>.
    */
   public String getName() {
     return name;
diff --git a/user/test/com/google/gwt/core/CoreSuite.java b/user/test/com/google/gwt/core/CoreSuite.java
new file mode 100644
index 0000000..17c5806
--- /dev/null
+++ b/user/test/com/google/gwt/core/CoreSuite.java
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2008 Google Inc.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.core;
+
+import com.google.gwt.core.client.JavaScriptExceptionTest;
+import com.google.gwt.junit.tools.GWTTestSuite;
+
+import junit.framework.Test;
+
+/**
+ * All I18N tests.
+ */
+public class CoreSuite {
+  public static Test suite() {
+    GWTTestSuite suite = new GWTTestSuite("All core tests");
+
+    // $JUnit-BEGIN$
+    suite.addTestSuite(JavaScriptExceptionTest.class);
+    // $JUnit-END$
+
+    return suite;
+  }
+}
diff --git a/user/test/com/google/gwt/core/client/JavaScriptExceptionTest.java b/user/test/com/google/gwt/core/client/JavaScriptExceptionTest.java
new file mode 100644
index 0000000..6397c5b
--- /dev/null
+++ b/user/test/com/google/gwt/core/client/JavaScriptExceptionTest.java
@@ -0,0 +1,144 @@
+/*
+ * Copyright 2008 Google Inc.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not
+ * use this file except in compliance with the License. You may obtain a copy of
+ * the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.google.gwt.core.client;
+
+import com.google.gwt.junit.client.GWTTestCase;
+
+/**
+ * Any JavaScript exceptions occurring within JSNI methods are wrapped as this
+ * class when caught in Java code. The wrapping does not occur until the
+ * exception passes out of JSNI into Java. Before that, the thrown object
+ * remains a native JavaScript exception object, and can be caught in JSNI as
+ * normal.
+ */
+public class JavaScriptExceptionTest extends GWTTestCase {
+
+  static native JavaScriptObject makeJSO() /*-{
+    return {
+      toString:function() {
+        return "jso";
+      },
+      name: "myName",
+      message: "myDescription",
+      extraField: "extraData"
+    };
+  }-*/;
+
+  static void throwJava(Throwable t) throws Throwable {
+    throw t;
+  }
+
+  static native void throwNative(Object e) /*-{
+    throw e;
+  }-*/;
+
+  static void throwSandwichJava(Object e) {
+    throwNative(e);
+  }
+
+  static native void throwSandwichNative(Throwable t) /*-{
+    @com.google.gwt.core.client.JavaScriptExceptionTest::throwJava(Ljava/lang/Throwable;)(t);
+  }-*/;
+
+  /**
+   * This test doesn't work in hosted mode yet; we'd need a way to throw true
+   * native objects as exceptions. Windows/IE is the deal killer right now on
+   * really making this work since there's no way to raise an exception of a
+   * true JS value. We could use JS lambdas around Java calls to get around this
+   * restriction.
+   */
+  public native void disabledTestJsExceptionSandwich() /*-{
+    var e = { };
+    try {
+      @com.google.gwt.core.client.JavaScriptExceptionTest::throwSandwichJava(Ljava/lang/Object;)(e);
+    } catch (t) {
+      @junit.framework.Assert::assertSame(Ljava/lang/Object;Ljava/lang/Object;)(e, t);
+    }
+  }-*/;
+
+  @Override
+  public String getModuleName() {
+    return "com.google.gwt.core.Core";
+  }
+
+  public void testJavaExceptionSandwich() {
+    RuntimeException e = new RuntimeException();
+    try {
+      throwSandwichNative(e);
+    } catch (Throwable t) {
+      assertSame(e, t);
+    }
+  }
+
+  public void testJso() {
+    JavaScriptObject jso = makeJSO();
+    try {
+      throwNative(jso);
+      fail();
+    } catch (JavaScriptException e) {
+      assertEquals("myName", e.getName());
+      assertEquals("myDescription", e.getDescription());
+      assertSame(jso, e.getException());
+      assertTrue(e.getMessage().contains("myName"));
+      assertTrue(e.getMessage().contains("myDescription"));
+      assertTrue(e.getMessage().contains("extraField"));
+      assertTrue(e.getMessage().contains("extraData"));
+    }
+  }
+
+  public void testNull() {
+    try {
+      throwNative(null);
+      fail();
+    } catch (JavaScriptException e) {
+      assertEquals("null", e.getName());
+      assertEquals("null", e.getDescription());
+      assertEquals(null, e.getException());
+      assertTrue(e.getMessage().contains("null"));
+    }
+  }
+
+  public void testObject() {
+    Object o = new Object() {
+      @Override
+      public String toString() {
+        return "myLameObject";
+      }
+    };
+    try {
+      throwNative(o);
+      fail();
+    } catch (JavaScriptException e) {
+      assertEquals(o.getClass().getName(), e.getName());
+      assertEquals("myLameObject", e.getDescription());
+      assertEquals(null, e.getException());
+      assertTrue(e.getMessage().contains(o.getClass().getName()));
+      assertTrue(e.getMessage().contains("myLameObject"));
+    }
+  }
+
+  public void testString() {
+    try {
+      throwNative("foobarbaz");
+      fail();
+    } catch (JavaScriptException e) {
+      assertEquals("String", e.getName());
+      assertEquals("foobarbaz", e.getDescription());
+      assertEquals(null, e.getException());
+      assertTrue(e.getMessage().contains("foobarbaz"));
+    }
+  }
+}