Fixed exception wrapping (#113)

Author: mfateev

src/main/java/com/uber/cadence/activity/Activity.java

@@ -117,8 +117,8 @@ public static String getDomain() {
      *
      * @return never returns as always throws.
      */
-    public static RuntimeException throwWrapped(Throwable e) {
-        return WorkflowInternal.throwWrapped(e);
+    public static RuntimeException wrap(Exception e) {
+        return WorkflowInternal.wrap(e);
     }
 
     /**

src/main/java/com/uber/cadence/internal/sync/CheckedExceptionWrapper.java renamed to src/main/java/com/uber/cadence/internal/common/CheckedExceptionWrapper.java

@@ -15,17 +15,17 @@
  *  permissions and limitations under the License.
  */
 
-package com.uber.cadence.internal.sync;
+package com.uber.cadence.internal.common;
 
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 
 /**
  * Do not reference directly by the application level code.
- * Use {@link com.uber.cadence.workflow.Workflow#throwWrapped(Throwable)} inside a workflow code and
- * {@link com.uber.cadence.activity.Activity#throwWrapped(Throwable)} inside an activity code instead.
+ * Use {@link com.uber.cadence.workflow.Workflow#wrap(Exception)} inside a workflow code and
+ * {@link com.uber.cadence.activity.Activity#wrap(Exception)} inside an activity code instead.
  */
-final class CheckedExceptionWrapper extends RuntimeException {
+public final class CheckedExceptionWrapper extends RuntimeException {
 
     private final static Field causeField;
 
@@ -39,39 +39,25 @@ final class CheckedExceptionWrapper extends RuntimeException {
     }
 
     /**
-     * Throws CheckedExceptionWrapper if e is checked exception.
+     * Returns CheckedExceptionWrapper if e is checked exception.
      * If there is a need to return a checked exception from an activity or workflow implementation
-     * throwWrapped it using this method. The library code will unwrap it automatically when propagating exception
+     * throw a wrapped exception it using this method. The library code will unwrap it automatically when propagating exception
      * to the caller.
-     * <p>
-     * Throws original exception if e is {@link RuntimeException} or {@link Error}.
-     * Never returns. But return type is not empty to be able to use it as:
      * <pre>
      * try {
      *     return someCall();
      * } catch (Exception e) {
-     *     throw CheckedExceptionWrapper.throwWrapped(e);
+     *     throw CheckedExceptionWrapper.wrap(e);
      * }
      * </pre>
-     * If throwWrapped returned void it wouldn't be possible to write <code>throw CheckedExcptionWrapper.throwWrapped</code>
-     * and compiler would complain about missing return.
-     *
-     * @return never returns as always throws.
      */
-    public static RuntimeException throwWrapped(Throwable e) {
-        throw getWrapped(e);
-    }
-
-    /**
-     * Similar to throwWrapped but just returns wrapped error without throwing it.
-     * Only Error is thrown as it should propagate up the stack without any delay.
-     */
-    public static RuntimeException getWrapped(Throwable e) {
+    public static RuntimeException wrap(Throwable e) {
+        // Errors are expected to propagate without any handling.
         if (e instanceof Error) {
             throw (Error) e;
         }
         if (e instanceof InvocationTargetException) {
-            return getWrapped(e.getCause());
+            return wrap(e.getCause());
         }
         if (e instanceof RuntimeException) {
             return (RuntimeException) e;
@@ -83,7 +69,7 @@ public static RuntimeException getWrapped(Throwable e) {
      * Removes CheckedException wrapper from the whole chain of Exceptions.
      * Assumes that wrapper always has a cause which cannot be a wrapper.
      */
-    public static Throwable unwrap(Throwable e) {
+    public static Exception unwrap(Exception e) {
         Throwable head = e;
         if (head instanceof CheckedExceptionWrapper) {
             head = head.getCause();
@@ -98,7 +84,11 @@ public static Throwable unwrap(Throwable e) {
             tail = current;
             current = tail.getCause();
         }
-        return head;
+        if (head instanceof Error) {
+            // Error should be propagated without any handling.
+            throw (Error)head;
+        }
+        return (Exception) head;
     }
 
     /**

src/main/java/com/uber/cadence/internal/common/WorkflowExecutionUtils.java

@@ -22,7 +22,6 @@
 import com.uber.cadence.client.WorkflowTerminatedException;
 import com.uber.cadence.client.WorkflowTimedOutException;
 import com.uber.cadence.common.RetryOptions;
-import com.uber.cadence.workflow.Workflow;
 import org.apache.thrift.TException;
 
 import java.lang.reflect.InvocationTargetException;
@@ -110,8 +109,7 @@ public static HistoryEvent getInstanceCloseEvent(Iface service, String domain,
                 response = SynchronousRetryer.retryWithResult(retryParameters,
                         () -> service.GetWorkflowExecutionHistory(r));
             } catch (TException e) {
-                // TODO: Refactor to avoid this ugly circular dependency.
-                throw Workflow.throwWrapped(e);
+                throw CheckedExceptionWrapper.wrap(e);
             }
             if (timeout != 0 && System.currentTimeMillis() - start > unit.toMillis(timeout)) {
                 throw new TimeoutException("WorkflowId=" + workflowExecution.getWorkflowId() +

src/main/java/com/uber/cadence/internal/replay/ReplayDecider.java

@@ -215,7 +215,8 @@ private void eventLoop() {
             }
             completed = true;
         } catch (Throwable e) {
-            failure = workflow.mapUnexpectedException(e);
+            // can cast as Error is caught above.
+            failure = workflow.mapUnexpectedException((Exception) e);
             completed = true;
         }
     }

src/main/java/com/uber/cadence/internal/replay/ReplayWorkflow.java

@@ -62,5 +62,5 @@ public interface ReplayWorkflow {
      * @param failure Unexpected failure cause
      * @return Serialized failure
      */
-    WorkflowExecutionException mapUnexpectedException(Throwable failure);
+    WorkflowExecutionException mapUnexpectedException(Exception failure);
 }

src/main/java/com/uber/cadence/internal/sync/ActivityInvocationHandler.java

@@ -48,7 +48,7 @@ public Object invoke(Object proxy, Method method, Object[] args) {
                 return "ActivityInvocationHandler";
             }
         } catch (NoSuchMethodException e) {
-            throw Workflow.throwWrapped(e);
+            throw Workflow.wrap(e);
         }
         if (!method.getDeclaringClass().isInterface()) {
             throw new IllegalArgumentException("Interface type is expected: " + method.getDeclaringClass());

src/main/java/com/uber/cadence/internal/sync/AllOfFuture.java

@@ -126,4 +126,9 @@ public List<G> get(long timeout, TimeUnit unit) throws TimeoutException {
     public List<G> get(long timeout, TimeUnit unit, List<G> defaultValue) {
         return impl.get(timeout, unit, defaultValue);
     }
+
+    @Override
+    public RuntimeException getFailure() {
+        return impl.getFailure();
+    }
 }

src/main/java/com/uber/cadence/internal/sync/AsyncInternal.java

@@ -247,8 +247,8 @@ private static <R> Promise<R> execute(boolean async, Functions.Func<R> func) {
             try {
                 func.apply();
                 return getAsyncInvocationResult();
-            } catch (Throwable e) {
-                return Workflow.newFailedPromise(Workflow.getWrapped(e));
+            } catch (Exception e) {
+                return Workflow.newFailedPromise(Workflow.wrap(e));
             } finally {
                 closeAsyncInvocation();
             }
@@ -257,8 +257,8 @@ private static <R> Promise<R> execute(boolean async, Functions.Func<R> func) {
             WorkflowInternal.newThread(false, () -> {
                 try {
                     result.complete(func.apply());
-                } catch (Throwable e) {
-                    result.completeExceptionally(Workflow.getWrapped(e));
+                } catch (Exception e) {
+                    result.completeExceptionally(Workflow.wrap(e));
                 }
             }).start();
             return result;

src/main/java/com/uber/cadence/internal/sync/CompletablePromiseImpl.java

@@ -128,6 +128,18 @@ public V get(long timeout, TimeUnit unit, V defaultValue) {
         return value;
     }
 
+    @Override
+    public RuntimeException getFailure() {
+        if (!completed) {
+            WorkflowThread.yield("Feature.get", () -> completed);
+        }
+        if (failure != null) {
+            unregisterWithRunner();
+            return failure;
+        }
+        return null;
+    }
+
     private void unregisterWithRunner() {
         if (registeredWithRunner) {
             runner.forgetFailedPromise(this);

src/main/java/com/uber/cadence/internal/sync/DeterministicRunnerImpl.java

@@ -17,6 +17,7 @@
 
 package com.uber.cadence.internal.sync;
 
+import com.uber.cadence.internal.common.CheckedExceptionWrapper;
 import com.uber.cadence.workflow.Promise;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;