sunng87 / diehard

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

Exceptions thrown from :on-failure or :on-retries-exceeded are swallowed #64

Closed devurandom closed 11 months ago

devurandom commented 11 months ago

Exceptions thrown from :on-failure or :on-retries-exceeded are swallowed. As a result, the following test fails:

(deftest throw-on-timeout-test
  (is (thrown-with-msg? Exception #":on-retries-exceeded"
                        (dh/with-retry {:retry-when          false
                                        :delay-ms            100
                                        :max-retries         10
                                        :on-failed-attempt   (fn [_ _] 
                                                               (println :on-failed-attempt))
                                        :on-failure          (fn [_ _] 
                                                               (prn :on-failure) 
                                                               (throw (ex-info ":on-failure" {})))
                                        :on-retries-exceeded (fn [_ _] 
                                                               (prn :on-retries-exceeded)
                                                               (throw (ex-info ":on-retries-exceeded" {})))}
                          false))))
sunng87 commented 11 months ago

hi @devurandom the retry-block won't throw exception that is thrown in listener. If you want to throw an exeption when retry exceeded, just throw it in with-retry, the exception of last attempt will be thrown.

devurandom commented 11 months ago

Thanks! The fact that exceptions thrown in the :on-... handlers do not make it out of the with-retry block and are not even logged was unexpected. Is this documented and I just missed it?

In the end I achieved the desired behaviour with:

(deftest throw-on-timeout-test
  (is (thrown-with-msg? Exception #"failure"
                        (or (dh/with-retry {:retry-when  false
                                            :delay-ms    100
                                            :max-retries 10}
                              false)
                            (throw (ex-info "failure" {}))))))
sunng87 commented 11 months ago

This behaviour is actually from the underlying library failsafe. I can add some description in diehard's docs.