Correct way to retry HttpClient requests with Polly
Asked Answered
P

2

4

I have an Azure function that makes a http call to a webapi endpoint. I'm following this example GitHub Polly RetryPolicy so my code has a similar structure. So in Startup.cs i have:

builder.Services.AddPollyPolicies(config); // extension methods setting up Polly retry policies
builder.Services.AddHttpClient("MySender", client =>
{
    client.BaseAddress = config.SenderUrl;
    client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
});

My retry policy looks like this:

public static class PollyRegistryExtensions
{
    public static IPolicyRegistry<string> AddBasicRetryPolicy(this IPolicyRegistry<string> policyRegistry, IMyConfig config)
    {
        var retryPolicy = Policy
            .Handle<Exception>()
            .OrResult<HttpResponseMessage>(r => !r.IsSuccessStatusCode)
            .WaitAndRetryAsync(config.ServiceRetryAttempts, retryCount => TimeSpan.FromMilliseconds(config.ServiceRetryBackOffMilliSeconds), (result, timeSpan, retryCount, context) =>
            {
                if (!context.TryGetLogger(out var logger)) return;

                logger.LogWarning(
                    $"Service delivery attempt {retryCount} failed, next attempt in {timeSpan.TotalMilliseconds} ms.");

            })
            .WithPolicyKey(PolicyNames.BasicRetry);

        policyRegistry.Add(PolicyNames.BasicRetry, retryPolicy);

        return policyRegistry;
    }
}

My client sender service receives IReadOnlyPolicyRegistry<string> policyRegistry and IHttpClientFactory clientFactory in its constructor. My code calling the client is the following:

var jsonContent =  new StringContent(JsonSerializer.Serialize(contentObj),
    Encoding.UTF8,
    "application/json");

HttpRequestMessage requestMessage = new HttpRequestMessage(HttpMethod.Post, "SendEndpoint")
{
    Content = jsonContent
};

requestMessage.Headers.Authorization = new AuthenticationHeaderValue("Bearer", authToken);
requestMessage.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));

var retryPolicy = _policyRegistry.Get<IAsyncPolicy<HttpResponseMessage>>(PolicyNames.BasicRetry)
                  ?? Policy.NoOpAsync<HttpResponseMessage>();

var context = new Context($"GetSomeData-{Guid.NewGuid()}", new Dictionary<string, object>
{
    { PolicyContextItems.Logger, _logger }
});

var httpClient = _clientFactory.CreateClient("MySender");

var response = await retryPolicy.ExecuteAsync(ctx =>
    httpClient.SendAsync(requestMessage), context);

When I attempt to test this with no endpoint service running then for the first retry attempt, the retry handler is fired and my logger records this first attempt. However, on the second retry attempt i get a error message saying:

The request message was already sent. Cannot send the same request message multiple times

