Add request retry functionality#23
Conversation
| (catch Exception e | ||
| (throw (unwrap-async-exception e))))))) | ||
|
|
||
| (def default-wrapped-request (middleware/wrap-request request*)) |
There was a problem hiding this comment.
This makes it so every request doesn't have to rebuild the entire middleware chain
| (reify Function | ||
| (apply [_ e] | ||
| (raise (unwrap-async-exception e)))))) | ||
| (try @resp-fut |
There was a problem hiding this comment.
Since .send is still async (it just does a .get internally), I don't think we're losing anything by handling it this way other than we must unwrap ExecutionException
| (-> (.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) |
There was a problem hiding this comment.
I'm happy to bring back the ->BodyHandler function, but confused me at first passing the as field and doing nothing with it.
|
|
||
| (-> (.sendAsync http-client http-request bh) | ||
| http-request (ring-request->HttpRequest req) | ||
| retry-handler (if (= :auto retry-handler) default-retry-handler retry-handler) |
There was a problem hiding this comment.
Should we call it
:retry-handler or just :retry?
Should we call it :auto or :default?
Personally, I'd like to make it the retries truly default like Apache HttpClient does, however, I realize that is a decent change to default behavior so opt-in makes sense.
| (reify Function | ||
| (apply [_ [resp e]] | ||
| (let [retry-fut (if retry-handler | ||
| (retry-handler resp e req retry-count) |
There was a problem hiding this comment.
Do you think we should put some of this in a map instead so it is more easily extensible in the future? Something like a retry-context map?
| ex) | ||
| ex)) | ||
|
|
||
| (def retry-exceptions |
There was a problem hiding this comment.
Do you think these should be in client.clj or should we move to a separate namespace?
| {: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" |
There was a problem hiding this comment.
Just making a quick doc change here. Doesn't make sense to do a multipart get request :).
1 participant
This is an initial crack to resolve #21. It's easily extendable and provides a reasonable retry strategy when
:autois supplied.