wojtekmach/req

Would it make sense to make retry `transient?` function public?

Opened this issue · 4 comments

I'm thinking that it would be nice to have the ability to define your own retry function, and in the retry function if the conditions that I care about don't match, default to

defp transient?(%Req.Response{status: status}) when status in [408, 429, 500, 502, 503, 504] do

I'll be happy to make a PR if this sounds reasonable.

This is very arbitrary but I'd like to keep Req.Steps to just have steps functions as it's already pretty big module. The downside is we can't have this function which I agree would be very convenient to have. Perhaps there is another way to solve this?

I had a look at Req.Steps to see if there's a more general pattern that we could take advantage of, but sadly it seems that only the transient? function would benefit from being exposed. Creating a whole new module e.g. Req.Conveniences would not look so great if there's only one function there of interest.

The retry function implementation can return a tuple {:delay, milliseconds}, maybe we could support new options {:retry, :transient} and {:retry, :safe_transient}?

    Req.new(
      ...,
      retry: fn
        _req, %Req.Response{status: 403} ->
          # Hypothetical service that returns HTTP 403 instead of 429 for "Too Many Requests" 
          {:delay, 30_000}

        _req, _resp ->
          {:retry, :safe_transient}
      end
    )

@wojtekmach how does the proposal sound? Is this something I could start working on that the Req library would be happy to accept?

Sorry I'm still considering it.