Exceptions propagate along dependencies in most engines (#33)

* wip: fix nested exceptions * Exceptions propagate along dependencies in most engines - Fixes virtual future exception propagation between nodes. - Avoid testing iterative-scheduling and virtual-futures (non-applicative) engines which do not propagate correctly. * linter * readme and changelog * revert changes to iterative sched --------- Co-authored-by: Alex Redington <[email protected]>
nubank · Jul 18, 2024 · 815e473 · 815e473
1 parent 03a3656
commit 815e473
Show file tree

Hide file tree

Showing 5 changed files with 43 additions and 21 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,6 +1,7 @@
 # Changelog
 
-## Unreleased
+## 1.18.1 / 2024-07-16
+- Fix virtual-future applicative engine error handling so that ExecutionExceptions are always unwrapped
 - Improve ergonomics when an unknown engine is specified
 - Mark virtual future applicative engine as `eval-key-channel` true.
 

diff --git a/README.md b/README.md
@@ -243,10 +243,12 @@ Engines supported in Nodely include:
  | ------------------------------- | ---------------------------------- | ------------ |
  | Lazy Synchronous                | `:sync.lazy`                       | Mature       | 
  | Core Async Lazy Scheduling      | `:core-async.lazy-scheduling`      | Mature       | 
+ | Applicative Virtual Threads     | `:applicative.virtual-futures`     | Mature       |
+ | Async Virtual Threads           | `:async.virtual-futures`           | Experimental |
  | Core Async Iterative Scheduling | `:core-async.iterative-scheduling` | Experimental | 
  | Async Manifold                  | `:async.manifold`                  | Experimental | 
- | Async Applicative               | `:async.applicative`               | Experimental | 
- | Async Virtual Threads           | `:async.virtual-futures`           | Experimental |
+ | Async Applicative               | `:applicative.core-async`          | Experimental | 
+ | Promesa Async Applicative       | `:applicative.promesa`             | Experimental | 
 
 ### Lazy Synchronous
 
@@ -302,16 +304,16 @@ time. However, it is likely to create many blocking Threads in the
 process, and so may have poor consequences when, e.g. serving many
 hundreds or thousands of requests per minute.
 
-### Async Applicative
+### Applicative
 
-This engine is implemented based on the category theory concept of Applicative Functors.
-Applicative Functors have been considered as good abstractions for asynchronous processes because parts of its structure do not imply a strict execution order.
-Because of that we were able to implement support to several engines with a single code base, just changing the required set of protocols.
-Currently Async Applicative supports the following sub-engines:
+Applicative engines are implemented based on the category theory concept of Applicative Functors.
+Applicative Functors have been considered as good abstractions for asynchronous processes because parts of its structure do not imply a strict execution order. Because of that we were able to implement support to several engines with a single code base, just changing the required set of protocols.
+Some examples of engines we have implemented using the applicative framework:
 
 * Synchronous
 * Core Async
 * Promesa
+* Virtual threads
 
 Setting a sub-engine (evaluation context) is done with the `:nodely.engine.applicative/context` option, which defaults to promesa.
 
@@ -322,7 +324,8 @@ Setting a sub-engine (evaluation context) is done with the `:nodely.engine.appli
                              :nodely.engine.applicative/context applicative.core-async/context})
 ```
 
-Everything related to the applicative engine is currently experimental and subject to change.
+We don't recommend usage of functions that are not part of the api namespace, those are subject to breaking changes.
+Instead use the engines that are provided by `nodely.api.v0`.
 
 ### Async Virtual Futures
 

diff --git a/project.clj b/project.clj
@@ -1,4 +1,4 @@
-(defproject dev.nu/nodely "1.18.0"
+(defproject dev.nu/nodely "1.18.1"
   :description "Decoupling data fetching from data dependency declaration"
   :url "https://github.com/nubank/nodely"
   :license {:name "MIT"}

diff --git a/src/nodely/engine/applicative/virtual_future.clj b/src/nodely/engine/applicative/virtual_future.clj
@@ -8,40 +8,44 @@
 
 (declare context)
 
+(defn deref-unwrapped
+  [it]
+  (try (deref it)
+       (catch java.util.concurrent.ExecutionException e
+         (throw (.getCause e)))))
+
 (extend-type GreenFuture
   mp/Contextual
   (-get-context [_] context)
 
   mp/Extract
   (-extract [it]
-    (try (deref it)
-         (catch java.util.concurrent.ExecutionException e
-           (throw (.getCause e))))))
+    (deref-unwrapped it)))
 
 (def context
   (reify
     mp/Context
     protocols/RunNode
     (-apply-fn  [_ f mv]
-      (vfuture (f (deref mv))))
+      (vfuture (f (deref-unwrapped mv))))
 
     mp/Functor
     (-fmap [mn f mv]
-      (vfuture (f (deref mv))))
+      (vfuture (f (deref-unwrapped mv))))
 
     mp/Monad
     (-mreturn [_ v]
       (vfuture v))
 
     (-mbind [mn mv f]
-      (vfuture (let [v (deref mv)]
-                 (deref (f v)))))
+      (vfuture (let [v (deref-unwrapped mv)]
+                 (deref-unwrapped (f v)))))
 
     mp/Applicative
     (-pure [_ v]
       (vfuture v))
 
     (-fapply [_ pf pv]
-      (vfuture (let [f (deref pf)
-                     v (deref pv)]
+      (vfuture (let [f (deref-unwrapped pf)
+                     v (deref-unwrapped pv)]
                  (f v))))))
diff --git a/test/nodely/api_test.clj b/test/nodely/api_test.clj
@@ -2,6 +2,7 @@
   (:refer-clojure :exclude [cond])
   (:require
    [clojure.core.async :as async]
+   [clojure.set :as set]
    [clojure.test :refer :all]
    [criterium.core :as criterium]
    [matcher-combinators.matchers :as matchers]
@@ -23,6 +24,12 @@
 (def sequence-node-env-with-missing-key
   {:y (>sequence inc ?x)})
 
+(def exceptions-all-the-way-down
+  {:a (>leaf (throw (ex-info "Oops!" {})))
+   :b (>leaf (inc ?a))
+   :c (>leaf (inc ?b))
+   :d (>leaf (inc ?c))})
+
 (def env-with-nine-sleeps {:a (blocking (>leaf (do (Thread/sleep 1000) :a)))
                            :b (blocking (>leaf (do (Thread/sleep 1000) :b)))
                            :c (blocking (>leaf (do (Thread/sleep 1000) :c)))
@@ -90,10 +97,17 @@
       (t/testing "sequence with missing key"
         (t/matching #"Missing key on env"
                     (try (api/eval-key sequence-node-env-with-missing-key :y {::api/engine engine-key})
-                         (catch clojure.lang.ExceptionInfo e (ex-message e))))))))
+                         (catch clojure.lang.ExceptionInfo e (ex-message e))))))
+
+    (t/testing "handling nested exceptions"
+      (t/matching #"Oops!"
+                  (try (api/eval-key exceptions-all-the-way-down :d {::api/engine engine-key})
+                       (catch clojure.lang.ExceptionInfo e (ex-message e)))))))
 
 (t/deftest api-test
-  (for [engine (keys api/engine-data)]
+  (for [engine (set/difference (set (keys api/engine-data))
+                               #{:core-async.iterative-scheduling
+                                 :async.virtual-futures})]
     (engine-test-suite engine)))
 
 (t/deftest incorrect-engine-id