Skip to content

Commit

Permalink
Use callback to trigger early return
Browse files Browse the repository at this point in the history 
This commit merges the exception predicate functionality into the existing
attempt callback function: if the callback returns `:again.core/fail` the
remaining execution attempts are skipped and `with-retries` rethrows the last
exception.

A few notes on this:

On second thought adding a second callback function (`exception-predicate`)
seemed like a bad design choice since to make the predicate more general it
would have required the same arguments as the existing `callback` function. At
which point it made sense to just enhance the existing callback functionality.

I decided to use a namespaced keyword (`:again.core/fail`) to signal the early
return:

1. there is no (or at least very little) risk of any existing callback function
out in the wild already returning that value, whereas `true` / `false` / `:fail`
/ `…` might be returned by an existing callback function accidentally

2. a keyword instead of `true` / `false` allows us to later add other
functionality by returning other keywords
  • Loading branch information
@liwp
liwp committed Apr 22, 2019
1 parent 0824779 commit cbde3bf
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 100 deletions.
42 changes: 29 additions & 13 deletions README.md
View file
Expand Up @@ -7,7 +7,7 @@ A Clojure library for retrying an operation based on a retry strategy.
## Clojars

```clj
[listora/again "0.2.0-SNAPSHOT"]
[listora/again "1.0.0-SNAPSHOT"]
```

---
Expand Down Expand Up @@ -71,6 +71,9 @@ functions.

### Advanced use case:

The advanced form allows you to pass in other options than just the retry
strategy.

```clj
(require '[again.core :as again])

Expand All @@ -87,46 +90,59 @@ functions.

(again/with-retries
{::again/callback log-attempt
::again/user-context (atom {})
::again/strategy [100 1000 10000]}
::again/strategy [100 1000 10000]
::again/user-context (atom {})}
(my-operation …))
```

The above example is contrived (there's no need to set `:retried?` in the user
context since the `:success` callback could just check if `::again/attempts` is
greater than 1), but it tries to show that:
greater than `1`), but it tries to show that:

- instead of a sequence of delays, `with-retries` also accepts a map as its
first argument
- the `:again.core/strategy` key is used to pass in the delay strategy
- the `:again.core/callback` key can be used to specify a function that will get
called after each form execution attempt
called after each attempt
- the `:again.core/user-context` key can be used to specify a user-defined
context object that will get passed to the callback function

The callback function and the context object allows (hopefully!) for arbitrary
monitoring implementations where the results of each attempt can be eg logged to
a monitoring system.

The callback is passed a map as its one argument:
The callback is called with a map as its only argument:

```clj
{
;; the number of form executions - a positive integer
::again/attempts …
:again.core/attempts …
;; the exception that was thrown when the execution failed (not present
;; in the `:success` case)
::again/exception …
;; the sum of all delays thus far
::again/slept …
:again.core/exception …
;; the sum of all delays thus far (in milliseconds)
:again.core/slept …
;; the result of the previous execution - `:success`, `:failure`, or
;; `:retry`
::again/status …
;; the `::again/user-context` value from the map passed to `with-retries`
::again/user-context …
:again.core/status …
;; the `:again.core/user-context` value from the map passed to `with-retries`
:again.core/user-context …
}
```

The callback can also return the `:again.core/fail` keyword to ignore the rest
of the retry strategy and throw the current exception from `with-retries` (That
is, it provides a mechanism for early termination). For example, the callback
could check the exception's `ex-data` and decide to fail the operation:

```clj
(again/with-retries
{::again/callback #(when (-> % ::again/exception ex-data :fail?) ::again/fail)
::again/strategy [100 1000 10000]}
(my-operation …))
```


### Generators:

