1

I am trying to get Polly to try again on timeout after 3 seconds and also when certain http codes are returned. However, it doesn't time out until after 100 seconds when the HttpClient times out.

Here is my code:

private static Polly.Wrap.AsyncPolicyWrap<HttpResponseMessage> GetPolicy()
{
    var timeoutPolicy = Policy.TimeoutAsync(3, Polly.Timeout.TimeoutStrategy.Optimistic);

    var retryPolicy = Policy
        .Handle<HttpRequestException>()
        .OrResult<HttpResponseMessage>(r =>
            r.StatusCode == HttpStatusCode.TooManyRequests ||
            r.StatusCode == HttpStatusCode.ServiceUnavailable ||
            r.StatusCode == HttpStatusCode.Forbidden)
        .WaitAndRetryAsync(3, i => TimeSpan.FromSeconds(3));

    var policy = retryPolicy.WrapAsync(timeoutPolicy);
    return policy;
}

Update

As requested, here is code where I am using the policy.

var pollyResponse = await GetPolicy().ExecuteAndCaptureAsync(() =>
      httpClient.SendAsync(GetMessage(HttpMethod.Delete, endpoint))
);

And the helper method that makes the HttpRequestMessage:

private HttpRequestMessage GetMessage<T>(HttpMethod method, string endpoint, T content)
{
    var message = new HttpRequestMessage
    {
        Method = method,
        RequestUri = new Uri(endpoint),
        Headers = {
                    { "MyCustomHeader", _value },
                    { HttpRequestHeader.Accept.ToString(), "application/json" }
                }
    };

    if (content != null)
    {
        var contentAsString = JsonSerializer.Serialize(content);
        message.Content = new StringContent(contentAsString);
    }

    return message;
}
Peter Csala
  • 17,736
  • 16
  • 35
  • 75
Niels Brinch
  • 3,033
  • 9
  • 48
  • 75
  • 1
    Suspect it's the ordering of policies but also the policies don't seem like they'll work well together as if you wanted to globally timeout after 3s but you're also waiting for 3s before a retry then your retries would never happen? – Matt Jul 28 '21 at 12:20
  • I intend to timeout after 3 seconds and then try again immediately. That number 3 you see there should be amount of times to retry afaik – Niels Brinch Jul 28 '21 at 20:56
  • 1
    @NielsBrinch Could you please share with us the usage part as well? CancellationToken handling is crucial in this scenario. – Peter Csala Jul 29 '21 at 07:50
  • 1
    @NielsBrinch By the way your retry policy will not trigger in case of Timeout. You have to add the following line as well: `Or()`. Timeout policy [throws its own exception...](https://github.com/App-vNext/Polly/wiki/Timeout#syntax) – Peter Csala Jul 29 '21 at 07:52

1 Answers1

5

First, let me share with you the revised version of your GetPolicy:

private static IAsyncPolicy<HttpResponseMessage> GetStrategy()
{
    var timeoutPolicy = Policy
        .TimeoutAsync<HttpResponseMessage>(3, TimeoutStrategy.Optimistic,
        onTimeoutAsync: (_, __, ___, ____) =>
        {
            Console.WriteLine("Timeout has occurred");
            return Task.CompletedTask;
        });

    var retryPolicy = Policy
        .Handle<HttpRequestException>()
        .Or<TimeoutRejectedException>()
        .OrResult<HttpResponseMessage>(r =>
            r.StatusCode == (HttpStatusCode)429 ||
            r.StatusCode == HttpStatusCode.ServiceUnavailable ||
            r.StatusCode == HttpStatusCode.Forbidden)
        .WaitAndRetryAsync(3, i => TimeSpan.FromSeconds(3),
        onRetryAsync: (_, __, ___) =>
        {
            Console.WriteLine("Retry will fire soon");
            return Task.CompletedTask;
        });

    return Policy.WrapAsync(retryPolicy, timeoutPolicy);
}
  • I've changed the return type because from the consumer perspective the PolicyWrap is just an implementation detail
    • You could also use the AsyncPolicy<T> abstract class as return type if you don't want to use an interface (IAsyncPolicy<T>)
  • I've added some debug logging (onTimeoutAsync, onRetryAsync) to be able to watch which policy triggers when
  • I've added an Or<TimeoutRejectedException>() builder function call on the retryPolicy to make sure that retry will be triggered in case of timeout
  • I've also changed your retryPolicy.WrapAsync to a PolicyWrap because with that the escalation chain is more explicit
    • The left most policy is the most outer
    • The right most policy is the most inner
  • I've also changed the timeoutPolicy (.TimeoutAsync < HttpResponseMessage > ) to align with the retry policy (both of them are wrapping a delegate which might return a Task<HttpResponseMessage>)

In order to be able to test our resilience strategy (note the naming) I've created the following helper method:

private static HttpClient client = new HttpClient();
public static async Task<HttpResponseMessage> CallOverloadedAPI(int responseDelay = 5000, int responseCode = 200)
{
    return await client.GetAsync($"http://httpstat.us/{responseCode}?sleep={responseDelay}");
}
  • It will issue a request against a website which will return with a specified status code after a predefined amount of time
    • If you haven't used this website before please visit: 1, 2

Now, let's call the website:

public static async Task Main()
{
    HttpResponseMessage response;
    try
    {
        response = await GetStrategy().ExecuteAsync(async () => await CallOverloadedAPI());
    }
    catch (Exception ex)
    {
        Console.WriteLine(ex.Message);
        Environment.Exit(-1);
    }
    Console.WriteLine("Finished");
}

The output:

Finished

Wait, what??? The thing is none of the policies have been triggered.

Why? Because after 5 seconds we have received a response with 200.

But, we have set up a timeout, right? Yes and no. :) Even though we have defined a timeout policy we haven't really connected that to the HttpClient

