I am using Polly's retry policy for my unsuccessful call. But it is not catching the exception and retrying.

Using:

• Polly 7.2.3

• .NET6.0

• Nsubstitute 4.2.2

Setup:

var delay = Backoff.DecorrelatedJitterBackoffV2(TimeSpan.FromMilliseconds(RetryDelay), RetryCount);

        
_retryPolicy = Policy.Handle<HttpRequestException>()
                     .Or<CustomException>()
                     .OrResult<string>(response => !string.IsNullOrEmpty(response))
                     .WaitAndRetryAsync(delay);

Usage:

public async Task ProcessRequest()
{
    var errors = await _retryPolicy.ExecuteAsync(async () => await this.RetryProcessRequest());
}

private async Task<string> RetryProcessRequest()
{
    var token = await _tokenInfrastructure.GetTokenAsync();

    return await _timezoneInfrastructure.ProcessRequest(token);
}

Unit test:

[Fact]
public async Task ProcessRequest_Throws()
{
    string errors = _fixture.Create<string>();
    var token = _fixture.Create<string>();

    // Retry policy configured to retry 3 times on failed call 
    var expectedReceivedCalls = 4;

    // this is throwing but Polly is not catching it and not retrying 
    _tokenInfrastructure.GetTokenAsync().Returns(Task.FromException<string>(new HttpRequestException())); 

    // this errors can be caught by Polly as configured and retrying
    _timezoneInfrastructure.ProcessRequest(token).Returns(errors); 

    await _timezoneOrchestration.Awaiting(o => o.ProcessRequest()).Should()
        .ThrowAsync<HttpRequestException>();

    await _tokenInfrastructure.Received(expectedReceivedCalls).GetTokenAsync();
    await _timezoneInfrastructure.Received(expectedReceivedCalls).ProcessRequest(Arg.Any<string>());
}

CodePudding user response：

After doing Rubber duck debugging found my mistake. Actually, Polly was configured well and retrying.

this line of code was never calling because above we were getting exceptions.

return await _timezoneInfrastructure.ProcessRequest(token);

In Unit tests, it was expecting some retry calls:

_timezoneInfrastructure.Received(expectedReceivedCalls).ProcessRequest(Arg.Any<string>());

CodePudding user response：

This post is not answer for the OP's question (the problem has already been addressed here). It is more like a set of suggestions (you can call it code review if you wish).

---

Exponential backoff

I'm glad to see that you are using the V2 of the backoff logic which utilizes the jitter in a proper way. My only concern here is that depending on the actual values of RetryDelay and RetryCount the sleepDuration might explode: It can easily reach several minutes. I would suggest two solutions in that case:

• Change factor parameter of the DecorrelatedJitterBackoffV2 from 2 (which is the default) to a lower number
• Or try to top the max sleepDuration, here I have detailed one way to do that

Combined Retry logic

Without knowing what does RetryProcessRequest do, it seems like this _retryPolicy smashes two different policies into one. I might be wrong, so this section could suggest something which is not applicable for your code.

I assume this part decorates the _tokenInfrastructure.GetTokenAsync call

_retryPolicy = Policy.Handle<HttpRequestException>()
                     .WaitAndRetryAsync(delay);

whereas this part decorates the _timezoneInfrastructure.ProcessRequest call

_retryPolicy = Policy.Handle<CustomException>()
                     .OrResult<string>(response => !string.IsNullOrEmpty(response))
                     .WaitAndRetryAsync(delay);

Based on your naming I assume that these are different downstream systems: tokenInfrastructure, timezoneInfrastructure. I would suggest to create separate policies for them. You might want to apply different Timeout for them or use separate Circuit Breakers.

Naming

I know naming is hard and I assume your method names (ProcessRequest, RetryProcessRequest or ProcessRequest_Throws) are dumyfied for StackOverflow. If not then please try to spend some time to come up with more expressive names.

Component testing

Your ProcessRequest_Throws test is not really a unit test. It is more likely a component test. You are testing there the integration between the Polly's policy and the decorated code.

If you would test only the correctness of the policy setup or test only the decorated code (with NoOpPolicy) then they were unit tests.