Skip to content

Commit

Permalink
Make exceptions work across Java/JavaScript boundary
Browse files Browse the repository at this point in the history
The essence of the change is, instead of throwing subclasses of Java
Throwable class in generated throw statements, we now throw the backing
JavaScript error object so the generated code looks like this:

 try {
   throw new SomeJavaException.backingJsObject; // Exceptions.unwrap
 } catch(e) {
   e = Exceptions.wrap(e);

   if (instanceof(e, SomeJavaException_ID)) {
     // catch block for SomeJavaException
   }
   // other catch blocks...
   else {
    throw e.backingJsObject; // Exceptions.unwrap
 }

As the propagated thrown object is a real JavaScript error, it plays
better with the browser dev tools and JavaScript callers (which become
more essential due to JsInterop).

There are two potentially breaking semantic changes with the patch:

 - If the original thrown object encapsulated by JavaScriptException
is not a JavaScript Error (e.g. a string as you can do that in JavaScript),
we no longer try to auto-fill the stacktrace by other means. The developers
needs to call fillInStackTrace in that case. I think this we was already the
behavior pre GWT 2.6 or 2.7 (see the original javadoc that was missed) and
preserving the newer behavior in the new design causes extra complications.

 - If you catch the exception in JSNI and pass it back to Java, it is not a
Java object anymore. This is unlikely to be a real issue in practice.

The patch is also breaks GIN as it uses its own hacky class loader
to load super-sourced versions of the files and ends up trying to load
java.lang.JsException which is forbidden by java due to package name.
I fixed GIN but this could be also worked around in released versions by
setting a compiler flag:
 -setProperty gin.classloading.exceptedPackages=com.google.gwt.core.client,
com.google.gwt.core.client.impl

Change-Id: Id21c182078075ca19bb0953136a8b9136adcba63
Review-Link: https://gwt-review.googlesource.com/#/c/14030/
  • Loading branch information
gkdn committed Dec 7, 2015
1 parent 297d775 commit 1d660d2
Show file tree
Hide file tree
Showing 22 changed files with 297 additions and 168 deletions.
18 changes: 0 additions & 18 deletions dev/core/src/com/google/gwt/dev/jjs/impl/CatchBlockNormalizer.java
Expand Up @@ -21,7 +21,6 @@
import com.google.gwt.dev.jjs.ast.JBinaryOperator; import com.google.gwt.dev.jjs.ast.JBinaryOperator;
import com.google.gwt.dev.jjs.ast.JBlock; import com.google.gwt.dev.jjs.ast.JBlock;
import com.google.gwt.dev.jjs.ast.JDeclarationStatement; import com.google.gwt.dev.jjs.ast.JDeclarationStatement;
import com.google.gwt.dev.jjs.ast.JDeclaredType;
import com.google.gwt.dev.jjs.ast.JExpression; import com.google.gwt.dev.jjs.ast.JExpression;
import com.google.gwt.dev.jjs.ast.JIfStatement; import com.google.gwt.dev.jjs.ast.JIfStatement;
import com.google.gwt.dev.jjs.ast.JInstanceOf; import com.google.gwt.dev.jjs.ast.JInstanceOf;
Expand All @@ -31,7 +30,6 @@
import com.google.gwt.dev.jjs.ast.JMethodBody; import com.google.gwt.dev.jjs.ast.JMethodBody;
import com.google.gwt.dev.jjs.ast.JMethodCall; import com.google.gwt.dev.jjs.ast.JMethodCall;
import com.google.gwt.dev.jjs.ast.JModVisitor; import com.google.gwt.dev.jjs.ast.JModVisitor;
import com.google.gwt.dev.jjs.ast.JNewInstance;
import com.google.gwt.dev.jjs.ast.JPrimitiveType; import com.google.gwt.dev.jjs.ast.JPrimitiveType;
import com.google.gwt.dev.jjs.ast.JProgram; import com.google.gwt.dev.jjs.ast.JProgram;
import com.google.gwt.dev.jjs.ast.JReferenceType; import com.google.gwt.dev.jjs.ast.JReferenceType;
Expand Down Expand Up @@ -143,26 +141,10 @@ public boolean visit(JMethodBody x, Context ctx) {
} }