So, how can I connect? Well, via CancellationToken

So, in case of timeout policy if a CancellationToken is in use then it can call its Cancel method to indicate the timeout fact to the HttpClient. And HttpClient will cancel the pending request.

Please note that, because we are using TimeoutPolicy the exception will be TimeoutRejectedException, not an OperationCanceledException.


So, let's modify our code to accept a CancellationToken

public static async Task<HttpResponseMessage> CallOverloadedAPI(int responseDelay = 5000, int responseCode = 200, CancellationToken token = default)
{
    return await client.GetAsync($"http://httpstat.us/{responseCode}?sleep={responseDelay}", token);
}

We have to adjust the usage side as well:

public static async Task Main()
{
    HttpResponseMessage response;
    try
    {
        response = await GetStrategy().ExecuteAsync(async (ct) => await CallOverloadedAPI(token: ct), CancellationToken.None);
    }
    catch (Exception ex)
    {
        Console.WriteLine(ex.Message);
        Environment.Exit(-1);
    }
    Console.WriteLine("Finished");
}

Now the output now will look like this:

Timeout has occurred
Retry will fire soon
Timeout has occurred
Retry will fire soon
Timeout has occurred
Retry will fire soon
Timeout has occurred
The delegate executed asynchronously through TimeoutPolicy did not complete within the timeout.

The last line is the Message of the TimeoutRejectedException.


Please note that if we remove the Or<TimeoutRejectedException>() call from the retryPolicy builder then the output will be the following:

Timeout has occurred
The delegate executed asynchronously through TimeoutPolicy did not complete within the timeout.

So, now retry will be triggered. There will be no escalation.


For the sake of completeness, here is the whole source code:

public static async Task Main()
{
    HttpResponseMessage response;
    try
    {
        response = await GetStrategy().ExecuteAsync(async (ct) => await CallOverloadedAPI(token: ct), CancellationToken.None);
    }
    catch (Exception ex)
    {
        Console.WriteLine(ex.Message);
        Environment.Exit(-1);
    }
    Console.WriteLine("Finished");
}

