Polly not catching Nsubstitute's mocked exception

Question

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>());
}

Please do not edit your question to include the potential fix. Please post an answer instead. — Peter Csala, Sep 08 '22 at 16:42
BTW are you sure that you want to retry when the response is **NOT** `null` or empty string? — Peter Csala, Sep 08 '22 at 16:48
really strange and badly designed but **YES** . `_timezoneInfrastructure` returns not empty string if something went wrong — Maruf, Sep 08 '22 at 16:59
If you don't mind I would post tomorrow an answer where I detail several thoughs about your code. There are a couple of things which bothers me — Peter Csala, Sep 08 '22 at 19:03

score 1 · Answer 1 · answered Sep 08 '22 at 16:55

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>());

score 1 · Answer 2 · answered Sep 09 '22 at 10:11

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.

Thank you taking your time and making suggestions! I totally agree with `naming` :). `Combined retry` logic is because, most of the time call is failing because of `token` is expired so I need to retry whole process — Maruf, Sep 13 '22 at 05:09

Polly not catching Nsubstitute's mocked exception

2 Answers2

Exponential backoff

Combined Retry logic

Naming

Component testing