sunng87 / diehard

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

`Fallback` doesn't support `.handleResult` #55

Open alexandergunnarson opened 1 year ago

alexandergunnarson commented 1 year ago

I'd like to throw an exception when retries are exceeded. Per https://failsafe.dev/faqs/#how-to-i-throw-an-exception-when-retries-are-exceeded, here's what they recommend (and what works on my machine):

// Retry on a null result
RetryPolicy<Connection> retryPolicy = RetryPolicy.<Connection>builder()
  .handleResult(null)
  .build();

// Fallback on a null result with a ConnectException
Fallback<Connection> fallback = Fallback.<Connection>builderOfException(e -> {
    return new ConnectException("Connection failed after retries");
  })
  .handleResult(null)
  .build();

Failsafe.with(fallback).compose(retryPolicy).get(this::getConnection);

This means diehard.core/fallback would have to be reworked slightly to support .handleResult etc.

Awesome library by the way! 🙌 Great functionality, and great abstraction over the Java lib.

alexandergunnarson commented 1 year ago

As a minimal example, I want the following to return :b, not :a:

(diehard/with-retry
  {:max-retries 2
   :delay-ms    500
   :retry-if    (fn [v _] (= v :a))
   :fallback    (fn [_ _] :b)}
  :a)

and I want this to throw the exception:

(diehard/with-retry
  {:max-retries 2
   :delay-ms    500
   :retry-if    (fn [v _] (= v :a))
   :fallback    (fn [_ _] (throw (ex-info "Stop the boat!" {})))}
  :a)
alexandergunnarson commented 1 year ago

Specifically, something like this could work (if you adapt it for diehard.core/fallback) — I'm using it in a project:

(defn- ->fallback-policy
  [{:as opts :keys [fallback retry-if retry-on retry-when]}]
  (when (some? fallback)
    (let [f       (if-not (fn? fallback) (constantly fallback) fallback)
          builder (Fallback/builder
                    ^CheckedFunction
                    (diehard.util/fn-as-checked-function
                      (fn [^ExecutionAttemptedEvent exec-event]
                        (diehard/with-event-context
                          exec-event
                          (f (.getLastResult exec-event) (.getLastException exec-event))))))]
      (when retry-if
        (.handleIf builder ^CheckedBiPredicate (diehard.util/bipredicate retry-if)))
      (when retry-on
        (.handle builder ^java.util.List (diehard.util/as-vector retry-on)))
      (when (contains? opts :retry-when)
        (if (fn? retry-when)
            (.handleResultIf builder (diehard.util/predicate retry-when))
            (.handleResult builder retry-when)))
      (.build builder))))
sunng87 commented 1 year ago

In diehard, the last exception is thrown when retries exceeds:

  (testing "max retry"
    (is (thrown? clojure.lang.ExceptionInfo
                 (with-retry {:max-retries 3}
                   (throw (ex-info "expected" {}))))))

Perhaps this will work for you.

alexandergunnarson commented 1 year ago

That's true, and that's useful, but different from the use case I'm describing. I want to have the option to throw on retries exceeded (say, when :retry-if returns truthy), even when no exception is thrown in the body:

(diehard/with-retry
  {:max-retries 2
   :retry-if    (fn [v _] (= v :a))
   :fallback    (fn [_ _] (throw (ex-info "Retries exceeded" {})))}
  ;; No exception thrown here
  :a)

Failsafe supports this (as does a "patched" version of diehard I'm using in a project); I'd love it diehard supported it as well.

sunng87 commented 1 year ago

I got your point. But using fallback for throwing this exception is a little hack because fallback is designed to provide a value. The throw exception, you have other ways to implement:

nlessa commented 1 year ago

Hello, wondering how I could "check the final return value of retry block and throw exception if result is not desired". Checking executions in the "retry-if" clause?