Auto retry failing requests

[Bertg]

We are a happy user of HTTP.rb, but the services we use are less "happy". Intermittently they will just thrown an HTTP 500 error, or time out on us.

It would be great if this library could provide an automatic retry mechanism (similar to Faraday's retry implementation).

We are willing to port the behaviour of retry behaviour of Faraday into this library if there is interest to have it merged. If not we would publish it as a gem that adds the functionality to HTTP.rb.

[ixti]

I have this functionality implemented as a client wrapper (ugly enough for me to be shy to open-source it)... But I think it's something nice to have.

Definitely would be happy to do a code review. And I can actually provide my implementation of this for the reference might be you'll find it good enough for the basis of your work.

@httprb/core WDYT?

[paul]

@ixti Seems like this might be best implemented as a "feature", to keep the main client code cleaner...

[Bertg]

@paul Excuse my ignorance here, but what exactly do you mean by "Feature" in this context?
@ixti If you'd like to share your code, I'd be happy to have a look at it

[ixti]

@paul I was thinking about that, but couldn't find a way to make it sane with Features unfortunately.

@Bertg at some point we introduced a thing called Feature which allow us to enhance behaviour of request/response. So feature is a class that accepts some initialization options and expose 2 public methods: wrap_response and wrap_request. See implementation of deflate and inflate for example:

• https://github.com/httprb/http/blob/master/lib/http/features/auto_deflate.rb
• https://github.com/httprb/http/blob/master/lib/http/features/auto_inflate.rb

PS: @paul @Bertg I'm going to prepare the code during this week and I believe it will be a good starter. I'm not married to the implementation I made, so if there will be better ways of doing it (I'm pretty sure there will be lots of things to improve as I built it long time ago) I'm totally fin with having better implementation.

[Bertg]

I've been looking into introducing this feature using the Feature.
I don't think that is the right way to go.

Based on what i can read in lib/http/client.rb:73-88 Feature is used to "modify" the request outgoing or the response incoming. And to "do something" on error. We don't want to modify the request or the response. And the on_error method does not allow us to change the request handling.

I think the correct place to handle this would be in lib/http/client.rb itself. I want to be able to handle Timeouts, HTTP status codes and other Exceptions. And it seems that this is the best place. On line lib/http/client.rb:95 where the connection gets closed when an exception happened, we could choose to add the retry logic instead.

Any feedback on this analysis?

[ixti]

Was overwhelmed at work, here's the code I use for this purpose:

3c148d3

Although as I was looking at that code, I think the best API should be non-obtrustive, some sort of wrapper for HTTP client:

http = HTTP::Retriable.new(HTTP)
http = HTTP::Retriable.new(HTTP.headers("Accept" => "application/json"))

I mean I think we should not "extend" HTTP client with #retriable methods or such, but wrap client.

[Bertg]

@ixti That is amazing work!

Would you mind if we extend it a bit more?

• Allow to define error codes to recover from
• Allow Exceptions to recover from
• Add an "on_retry" callback, analogous with on_error
• Add a "should_retry(request, response, exception)" method, that - if set - makes the decision to retry or not

[ixti]

@Bertg Sure. Feel free to do so!

[Bertg]

I've started the work here: master...younited:retry

If you have any comments so far, would be great!

[Bertg]

Closing this in favour of the PR #598