Unknown severity level causes NPE
$ cat .clojure
/opt/clojure/clojure.jar:/opt/clojure-contrib/clojure-contrib.jar:lib/commons-logging-1.1.1.jar
$ clj
Clojure 1.1.0-alpha-SNAPSHOT
user=> (use 'clojure.contrib.logging)
nil
user=> (log :info "Hello, world.")
Sep 21, 2009 3:44:03 PM clojure.contrib.logging$eval__92$impl_write_BANG___100 invoke
INFO: Hello, world.
nil
user=> (log :bar "Hello, world.")
java.lang.NullPointerException (NO_SOURCE_FILE:0)
I hit this when using :severe as a log level, which should actually be :error.
Leave a comment
file:bCd_BmPWmr3OVpeJe5afGb: Patch.
Added a patch. This does three things:
The best test is
- Examines the level in (log). If it's a keyword at compile-time, transform it now; otherwise, transform it later.
- Transformation maps j.u.l terms to those used by c.c.l. This is handy, though the mapping (of course) does not round-trip perfectly.
- Unrecognized terms cause an exception to be thrown.
The best test is
(require ['clojure.contrib.logging :as 'log])
(log/log ((fn []:severe)) "hi") ; Test runtime level computation.
(log/log :boo "ho")
(log/log :fine "ha")
(log/log :info "ha")
file:aOuXXKPWqr3OVpeJe5afGb: Correct quoting.
The only valid levels are the ones specified in the documentation, those levels in common use across multiple logging implementations. If one uses an invalid value one should not expect it to work.
The "correct" solution would be to provide a better exception in this error case.
The "correct" solution would be to provide a better exception in this error case.
"If one uses an invalid value one should not expect it to work"
No, but one should expect something more useful than a NPE. The patch I attached throws an exception when an invalid value is used (indeed, at compile time if it's a constant keyword).
c.c.l already translates from one set of log levels to those in use by the underlying implementation (e.g., if you're using Java Logging it'll convert :debug to FINE). This patch ensures that you can use the other set of names, too, which seems perfectly reasonable, but I can understand if you disagree. In that case, simply change translate-log-level to something like
(defn translate-log-level [level]
(or (#{:trace :debug :info :warn :error :fatal} level)
(throw (new Exception (str "Unknown log level " (prn-str level))))))
and commit. I'm not particularly fussed either way, but the validation needs to be done.
No, but one should expect something more useful than a NPE. The patch I attached throws an exception when an invalid value is used (indeed, at compile time if it's a constant keyword).
c.c.l already translates from one set of log levels to those in use by the underlying implementation (e.g., if you're using Java Logging it'll convert :debug to FINE). This patch ensures that you can use the other set of names, too, which seems perfectly reasonable, but I can understand if you disagree. In that case, simply change translate-log-level to something like
(defn translate-log-level [level]
(or (#{:trace :debug :info :warn :error :fatal} level)
(throw (new Exception (str "Unknown log level " (prn-str level))))))
and commit. I'm not particularly fussed either way, but the validation needs to be done.