Fork me on GitHub
#duct
<
2020-07-09
>
erwinrooijakkers09:07:09

Is it possible that module.web’s log-error does not log the exception when using clojure.tools.logging? clojure.tools.logging needs exception as first argument to be properly handled (e.g., the Sentry logback appender processing the stracktrace properly to be shown in Sentry) module.web has this handler:

(defn- log-error [logger ex] 
  (logger/log logger :error ::handler-error ex))
https://github.com/duct-framework/module.web/blob/6aa5ccb38280a872d45eaddbfab8e9fa1013ca5a/src/duct/middleware/web.clj#L18 The ClojureLogger is implemented like this:
(defrecord ClojureLogger []
  logger/Logger
  (-log [_ level ns _ _ _ event data]
    (let [level  (if (= level :report) :info level)
          logger (impl/get-logger log/*logger-factory* ns)]
      (cond
        (instance? Throwable event)
        (log/log* logger level event nil)
        (instance? Throwable data)
        (log/log* logger level data (pr-str event)) 
        (nil? data)
        (log/log* logger level nil (pr-str event))
        :else
        (log/log* logger level nil (pr-str event data))))))
So it seems that the log-error moves the exception as the 3th argument, which is an underscore in the implementation and thus event or data argument is not recognized as a throwable and the _ is ignored?

erwinrooijakkers09:07:25

If this is the case I will create an issue, or do I overlook something?

walterl10:07:35

clojure.tools.logging/log* (note the *) takes a throwable as the third arg, according to its docs: https://clojure.github.io/tools.logging/#clojure.tools.logging/log*, so all seems in order to me

erwinrooijakkers11:07:36

Thanks for that check 🙂 I referred to the arguments of -log

walterl11:07:20

Looks like -log is only called by log-form (https://github.com/duct-framework/logger/blob/master/src/duct/logger.clj#L12), which puts the arguments in the right places. Am I missing something?

erwinrooijakkers12:07:39

Ah log-form great

erwinrooijakkers12:07:51

If that one is called also for other implementation of Logger then it should insert those two arguments

erwinrooijakkers12:07:29

namespace and a delayed random/uuid

erwinrooijakkers12:07:51

(-log [_ level ns _ _ _ event data] is the signature of the ClojureLogger (https://github.com/duct-framework/logger.clojure/blob/master/src/duct/logger/clojure.clj)

erwinrooijakkers12:07:21

log-form has:

this level ns delay event data

walterl13:07:25

I think this is where you may be misreading it. log-form takes only [logger level event data form], but calls -log with 8 args, as per the Logger protocol: [logger level ns-str file line id event data]

erwinrooijakkers12:07:02

ClojureLogger has: this level ns _ _ _ event data

erwinrooijakkers12:07:17

So there seems to be a mismatch in argument count if log-form is used

walterl13:07:25

I think this is where you may be misreading it. log-form takes only [logger level event data form], but calls -log with 8 args, as per the Logger protocol: [logger level ns-str file line id event data]

erwinrooijakkers13:07:45

Ah you are right

erwinrooijakkers13:07:57

I missed the file and line

erwinrooijakkers13:07:20

Okay this means that it fits like this;

erwinrooijakkers13:07:40

log-form:

this level ns file line delay event data
ClojureLogger:
_    level ns _    _    _     event data
So that’s fine

👍 3