private class UnwrapJavaScriptExceptionVisitor extends JModVisitor { private class UnwrapJavaScriptExceptionVisitor extends JModVisitor {
JDeclaredType jseType =
program.getFromTypeMap("com.google.gwt.core.client.JavaScriptException");
JMethod unwrapMethod = program.getIndexedMethod(RuntimeConstants.EXCEPTIONS_UNWRAP); JMethod unwrapMethod = program.getIndexedMethod(RuntimeConstants.EXCEPTIONS_UNWRAP);


@Override @Override
public void endVisit(JThrowStatement x, Context ctx) { public void endVisit(JThrowStatement x, Context ctx) {
assert jseType != null;

JExpression expr = x.getExpr();

// Optimization: unwrap not needed if "new BlahException()"
if (expr instanceof JNewInstance && !expr.getType().equals(jseType)) {
return;
}

// Optimization: unwrap not needed if expression can never be JavaScriptException
if (program.typeOracle.castFailsTrivially((JReferenceType) expr.getType(), jseType)) {
return;
}

// throw x; -> throw Exceptions.unwrap(x); // throw x; -> throw Exceptions.unwrap(x);
ctx.replaceMe(createUnwrappedThrow(x)); ctx.replaceMe(createUnwrappedThrow(x));
} }
Expand Down
Expand Up @@ -27,42 +27,28 @@ final class Exceptions {


@DoNotInline // This frame can be useful in understanding the native stack @DoNotInline // This frame can be useful in understanding the native stack
static Object wrap(Object e) { static Object wrap(Object e) {
// Although this is impossible to happen in code generated from Java (as we always unwrap
// before throwing), there are code out there where the Java exception is instantiated and
// thrown in native code, hence we may receive it already wrapped.
if (e instanceof Throwable) { if (e instanceof Throwable) {
return e; return e;
} }


JavaScriptException jse = getCachedJavaScriptException(e); Throwable javaException = getJavaException(e);
if (jse == null) { if (javaException == null) {
jse = new JavaScriptException(e); javaException = new JavaScriptException(e);
StackTraceCreator.captureStackTrace(jse, e); StackTraceCreator.captureStackTrace(javaException);
cacheJavaScriptException(e, jse);
} }

return javaException;
return jse;
} }


static Object unwrap(Object e) { @DoNotInline // This method shouldn't be inlined and pruned as JsStackEmulator needs it.
if (e instanceof JavaScriptException) { static native Object unwrap(Object t)/*-{
JavaScriptException jse = ((JavaScriptException) e); return t.@Throwable::backingJsObject;
if (jse.isThrownSet()) {
return jse.getThrown();
}
}
return e;
}

private static native JavaScriptException getCachedJavaScriptException(Object e) /*-{
return e && e.__gwt$exception;
}-*/; }-*/;


private static native void cacheJavaScriptException(Object e, JavaScriptException jse) /*-{ private static native Throwable getJavaException(Object e)/*-{
if (e && typeof e == 'object') { return e && e["__java$exception"];
try {
e.__gwt$exception = jse;
} catch (ignored) {
// See https://code.google.com/p/google-web-toolkit/issues/detail?id=8449
}
}
}-*/; }-*/;


static AssertionError makeAssertionError() { static AssertionError makeAssertionError() {
Expand Down
4 changes: 2 additions & 2 deletions dev/core/super/javaemul/internal/ConsoleLogger.java
Expand Up @@ -68,7 +68,7 @@ function stringify(fnStack) {
} }
return "\t" + fnStack.join("\n\t"); return "\t" + fnStack.join("\n\t");
} }
var backingError = t.__gwt$backingJsError; var backingError = t.backingJsObject;
return backingError && (backingError.stack || stringify(backingError.fnStack)); return backingError && (backingError.stack || stringify(t.fnStack));
}-*/; }-*/;
} }
4 changes: 4 additions & 0 deletions dev/core/super/javaemul/internal/JsUtils.java
Expand Up @@ -37,6 +37,10 @@ public static native String unsafeCastToString(Object string) /*-{
return string; return string;
}-*/; }-*/;


public static native void setProperty(Object map, String key, Object value) /*-{
map[key] = value;
}-*/;

