Skip to content

Commit

Permalink
Merge pull request #17 from kepler16/artem/tre-856-do-not-swallow-exc…
Browse files Browse the repository at this point in the history
…eptions-in-gx

feat(TRE-856): do not swallow exceptions in gx
  • Loading branch information
armed authored Oct 24, 2022
2 parents 2e1f8f1 + ee405fc commit 6b2eb09
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 55 deletions.
3 changes: 2 additions & 1 deletion .clj-kondo/funcool/promesa/config.edn
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,5 @@
promesa.core/plet clojure.core/let
promesa.core/loop clojure.core/loop
promesa.core/recur clojure.core/recur
promesa.core/with-redefs clojure.core/with-redefs}}
promesa.core/with-redefs clojure.core/with-redefs
promesa.core/doseq clojure.core/doseq}}
5 changes: 3 additions & 2 deletions src/k16/gx/beta/core.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -232,13 +232,13 @@
(props-fn arg-map)
(catch #?(:clj Throwable :cljs :default) e
(gx.err/throw-gx-err "Props function error"
{:ex-message (impl/error-message e)
{:ex-message (gx.err/gather-error-messages e)
:args arg-map}))))

(defn- wrap-error
[e arg-map]
(gx.err/gx-err-data "Signal processor error"
{:ex-message (impl/error-message e)
{:ex-message (gx.err/gather-error-messages e)
:ex e
:args arg-map}))

Expand Down Expand Up @@ -405,6 +405,7 @@

:else gxm))))))

#_{:clj-kondo/ignore [:unresolved-symbol]}
(comment
(def graph {:options {:port 8080}
:router {:some "router"}
Expand Down
38 changes: 35 additions & 3 deletions src/k16/gx/beta/errors.cljc
Original file line number Diff line number Diff line change
@@ -1,10 +1,35 @@
(ns k16.gx.beta.errors)

(defrecord ErrorContext [error-type node-key node-contents signal-key])
(defrecord ErrorContext [error-type node-key
node-contents signal-key
causes])

(def ^:dynamic *err-ctx*
"Error context is used for creating/throwing exceptions with contextual data"
(map->ErrorContext {:error-type :general}))
(map->ErrorContext {:error-type :general :causes []}))

(defn gather-error-messages
[ex]
#?(:clj (->> ex
(iterate ex-cause)
(take-while some?)
(mapv ex-message)
(interpose "; ")
(apply str))
:cljs (cond
(instance? cljs.core/ExceptionInfo ex)
(ex-message ex)

(instance? js/Error ex)
(ex-message ex)

:else ex)))

(defn add-err-cause
"Adds cause to error context, evaluates to nil"
[cause]
(set! *err-ctx* (update *err-ctx* :causes conj cause))
nil)

(defn gx-err-data
([internal-data]
Expand Down Expand Up @@ -48,14 +73,21 @@
(flatten)
(apply str)))

(defn- cause->str
[{:keys [title data exception]}]
(str "cause(" title " = " data "): " (gather-error-messages exception)))

(defn humanize-error
[{:keys [node-key signal-key message]} & rest-of-error]
[{:keys [node-key signal-key message causes]} & rest-of-error]
(let [rest-of-error (filter seq rest-of-error)]
(apply str (concat [(or message "Error") ": "
(tokenize "node = " node-key
"signal = " signal-key)]
(when (seq rest-of-error)
(conj (interpose "\n\t" rest-of-error)
"\n\t"))
(when (seq causes)
(conj (interpose "\n\t" (map cause->str causes))
"\n\t"))))))

(defmulti humanize :error-type)
Expand Down
43 changes: 14 additions & 29 deletions src/k16/gx/beta/impl.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -142,23 +142,6 @@
[& maps]
(reduce merger maps))

(defn error-message
[ex]
#?(:clj (->> ex
(iterate ex-cause)
(take-while some?)
(mapv ex-message)
(interpose "; ")
(apply str))
:cljs (cond
(instance? cljs.core/ExceptionInfo ex)
(ex-message ex)

(instance? js/Error ex)
(ex-message ex)

:else ex)))