* `additive-strategy` - an infinite sequence of incrementally increasing delays
Expand Down
48 changes: 23 additions & 25 deletions src/again/core.clj
View file
Expand Up @@ -95,9 +95,7 @@
"Turn a strategy-sequence to an options-map (if it's not a map already), and
define a default callback function."
[strategy-or-options]
(let [noop (fn [& _])
default-options {::callback noop
::exception-predicate (constantly true)}
(let [default-options {::callback (constantly nil)}
options (if (map? strategy-or-options)
strategy-or-options
{::strategy strategy-or-options})]
Expand All @@ -107,7 +105,6 @@
[strategy-or-options f]
(let [{callback ::callback
delays ::strategy
should-retry? ::exception-predicate
:as options}
(build-options strategy-or-options)

Expand All @@ -123,16 +120,15 @@
callback)
res)
(catch Exception e
(when-not (should-retry? e) (throw e))

(-> callback-state
(assoc
::exception e
::status (if delay :retry :failure))
callback)
(if delay
(sleep delay)
(throw e))
(let [retry? (-> callback-state
(assoc
::exception e
::status (if delay :retry :failure))
callback
(not= ::fail))]
(if (and delay retry?)
(sleep delay)
(throw e)))
nil))]
res
(recur delays (-> callback-state
Expand All @@ -148,24 +144,26 @@
number of elements in the `strategy` plus one. A simple retry stategy would
be: [100 100 100 100] which results in the operation being retried four times,
for a total of five attempts, with 100ms sleeps in between attempts. Note:
that infinite strategies are supported, but maybe not encouraged…
infinite strategies are supported, but maybe not encouraged…
Strategies can be built with the provided builder fns, eg `linear-strategy`,
and modified with the provided manipulator fns, eg `clamp-delay`, but you can
also create any custom seq of delays that suits your use case.
Instead of a simple delay sequence, you can also pass in the following type of
options map:
options map as the first argument to `with-retries`:
{:again.core/callback <fn>
:again.core/user-context <anything, but probably an atom>
:again.core/exception-predicate <fn>
:again.core/strategy <delay strategy>}
:again.core/strategy <delay strategy>
:again.core/user-context <anything, but probably an atom>}
`:again.core/callback` is a callback function that is called after each
attempt. `:again.core/user-context` is an opaque value that is passed to the
callback function as an argument. And `:again.core/strategy` is the sequence
of delays.
attempt.
`:again.core/strategy` is the sequence of delays (ie retry strategy).
`:again.core/user-context` is an opaque value that is passed to the callback
function as an argument.
The callback function is called with the following type of map as its only
argument:
Expand All @@ -176,8 +174,8 @@
:again.core/status <the result of the last attempt: :success, :failure, or :retry
:again.core/user-context <the user context from the options map>}
The exception-predicate is a function that is called with an exception. It
can return truthy or falsey. If truthy, the exception is retried. This
function defaults to `(constantly true)`"
The callback function can return `:again.core/fail` to instruct `with-retries`
to ignore the rest of the retry strategy and rethrow the previous
exception (ie return early)."
[strategy-or-options & body]
`(with-retries* ~strategy-or-options (fn [] ~@body)))
153 changes: 91 additions & 62 deletions test/again/core_test.clj
View file
Expand Up @@ -136,13 +136,33 @@

(defn new-callback-fn
"Returns a map consisting of the following fields:
- `callback` - a callback function to pass to `with-retries`
- `args` - an atom recording the arguments passed to `callback`"
[]
(let [args (atom [])
callback #(swap! args conj %)]
- `callback` - a callback function to pass to `with-retries` that will fail
the operation early after the `n`th call
- `args` - an atom recording the arguments passed to `callback`"
[& [n]]
(let [n (or n Integer/MAX_VALUE)
attempts (atom 0)
args (atom [])
callback #(let [i (swap! attempts inc)]
(swap! args conj %)
(when (< n i)
::a/fail))]
{:args args :callback callback}))

(defspec spec-with-retries
200
(prop/for-all
[strategy (gen/vector gen/s-pos-int)]
(let [{:keys [attempts f]} (new-failing-fn)
delays (atom [])]
(with-redefs [a/sleep #(swap! delays conj %)]
(try
(with-retries strategy (f))
(catch Exception _)))

(and (= @attempts (inc (count strategy)))
(= @delays strategy)))))

(deftest test-with-retries
(with-redefs [a/sleep (constantly nil)]
(testing "with-retries"
Expand Down Expand Up @@ -222,84 +242,93 @@
::a/exception exception
::a/slept 123
::a/status :failure})
"calls callback hook with permanent failure"))))))
"calls callback hook with permanent failure")))

(testing "with early failure"
(let [{:keys [exception f]} (new-failing-fn)
{:keys [args callback]} (new-callback-fn 1)]
(is (thrown?
Exception
(with-retries
{::a/callback callback
::a/strategy [1 2 3]}
(f)))
"throws exception")

(is (= (count @args) 2) "calls callback hook three twice")
(is (= (first @args)
{::a/attempts 1
::a/exception exception
::a/slept 0
::a/status :retry})
"first callback call")
(is (= (second @args)
{::a/attempts 2
::a/exception exception
::a/slept 1
::a/status :retry})
"last callback call"))))))

(defmulti log-attempt ::a/status)

(defmethod log-attempt :retry [s]
(swap! (::a/user-context s) assoc :retried? true)
(println "RETRY" s))
(if (< (count @(::a/user-context s)) 1)
(swap! (::a/user-context s) conj :retry)
(do
(swap! (::a/user-context s) conj :fail)
::a/fail)))

(defmethod log-attempt :success [s]
(if (-> s ::a/user-context deref :retried?)
(println "SUCCESS after" (::a/attempts s) "attempts" s)
(println "SUCCESS on first attempt" s)))
(defmethod log-attempt :failure [s] (println "FAILURE" s))
(swap! (::a/user-context s) conj :success))

(defmethod log-attempt :failure [s]
(swap! (::a/user-context s) conj :failure))

(defmethod log-attempt :default [s] (assert false))

(deftest test-multimethod-callback
(with-redefs [a/sleep (constantly nil)]
(testing "multi-method-callback"
(testing "with success"
(let [{:keys [f]} (new-failing-fn 2)
user-context (atom [])]
(with-retries
{::a/callback log-attempt
::a/strategy [1 2]
::a/user-context user-context}
(f))
(is (= (count @user-context) 2) "multimethod is called twice")
(is (= (first @user-context) :retry) "first call is a retry")
(is (= (second @user-context) :success) "second call is a success")))

(testing "with failure"
(let [{:keys [exception f]} (new-failing-fn)]
(let [{:keys [exception f]} (new-failing-fn)
user-context (atom [])]
(try
(with-retries
{::a/callback log-attempt
::a/strategy [1]
::a/user-context (atom {})}
::a/user-context user-context}
(f))
(catch Exception e
(when (not= e exception)
(println "Unexpected exception:" e)
(.printStackTrace e))))))
(is (= e exception) "Unexpected exception")))
(is (= (count @user-context) 2) "multimethod is called twice")
(is (= (first @user-context) :retry) "first call is a retry")
(is (= (second @user-context) :failure) "second call is a failure")))

(testing "with success"
(let [{:keys [exception f]} (new-failing-fn 2)]
(testing "with early failure"
(let [{:keys [exception f]} (new-failing-fn)
user-context (atom [])]
(try
(with-retries
{::a/callback log-attempt
::a/strategy [1]
::a/user-context (atom {})}
::a/strategy [1 2]
::a/user-context user-context}
(f))
(catch Exception e
(when (not= e exception)
(println "Unexpected exception:" e)
(.printStackTrace e)))))))))
(is (= e exception) "Unexpected exception")))
(is (= (count @user-context) 2) "multimethod is called three times")
(is (= (first @user-context) :retry) "first call is a retry")
(is (= (second @user-context) :fail) "second call is a fail"))))))

(deftest test-exception-filter
(with-redefs [a/sleep (constantly nil)]
(let [call-count (atom 0)
retry-exception? (comp :retry? ex-data)

test-conditional-retries
(fn [expected-call-count strategy retry?]
(try
(reset! call-count 0)
(with-retries {::a/strategy strategy
::a/exception-predicate retry-exception?}
(do
(swap! call-count inc)
(throw (ex-info "failure" {:retry? retry?}))))
(catch Exception e
(is (= "failure" (.getMessage e)))
(is (= expected-call-count @call-count)
(str "when retry is " retry?
" and strategy is " (pr-str strategy)
" should have tried " expected-call-count
" time(s)")))))]
(test-conditional-retries 1 [1 2 3] false)
(test-conditional-retries 1 [1 2 3 4] false)
(test-conditional-retries 4 [1 2 3] true)
(test-conditional-retries 5 [1 2 3 4] true))))

(defspec spec-with-retries
200
(prop/for-all
[strategy (gen/vector gen/s-pos-int)]
(let [{:keys [attempts f]} (new-failing-fn)
delays (atom [])]
(with-redefs [a/sleep #(swap! delays conj %)]
(try
(with-retries strategy (f))
(catch Exception _)))

(and (= @attempts (inc (count strategy)))
(= @delays strategy)))))

0 comments on commit cbde3bf

Please sign in to comment.