public static native int getIntProperty(Object map, String key) /*-{ public static native int getIntProperty(Object map, String key) /*-{
return map[key]; return map[key];
}-*/; }-*/;
Expand Down
8 changes: 4 additions & 4 deletions dev/core/test/com/google/gwt/dev/js/JsStackEmulatorTest.java
Expand Up @@ -146,7 +146,7 @@ public void testSimpleThrow() throws Exception {
checkOnModuleLoad(program, "function onModuleLoad(){" + checkOnModuleLoad(program, "function onModuleLoad(){" +
"var stackIndex;$stack[stackIndex=++$stackDepth]=onModuleLoad;" + "var stackIndex;$stack[stackIndex=++$stackDepth]=onModuleLoad;" +
"$location[stackIndex]='EntryPoint.java:'+'3',$clinit_EntryPoint();" + "$location[stackIndex]='EntryPoint.java:'+'3',$clinit_EntryPoint();" +
"throw $location[stackIndex]='EntryPoint.java:'+'4',new RuntimeException" + "throw unwrap(($location[stackIndex]='EntryPoint.java:'+'4',new RuntimeException))" +
"}"); "}");
} }


Expand All @@ -170,9 +170,9 @@ public void testThrowWithInlineMethodCall() throws Exception {
checkOnModuleLoad(program, "function onModuleLoad(){" + checkOnModuleLoad(program, "function onModuleLoad(){" +
"var stackIndex;$stack[stackIndex=++$stackDepth]=onModuleLoad;" + "var stackIndex;$stack[stackIndex=++$stackDepth]=onModuleLoad;" +
"$location[stackIndex]='EntryPoint.java:'+'6',$clinit_EntryPoint();" + "$location[stackIndex]='EntryPoint.java:'+'6',$clinit_EntryPoint();" +
"throw new RuntimeException(" + "throw unwrap(new RuntimeException(" +
"($tmp=($location[stackIndex]='EntryPoint.java:'+'4',thing).toString()," + "($tmp=($location[stackIndex]='EntryPoint.java:'+'4',thing).toString()," +
"$location[stackIndex]='EntryPoint.java:'+'7',$tmp))" + "$location[stackIndex]='EntryPoint.java:'+'7',$tmp)))" +
"}"); "}");
} }


