lestrrat-go / httprc

Quasi Up-to-date HTTP In-memory Cache
MIT License
17 stars 5 forks source link

Add support for automatic retries in case of failure #12

Open ItalyPaleAle opened 8 months ago

ItalyPaleAle commented 8 months ago

I'm happy to provide a PR for this

Would it be possible to add an option to httprc to automatically retry a request that fails? For example, due to transient network failures, server responding with an error status code, etc.

Note that we use httprc via github.com/lestrrat-go/jwx/v2/jwk

lestrrat commented 8 months ago

I think the API change that I'm currently working on would be able to provide such capabilities, because it now allows you to pass a http.Client (well, something that supports Do(*http.Request) (*http.Response, error), though for github.com/lestrrat-go/jwx I don't think this can be introduced without API breakage, which means you will only be able to get this if you use v3, which is the next version.

Does that work for you?

ItalyPaleAle commented 8 months ago

That could potentially work, although it would require consumers to create their own logic to handle retries.

What I was thinking was adding an option to configure how the operation could be retried, letting jwx/httprc handle the retries (checking for network failures, then checking for status codes that are retriable).

In one possible design, assuming you'd be ok with accepting a new dependency, it could be adding an option to accept a backoff.Backoff from github.com/cenkalti/backoff/v4. We use this library extensively and it allows passing a configuration for retries with backoffs, including linear and exponential ones.

In another design, which wouldn't require an external dependency,it could be as simple as passing a "MaxRetries" option, letting httprc figure out how to do the back-off.

lestrrat commented 8 months ago

TBH I'm a bit meh about baking in retries, because retries usually tend to be fairly elaborate and its usage varies subtly depending on your requirements and environment. This in turn adds a bunch of possible failure modes and configuration possibilities. I've been burned before (implementation is not really the problem, I'm more concerned with the long term support issue), and have removed them when developing jwx v2

(sidenote: I also have a slight beef with passing closures to Retry() :)

ItalyPaleAle commented 8 months ago

Understood (also I didn't know you had your own package) :)