Implementing timeout in async retry operations

Question

I wrote an async method with retry logic. It works just fine, however recently I wanted to add a timeout for each try in case the operation takes too long.

public static async Task<Result> PerformAsync(Func<Task> Delegate,
    Func<Exception, bool> FailureCallback = null, int Timeout = 30000,
    int Delay = 1000, int Threshold = 10)
{
    if (Delegate == null)
    {
        throw new ArgumentNullException(nameof(Delegate));
    }

    if (Threshold < 1)
    {
        throw new ArgumentOutOfRangeException(nameof(Threshold));
    }

    CancellationTokenSource Source = new CancellationTokenSource();
    CancellationToken Token = Source.Token;

    bool IsSuccess = false;

    for (int Attempt = 0; Attempt <= Threshold && !Source.IsCancellationRequested;
        Attempt++)
    {
        try
        {
            await Delegate();

            Source.Cancel();

            IsSuccess = true;

            break;
        }

        catch (Exception E)
        {
            Exceptions.Add(E);

            if (FailureCallback != null)
            {
                bool IsCanceled =
                    Application.Current.Dispatcher.Invoke(new Func<bool>(() =>
                {
                    return !FailureCallback(E);
                }));

                if (IsCanceled)
                {
                    Source.Cancel();

                    IsSuccess = false;

                    break;
                }
            }
        }

        await Task.Delay(Delay);
    }

    return new Result(IsSuccess, new AggregateException(Exceptions));
}

I've been trying various solutions all over the web, but for whatever reason I've never managed to set timeout for each try individually.

I tried to do this using Task.WhenAny() with Task.Delay(Timeout), but when I launch my program, FailureCallback is called only once and if another try fails, FailureCallback is not called.

Answer 1

Ok, lets start. First of all, the intended usage of a CancellationToken isn't to cancel locally a loop, that's a waste, a CancellationToken reserves some resources and in your case you can simply usea boolean.

bool IsSuccess = false;
bool IsCancelled = false;

for (int Attempt = 0; Attempt <= Threshold; Attempt++)
{

    try
    {
        await Delegate();
        IsSuccess = true;
        //You are breaking the for loop, no need to test the boolean
        //in the for conditions
        break;
    }

    catch (Exception E)
    {
        Exceptions.Add(E);

        if (FailureCallback != null)
        {
            IsCancelled = Application.Current.Dispatcher.Invoke(new Func<bool>(() =>
            {
                    return !FailureCallback(E);
            }));

            //You are breaking the for loop, no need to test the boolean
            //in the for conditions

            if(IsCancelled)
                break;

        }
    }

    await Task.Delay(Delay);
}

//Here you have "IsSuccess" and "IsCancelled" to know what happened in the loop
//If IsCancelled is true the operation was cancelled, if IsSuccess is true
//the operation was success, if both are false the attempt surpased threshold.

Second, you must update your delegate to be cancellable, that's the real intended usage of CancellationToken, make your delegate to expect a CancellationToken and use it properly inside the function.

public static async Task<Result> PerformAsync(Func<CancellationToken, Task> Delegate, //..

//This is an example of the Delegate function
public Task MyDelegateImplemented(CancellationToken Token)
{

    //If you have a loop check if it's cancelled in each iteration
    while(true)
    {
        //Throw a TaskCanceledException if the cancellation has been requested
        Token.ThrowIfCancellationRequested();

        //Now you must propagate the token to any async function
        //that accepts it
        //Let's suppose you are downloading a web page

        HttpClient client;

        //...

        await client.SendAsync(message, Token)

    }

}

Finally, now that your task is cancellable you can implement the timeout like this:

//This is the "try" in your loop
try
{
    var tokenSource = new CancellationTokenSource();

    var call = Delegate(tokenSource.Token);
    var delay = Task.Delay(timeout, tokenSource.Token);

    var finishedTask = await Task.WaitAny(new Task[]{ call, delay });

    //Here call has finished or delay has finished, one will
    //still be running so you need to cancel it

    tokenSource.Cancel();
    tokenSource.Dispose();

    //WaitAny will return the task index that has finished
    //so if it's 0 is the call to your function, else it's the timeout

    if(finishedTask == 0)
    {
        IsSuccess = true;
        break;
    }
    else
    {
        //Task has timed out, handle the retry as you need.
    }

}