Violation of pre-condition not caught by error handler

[mzuther]

Hi!

I am using chime 0.3.2 with Clojure 1.10.1 and have found that the :error-handler function currently does not catch violations of pre-conditions.

From looking at the code, chime seems to only catch java.lang.Exception:

(chime/chime-at
    (take 3 (rest (chime/periodic-seq (java.time.Instant/now)
                                      (java.time.Duration/ofSeconds 1))))
    (fn [timestamp] (throw (Exception. (str timestamp))))
    {:error-handler (fn [e] (println (str e)))})

but not java.lang.Error:

(chime/chime-at
    (take 3 (rest (chime/periodic-seq (java.time.Instant/now)
                                      (java.time.Duration/ofSeconds 1))))
    (fn [timestamp] (throw (AssertionError. (str timestamp))))
    {:error-handler (fn [e] (println (str e)))})

Could you please change the code and catch java.lang.Throwable, the common ancestor of errors and exceptions?

Thanks a lot! 😄

Martin

[jarohen]

Hey Martin, thanks for the report 😄

I'm not a fan of catching Error, in general - from its Javadoc:

An Error is a subclass of Throwable that indicates serious problems that a reasonable application should not try to catch.

If your application is throwing these AssertionErrors in production usage, would it be an option to explicitly catch them in your schedule fn?

That said, Chime should try to behave reasonably in that case - if anything, we would want to ensure that it shuts down cleanly, where possible. Will see what it does!

James

[mzuther]

Hey James,

the name error-handler is a bit misleading and drove me in the wrong direction. You are right, catching all Errors is usually a bad idea.

I still think that chime should "handle" an Error in some way, though. Currently, there is a distinction between code throwing an Error and an Exception:

1. Exception, no error-handler:
   • debug message appears
   • chime continues
2. Exception, with error-handler:
   • problem is handled by error-handler
   • loop may be terminated
3. Error:
   • problematic code just misbehaves without a warning
   • to leave the loop, you have to catch the Error and re-throw it as an Exception

I think that an Error should just fall through chime (thus exiting the loop). This way, it could be handled either in the schedule fn or the wrapping code.

And a short explanation of this behaviour in the README wouldn't hurt... 😀

Thanks,

Martin

[jarohen]

Agreed - have implemented that in the above commit. I've also deprecated :error-handler, replacing it with :exception-handler.

[mzuther]

Thanks a lot, that sounds great! 😄