'Retry' mechanism with callback

Question

We've had a situation whereby we have to cater for certain exceptions and retry a particular method whenever these exceptions occur. This is required in various parts of the system. In addition, we (might) want to invoke a 'callback' upon the need to retry.

I'll show you the code, then give you an example of useage:

    public static void InvokeActionWithRetry(Action action, int retryAttempts, Action retryCallback = null, params Type[] exceptionTypes)
    {
        do
        {
            try {
                action();
                break;
            }
            catch (Exception ex)
            {
                   if ((exceptionTypes?.Length > 0 && exceptionTypes.Contains(ex.GetType()))
                        || retryAttempts == 0)
                    throw;

                if (retryCallback != null)
                    retryCallback();
            }

            retryAttempts--;
        }
        while (retryAttempts >= 0);
    }

So, an example (although not a real-world one), may be:

ActionHelper.InvokeActionWithRetry(() => {
    Print(document);
},
5,
() => {
    RebootPrinter();
},
typeof(PrinterNeedsRestartException));

Is there anything that can be improved with the static method I've written?

I'm pretty new to using actions/funcs so I'm open to suggestions about how I can refactor this.

I also feel the type evaluation is a little 'hacky'.

EDIT:

I just noticed I'm not checking the types collection passed in prior to calling Contains on it. Naughty. I'll rework that bit and update the question.

Answer 1

Consider this example:

InvokeActionWithRetry(() => 
        { 
            Console.WriteLine("action"); 
            throw new Exception(); 
        }, 
    3);

It prints 'action' four times. That's not what I expect from the API (but is what I expected from reading the code). I'd expect the number I pass in to be the total number of invocations - not the number of retries after the first attempt.

---

You need to check this arguments for this method as there is a lot of scope to pass in the weird values. e.g. what if retryAttempts is a negative number? That doesn't make sense!

You also need to check action for null.

---

Answer 2

You should specify the exceptions to retry on instead of the exceptions to fail on. This will prevent you from retrying on unexpected/newly introduced exceptions.

Don't reuse parameters.

If you're using C# 6 you should use exception filtering instead of catch {...throw;}.

Depending on the amount of exceptions you plan to filter on it might be better to create a HashSet<T> from the params.

public static void InvokeActionWithRetry(Action action, int attempts, Action retryCallback = null, params Type[] exceptionTypes)
{
    if(action == null) throw new ArgumentNullException(nameof(action));
    if(attempts < 0) throw new ArgumentOutOfRangeException(nameof(attempts), nameof(attempts) + " must be positive");

    var exceptionFilter = (exceptionTypes?.Length > 0) 
        ? new HashSet<Type>(exceptionTypes) 
        : new HashSet<Type>();

    var lastAttempt = attempts - 1;
    for(int attempt = 0; attempt < attempts; attempt++)
    {
        try {
            action();
            break;
        }
        catch (Exception ex) when (exceptionFilter.Contains(ex.GetType()) && attempt != lastAttempt)
        {
            if (retryCallback != null)
                retryCallback();
        }
    }
}

Answer 3

1. Functionality extension - besides already mentioned answers, I would extend your functionality with a back-off mechanism which can prove useful in some situations (e.g. a SQL deadlock). Since the options are quite a few now, these can be contained into a special class:

    public class ActionRetryOptions
    {
        // general data
        public Action Action { get; set; }
        public uint AttemptCount { get; set; } = 3;

        // retry data
        public Action RetryCallback { get; set; }

        // backoff mechanism
        public bool UseBackoff { get; set; } = false;
        public TimeSpan BackoffInitialInterval { get; set; } = TimeSpan.FromSeconds(1);
        public int BackoffFactor { get; set; } = 1;

        // fail fast
        public IList<Type> FailFastExceptionTypes { get; set; }
    }

I have also added some defaults to make it easier to use for the caller.

2. Changed function to support back-off

[edit] Changed BackoffInterval from int to TimeSpan as suggested by RobH

    public static void InvokeActionWithRetry(ActionRetryOptions retryOptions)
    {
        if (retryOptions == null)
            throw new ArgumentNullException(nameof(retryOptions));

        if (retryOptions.Action == null)
            throw new ArgumentNullException(nameof(retryOptions.Action));

        if (retryOptions.BackoffFactor < 1)
            throw new ArgumentException("BackoffInterval must greater or equal to 1");

        // backoff initialization
        int backOffTime = (int) retryOptions.BackoffInitialInterval.TotalMilliseconds;
        var random = new Random();

        for (uint attempt = retryOptions.AttemptCount; attempt > 0; attempt --)
        {
            try
            {
                retryOptions.Action();
                break;
            }
            catch (Exception ex)
            {
                if ((retryOptions.FailFastExceptionTypes?.Count > 0 && retryOptions.FailFastExceptionTypes.Contains(ex.GetType()))
                     || attempt <= 1)
                    throw;

                // back-off
                if (retryOptions.UseBackoff)
                {
                    int sleepTime = (int)(backOffTime * random.NextDouble());
                    Thread.Sleep(sleepTime);
                    backOffTime *= retryOptions.BackoffFactor;
                }

                if (retryOptions.RetryCallback != null)
                    retryOptions.RetryCallback();
            }
        }
    }

I have considered a simple back-off where I start with a certain period and multiply by a factor.

3. Simple test

   static void Main(string[] args)
   {
        try
        {
            var retryOptions = new ActionRetryOptions()
            {
                Action = () =>
                    {
                        Console.WriteLine("Action with error");
                        throw new Exception();
                    },
                UseBackoff = true,
                BackoffFactor = 2,
                AttemptCount = 5,
                RetryCallback = () =>
                {
                    Console.WriteLine("Retry callback");
                },
                FailFastExceptionTypes = new List<Type>() { typeof(SqlException) }
            };

            InvokeActionWithRetry(retryOptions);
        }
        catch (Exception exc)
        {
            Console.WriteLine("Unhandled exception - " + exc.ToString());
        }
        Console.ReadLine();
    }