Expand Down Expand Up @@ -226,7 +226,7 @@ public void testTryCatch() throws Exception {
checkOnModuleLoad(program, "function onModuleLoad(){" + checkOnModuleLoad(program, "function onModuleLoad(){" +
"var stackIndex;$stack[stackIndex=++$stackDepth]=onModuleLoad;" + "var stackIndex;$stack[stackIndex=++$stackDepth]=onModuleLoad;" +
"$location[stackIndex]='EntryPoint.java:'+'3',$clinit_EntryPoint();var e,s;" + "$location[stackIndex]='EntryPoint.java:'+'3',$clinit_EntryPoint();var e,s;" +
"try{throw $location[stackIndex]='EntryPoint.java:'+'5',new RuntimeException" + "try{throw unwrap(($location[stackIndex]='EntryPoint.java:'+'5',new RuntimeException))" +
"}catch($e0){$e0=wrap($e0);" + "}catch($e0){$e0=wrap($e0);" +
"$stackDepth=($location[stackIndex]='EntryPoint.java:'+'6',stackIndex);" + "$stackDepth=($location[stackIndex]='EntryPoint.java:'+'6',stackIndex);" +
"if(instanceOf($e0,'java.lang.RuntimeException')){" + "if(instanceOf($e0,'java.lang.RuntimeException')){" +
Expand Down
1 change: 1 addition & 0 deletions tools/api-checker/config/gwt27_28userApi.conf
Expand Up @@ -91,6 +91,7 @@ excludedFiles_new **/linker/**\
:**/server/**\ :**/server/**\
:**/tools/**\ :**/tools/**\
:**/vm/**\ :**/vm/**\
:user/src/com/google/gwt/core/client/impl/JavaScriptExceptionBase.java\
:user/src/com/google/gwt/core/client/impl/WeakMapping.java\ :user/src/com/google/gwt/core/client/impl/WeakMapping.java\
:user/src/com/google/gwt/core/shared/impl/ThrowableTypeResolver.java\ :user/src/com/google/gwt/core/shared/impl/ThrowableTypeResolver.java\
:user/src/com/google/gwt/i18n/**/impl/cldr/**\ :user/src/com/google/gwt/i18n/**/impl/cldr/**\
Expand Down
1 change: 1 addition & 0 deletions user/BUILD
Expand Up @@ -310,6 +310,7 @@ java_library(
"src/com/google/gwt/user/client/impl/DomImpl.java", "src/com/google/gwt/user/client/impl/DomImpl.java",
"src/com/google/gwt/user/client/HistoryListener.java", "src/com/google/gwt/user/client/HistoryListener.java",
"src/com/google/gwt/user/client/Cookies.java", "src/com/google/gwt/user/client/Cookies.java",
"src/com/google/gwt/core/client/impl/JavaScriptExceptionBase.java",
"src/com/google/gwt/user/client/ui/AcceptsOneWidget.java", "src/com/google/gwt/user/client/ui/AcceptsOneWidget.java",
"src/com/google/gwt/user/client/ui/Widget.java", "src/com/google/gwt/user/client/ui/Widget.java",
"src/com/google/gwt/user/client/ui/IsWidget.java", "src/com/google/gwt/user/client/ui/IsWidget.java",
Expand Down
14 changes: 8 additions & 6 deletions user/src/com/google/gwt/core/client/JavaScriptException.java
Expand Up @@ -15,6 +15,8 @@
*/ */
package com.google.gwt.core.client; package com.google.gwt.core.client;


import com.google.gwt.core.client.impl.JavaScriptExceptionBase;

/** /**
* Any JavaScript exceptions occurring within JSNI methods are wrapped as this * Any JavaScript exceptions occurring within JSNI methods are wrapped as this
* class when caught in Java code. The wrapping does not occur until the * class when caught in Java code. The wrapping does not occur until the
Expand All @@ -30,7 +32,7 @@
* determined, {@link #fillInStackTrace()} can be called in the associated catch * determined, {@link #fillInStackTrace()} can be called in the associated catch
* block to create a stack trace corresponding to the location where the * block to create a stack trace corresponding to the location where the
* JavaScriptException object was created. * JavaScriptException object was created.
* *
* <pre> * <pre>
* try { * try {
* nativeMethod(); * nativeMethod();
Expand All @@ -41,7 +43,7 @@
* } * }
* </pre> * </pre>
*/ */
public final class JavaScriptException extends RuntimeException { public final class JavaScriptException extends JavaScriptExceptionBase {


private static final Object NOT_SET = new Object(); private static final Object NOT_SET = new Object();


Expand Down Expand Up @@ -108,14 +110,13 @@ public JavaScriptException(Object e) {
* trace * trace
*/ */
public JavaScriptException(Object e, String description) { public JavaScriptException(Object e, String description) {
// Stack trace is writeable for outside just for classic devmode but otherwise it is not and we super(e);
// don't want unnecessary fillInStackTrace calls from super constructor as well.
super(null, null, true, !GWT.isScript());
this.e = e; this.e = e;
this.description = description; this.description = description;
} }


public JavaScriptException(String name, String description) { public JavaScriptException(String name, String description) {
super(null);
this.message = "JavaScript " + name + " exception: " + description; this.message = "JavaScript " + name + " exception: " + description;
this.name = name; this.name = name;
this.description = description; this.description = description;
Expand All @@ -128,9 +129,10 @@ public JavaScriptException(String name, String description) {
* @param message the detail message * @param message the detail message
*/ */
protected JavaScriptException(String message) { protected JavaScriptException(String message) {
super(message); super(null);
this.message = this.description = message; this.message = this.description = message;
this.e = NOT_SET; this.e = NOT_SET;
fillInStackTrace();
} }


/** /**
Expand Down
6 changes: 6 additions & 0 deletions user/src/com/google/gwt/core/client/impl/Impl.java
Expand Up @@ -27,6 +27,12 @@
*/ */
public final class Impl { public final class Impl {


static {
if (GWT.isClient() && StackTraceCreator.collector != null) {
// Just enforces loading of StackTraceCreator early on, nothing else to do here...
}
}

private static final int WATCHDOG_ENTRY_DEPTH_CHECK_INTERVAL_MS = 2000; private static final int WATCHDOG_ENTRY_DEPTH_CHECK_INTERVAL_MS = 2000;


/** /**
Expand Down
@@ -0,0 +1,26 @@
/*
* Copyright 2015 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.impl;

/**
* A helper so that we can change the parent of JavaScriptException via super-source. Otherwise we
* would need to copy whole content of the class and also dev mode would choke.
*/
public class JavaScriptExceptionBase extends RuntimeException {
public JavaScriptExceptionBase(Object e) {
super();
}
}

0 comments on commit 1d660d2

Please sign in to comment.