sunng87 / diehard

Clojure resilience library for flexible retry, circuit breaker and rate limiter
Eclipse Public License 2.0
330 stars 27 forks source link

Allow retry on Throwable #36

Closed msassak closed 4 years ago

msassak commented 4 years ago

Currently diehard restricts retry-on to Exception and its subclasses:

https://github.com/sunng87/diehard/blob/master/src/diehard/spec.clj#L17

This is in keeping with the recommendation from the Java docs:

The class Exception and its subclasses are a form of Throwable that indicates conditions that a reasonable application might want to catch.

https://docs.oracle.com/javase/8/docs/api/java/lang/Exception.html

But this advice is (sadly) contradicted by experience. In the wild, much code throw exceptions that are not subclasses of Exception (Clojure's pre and post assertions, for example) or catches Throwable, usually in a server context to prevent thread death, for example:

As diehard is used in exactly these situations, I believe the spec should be relaxed to (isa? % Throwable).

msassak commented 4 years ago

To illustrate what I mean about Clojure's pre and post conditions:

=> (defn y [a] {:pre [(number? a)]} a)
#'user/y
=> (y 123)
123
=> (y "123")

Execution error (AssertionError) at user/y (form-init3166981006581329906.clj:1).
Assert failed: (number? a)
=> (isa? (type *e) Exception)
false
=> (isa? (type *e) Throwable)
true
sunng87 commented 4 years ago

Makes sense. Let me create a fix for this.

sunng87 commented 4 years ago

@msassak could you please check if https://github.com/sunng87/diehard/commit/4628fa5cd563be148c4fc5862ecf4c6db35a93a6 work for you?

msassak commented 4 years ago

@sunng87 I verified it works. Thank you!

sunng87 commented 4 years ago

Thank you for quick verification.

Currently the release was blocked by a leiningen issue. I will release a new version once it fixed.

sunng87 commented 4 years ago

released in 0.9.3