private static AsyncPolicy<HttpResponseMessage> GetStrategy()
{
    var timeoutPolicy = Policy
        .TimeoutAsync<HttpResponseMessage>(3, TimeoutStrategy.Optimistic,
        onTimeoutAsync: (_, __, ___, ____) =>
        {
            Console.WriteLine("Timeout has occurred");
            return Task.CompletedTask;
        });

    var retryPolicy = Policy
        .Handle<HttpRequestException>()
        .Or<TimeoutRejectedException>()
        .OrResult<HttpResponseMessage>(r =>
            r.StatusCode == (HttpStatusCode)429 ||
            r.StatusCode == HttpStatusCode.ServiceUnavailable ||
            r.StatusCode == HttpStatusCode.Forbidden)
        .WaitAndRetryAsync(3, i => TimeSpan.FromSeconds(3),
        onRetryAsync: (_, __, ___) =>
        {
            Console.WriteLine("Retry will fire soon");
            return Task.CompletedTask;
        });

    return Policy.WrapAsync(retryPolicy, timeoutPolicy);
}

private static HttpClient client = new HttpClient();
public static async Task<HttpResponseMessage> CallOverloadedAPI(int responseDelay = 5000, int responseCode = 200, CancellationToken token = default)
{
    return await client.GetAsync($"http://httpstat.us/{responseCode}?sleep={responseDelay}", token);
}
Peter Csala
  • 17,736
  • 16
  • 35
  • 75
  • 1
    I have tried to implement your code. However, in your example, it looks like ct here `ExecuteAsync(async (ct) =>` is a `CancellationToken` but for me its a `PollyContext`. Am I missing something? – Niels Brinch Jul 30 '21 at 09:01
  • @NielsBrinch Hmm, that's strange. Let me check. Which version of Polly are you using? – Peter Csala Jul 30 '21 at 09:15
  • @NielsBrinch Please try to specify the type of `ct` and hopefully it will find the correct overload. Like this: `ExecuteAsync(async (CancellationToken ct) => await CallOverloadedAPI(token: ct), CancellationToken.None)` – Peter Csala Jul 30 '21 at 09:17
  • @NielsBrinch It is still weird for me because from the `CancellationToken.None` it should able to determine [which overload to use](https://github.com/App-vNext/Polly/blob/master/src/Polly/AsyncPolicy.ExecuteOverloads.cs). `public Task ExecuteAsync(Func action, Context context)` vs `public Task ExecuteAsync(Func action, CancellationToken cancellationToken)` – Peter Csala Jul 30 '21 at 09:20
  • @NielsBrinch You should change this line: `await GetPolicy().ExecuteAndCaptureAsync(() => httpClient.SendAsync(GetMessage(HttpMethod.Delete, endpoint)))` to this: `await GetPolicy().ExecuteAndCaptureAsync(async (CancellationToken ct) => await httpClient.SendAsync(GetMessage(HttpMethod.Delete, endpoint), ct), CancellationToken.None)` Please note that the decorated delegate should be async as well. – Peter Csala Jul 30 '21 at 10:37
  • @NielsBrinch Did the suggested change work for you? – Peter Csala Aug 02 '21 at 07:45
  • 1
    Thank you so much for following up. Yes, from a functional and compile stand point, the new changes worked. However, whether the policies are applied exactly right in the real world, I haven't been able to test yet. – Niels Brinch Aug 03 '21 at 12:25
  • Actually it did not work so well in the real world. I got millions of timeouts after about 20 seconds. The endpoints respond after 250ms in average when called from any other source than these polly requests. I haven't yet had time to look into it further. – Niels Brinch Aug 04 '21 at 06:39
  • 1
    Did you pass the CancellationToken to the `SendAsync` call? `httpClient.SendAsync(GetMessage(HttpMethod.Delete, endpoint), ct)` – Peter Csala Aug 04 '21 at 07:11
  • Yes - and I have been able to investigate now. It was caused by something else. And the 20-something-seconds can be explained more or less by the retries inside Polly (even though it is a bit high). But without the new policy (strategy) I have tested and it uses the 100 seconds from HttpClient. So all in all: It works. Not sure if it's 100% accurate. – Niels Brinch Aug 04 '21 at 07:25
  • 1
    @NielsBrinch The observed 20 seconds can be explained like this: 0th (initial) attempt fails after 3 seconds; 3 seconds penalty; 1st retry attempt fails after 3 seconds; 3 seconds penalty ... All in all you have 4 timed out requests (4*3 seconds) and three-times you have a penalty (3*3) >> 21 seconds in total – Peter Csala Aug 04 '21 at 07:42