Fixed JsonDataConverter to correctly report non serializable exceptions (#238)

Author: mfateev

src/main/java/com/uber/cadence/converter/DataConverterException.java

@@ -32,10 +32,14 @@ public DataConverterException(byte[] content, Type[] valueTypes, Throwable cause
     super(toMessage(null, content, valueTypes), cause);
   }
 
-  public DataConverterException(Exception cause) {
+  public DataConverterException(Throwable cause) {
     super(cause);
   }
 
+  public DataConverterException(String message, Throwable cause) {
+    super(message, cause);
+  }
+
   public DataConverterException(String message, byte[] content, Type[] valueTypes) {
     super(toMessage(message, content, valueTypes));
   }

src/main/java/com/uber/cadence/converter/JsonDataConverter.java

@@ -120,12 +120,27 @@ public byte[] toData(Object... values) throws DataConverterException {
         if (value instanceof TBase) {
           return newThriftSerializer().toString((TBase) value).getBytes(StandardCharsets.UTF_8);
         }
-        String json = gson.toJson(value);
-        return json.getBytes(StandardCharsets.UTF_8);
+        try {
+          String json = gson.toJson(value);
+          return json.getBytes(StandardCharsets.UTF_8);
+        } catch (Throwable e) {
+          if (value instanceof Throwable) {
+            // Keep original exception stack trace, but replace it with a DataConverterException
+            // which is guaranteed to be serializable.
+            DataConverterException ee =
+                new DataConverterException("Failure serializing exception: " + value.toString(), e);
+            ee.setStackTrace(((Throwable) value).getStackTrace());
+            String json = gson.toJson(ee);
+            return json.getBytes(StandardCharsets.UTF_8);
+          }
+          throw e;
+        }
       }
       String json = gson.toJson(values);
       return json.getBytes(StandardCharsets.UTF_8);
-    } catch (Exception e) {
+    } catch (DataConverterException e) {
+      throw e;
+    } catch (Throwable e) {
       throw new DataConverterException(e);
     }
   }

src/test/java/com/uber/cadence/workflow/WorkflowTest.java

@@ -57,8 +57,11 @@
 import com.uber.cadence.worker.WorkflowImplementationOptions;
 import com.uber.cadence.workflow.Functions.Func;
 import com.uber.cadence.workflow.Functions.Func1;
+import java.io.File;
+import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.io.InputStream;
 import java.lang.management.ManagementFactory;
 import java.lang.reflect.Type;
 import java.time.Duration;
@@ -3851,6 +3854,103 @@ public void testGenericParametersWorkflow() throws ExecutionException, Interrupt
     assertEquals(expectedResult, result);
   }
 
+  public static class NonSerializableException extends RuntimeException {
+    private final InputStream file; // gson chokes on this field
+
+    public NonSerializableException() {
+      try {
+        file = new FileInputStream(File.createTempFile("foo", "bar"));
+      } catch (IOException e) {
+        throw Activity.wrap(e);
+      }
+    }
+  }
+
+  public interface NonSerializableExceptionActivity {
+
+    @ActivityMethod(scheduleToCloseTimeoutSeconds = 5)
+    void execute();
+  }
+
+  public static class NonSerializableExceptionActivityImpl
+      implements NonSerializableExceptionActivity {
+
+    @Override
+    public void execute() {
+      throw new NonSerializableException();
+    }
+  }
+
+  public static class TestNonSerializableExceptionInActivityWorkflow implements TestWorkflow1 {
+
+    @Override
+    public String execute(String taskList) {
+      NonSerializableExceptionActivity activity =
+          Workflow.newActivityStub(NonSerializableExceptionActivity.class);
+      try {
+        activity.execute();
+      } catch (ActivityFailureException e) {
+        return e.getMessage();
+      }
+      return "done";
+    }
+  }
+
+  @Test
+  public void testNonSerializableExceptionInActivity() {
+    worker.registerActivitiesImplementations(new NonSerializableExceptionActivityImpl());
+    startWorkerFor(TestNonSerializableExceptionInActivityWorkflow.class);
+    TestWorkflow1 workflowStub =
+        workflowClient.newWorkflowStub(
+            TestWorkflow1.class, newWorkflowOptionsBuilder(taskList).build());
+
+    String result = workflowStub.execute(taskList);
+    assertTrue(result.contains("NonSerializableException"));
+  }
+
+  public interface NonSerializableExceptionChildWorkflow {
+
+    @WorkflowMethod
+    String execute(String taskList);
+  }
+
+  public static class NonSerializableExceptionChildWorkflowImpl
+      implements NonSerializableExceptionChildWorkflow {
+
+    @Override
+    public String execute(String taskList) {
+      throw new NonSerializableException();
+    }
+  }
+
+  public static class TestNonSerializableExceptionInChildWorkflow implements TestWorkflow1 {
+
+    @Override
+    public String execute(String taskList) {
+      NonSerializableExceptionChildWorkflow child =
+          Workflow.newChildWorkflowStub(NonSerializableExceptionChildWorkflow.class);
+      try {
+        child.execute(taskList);
+      } catch (ChildWorkflowFailureException e) {
+        return e.getMessage();
+      }
+      return "done";
+    }
+  }
+
+  @Test
+  public void testNonSerializableExceptionInChildWorkflow() {
+    startWorkerFor(
+        TestNonSerializableExceptionInChildWorkflow.class,
+        NonSerializableExceptionChildWorkflowImpl.class);
+    TestWorkflow1 workflowStub =
+        workflowClient.newWorkflowStub(
+            TestWorkflow1.class, newWorkflowOptionsBuilder(taskList).build());
+
+    String result = workflowStub.execute(taskList);
+    assertTrue(result.contains("NonSerializableException"));
+  }
+
   public interface TestLargeWorkflow {
 
     @WorkflowMethod