Retry loop for asynchronous HTTP requests

Question

I've got a method that needs to PUT data to a web API. Sometimes the connection fails, so I needed a way to do retries, but if the retries fail, I still need to capture the exception and re-throw it.

I've got something that I "believe" is working, but I've got a stupid throw at the end. Is there a better, more concise way to do this when working with async tasks?

private const int TotalNumberOfAttempts = 10;
public static async Task<HttpResponseMessage> PutWithRetriesAsync(string url,
 HttpContent content,
 AuthenticationHeaderValue authenticationHeaderValue,
 MediaTypeWithQualityHeaderValue mediaTypeWithQualityHeaderValue)
{
 var numberOfAttempts = 0;
 ExceptionDispatchInfo capturedException;
 do
 {
 try
 {
 return await PutAsync(url, content, authenticationHeaderValue, mediaTypeWithQualityHeaderValue);
 }
 catch (AggregateException ex)
 {
 capturedException = ExceptionDispatchInfo.Capture(ex);
 numberOfAttempts++;
 }
 } while (numberOfAttempts < TotalNumberOfAttempts);
 if (capturedException != null)
 {
 capturedException.Throw();
 }
 throw new Exception("That will never be thrown");
}

Answer 1

I would rewrite your code into a while (true) loop, that can only be exited using the return in your try or using a throw; inside a condition in your catch:

while (true)
{
 try
 {
 return await PutAsync(url, content, authenticationHeaderValue, mediaTypeWithQualityHeaderValue);
 }
 catch
 {
 numberOfAttempts++;
 if (numberOfAttempts >= TotalNumberOfAttempts)
 throw;
 }
}

This way, you don't need ExceptionDispatchInfo or the useless (but required by the compiler) throw at the end.

I also changed catch (AggregateException ex) to catch all exceptions, because await usually doesn't throw AggregateException (unlike task.Wait() or task.Result).

• Chase Florell: This is a good approach. I had originally done the same thing with a do, but without the ExceptionDispatchInfo. I was just under the impression that the EDI was the right way to deal with async exceptions.
• svick: @ChaseFlorell If you need to catch it in one place and then rethrow it somewhere else, yes. But you don't need to do that here, throw; will work as well and it's simpler.
• Chase Florell: ah, makes sense. So this approach is equally as valid. gist.github.com/ChaseFlorell/9524b2e8a85dfa5b563e
• svick: @ChaseFlorell Not really, throw ex; on an exception that you caught will overwrite the stack trace of the exception. The entire point of EDI is to avoid that. So, you should use just throw; when you can. And when you can't, use EDI, but don't rethrow an exception using throw ex;.
• Chase Florell: makes perfect sense.

Answer 2

How about:

private const int TotalNumberOfAttempts = 10;
public static async Task<HttpResponseMessage> PutWithRetriesAsync(string url,
 HttpContent content,
 AuthenticationHeaderValue authenticationHeaderValue,
 MediaTypeWithQualityHeaderValue mediaTypeWithQualityHeaderValue)
{
 var exceptions = new List<Exception>();
 for(int i = 0; i < TotalNumberOfAttempts; i++)
 {
 try
 {
 return await PutAsync(url, content, authenticationHeaderValue, mediaTypeWithQualityHeaderValue);
 }
 catch (AggregateException ex)
 {
 exceptions.Add(ex);
 }
 } 
 throw new AggregateException(exceptions);
}

You can easily abstract this functionality into an extension method to provide retry logic for any arbitrary Func<Task>.

• svick: Why are you rethrowing all of the exceptions? I doubt they will give you any additional information, rethrowing just the last one should be enough.
• Ami: This kind of retry logic is very dangerous and is rarely a good idea. The nature of the connectivity/network failure OP is up against will determine whether or not retry logic makes sense here and what to do with the exceptions he accumulates.