Enhance exception handling

Signed-off-by: Greg Haskins <[email protected]>

Author: ghaskins

src/temporal/client/core.clj

@@ -158,7 +158,10 @@ defworkflow once the workflow concludes.
   [{:keys [^WorkflowStub stub] :as workflow}]
   (-> (.getResultAsync stub u/bytes-type)
       (p/then nippy/thaw)
-      (p/catch e/slingshot? e/recast-stone)))
+      (p/catch e/slingshot? e/recast-stone)
+      (p/catch (fn [e]
+                 (log/error e)
+                 (throw e)))))
 
 (defn query
   "

src/temporal/internal/activity.clj

@@ -107,12 +107,9 @@
     (log/trace activity-id "calling" f "with args:" a)
     (try+
      (result-> activity-id (f ctx a))
-     (catch Exception e
-       (log/error e)
-       (throw e))
      (catch Object o
        (log/error &throw-context)
-       (e/freeze &throw-context)))))
+       (e/forward &throw-context)))))
 
 (defn dispatcher [ctx dispatch]
   (reify DynamicActivity

src/temporal/internal/exceptions.clj

@@ -2,6 +2,7 @@
 
 (ns temporal.internal.exceptions
   (:require [slingshot.slingshot :refer [throw+]]
+            [taoensso.timbre :as log]
             [temporal.exceptions :as e]
             [temporal.internal.utils :as u])
   (:import [io.temporal.failure ApplicationFailure]))
@@ -12,13 +13,28 @@
   (and (instance? ApplicationFailure (ex-cause ex))
        (= exception-type (.getType (cast ApplicationFailure (ex-cause ex))))))
 
-(defn freeze
-  [{{:keys [::e/non-retriable?] :or {non-retriable? false}} :object :as context}]
-  (let [o (u/->objarray context)
-        t (if non-retriable?
-            (ApplicationFailure/newNonRetryableFailure nil exception-type o)
-            (ApplicationFailure/newFailure nil exception-type o))]
-    (throw t)))
+(defn forward
+  [{:keys [message wrapper] {:keys [::e/non-retriable?] :or {non-retriable? false} :as object} :object :as context}]
+  (log/trace "forward:" context)
+  (cond
+    (and (map? object) (empty? object) (true? (some->> wrapper (instance? Throwable))))
+    (do
+      (log/trace "recasting wrapped exception")
+      (throw wrapper))
+
+    (instance? Throwable object)
+    (do
+      (log/trace "recasting throwable")
+      (throw object))
+
+    :else
+    (do
+      (log/trace "recasting stone within ApplicationFailure")
+      (let [o (u/->objarray context)
+            t (if non-retriable?
+                (ApplicationFailure/newNonRetryableFailure message exception-type o)
+                (ApplicationFailure/newFailure message exception-type o))]
+        (throw t)))))
 
 (defn recast-stone [ex]
   (let [stone (->> ex ex-cause (cast ApplicationFailure) (.getDetails) u/->args :object)]

src/temporal/internal/workflow.clj

@@ -64,9 +64,6 @@
              (f a))]
      (log/trace workflow-id "result:" r)
      (nippy/freeze r))
-   (catch Exception e
-     (log/error e)
-     (throw e))
    (catch Object o
      (log/error &throw-context)
-     (e/freeze &throw-context))))
+     (e/forward &throw-context))))

test/temporal/test/exception.clj

@@ -0,0 +1,36 @@
+;; Copyright © Manetu, Inc.  All rights reserved
+
+(ns temporal.test.exception
+  (:require [clojure.test :refer :all]
+            [taoensso.timbre :as log]
+            [temporal.client.core :as c]
+            [temporal.workflow :refer [defworkflow]]
+            [temporal.activity :refer [defactivity] :as a]
+            [temporal.test.utils :as t]))
+
+(use-fixtures :once t/wrap-service)
+
+(defactivity exception-activity
+  [ctx args]
+  (log/info "exception-activity:" args)
+  (throw (ex-info "test 1" {})))
+
+(defworkflow indirect-exception-workflow
+  [args]
+  (log/info "indirect-exception-workflow:" args)
+  @(a/invoke exception-activity args {:retry-options {:maximum-attempts 1}}))
+
+(defworkflow direct-exception-workflow
+  [args]
+  (log/info "direct-exception-workflow:" args)
+  (throw (ex-info "test 2" {})))
+
+(deftest the-test
+  (testing "Verifies that we can throw exceptions indirectly from an activity"
+    (let [workflow (t/create-workflow indirect-exception-workflow)]
+      (c/start workflow {})
+      (is (thrown? Exception @(c/get-result workflow)))))
+  (testing "Verifies that we can throw exceptions directly from a workflow"
+    (let [workflow (t/create-workflow direct-exception-workflow)]
+      (c/start workflow {})
+      (is (thrown? Exception @(c/get-result workflow))))))

test/temporal/test/slingshot.clj

@@ -30,7 +30,7 @@
    (catch [:type ::test1] _
      (log/info "caught stone 1")
      (try+
-      @(a/invoke slingshot-retriable-activity args)
+      @(a/invoke slingshot-retriable-activity args {:retry-options {:maximum-attempts 2}})
       (catch [:type ::test2] _
         (log/info "caught stone 2")
         (throw+ {:type ::test3}))))))
@@ -39,8 +39,8 @@
   (testing "Verifies that we can catch slingshot stones across activity/workflow boundaries"
     (let [workflow (t/create-workflow slingshot-workflow)]
       (c/start workflow {})
-      (try+
-       @(c/get-result workflow)
-       (throw (ex-info "should not get here" {}))
-       (catch [:type ::test3] _
-         (log/info "caught stone 3"))))))
+      (is (= :ok (try+
+                  @(c/get-result workflow)
+                  (throw (ex-info "should not get here" {}))
+                  (catch [:type ::test3] _
+                    :ok)))))))