Occasionally I have a need to retry an operation several times before giving up. My code is like...
int retries = 3;
while(true) {
try {
DoSomething();
break; // success!
} catch {
if(--retries == 0) throw;
else Thread.Sleep(1000);
}
}
I would like to rewrite this in a general retry function like...
TryThreeTimes(DoSomething);
Is it possible in C#? What would be the code for the TryThreeTimes() method?
public delegate void ThingToTryDeletage();
public static void TryNTimes(ThingToTryDelegate, int N, int sleepTime)
{
while(true)
{
try
{
ThingToTryDelegate();
} catch {
if( --N == 0) throw;
else Thread.Sleep(time);
}
}
public void TryThreeTimes(Action action)
{
int retries = 3;
while(true) {
try {
action();
break; // success!
} catch {
if(--retries == 0) throw;
else Thread.Sleep(1000);
}
}
}
Then you would call:
TryThreeTimes(DoSomething);
...or alternatively...
TryThreeTimes(() => DoSomethingElse(withLocalVariable));
You might like to extract the magic number '3' and the timeout period as arguments.
Blanket catch statements that simply retry the same call can be dangerous if used as a general exception handling mechanism. Having said that, here's a lambda-based retry wrapper that you can use with any method. I chose to factor the number of retries and the retry timeout out as parameters for a bit more flexibility:
public static class RetryUtility
{
public static void RetryAction( Action action, int numRetries, int retryTimeout )
{
if( action == null )
throw new ArgumenNullException("action"); // slightly safer...
do
{
try { action(); return; }
catch
{
if( numRetries <= 0 ) throw; // improved to avoid silent failure
else Thread.Sleep( retryTimeout );
}
} while( numRetries-- > 0 );
}
}
You can now use this utility method to perform retry logic:
RetryUtility.RetryAction( () => SomeFunctionThatCanFail(), 3, 1000 );
You might also consider adding the exception type you want to retry for. For instance is this a timeout exception you what to retry? A database exception?
RetryForExcpetionType(DoSomething, typeof(TimeoutException), 5, 1000);
public static void RetryForExcpetionType(Action action, Type retryOnExceptionType, int numRetries, int retryTimeout)
{
if (action == null)
throw new ArgumentNullException("action");
if (retryOnExceptionType == null)
throw new ArgumentNullException("retryOnExceptionType");
while (true)
{
try
{
action();
return;
}
catch(Exception e)
{
if (--numRetries <= 0 || !retryOnExceptionType.IsAssignableFrom(e.GetType()))
throw;
if (retryTimeout > 0)
System.Threading.Thread.Sleep(retryTimeout);
}
}
}
You might also note that all of the other examples have a similar issue with testing for retries == 0 and either retry infinity or fail to raise exceptions when given a negative value. Also Sleep(-1000) will fail in the catch blocks above. Depends on how 'silly' you expect people to be but defensive programming never hurts.
Or how about doing it a bit neater....
int retries = 3;
while (retries > 0)
{
if (DoSomething())
{
retries = 0;
}
else
{
retries--;
}
}
I believe throwing exceptions should generally be avoided as a mechanism unless your a passing them between boundaries (such as building a library other people can use). Why not just have the DoSomething() command return true if it was successful and false otherwise?
EDIT: And this can be encapsulated inside a function like others have suggested as well. Only problem is if you are not writing the DoSomething() function yourself
I'd implement this:
public static bool Retry(int maxRetries, Func<bool, bool> method)
{
while (maxRetries > 0)
{
if (method(maxRetries == 1))
{
return true;
}
maxRetries--;
}
return false;
}
I wouldn't use exceptions the way they're used in the other examples. It seems to me that if we're expecting the possibility that a method won't succeed, its failure isn't an exception. So the method I'm calling should return true if it succeeded, and false if it failed.
Why is it a Func<bool, bool> and not just a Func<bool>? So that if I want a method to be able to throw an exception on failure, I have a way of informing it that this is the last try.
So I might use it with code like:
Retry(5, delegate(bool lastIteration)
{
// do stuff
if (!succeeded && lastIteration)
{
throw new InvalidOperationException(...)
}
return succeeded;
});
or
if (!Retry(5, delegate(bool lastIteration)
{
// do stuff
return succeeded;
}))
{
Console.WriteLine("Well, that didn't work.");
}
If passing a parameter that the method doesn't use proves to be awkward, it's trivial to implement an overload of Retry that just takes a Func<bool> as well.
This is possibly a bad idea. First, it is emblematic of the maxim "the definition of insanity is doing the same thing twice and expecting different results each time". Second, this coding pattern does not compose well with itself. For example:
Suppose your network hardware layer resends a packet three times on failure, waiting, say, a second between failures.
Now suppose the software layer resends an notification about a failure three times on packet failure.
Now suppose the notification layer reactivates the notification three times on an notification delivery failure.
Now suppose the error reporting layer reactivates the notification layer three times on a notification failure.
And now suppose the web server reactivates the error reporting three times on error failure.
And now suppose the web client resends the request three times upon getting an error from the server.
Now suppose the line on the network switch that is supposed to route the notification to the administrator is unplugged. When does the user of the web client finally get their error message? I make it at about twelve minutes later.
Lest you think this is just a silly example: we have seen this bug in customer code, though far, far worse than I've described here. In the particular customer code, the gap between the error condition happening and it finally being reported to the user was several weeks because so many layers were automatically retrying with waits. Just imagine what would happen if there were ten retries instead of three.
Usually the right thing to do with an error condition is report it immediately and let the user decide what to do. If the user wants to create a policy of automatic retries, let them create that policy at the appropriate level in the software abstraction.
Allowing for functions and retry messages
public static T RetryMethod<T>(Func<T> method, int numRetries, int retryTimeout, Action onFailureAction)
{
Guard.IsNotNull(method, "method");
T retval = default(T);
do
{
try
{
retval = method();
return retval;
}
catch
{
onFailureAction();
if (numRetries <= 0) throw; // improved to avoid silent failure
Thread.Sleep(retryTimeout);
}
} while (numRetries-- > 0);
return retval;
}