From 04daf6ac57a5c2106afd3dfd9ec589fc83bd715a Mon Sep 17 00:00:00 2001
From: Dan Holmsand <holmsand@gmail.com>
Date: Sun, 13 Sep 2015 11:00:07 +0200
Subject: [PATCH] Exceptions from reactions are now thrown on deref only

---
 src/reagent/ratom.cljs               | 30 +++++++++++++++++++++-------
 test/reagenttest/testratom.cljs      | 26 +++++++++++++-----------
 test/reagenttest/testratomasync.cljs | 27 ++++++++++++++-----------
 3 files changed, 52 insertions(+), 31 deletions(-)

diff --git a/src/reagent/ratom.cljs b/src/reagent/ratom.cljs
index 6c11cd2..d54aaa8 100644
--- a/src/reagent/ratom.cljs
+++ b/src/reagent/ratom.cljs
@@ -206,6 +206,7 @@
 (def ^:const clean 0)
 (def ^:const maybe-dirty 1)
 (def ^:const dirty 2)
+(def ^:const failed 3)
 
 (deftype Reaction [f ^:mutable state ^:mutable ^number dirtyness
                    ^:mutable watching ^:mutable watches
@@ -221,7 +222,7 @@
                nil watches)
     nil)
 
-  (-add-watch [this key f]
+  (-add-watch [_ key f]
     (set! watches (check-watches watches (assoc watches key f))))
 
   (-remove-watch [this key]
@@ -234,7 +235,7 @@
   (-reset! [a newval]
     (let [oldval state]
       (set! state newval)
-      (when on-set
+      (when (some? on-set)
         (set! dirtyness dirty)
         (on-set oldval newval))
       (-notify-watches a oldval newval)
@@ -260,14 +261,11 @@
   (-check-clean [this]
     (when (== dirtyness maybe-dirty)
       (let [ar auto-run]
-        ;; TODO: try/catch
         (set! auto-run nil)
         (doseq [w watching]
           (when (and (instance? Reaction w)
                      (not (-check-clean w)))
-            (if-some [ar (.-auto-run w)]
-              (ar w)
-              (run w))))
+            (._try-run this w)))
         (set! auto-run ar))
       (when (== dirtyness maybe-dirty)
         (set! dirtyness clean)))
@@ -299,6 +297,17 @@
     (set! watching derefed)
     nil)
 
+  Object
+  (_try-run [_ parent]
+    (try
+      (if-some [ar (.-auto-run parent)]
+        (ar parent)
+        (run parent))
+      (catch :default e
+        (set! (.-dirtyness parent) failed)
+        (set! (.-state parent) e)
+        (set! dirtyness dirty))))
+
   IRunnable
   (run [this]
     (let [oldstate state
@@ -319,6 +328,11 @@
   IDeref
   (-deref [this]
     (-check-clean this)
+    (when (== dirtyness failed)
+      (let [e state]
+        (set! dirtyness dirty)
+        (set! state nil)
+        (throw e)))
     (if (and (nil? auto-run) (nil? *ratom-context*))
       (when-not (== dirtyness clean)
         (let [oldstate state
@@ -361,7 +375,9 @@
 
 ;;; Queueing
 
-(defonce render-queue nil) ;; Gets set up from batching
+;; Gets set up from batching
+;; TODO: Refactor so that isn't needed
+(defonce render-queue nil)
 
 (def dirty-queue nil)
 
diff --git a/test/reagenttest/testratom.cljs b/test/reagenttest/testratom.cljs
index 1f983b2..ada284a 100644
--- a/test/reagenttest/testratom.cljs
+++ b/test/reagenttest/testratom.cljs
@@ -247,15 +247,17 @@
     (is (= @b 6))
     (is (= runs (running)))))
 
-;; (deftest catching
-;;   (let [runs (running)
-;;         a (rv/atom false)
-;;         catch-count (atom 0)
-;;         b (reaction (if @a (throw {})))
-;;         c (run! (try @b (catch js/Object e
-;;                           (swap! catch-count inc))))]
-;;     (is (= @catch-count 0))
-;;     (reset! a false)
-;;     (is (= @catch-count 0))
-;;     (reset! a true)
-;;     (is (= @catch-count 1))))
+(deftest catching
+  (let [runs (running)
+        a (rv/atom false)
+        catch-count (atom 0)
+        b (reaction (if @a (throw (js/Error. "fail"))))
+        c (run! (try @b (catch :default e
+                          (swap! catch-count inc))))]
+    (is (= @catch-count 0))
+    (reset! a false)
+    (is (= @catch-count 0))
+    (reset! a true)
+    (is (= @catch-count 1))
+    (reset! a false)
+    (is (= @catch-count 1))))
diff --git a/test/reagenttest/testratomasync.cljs b/test/reagenttest/testratomasync.cljs
index 7678301..b45fefa 100644
--- a/test/reagenttest/testratomasync.cljs
+++ b/test/reagenttest/testratomasync.cljs
@@ -271,15 +271,18 @@
     (is (= @b 6))
     (is (= runs (running)))))
 
-;; (deftest catching
-;;   (let [runs (running)
-;;         a (rv/atom false)
-;;         catch-count (atom 0)
-;;         b (reaction (if @a (throw {})))
-;;         c (run! (try @b (catch js/Object e
-;;                           (swap! catch-count inc))))]
-;;     (is (= @catch-count 0))
-;;     (reset! a false)
-;;     (is (= @catch-count 0))
-;;     (reset! a true)
-;;     (is (= @catch-count 1))))
+(deftest catching
+  (let [runs (running)
+        a (rv/atom false)
+        catch-count (atom 0)
+        b (reaction (if @a (throw (js/Error. "reaction fail"))))
+        c (ar (fn [] (try @b (catch js/Object e
+                               (swap! catch-count inc)))))]
+    (is (= @catch-count 0))
+    (reset! a false)
+    (sync)
+    (is (= @catch-count 0))
+    (reset! a true)
+    (is (= @catch-count 0))
+    (sync)
+    (is (= @catch-count 1))))