I know that other people have encountered a similar problem (see Retrying HttpClient Unsuccessful Requests and the solution seems to be do what i'm doing (i.e. use HttpClientFactory). However, i DON'T get this problem if i define my retry policy as part of the configuration in Startup as so:

builder.Services.AddHttpClient("MyService", client =>
    {
        client.BaseAddress = config.SenderUrl;
        client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
    }).AddPolicyHandler(GetRetryPolicy());

static IAsyncPolicy<HttpResponseMessage> GetRetryPolicy()
{
    return HttpPolicyExtensions
        .HandleTransientHttpError()
        .OrResult(msg => msg.StatusCode == System.Net.HttpStatusCode.NotFound)
        .WaitAndRetryAsync(3, retryAttempt => TimeSpan.FromMilliseconds(1000));
}

and simply call my service as so:

var response = await httpClient.SendAsync(requestMessage);

BUT doing it this way i lose the ability to pass my logger in the retry policy context (which is the whole reason i'm injecting in IReadOnlyPolicyRegistry<string> policyRegistry - I can not do this at startup). Another benefit is for unit testing - i can simply inject in the same collection with the same policy without copying and pasting a whole bunch of code and making the unit test redundant since i'm no longer testing my service. Having the policy defined in the startup makes this impossible. So my question is, is there a way to not get this duplicate request error using this approach ?

Psychedelic answered 9/3, 2021 at 19:24 Comment(0)
W
4

You are getting a bit too much caught up in Polly and how to configure it, and forgetting a few basic aspects. Don't worry, this is too easy to do!

First, you cannot send the same HttpRequestMessage more than once. See this extensive Q&A on the subject. It's also documented officially, though the documentation is a bit opaque on the reason.

Second, as you have it coded, the request you created was captured once by the lambda, and then reused over and over again.

For your particular case, I would move the creation of the request inside the lambda that you are passing to ExecuteAsync. This gives you a new request each time.

Modifying your code,

var jsonContent =  new StringContent(
    JsonSerializer.Serialize(contentObj),
    Encoding.UTF8,
    "application/json");

var retryPolicy = _policyRegistry.Get<IAsyncPolicy<HttpResponseMessage>>PolicyNames.BasicRetry)
    ?? Policy.NoOpAsync<HttpResponseMessage>();

var context = new Context(
    $"GetSomeData-{Guid.NewGuid()}",
    new Dictionary<string, object>
    {
        { PolicyContextItems.Logger, _logger }
    });

var httpClient = _clientFactory.CreateClient("MySender");

var response = await retryPolicy.ExecuteAsync(ctx =>
{
    var requestMessage = new HttpRequestMessage(HttpMethod.Post, "SendEndpoint")
    {
        Content = jsonContent
    };

    requestMessage.Headers.Authorization = new AuthenticationHeaderValue("Bearer", authToken);
    requestMessage.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
    httpClient.SendAsync(requestMessage), context);
}

The other captures: logger, authToken, might be OK if they don't change request over request, but you may need to move other variables inside the lambda as well.

Not using Polly at all makes most of this thought process unnecessary, but with Polly, you have to remember retries and policy are occurring across time and context.

Whatnot answered 9/3, 2021 at 21:2 Comment(1)
Thank you ! I had to slightly tweak your answer : var response = await retryPolicy.ExecuteAsync(ctx => DoRequest(jsonContent, authToken), context); where DoRequest contains the bulk of the original code.Psychedelic
A
8

Here's an alternative solution (which I prefer).

The PolicyHttpMessageHandler added by AddPolicyHandler will create a Polly Context if one isn't already attached. So you can add a MessageHandler that creates a Context and attaches the logger:

public sealed class LoggerProviderMessageHandler<T> : DelegatingHandler
{
    private readonly ILogger _logger;

    public LoggerProviderMessageHandler(ILogger<T> logger) => _logger = logger;

    protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
    {
        var httpClientRequestId = $"GetSomeData-{Guid.NewGuid()}";
        var context = new Context(httpClientRequestId);
        context[PolicyContextItems.Logger] = _logger;
        request.SetPolicyExecutionContext(context);

        return await base.SendAsync(request, cancellationToken);
    }
}

A little extension method for registration makes it nice:

public static IHttpClientBuilder AddLoggerProvider<T>(this IHttpClientBuilder builder)
{
    if (!services.Any(x => x.ServiceType == typeof(LoggerProviderMessageHandler<T>)))
        services.AddTransient<LoggerProviderMessageHandler<T>>();
    return builder.AddHttpMessageHandler<LoggerProviderMessageHandler<T>>();
}

And then you can use it as such (note that it must be before the AddPolicyHandler so that it creates the Context first):

builder.Services.AddHttpClient("MyService", client =>
{
    client.BaseAddress = config.SenderUrl;
    client.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
})
    .AddLoggerProvider<MyService>()
    .AddPolicyHandler(GetRetryPolicy());

At runtime, LoggerProviderMessageHandler<MyService> gets an ILogger<MyService>, creates a Polly Context containing that logger, and then invokes PolicyHttpMessageHandler, which uses the existing Polly Context, so your retry policy can successfully use context.TryGetLogger.

Antagonism answered 9/3, 2021 at 22:6 Comment(1)
Got your method working - very elegant! The only downside is that it's not quite as unit test friendly as with having IReadOnlyPolicyRegistry<string> policyRegistry passed in but it does make the service implementation much cleaner.Psychedelic
W
4

You are getting a bit too much caught up in Polly and how to configure it, and forgetting a few basic aspects. Don't worry, this is too easy to do!

First, you cannot send the same HttpRequestMessage more than once. See this extensive Q&A on the subject. It's also documented officially, though the documentation is a bit opaque on the reason.

Second, as you have it coded, the request you created was captured once by the lambda, and then reused over and over again.

For your particular case, I would move the creation of the request inside the lambda that you are passing to ExecuteAsync. This gives you a new request each time.

Modifying your code,

var jsonContent =  new StringContent(
    JsonSerializer.Serialize(contentObj),
    Encoding.UTF8,
    "application/json");

var retryPolicy = _policyRegistry.Get<IAsyncPolicy<HttpResponseMessage>>PolicyNames.BasicRetry)
    ?? Policy.NoOpAsync<HttpResponseMessage>();

var context = new Context(
    $"GetSomeData-{Guid.NewGuid()}",
    new Dictionary<string, object>
    {
        { PolicyContextItems.Logger, _logger }
    });

var httpClient = _clientFactory.CreateClient("MySender");

var response = await retryPolicy.ExecuteAsync(ctx =>
{
    var requestMessage = new HttpRequestMessage(HttpMethod.Post, "SendEndpoint")
    {
        Content = jsonContent
    };

    requestMessage.Headers.Authorization = new AuthenticationHeaderValue("Bearer", authToken);
    requestMessage.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));
    httpClient.SendAsync(requestMessage), context);
}

The other captures: logger, authToken, might be OK if they don't change request over request, but you may need to move other variables inside the lambda as well.

Not using Polly at all makes most of this thought process unnecessary, but with Polly, you have to remember retries and policy are occurring across time and context.

Whatnot answered 9/3, 2021 at 21:2 Comment(1)
Thank you ! I had to slightly tweak your answer : var response = await retryPolicy.ExecuteAsync(ctx => DoRequest(jsonContent, authToken), context); where DoRequest contains the bulk of the original code.Psychedelic

© 2022 - 2024 — McMap. All rights reserved.