From 10168603951aa5f34f489e30f84b133f3045445c Mon Sep 17 00:00:00 2001 From: Vincent Pizzo Date: Fri, 18 Dec 2020 14:12:47 -0800 Subject: [PATCH] Add request retry functionality --- README.md | 27 +++++-- src/hato/client.clj | 163 ++++++++++++++++++++++++++------------ test/hato/client_test.clj | 104 +++++++++++++++++++++++- 3 files changed, 235 insertions(+), 59 deletions(-) diff --git a/README.md b/README.md index c4fff24..bc6cf47 100644 --- a/README.md +++ b/README.md @@ -210,6 +210,19 @@ request and returns a response. Convenience wrappers are provided for the http v - `:http-1.1` prefer HTTP/1.1 - `:http-2` (default) tries to upgrade to HTTP/2, falling back to HTTP/1.1 +`retry-handler` Sets an optional retry handler which can be used for retrying requests and implementing more complex + retry strategies such as exponential backoff, retrying on 503s, etc. No retries will be attempted by default. `:auto` + may be supplied which will retry idempotent requests up to 3 times on `IOException` while skipping some other + exceptions such as `InterruptedIOException`, `UnknownHostException`, or `SSLException`. + + A retry handler may also be a simple function that receives the following arguments: + + response => The response (nil when ex is present) + ex => The exception (nil when response is present) + req => The original request + retry-count => The current retry iteration (starts at 0) + + The function should return a boolean or a `CompletableFuture` that resolves to a boolean (useful for backoffs/sleep). ## Usage examples @@ -410,13 +423,13 @@ To send a multipart request, `:multipart` may be supplied as a sequence of maps the `:body` of the request with an `InputStream` of the supplied parts. ```clojure -(hc/get "http://moo.com" - {:multipart [{:name "title" :content "My Awesome Picture"} - {:name "Content/type" :content "image/jpeg"} - {:name "foo.txt" :part-name "eggplant" :content "Eggplants"} - {:name "file" :content (io/file ".test-data")} - {:name "data" :content (.getBytes "hi" "UTF-8") :content-type "text/plain" :file-name "data.txt"} - {:name "jsonParam" :content (io/file ".test-data") :content-type "application/json" :file-name "data.json"}]}) +(hc/post "http://moo.com" + {:multipart [{:name "title" :content "My Awesome Picture"} + {:name "Content/type" :content "image/jpeg"} + {:name "foo.txt" :part-name "eggplant" :content "Eggplants"} + {:name "file" :content (io/file ".test-data")} + {:name "data" :content (.getBytes "hi" "UTF-8") :content-type "text/plain" :file-name "data.txt"} + {:name "jsonParam" :content (io/file ".test-data") :content-type "application/json" :file-name "data.json"}]}) ``` diff --git a/src/hato/client.clj b/src/hato/client.clj index 1f39a45..371e71a 100644 --- a/src/hato/client.clj +++ b/src/hato/client.clj @@ -6,19 +6,19 @@ [hato.middleware :as middleware] [clojure.java.io :as io]) (:import - (java.net.http - HttpClient$Redirect - HttpClient$Version - HttpResponse$BodyHandlers - HttpRequest$BodyPublisher - HttpRequest$BodyPublishers HttpResponse HttpClient HttpRequest HttpClient$Builder HttpRequest$Builder) - (java.net CookiePolicy CookieManager URI ProxySelector Authenticator PasswordAuthentication CookieHandler) - (javax.net.ssl KeyManagerFactory TrustManagerFactory SSLContext) - (java.security KeyStore) - (java.time Duration) - (java.util.function Function Supplier) - (java.io File InputStream) - (clojure.lang ExceptionInfo))) + (java.net.http + HttpClient$Redirect + HttpClient$Version + HttpResponse$BodyHandlers + HttpRequest$BodyPublisher + HttpRequest$BodyPublishers HttpResponse HttpClient HttpRequest HttpClient$Builder HttpRequest$Builder) + (java.net CookiePolicy CookieManager URI ProxySelector Authenticator PasswordAuthentication CookieHandler UnknownHostException) + (javax.net.ssl KeyManagerFactory TrustManagerFactory SSLContext SSLException) + (java.security KeyStore) + (java.time Duration) + (java.util.function Function Supplier BiFunction) + (java.io File InputStream InterruptedIOException IOException) + (java.util.concurrent CompletableFuture CompletionException ExecutionException))) (defn- ->Authenticator [v] @@ -30,15 +30,6 @@ (getPasswordAuthentication [] (PasswordAuthentication. user (char-array pass)))))))) -(defn- ->BodyHandler - "Returns a BodyHandler. - - Always returns InputStream that are coerced in middleware. - - https://docs.oracle.com/en/java/javase/11/docs/api/java.net.http/java/net/http/HttpResponse.BodyHandler.html" - [_] - (HttpResponse$BodyHandlers/ofInputStream)) - (defn- ->BodyPublisher "Returns a BodyPublisher. @@ -284,42 +275,114 @@ (->BodyPublisher req)) .build))) +(defn unwrap-async-exception + "Helper function to unwrap common async exceptions to get the root cause such as + an IOException." + [^Exception ex] + (if (or (instance? CompletionException ex) + (instance? ExecutionException ex)) + (if-let [cause (.getCause ex)] + (unwrap-async-exception cause) + ex) + ex)) + +(def retry-exceptions + "Common exceptions to retry requests on." + #{IOException}) + +(def non-retry-exceptions + "Common exceptions to not retry requests on. Borrowed from Apache HttpClient's + `DefaultHttpRequestRetryHandler`." + #{InterruptedIOException UnknownHostException SSLException}) + +(defn idempotent-request? + "Checks to see if the request is idempotent. It is considered idempotent if the + request method is one of `:get`, `:head`, or `:options`." + [req] + (contains? #{:get :head :options} (:request-method req))) + +(defn make-retry-handler + "Constructs a basic retry handler. Options are: + + :retry-exceptions => A collection of exceptions required for us to retry the request on. + Defaults to `retry-exceptions`. + :non-retry-exceptions => A collection of exceptions to cause us to skip retrying a specific + request. Defaults to `non-retry-exceptions`. + :max-retry-count => The maximum number of retries before giving up. Defaults to 3." + [{:keys [retry-exceptions non-retry-exceptions max-retry-count] + :or {retry-exceptions retry-exceptions + non-retry-exceptions non-retry-exceptions + max-retry-count 3}}] + (fn [_resp ex req retry-count] + (and (idempotent-request? req) + (some #(instance? % ex) retry-exceptions) + (not (some #(instance? % ex) non-retry-exceptions)) + (< retry-count max-retry-count)))) + +(def default-retry-handler + "A default retry handler which will retry if there is an error with any known retriaretryble + exceptions while skipping non-retry exceptions. By default, this will only retry idempotent + requests (i.e. GET/HEAD/OPTIONS)." + (make-retry-handler {})) + +(defn request-with-retries* + "Makes a request retrying on failures as defined by retry-handler" + [http-client http-request body-handler req retry-handler retry-count] + (-> (.sendAsync http-client http-request body-handler) + (.handle + (reify BiFunction + (apply [_ resp e] + [resp (unwrap-async-exception e)]))) + (.thenCompose + (reify Function + (apply [_ [resp e]] + (let [retry-fut (if retry-handler + (retry-handler resp e req retry-count) + (CompletableFuture/completedFuture false))] + (-> (if (instance? CompletableFuture retry-fut) + ^CompletableFuture retry-fut + (CompletableFuture/completedFuture retry-fut)) + (.thenCompose + (reify Function + (apply [_ retry?] + (if retry? + (request-with-retries* http-client http-request body-handler req retry-handler (inc retry-count)) + (if e + (CompletableFuture/failedFuture e) + (CompletableFuture/completedFuture + (response-map {:request req + :http-client http-client + :response resp})))))))))))))) + (defn request* - [{:keys [http-client async? as] + [{:keys [http-client async? retry-handler] :as req} & [respond raise]] (let [^HttpClient http-client (if (instance? HttpClient http-client) http-client (build-http-client http-client)) - http-request (ring-request->HttpRequest req) - bh (->BodyHandler as)] - (if-not async? - (let [resp (.send http-client http-request bh)] - (response-map - {:request req - :http-client http-client - :response resp})) - - (-> (.sendAsync http-client http-request bh) + http-request (ring-request->HttpRequest req) + retry-handler (if (= :auto retry-handler) default-retry-handler retry-handler) + body-handler (HttpResponse$BodyHandlers/ofInputStream) + resp-fut (request-with-retries* http-client http-request body-handler req retry-handler 0)] + (if async? + (-> resp-fut (.thenApply - (reify Function - (apply [_ resp] - (respond - (response-map - {:request req - :http-client http-client - :response resp}))))) + (reify Function + (apply [_ resp-map] + (respond resp-map)))) (.exceptionally - (reify Function - (apply [_ e] - (let [cause (.getCause ^Exception e)] - (if (instance? ExceptionInfo cause) - (raise cause) - (raise e)))))))))) + (reify Function + (apply [_ e] + (raise (unwrap-async-exception e)))))) + (try @resp-fut + (catch Exception e + (throw (unwrap-async-exception e))))))) + +(def default-wrapped-request (middleware/wrap-request request*)) (defn request [req & [respond raise]] - (let [wrapped (middleware/wrap-request request*)] - (if (:async? req) - (wrapped req (or respond identity) (or raise #(throw %))) - (wrapped req)))) + (if (:async? req) + (default-wrapped-request req (or respond identity) (or raise #(throw %))) + (default-wrapped-request req))) (defn- configure-and-execute "Convenience wrapper" diff --git a/test/hato/client_test.clj b/test/hato/client_test.clj index 7abe2a4..68c7ada 100644 --- a/test/hato/client_test.clj +++ b/test/hato/client_test.clj @@ -7,10 +7,11 @@ [cheshire.core :as json]) (:import (java.io InputStream) (java.net ProxySelector CookieHandler CookieManager) - (java.net.http HttpClient$Redirect HttpClient$Version HttpClient) + (java.net.http HttpClient$Redirect HttpClient$Version HttpClient HttpTimeoutException) (java.time Duration) (javax.net.ssl SSLContext) - (java.util UUID))) + (java.util UUID) + (java.util.concurrent CompletableFuture))) (deftest test-build-http-client (testing "authenticator" @@ -289,5 +290,104 @@ (let [r (get "https://nghttp2.org/httpbin/get" {:as :json})] (is (= :http-2 (:version r)))))) +(defn slow-response + [call-count-atom sleep-time-ms] + (swap! call-count-atom inc) + (Thread/sleep sleep-time-ms) + {:status 200 + :headers {"Content-Type" "application/json"} + :body (json/generate-string {:sleep sleep-time-ms})}) + +(defmacro exception-cause + "Evaluates the form, and returns the unwrapped exception." + [form] + `(try ~form + (catch Exception e# + (unwrap-async-exception e#)))) + +(deftest ^:integration sync-retry-handlers + (testing "Sync non-idempotent requests are not retried" + (let [call-count (atom 0)] + (with-server (fn [_] (slow-response call-count 200)) + (is (thrown? HttpTimeoutException (post "http://localhost:1234" {:timeout 100 :retry-handler :auto}))) + (is (= 1 @call-count))))) + + (testing "Sync requests will make 4 requests total then fail" + (let [call-count (atom 0)] + (with-server (fn [_] (slow-response call-count 200)) + (is (thrown? HttpTimeoutException (get "http://localhost:1234" {:timeout 100 :retry-handler :auto}))) + (is (= 4 @call-count))))) + + (testing "Sync requests will make 2 requests total when second request is a success" + (let [call-count (atom 0)] + (with-server (fn [_] (if (zero? @call-count) + (slow-response call-count 200) + (slow-response call-count 1))) + (is (= 200 (:status (get "http://localhost:1234" {:timeout 100 :retry-handler :auto})))) + (is (= 2 @call-count))))) + + (testing "Sync requests honor custom predicate-like retry handlers" + (let [call-count (atom 0)] + (with-server (fn [_] (slow-response call-count 200)) + (is (thrown? HttpTimeoutException (get "http://localhost:1234" {:timeout 100 + :retry-handler (fn [_resp _ex _req retry-count] + (< retry-count 9))}))) + (is (= 10 @call-count))))) + + (testing "Sync requests honor custom retry handlers that return a CompletableFuture" + (let [call-count (atom 0)] + (with-server (fn [_] (slow-response call-count 200)) + (is (thrown? HttpTimeoutException (get "http://localhost:1234" + {:timeout 100 + :retry-handler (fn [_resp _ex _req retry-count] + (CompletableFuture/completedFuture (< retry-count 1)))}))) + (is (= 2 @call-count)))))) + +(deftest ^:integration async-retry-handlers + (testing "Async non-idempotent requests are not retried" + (let [call-count (atom 0)] + (with-server (fn [_] (slow-response call-count 200)) + (is (instance? HttpTimeoutException (exception-cause @(post "http://localhost:1234" {:async? true + :timeout 100 + :retry-handler :auto})))) + (is (= 1 @call-count))))) + + (testing "Async requests will make 4 requests total then fail" + (let [call-count (atom 0)] + (with-server (fn [_] (slow-response call-count 200)) + (is (instance? HttpTimeoutException (exception-cause @(get "http://localhost:1234" {:async? true + :timeout 100 + :retry-handler :auto})))) + (is (= 4 @call-count))))) + + (testing "Async requests will make 2 requests total when second request is a success" + (let [call-count (atom 0)] + (with-server (fn [_] (if (zero? @call-count) + (slow-response call-count 200) + (slow-response call-count 1))) + (is (= 200 (:status @(get "http://localhost:1234" {:async? true + :timeout 100 + :retry-handler :auto})))) + (is (= 2 @call-count))))) + + (testing "Async requests honor custom predicate-like retry handlers" + (let [call-count (atom 0)] + (with-server (fn [_] (slow-response call-count 200)) + (is (instance? HttpTimeoutException (exception-cause @(get "http://localhost:1234" + {:timeout 100 + :retry-handler (fn [_resp _ex _req retry-count] + (< retry-count 9))})))) + (is (= 10 @call-count))))) + + (testing "Async requests honor custom retry handlers that return a CompletableFuture" + (let [call-count (atom 0)] + (with-server (fn [_] (slow-response call-count 200)) + (is (instance? HttpTimeoutException (exception-cause + @(get "http://localhost:1234" + {:timeout 100 + :retry-handler (fn [_resp _ex _req retry-count] + (CompletableFuture/completedFuture (< retry-count 1)))})))) + (is (= 2 @call-count)))))) + (comment (run-tests)) \ No newline at end of file