Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How can I improve this exception retry scenario?

I have a web service method I am calling which is 3rd party and outside of my domain. For some reason every now and again the web service fails with a gateway timeout. Its intermittent and a call to it directly after a failed attempt can succeed.

Now I am left with a coding dilemma, I have code that should do the trick, but the code looks like amateur hour, as you'll see below.

Is this really bad code, or acceptable given the usage? If its not acceptable, how can I improve it?

Please try hard to keep a straight face while looking at it.

try
{
    MDO = OperationsWebService.MessageDownload(MI);
}
catch
{
    try
    {
        MDO = OperationsWebService.MessageDownload(MI);
    }
    catch
    {
        try
        {
            MDO = OperationsWebService.MessageDownload(MI);
        }
        catch
        {
            try
            {
                MDO = OperationsWebService.MessageDownload(MI);
            }
            catch 
            {
                try
                {
                    MDO = OperationsWebService.MessageDownload(MI);
                }
                catch (Exception ex)
                {
                    // 5 retries, ok now log and deal with the error.
                }
            }
        }
    }
}
like image 674
JL. Avatar asked Feb 16 '10 02:02

JL.


2 Answers

You can do it in a loop.

Exception firstEx = null;
for(int i=0; i<5; i++) 
{
    try
    {
        MDO = OperationsWebService.MessageDownload(MI);
        firstEx = null;
        break; 
    }
    catch(Exception ex)
    {
        if (firstEx == null) 
        {
            firstEx = ex;
        }
        Thread.Sleep(100 * (i + 1));
    }
}
if (firstEx != null) 
{
    throw new Exception("WebService call failed after 5 retries.", firstEx);
}
like image 103
Samuel Neff Avatar answered Nov 06 '22 01:11

Samuel Neff


Here's another way you might try:

// Easier to change if you decide that 5 retries isn't right for you
Exception exceptionKeeper = null;
for (int i = 0; i < MAX_RETRIES; ++i)
{
    try
    {
       MDO = OperationsWebService.MessageDownload(MI);
       break;  // correct point from Joe - thanks.
    }
    catch (Exception ex)
    {
        exceptionKeeper = ex;
        // 5 retries, ok now log and deal with the error.
    }  
}

I think it documents the intent better. It's less code as well; easier to maintain.

like image 12
duffymo Avatar answered Nov 06 '22 02:11

duffymo