Polly Retry policy with Function is not waiting for result

Question

I am trying to convert my existing function to Polly Retry policy

public static T Execute<T>(Func<T> getTask) where T : Task
{
    var retryCount = 3;
    while (retryCount-- > 0)
    {
        try
        {
            getTask().Wait();
            return getTask();
        } catch(Exception ex){
            // handle retry
        }
    }
}

Converted to this

public static T Execute<T>(Func<T> func) where T : Task
{
    var task = func();
    Policy.Handle<HttpRequestException>()
        .Or<TimeoutException>()
        .WaitAndRetryAsync(
            3,
            retryAttempt => TimeSpan.FromSeconds(Math.Pow(5, retryAttempt)),
            (exception, timeSpan, retryCount, context) =>
            {
                //do some logging
            })
        .ExecuteAsync(func).Wait();
    return task;
}

and test code is

var retryCount = 0;
var res = HttpRetryWrapper.Execute(() => Task.Factory.StartNew<string>(function: () =>
{
    if (++retryCount == 3)
    {
        return "fake";
    }
    throw new TimeoutException();
}));

when I assert the res value, I am not getting the right result. Debugging traces me into point where the Execution is not waiting for results properly.

The number of calls to test function is correct. However the logging is messed up and the final result is not having result fake

Trapper

Pretty much never use Wait. Try making the function async and awaiting inside – Hexamethylenetetramine

You also just return the task local var, which is not used by your policy. Polly is calling func on its own. – Hexamethylenetetramine

@Hexamethylenetetramine this is the one contract which I can't break public static T Execute<T>(Func<T> func) where T : Task – Trapper

Answer 1

For a helper method for async executions through a hard-coded Polly policy, where the executions asynchronously return a type TResult, via a Task<TResult>, you could adopt:

public static Task<TResult> ExecuteAsync<TResult>(Func<Task<TResult>> func) 
{
    return Policy.Handle<HttpRequestException>()
        .Or<TimeoutException>()
        .WaitAndRetryAsync(
            3,
            retryAttempt => TimeSpan.FromSeconds(Math.Pow(5, retryAttempt)),
            (exception, timeSpan, retryCount, context) =>
            {
                //do some logging
            })
        .ExecuteAsync<TResult>(func); // This is an async-await-eliding contraction of: .ExecuteAsync<TResult>(async () => await func());
    }

(Given the policy used is the same every time, you could consider also storing the policy in a static field and creating it only once.)

Note: This (intentionally) doesn't conform to the contract stated in your comment to your original question, as being unbreakable:

public static T Execute<T>(Func<T> func) where T : Task

The clause where T : Task initially looks appealing for an or async async-like method designed to work with either Task or Task<T>. Jon Skeet explains here and here why it doesn't work with async. Your proposed helper method signature is not in-itself async:

public static T Execute<T>(Func<T> func) where T : Task

However, introducing .ExecuteAsync(async () => await func()); in your example code forces a similar issue. The Polly .ExecuteAsync(...) overload, precisely in order to play nicely with async/await, exists in two main forms:

(1) Task ExecuteAsync(Func<Task> func)

(2) Task<TResult> ExecuteAsync<TResult>(Func<Task<TResult>> func)

The compiler has to select one or the other at compile time: it can't (in your example code) compile it to be either (1) or (2) at runtime. Since it only knows T : Task it selects (1), which returns Task. Hence the error you see in your comment to @JamesFaix's answer: Cannot implicitly convert type Task to T.

If you want helper patterns of this form, which callers can use for either Task or Task<TResult> -returning calls, you would have to declare both:

class HttpRetryWrapper
{
    private static policy = Policy.Handle<HttpRequestException>()
        .Or<TimeoutException>()
        .WaitAndRetryAsync(
            3,
            retryAttempt => TimeSpan.FromSeconds(Math.Pow(5, retryAttempt)),
            (exception, timeSpan, retryCount, context) =>
            {
                //do some logging
            });

    public static Task ExecuteAsync(Func<Task> func) 
    {
        return policy.ExecuteAsync(func);
    }

    public static Task<TResult> ExecuteAsync<TResult>(Func<Task<TResult>> func) 
    {
        return policy.ExecuteAsync<TResult>(func);
    }
}

---

Finally, if this is for wrapping calls placed through HttpClient, the recommended pattern is to place the Polly policies in a DelegatingHandler as described in @MuhammedRehanSaeed's answer here. ASP.NET Core 2.1 supports concise declaration for creating such DelegatingHandlers using IHttpClientFactory.

Rounds

Answer 2

I think what you want is

public static T Execute<T>(Func<T> func) where T : Task
{
    return Policy.Handle<HttpRequestException>()
        .Or<TimeoutException>()
        .WaitAndRetryAsync(
            3,
            retryAttempt => TimeSpan.FromSeconds(Math.Pow(5, retryAttempt)),
            (exception, timeSpan, retryCount, context) =>
            {
                //do some logging
            })
        .ExecuteAsync(func).Wait();
}

In your code example, you were invoking func once and returning that, and then in between defining and invoking a policy, but not returning the result of invoking that policy.

If you want to avoid Wait I think you could also do

public static T Execute<T>(Func<T> func) where T : Task
{
    return Policy.Handle<HttpRequestException>()
        .Or<TimeoutException>()
        .WaitAndRetryAsync(
            3,
            retryAttempt => TimeSpan.FromSeconds(Math.Pow(5, retryAttempt)),
            (exception, timeSpan, retryCount, context) =>
            {
                //do some logging
            })
        .ExecuteAsync(async () => await func());
}

Hexamethylenetetramine

I started with that. ran into issue Cannot implicitly convert type Task to T. When I explicit cast, running into run time exceptions – Trapper

Maybe you just want Execute and not ExecuteAsync. – Hexamethylenetetramine