(def locals #{'gx/ref 'gx/ref-keys})

(defn local-form?
Expand Down Expand Up @@ -192,21 +175,23 @@
:else x))
form))

#?(:clj
(defn quiet-requiring-resolve
[sym]
(try
(requiring-resolve sym)
(catch Throwable _ nil))))

(defn resolve-symbol
[sym]
(when (symbol? sym)
#?(:cljs (namespace-symbol sym)
:clj (some-> sym
(namespace-symbol)
(quiet-requiring-resolve)
(var-get)))))
(if-let [nss #?(:cljs (namespace-symbol sym)
:clj (try
(some-> sym
(namespace-symbol)
(requiring-resolve)
(var-get))
(catch Throwable e
(gx.err/add-err-cause
{:title :symbol-cannot-be-resolved
:data sym
:exception e}))))]
nss
(gx.err/add-err-cause {:title :symbol-cannot-be-resolved
:data sym}))))

(defn form->runnable [form-def]
(let [props* (atom #{})
Expand Down
50 changes: 34 additions & 16 deletions test/k16/gx/beta/core_test.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@
:node-contents '(throw (ex-info "foo" (gx/ref :a))),
:message "Special forms are not supported",
:internal-data
{:form-def '(throw (ex-info "foo" (gx/ref :a))), :token 'throw}}
{:form-def '(throw (ex-info "foo" (gx/ref :a))), :token 'throw}
:causes []}
failure))))

(deftest component-support-test
Expand Down Expand Up @@ -170,7 +171,8 @@
"circular :a -> :b -> :a")},
:message "Dependency errors",
:error-type :deps-sort,
:signal-key :gx/start}
:signal-key :gx/start
:causes []}
failure))))

(defn my-props-fn
Expand Down Expand Up @@ -236,7 +238,7 @@
:node-contents '(throw "starting"),
:message "Special forms are not supported",
:internal-data {:form-def '(throw "starting"), :token 'throw}}
(-> gx-norm :failures first)))))
(-> gx-norm :failures first (dissoc :causes))))))

(testing "unresolved symbol failure"
(let [graph {:a {:nested-a 1}
Expand All @@ -255,7 +257,7 @@
:internal-data
{:form-def '(some-not-found-symbol "stopping"),
:token 'some-not-found-symbol}}
failure))))
(dissoc failure :causes)))))
#?(:clj
(testing "processor failure"
(let [graph {:a {:nested-a 1}
Expand All @@ -274,7 +276,8 @@
:node-key :b,
:node-contents #:gx{:start '(/ (gx/ref :z) 0),
:stop '(println "stopping")},
:signal-key :gx/start}
:signal-key :gx/start
:causes []}
{:internal-data
{:ex-message
(str "class clojure.lang.Keyword cannot be cast"
Expand All @@ -287,7 +290,8 @@
:error-type :node-signal,
:node-key :c,
:node-contents '(inc :bar),
:signal-key :gx/start})
:signal-key :gx/start
:causes []})
p-gx-started (gx/signal gx-norm :gx/start)]
(is (= expect
(->> @p-gx-started
Expand Down Expand Up @@ -318,7 +322,8 @@
:node-contents
#:gx{:component 'k16.gx.beta.core-test/props-validation-component,
:start #:gx{:props-fn 'k16.gx.beta.core-test/my-props-fn}},
:signal-key :gx/start}
:signal-key :gx/start
:causes []}
(first (:failures gx-started))))))))

(comment
Expand All @@ -344,21 +349,24 @@
:error-type :node-signal,
:node-key :d,
:node-contents '(gx/ref :c),
:signal-key :gx/start}
:signal-key :gx/start
:causes []}
{:internal-data {:dep-node-keys '(:b)},
:message "Failure in dependencies",
:error-type :node-signal,
:node-key :c,
:node-contents '(gx/ref :b),
:signal-key :gx/start}
:signal-key :gx/start
:causes []}
{:internal-data
{:ex-message "Divide by zero",
:args {:props {:a 1}, :state :uninitialised, :value nil, :instance nil}},
:message "Signal processor error",
:error-type :node-signal,
:node-key :b,
:node-contents '(/ (gx/ref :a) 0),
:signal-key :gx/start})
:signal-key :gx/start
:causes []})
p-gx-started (gx/signal gx-map :gx/start)
failures (-> (:failures @p-gx-started)
(vec)
Expand All @@ -381,7 +389,8 @@
:signal-key :gx/start}
(-> (:failures gx-map)
(first)
(update :internal-data dissoc :ex)))))))))
(update :internal-data dissoc :ex)
(dissoc :causes)))))))))

