Skip to content

Commit

Permalink
[fix] [#21] Work around issue with use in Cljs core.async/go bodies
Browse files Browse the repository at this point in the history
Problem:
  (clojure.core.async/go (taoensso.telemere/log! "hello")) ; Compiles fine
  (cljs.core.async/go    (taoensso.telemere/log! "hello")) ; Compile fails

I could try to get to the bottom of exactly what's going on - but ultimately
IOC mechanisms like `go` are always going to be a bit fragile, especially for
heavily-optimized/unusual code.

In this case, the problem is thankfully only with Cljs - and Telemere's Cljs
performance isn't too critical - so I think we can afford to just bypass any
potential fiddling by the `go` macro by wrapping Cljs Telemere expansions in
an IIFE ((fn [] ...)).

Downside is the (small) added cost of a function construction and call.
Upside   is avoiding potential issues with core.async and other similar
IOC-style systems (Electric Clojure, etc.)
  • Loading branch information
ptaoussanis committed Sep 20, 2024
1 parent 568906c commit cbab57b
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 34 deletions.
3 changes: 2 additions & 1 deletion projects/main/project.clj
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@
*unchecked-math* false #_:warn-on-boxed}

:dependencies
[[org.clojure/test.check "1.1.1"]
[[org.clojure/core.async "1.6.681"]
[org.clojure/test.check "1.1.1"]
[org.clojure/tools.logging "1.3.0"]
[org.slf4j/slf4j-api "2.0.16"]
[com.taoensso/telemere-shell "1.0.0-SNAPSHOT"]
Expand Down
71 changes: 38 additions & 33 deletions projects/main/src/taoensso/telemere/impl.cljc
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,13 @@
(sig-middleware# signal#) ; Apply signal middleware, can throw
(do signal#)))))

;; Could avoid double `run-form` expansion with a fn wrap (>0 cost)
;; (let [run-fn-form (when run-form `(fn [] (~run-form)))]
;; `(let [~'run-fn-form ~run-fn-form]
;; (if-not ~allow?
;; (run-fn-form)
;; (let [...]))))

into-let-form
(enc/cond!
(not trace?) ; Don't trace
Expand Down Expand Up @@ -734,39 +741,37 @@
(enc/try*
(do (RunResult. ~run-form nil (- (enc/now-nano*) t0#)))
(catch :all t# (RunResult. nil t# (- (enc/now-nano*) t0#)))
(finally (.close otel-scope#))))))])]

;; Could avoid double `run-form` expansion with a fn wrap (>0 cost)
;; (let [run-fn-form (when run-form `(fn [] (~run-form)))]
;; `(let [~'run-fn-form ~run-fn-form]
;; (if-not ~allow?
;; (run-fn-form)
;; (let [...]))))

;; Unless otherwise specified, allow errors to throw on call
`(let [~'__kind ~kind-form
~'__ns ~ns-form
~'__id ~id-form
~'__level ~level-form]

(enc/if-not ~allow?
~run-form
(let [~'__inst ~inst-form
~'__thread ~thread-form
~'__root0 ~root-form0 ; ?{:keys [id uid]}

~@into-let-form ; Inject conditional bindings
signal# ~signal-delay-form]

(dispatch-signal!
;; Unconditionally send same wrapped signal to all handlers.
;; Each handler will use wrapper for handler filtering,
;; unwrapping (realizing) only allowed signals.
(WrappedSignal. ~'__ns ~'__kind ~'__id ~'__level signal#))

(if ~'__run-result
( ~'__run-result signal#)
true)))))))))
(finally (.close otel-scope#))))))])

final-form
;; Unless otherwise specified, allow errors to throw on call
`(let [~'__kind ~kind-form
~'__ns ~ns-form
~'__id ~id-form
~'__level ~level-form]

(enc/if-not ~allow?
~run-form
(let [~'__inst ~inst-form
~'__thread ~thread-form
~'__root0 ~root-form0 ; ?{:keys [id uid]}

~@into-let-form ; Inject conditional bindings
signal# ~signal-delay-form]

(dispatch-signal!
;; Unconditionally send same wrapped signal to all handlers.
;; Each handler will use wrapper for handler filtering,
;; unwrapping (realizing) only allowed signals.
(WrappedSignal. ~'__ns ~'__kind ~'__id ~'__level signal#))

(if ~'__run-result
( ~'__run-result signal#)
true))))]

(if cljs?
`((fn [] ~final-form)) ; IIFE wrap for use in `go` and other IOC-style bodies
(do final-form)))))))

(comment
(with-signal (signal! {:level :warn :let [x :x] :msg ["Test" "message" x] :data {:a :A :x x} :run (+ 1 2)}))
Expand Down
7 changes: 7 additions & 0 deletions projects/main/test/taoensso/telemere_tests.cljc
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
(ns taoensso.telemere-tests
(:require
[clojure.test :as test :refer [deftest testing is]]
[clojure.core.async :as async]
[taoensso.encore :as enc :refer [throws? submap?] :rename {submap? sm?}]
[taoensso.encore.signals :as sigs]
[taoensso.telemere :as tel]
Expand Down Expand Up @@ -658,6 +659,12 @@
;;
(do (enc/set-var-root! impl/*sig-handlers* nil) :unset-handler)])))])

;;;;

(deftest _core-async
(testing "Signals in go macros"
[(async/go (tel/log! "hello"))]))

;;;; Interop

(comment (def ^org.slf4j.Logger sl (org.slf4j.LoggerFactory/getLogger "my.class")))
Expand Down

0 comments on commit cbab57b

Please sign in to comment.