search

mrkkrp / req

An HTTP client library
Other
337 stars 40 forks source link

Monadic Retry Policies #55

Closed jkachmar closed 4 years ago

jkachmar commented 5 years ago

I'm finding myself in a situation where Monadic retries (of the RetryPolicyM sort) would be potentially useful, but I don't have access to them within Req's httpConfigRetryPolicy or httpConfigRetryJudge.

My specific use case would be to either perform some kind of logging action and/or update a counter (e.g. TVar shared with the rest of my program) on each failed request.

Do you feel like the design of req is amenable to this sort of thing? The only way I can think of this would be to do what RetryPolicyM does and parameterize it around some m that represents the context you're operating in...

mrkkrp commented 5 years ago

I'm not sure how common this need is, yet it'd cause a lot of breaking changes. If we need to allow access to MonadHttp instance then HttpConfig should be parametrized over some m to contain RetryPolicyM m inside...

jkachmar commented 5 years ago

Yeah, I'm not entirely convinced that it's worth it versus simply "disabling" the built-in retry policy (httpConfigRetryPolicy = limitRetries 0 or something) and then using the retry package directly with my own RetryPolicyM (parameterized with my application's type) and retrying.

It wouldn't be much extra work, just that I'd have to replicate some of the req functionality on my own.

ramirez7 commented 4 years ago

How about changing from

httpConfigRetryPolicy :: RetryPolicy

to

httpConfigRetryPolicy :: RetryPolicyM IO

since Req is IO anyways (and MonadHttp requires MonadIO). This retry policy can then be lifted internally to Req via natTransformRetryPolicy liftIO.

This wouldn't require any breaking changes, but it would fix the largest drawback (imo) of the current approach: fullJitterBackoff cannot be used due to its requirement of IO.

mrkkrp commented 4 years ago

@ramirez7 Do you want to prepare a PR?

ramirez7 commented 4 years ago

:+1: yep I'm on it