(def ^:export push-down-props-component
{:gx/props '(gx/ref-keys [:a])
Expand Down Expand Up @@ -420,7 +429,9 @@
:node-contents
#:gx{:component 'k16.gx.beta.core-test/non-existent}
:internal-data
{:component 'k16.gx.beta.core-test/non-existent}}
{:component 'k16.gx.beta.core-test/non-existent}
:causes [{:title :symbol-cannot-be-resolved
:data 'k16.gx.beta.core-test/non-existent}]}
(first (:failures gx-map)))))

(let [graph {:c {:gx/component 'k16.gx.beta.core-test/invalid-component}}
Expand All @@ -433,7 +444,8 @@
#:gx{:component 'k16.gx.beta.core-test/invalid-component}
:internal-data
{:component {:some "invalid component"}
:schema-error #{[:some ["disallowed key"]]}}}
:schema-error #{[:some ["disallowed key"]]}}
:causes []}
(-> gx-map
:failures
first
Expand All @@ -451,7 +463,8 @@
{:component #:gx{:start #:gx{:processor "non callable val"}}
:schema-error
#{[:gx/start
#:gx{:processor ["should be an ifn"]}]}}}
#:gx{:processor ["should be an ifn"]}]}}
:causes []}
(-> gx-map
:failures
first
Expand Down Expand Up @@ -552,13 +565,18 @@

(deftest unserolvable-symbol-test
(let [graph {:a 'foo.bar/baz}
norm (gx/normalize {:graph graph})]
norm (gx/normalize {:graph graph})
failure (first (:failures norm))
cause (first (:causes failure))]
#?(:clj
(is (instance? java.io.FileNotFoundException (:exception cause))))
(is (= :symbol-cannot-be-resolved (:title cause)))
(is (= {:message "Unable to resolve symbol",
:error-type :normalize-node,
:node-key :a,
:node-contents 'foo.bar/baz,
:internal-data {:form-def 'foo.bar/baz, :token 'foo.bar/baz}}
(first (:failures norm))))))
(dissoc failure :causes)))))

(def ^:export proc-instance-component
{:gx/start {:gx/processor (fn [{:keys [props]}]
Expand Down
12 changes: 8 additions & 4 deletions test/k16/gx/beta/system_test.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -43,27 +43,31 @@
"circular :a -> :b -> :a"]},
:message "Dependency errors",
:error-type :deps-sort,
:signal-key :gx/start}
:signal-key :gx/start
:causes []}
{:internal-data
{:errors
[":c depends on :z, but :z doesn't exist"
"circular :a -> :b -> :a"]},
:message "Dependency errors",
:error-type :deps-sort,
:signal-key :gx/suspend}
:signal-key :gx/suspend
:causes []}
{:internal-data
{:errors
[":c depends on :z, but :z doesn't exist"
"circular :a -> :b -> :a"]},
:message "Dependency errors",
:error-type :deps-sort,
:signal-key :gx/resume}
:signal-key :gx/resume
:causes []}
{:internal-data
{:errors
[":c depends on :z, but :z doesn't exist"
"circular :a -> :b -> :a"]},
:message "Dependency errors",
:error-type :deps-sort,
:signal-key :gx/stop}]}
:signal-key :gx/stop
:causes []}]}
(ex-data err))))))

1 comment on commit 6b2eb09

@vercel
Copy link

@vercel vercel bot commented on 6b2eb09 Oct 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

gx – ./

gx-kepler16.vercel.app
gx-git-master-kepler16.vercel.app
gx.kepler16.com

Please sign in to comment.