Search code examples
c#pollyretry-logic

Polly Try and Retry wrapper function with generic


I have a wrapper function, which takes in function making a database call returning IEnumerable<T>.

This type T can be any of my specific class that stores returned integer from the database.

I made it work, but currently it only accepts specific IEnumerable<MyOwnClass>. How to make it so that I can pass any IEnumerable<T>of any of my classes?

And also how to handle the result in OnRetry? As it should have access to parameter of the IEnumerable<T> which is named differently in each class.


public static async ValueTask<TResult> KeepTrying<TResult>(Func<TResult> func, int expectedInteger) where TResult : IEnumerable<MyOwnClass>
{

var pipeline = new ResiliencePipelineBuilder<IEnumerable<MyOwnClass>>()
.AddRetry( new RetryStrategyOptions<IEnumerable<MyOwnClass>>(){

    ShouldHandle = new PredicateBuilder<IEnumerable<MyOwnClass>>().Handle<NotFoundException>()
    .HandleResult(result => result.First().MyInteger != expectedInteger),
    MaxRetryAttempts = 3,
    Delay = TimeSpan.FromSeconds(2)
})
.Build();

return await pipeline.ExecuteAsync(token => new ValueTask<TResult>(func()));

}

Calling this wrapper:

var result = await KeepTrying(() => DB.MyFunction(), expectedInteger: 100);

Answers to Update 2


  1. Without proper context I can't tell when NotFoundException is thrown. But by educated guess is that it is thrown when the database function would otherwise return null. But null and empty collection are not the same.

Correct I first check if it's empty:

if (result.Count == 0)
    throw new NotFoundException("Some error");

return result;

Is this good enough check for the error-prone result.First().MyInteger?

  1. I'm not 100% sure what do mean by this. Could you please rephrase it? (Regarding handling other types)

Sure, what I have done so far is working, but I am well aware this might be very questionable design.

Sometimes I want to return from database not only integer, but also bool, or string.

