Pathoschild / FluentHttpClient

A modern async HTTP client for REST APIs. Its fluent interface lets you send an HTTP request and parse the response in one go.
MIT License
345 stars 52 forks source link

Multiple retry configs #99

Closed Jericho closed 3 years ago

Jericho commented 3 years ago

I would like to propose an enhancement to the RetryCoordinator that would allow multiple retry configurations. The current coordinator only allows a single configuration which is perfect for relatively simple situations where you want to automatically retry when a given HTTP code is returned in the response, such as HTTP 429 for example.

If you have a more complex situation where there are multiple reasons to retry a given request your only option currently is to pepper the code in your retry config with multiple if .. else if ... else which works but it can get quite messy really quick, particularly if each scenario has different number of retries, different wait time between retries, etc.

I propose allowing the retry coordinator to accept multiple IRetryConfig which would allow developers to create one implementation of IRetryConfig for each possible retry scenario. This would allow each scenario can have their own max number of retries, wait time between retries, etc. Each scenario would be cleanly separated in their own class that implements IRetryConfig

So, for example, let's say I have three distinct scenarios where I want to retry a given request:

  1. the http status code of the response is HTTP 429 which means that I have sent too many requests in a given amount of time
  2. The response contains an exception caused by the fact that the bearer token I provided with my request has expired (in this scenario I will want to renew my token before retrying)
  3. The response contains an exception caused by a timeout when querying a database

My proposal would allow writing three distinct retry configuration classes like so:

internal class Http429RetryConfig : IRetryConfig { ... code omitted for brevity ... }
internal class TokenExpiredRetryConfig: IRetryConfig { ... code omitted for brevity ... }
internal class DatabaseTimeoutRetryConfig: IRetryConfig { ... code omitted for brevity ... }

and I would be able to configure my fluent client like so:

var retryConfigs = new[]
{
    new Http429RetryConfig(),
    new TokenExpiredRetryConfig(),
    new DatabaseTimeoutRetryConfig()
};
fluentClient.SetRequestCoordinator(retryConfigs);

at runtime, the RetryCoordinator would loop through the retry configurations looking for the first one (if any) that indicates the current request should be retried.

I have already implemented this enhancement in my fork and I have been successfully using it for several months. I will submit a PR so you can review and decide if this is something you want to incorporate in FluentHttpClient.

Pathoschild commented 3 years ago

Thanks, this looks useful!

Note that the retry count is shared with this approach. So let's say we have two retry configs: DatabaseTimeoutRetryConfig (retry after 1 second) and RetryWithBackoffConfig (retry after 5, 30, and 60 seconds). If we get a database timeout followed by a different error, the second retry will be handled by RetryWithBackoffConfig which will skip straight to 30 seconds. That also affects the maximum number of retries; e.g. the first config retries a few times, then the second one throws a max-retries-exceeded error without ever retrying itself.

That's a bit of an edge case though, and I'm fine with that as long as it's documented, since custom coordinators provides a way to handle it more granularly if needed.

Jericho commented 3 years ago

I agree it's an edge case but thanks for pointing it out. The logic I designed does not account for the possibility for a request to fail for a given reason, and to fail again when retried but for a different reason.

Pathoschild commented 3 years ago

Thanks! Merged into develop for the upcoming FluentHttpClient 4.1.