So what I did is I added new property to IMyInteger (actually renamed it to IReturnedValueFromDatabase as it's not only int).

public interface IReturnedValueFromDatabase
{
    public int ReturnedInteger { get; set; } // was MyInteger
    public bool ReturnedBool { get; set; }
    public string ReturnedString { get; set; }
}

Now I added few things to KeepTrying():

Added new nullable params so in HandleResult I know which one should be checked:


public static async ValueTask<TResult> KeepTrying<TResult>(..., int? expectedNumber = null, bool? expectedBool = null, bool handleResult = true) where TResult : IReturnedValueFromDb
{
    // skipping to .HandleResult()
    .HandleResult(result => {
   
        if (!handleResult)
           // Sometimes I want to only handle Exception and not to check result, so added this check, but maybe there is better way?
           return false;

        if (expectedNumber != null)
           // I am passing int so lets check int
           return result.First().ReturnedInteger != expectedNumber;

        if (expectedBool != null)
           // I am passing bool so lets check bool
           return result.First().ReturnedBool != expectedBool;

        // all else fails, throw exception
        throw new InvalidOperationException("Unable to determine result");
    })
}

Hopefully I am not too confusing again. Basically I return different values from DB so want to make the function as generic as possible but remain DRY. I don't know if this is terrible design and maybe I should create different functions for each type, or there is other way.

  1. Regarding exception thrown from KeepTrying()

When I run it in test, and KeepTrying would not return anything, test wouldnt fail, so I assume it doesn't throw an exception, but it probably only gets lost somewhere. I guess I can either use try/except on KeepTrying as you proposed or in the test where I call KeepTrying (so that I can remove following Assert.That statement). But this is not that important right now.

  1. using Execute instead of ExecuteAsync

If it's not a problem, I can use Execute as you are suggesting. I still got lots to learn about these things. Thanks so far!

Update 3


You can avoid throwing exception. You can simply use .Any inside the HandleResult

Sorry this might have been confusing because of same naming result - it was excerpt from my database function where I throw exception if result.Count == 0, or return result otherwise.

I have created dotnetfiddle with current version

Hence I am now also getting confused, not sure if you meant exception inside .HandleResult() - can I remove flag bool? handleResult = true from KeepTrying() and inside HandleResult do instead:

if (!result.Any()) // In case I don't want to check result?
    return false;

I think it would be best if you could just rewrite the fiddle with your suggestions to avoid further confusion.

using property selector

Sure, I can try, at least its working now. I will keep adjusting and see.

Based on this description my educated guess is that you are not awaiting the KeepTrying method inside your test. await it and it should throw the exception.

I am actually using await (see Test class in fiddle) - if I remove await, I get error that Can not resolve symbol 'First' from nextline Assert.That(myNumber.First().ReturnedInteger, which is strange, because I changed KeepTrying() to sync, but it might be because Test is async?

The strange behaviour is that when result is incorrect, it will correctly retry, but will not throw exception, thats why I am also using Assert.That in following line - if KeepTrying would throw exception I could remove it.

I would get exception thrown from HandleResult() only when I passed one of the arguments as null, for example expectedNumber = null. There are some strange behaviours but I can avoid these if I only use the function as expected - maybe one more question:

For DB functions when I am not checking results, where I just want to make sure something was returned (i.e. no NotFoundException was thrown), is my handleResult flag good enough solution to ignore checking solution?

Anyway, if it's too confusing at this point, don't bother, you already helped me ton and made the function work! Thanks a lot for that.


Solution

  • How to make it so that I can pass any IEnumerable<T> of any of my classes?

    You should change the signature of your method

    from

    ValueTask<TResult> KeepTrying<TResult>(
       Func<TResult> func, int expectedInteger) 
       where TResult : IEnumerable<MyOwnClass>
    

    to

    ValueTask<IEnumerable<TResult>> KeepTrying<TResult>(
      Func<IEnumerable<TResult>> func, int expectedInteger) 
      where TResult : IMyInteger
    

    So, basically your TResult is not the enumerable collection rather its generic type parameter.

    static async ValueTask<IEnumerable<TResult>> KeepTrying<TResult>(
      Func<IEnumerable<TResult>> func, int expectedInteger) 
      where TResult : IMyInteger
    {
        var pipeline = new ResiliencePipelineBuilder<IEnumerable<TResult>>()
        .AddRetry(new()
        {
            ShouldHandle = new PredicateBuilder<IEnumerable<TResult>>()
                .Handle<NotFoundException>()
                .HandleResult(result => result.First().MyInteger != expectedInteger),
            MaxRetryAttempts = 3,
            Delay = TimeSpan.FromSeconds(2)
        })
        .Build();
    
        return await pipeline.ExecuteAsync(_ => ValueTask.FromResult(func()));
    }
    
    • Please bear in mind that result.First().MyInteger is quite error-prone, because it assumes that the collection always has at least 1 element in it

    The IMyInteger is a simple interface which defines MyInteger property's getter

    interface IMyInteger
    {
        public int MyInteger { get; }
    }
    

    And also how to handle the result in OnRetry?

    OnRetry = static args =>
    {
         Console.WriteLine(args.Outcome.Result);
         return default;
    }
    

    args has an Outcome property. Either its Exception or its Result property is populated depending on which Handle method triggered a retry.


    For the shake of completeness here is full sample app

    await KeepTrying<A>(() => [new ()], 1); //no retry
    await KeepTrying<B>(() => [new ()], 1); //three retries
    
    async ValueTask<IEnumerable<TResult>> KeepTrying<TResult>(
      Func<IEnumerable<TResult>> func, int expectedInteger) 
      where TResult : IMyInteger
    {
        var pipeline = new ResiliencePipelineBuilder<IEnumerable<TResult>>()
        .AddRetry(new()
        {
            ShouldHandle = new PredicateBuilder<IEnumerable<TResult>>()
                .Handle<NotFoundException>()
                .HandleResult(result => result.First().MyInteger != expectedInteger),
            MaxRetryAttempts = 3,
            Delay = TimeSpan.FromSeconds(2),
            OnRetry = static args =>
            {
                Console.WriteLine(args.Outcome.Result);
                return default;
            }
        })
        .Build();
    
        return await pipeline.ExecuteAsync(_ => ValueTask.FromResult(func()));
    }    
    
    interface IMyInteger
    {
        public int MyInteger { get; }
    }
    
    class A : IMyInteger
    {
        public int MyInteger { get; } = 1;
    }
    
    class B : IMyInteger
    {
        public int MyInteger { get; } = 2;
    }
    

    UPDATE #1

    I forgot to mention to please prefer Execute over ExecuteAsync in this scenario:

    return ValueTask.FromResult(pipeline.Execute(func));
    

    UPDATE #2

    1. If result.First().MyInteger is error prone, whats the better way? I am first handling NotFoundException so it should be indeed there. Or thats not enough?

    Without proper context I can't tell when NotFoundException is thrown. But by educated guess is that it is thrown when the database function would otherwise return null. But null and empty collection are not the same.

    If the database function can return an empty collection, like new MyOwnClass[] then .First will thrown an InvalidOperationException. FirstOrDefault won't throw any exception if the collection is empty but would return with default(T).

    1. Now Im trying to implement not only integer, but also bool and string - can I expand HandleResult and insert if statements? If (expectedNumber != null) check number, etc? If you understand what I mean, if not I can reply later on PC.

    I'm not 100% sure what do mean by this. Could you please rephrase it?

    1. How to throw an exception from this KeepTrying function if it fails?

    If all retry attempts fail then the retry strategy will propagate the last exception. Look at this sequence diagram which depicts exactly this scenario. So, the Execute{Async} will do it for you without asking.

    If you want to change it the exception to a custom one then

    try
    {
       return await pipeline.ExecuteAsync(token => new ValueTask<TResult>(func()));
       //OR
       //return ValuTask.FromResult(pipeline.Execute(func));
    }
    catch(NotFoundException)
    {
       return ValueTask.FromException(new CustomException(ex));
    }
    

    An alternative solution could utilize the ExecuteOutcomeAsync

    1. Regarding using Execute instead of ExecuteAsync, would you care explaining the reason? I an running tests where everything is async, should I still use just Execute?

    Even though Polly V8 is async by its core it does not mean that you have to always use ExecuteAsync. Execute is designed to decorate synchronous code invocation. So, Execute is absolutely a good choice to decorate your func().

    Also your KeepTrying is implemented in a synchronous fashion so, it feels more natural to use Execute for retry as well.

    Dotnet fiddle: https://dotnetfiddle.net/M0lViU


    UPDATE #3

    if (result.Count == 0)
       throw new NotFoundException("Some error");
    
    return result;
    

    Is this good enough check for the error-prone result.First().MyInteger?

    You can avoid throwing exception. You can simply use .Any inside the HandleResult something like this:

    ShouldHandle = new PredicateBuilder<IEnumerable<TResult>>()
        .HandleResult(result => !result.Any() || result.First().MyInteger != expectedInteger)
    

    So what I did is I added new property to IMyInteger (actually renamed it to IReturnedValueFromDatabase as it's not only int).

    Instead of having multiple if statements you can pass a property selector to your function: Expression<Func<IReturnedValueFromDatabase,TProperty>> selector

    So, the signature of your method could look like this:

    ValueTask<IEnumerable<TResult>> KeepTrying<TResult, TProperty>(
      Func<IEnumerable<TResult>> func,
      Expression<Func<TResult,TProperty>> selector 
      TProperty expectedValue) 
      where TResult : IReturnedValueFromDatabase
    

    I know I know it's getting more and more complicated, but if you wish to have a really generic solution then that's the way how you should approach the problem.

    When I run it in test, and KeepTrying would not return anything, test wouldnt fail, so I assume it doesn't throw an exception, but it probably only gets lost somewhere.

    Based on this description my educated guess is that you are not awaiting the KeepTrying method inside your test. await it and it should throw the exception.


    UPDATE #4

    The ultimate solution which incorporates all requests and concerns:
    https://dotnetfiddle.net/UydLSY