如何改进此异常重试方案？

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.

我有一个我正在调用的Web服务方法，它是第三方和我的域之外。出于某种原因，Web服务偶尔会出现网关超时。它的间歇性和在尝试失败后直接调用它可以成功。

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.
                }
            }
        }
    }
}

10 个解决方案

#1

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);
}

#2

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.

我认为它更好地记录了意图。代码也少了;更容易维护。

#3

All of the answers so far assume that the reaction to any exception should be to retry the operation. This is a good assumption right up until it's a false assumption. You could easily be retrying an operation that is damaging your system, all because you didn't check the exception type.

到目前为止，所有答案都假设对任何异常的反应都应该是重试操作。这是一个很好的假设，直到它是一个错误的假设。您可以轻松地重试损坏系统的操作，因为您没有检查异常类型。

You should almost never use a bare "catch", nor "catch (Exception ex). Catch a more-specific exception - one you know you can safely recover from.

你几乎不应该使用一个简单的“catch”，也不应该使用“catch（Exception ex）。”捕获一个更具体的例外 - 你知道可以安全恢复的例外。

#4

Try a loop, with some kind of limit:

尝试一个循环，有一些限制：

int retryCount = 5;
var done = false;
Exception error = null;
while (!done && retryCount > 0)
{
    try
    {
        MDO = OperationsWebService.MessageDownload(MI);
        done = true;
    }
    catch (Exception ex)
    {
        error = ex;
    }
    if (done)
        break;

    retryCount--;
}

#5

You should use recursion (or a loop), and should only retry if you got the error you expected.

您应该使用递归（或循环），并且只应在您遇到预期错误时重试。

For example:

例如：

static void TryExecute<TException>(Action method, Func<TException, bool> retryFilter, int maxRetries) where TException : Exception {
    try {
        method();
    } catch(TException ex) {
        if (maxRetries > 0 && retryFilter(ex))
            TryExecute(method, retryFilter, maxRetries - 1);
        else
            throw;
    }
}

EDIT: With a loop:

编辑：循环：

static void TryExecute<TException>(Action method, Func<TException, bool> retryFilter, int maxRetries) where TException : Exception {
    while (true) {
        try {
            method();
            return;
        } catch(TException ex) {
            if (maxRetries > 0 && retryFilter(ex))
                maxRetries--;
            else
                throw;
        }
    }
}

---

You can try to prevent future errors in retryFilter, perhaps by Thread.Sleep.

您可以尝试通过Thread.Sleep来防止retryFilter中的未来错误。

If the last retry fails, this will throw the last exception.

如果上次重试失败，则会抛出最后一个异常。

#6

Here is some retry logic we are using. We don't do this a lot and I was going to pull it out and document it as our Retry Pattern/Standard. I had to wing it when I first wrote it so I came here to see if I was doing it correctly. Looks like I was. The version below is fully commented. See below that for an uncommented version.

这是我们正在使用的一些重试逻辑。我们不会这么做很多，我会把它拉出来并将其记录为我们的重试模式/标准。我第一次写这篇文章的时候不得不放弃它，所以我来到这里看看我是否正确地做到了。看起来像我。以下版本已完全注释。有关未注释的版本，请参见下文。

#region Retry logic for SomeWebService.MyMethod
// The following code wraps SomeWebService.MyMethod in retry logic
// in an attempt to account for network failures, timeouts, etc.

// Declare the return object for SomeWebService.MyMethod outside of
// the following for{} and try{} code so that we have it afterwards.
MyMethodResult result = null;

// This logic will attempt to retry the call to SomeWebService.MyMethod
for (int retryAttempt = 1; retryAttempt <= Config.MaxRetryAttempts; retryAttempt++)
{
    try
    {
        result = SomeWebService.MyMethod(myId);

        // If we didn't get an exception, then that (most likely) means that the
        // call was successful so we can break out of the retry logic.
        break;
    }
    catch (Exception ex)
    {
        // Ideally we want to only catch and act on specific
        // exceptions related to the failure. However, in our
        // testing, we found that the exception could be any type
        // (service unavailable, timeout, database failure, etc.)
        // and attempting to trap every exception that was retryable
        // was burdensome. It was easier to just retry everything
        // regardless of the cause of the exception. YMMV. Do what is
        // appropriate for your scenario.

        // Need to check to see if there will be another retry attempt allowed.
        if (retryAttempt < Config.MaxRetryAttempts)
        {
            // Log that we are re-trying
            Logger.LogEvent(string.Format("Retry attempt #{0} for SomeWebService.MyMethod({1})", retryAttempt, myId);

            // Put the thread to sleep. Rather than using a straight time value for each
            // iteration, we are going to multiply the sleep time by how many times we
            // have currently tried to call the method. This will allow for an easy way to
            // cover a broader range of time without having to use higher retry counts or timeouts.
            // For example, if MaxRetryAttempts = 10 and RetrySleepSeconds = 60, the coverage will
            // be as follows:
            // - Retry #1 - Sleep for 1 minute
            // - Retry #2 - Sleep for 2 minutes (covering three minutes total)
            // - Retry #10 - Sleep for 10 minutes (and will have covered almost an hour of downtime)
            Thread.Sleep(retryAttempt * Config.RetrySleepSeconds * 1000);
        }
        else
        {
            // If we made it here, we have tried to call the method several
            // times without any luck. Time to give up and move on.

            // Moving on could either mean:
            // A) Logging the exception and moving on to the next item.
            Logger.LogError(string.Format("Max Retry Attempts Exceeded for SomeWebService.MyMethod({0})", MyId), ex);
            // B) Throwing the exception for the program to deal with.
            throw new Exception(string.Format("Max Retry Attempts Exceeded for SomeWebService.MyMethod({0})", myId), ex);
            // Or both. Your code, your call.
        }
    }
}
#endregion

I like Samuel Neff's example of using an exception variable to see if it completely failed or not. That would have made some of the evaluations in my logic a little simpler. I could go either way. Not sure that either way has a significant advantage over the other. However, at this point in time, I'm not going to change how we do it. The important thing is to document what you are doing and why so that some idiot doesn't come through behind you and muck with everything.

我喜欢Samuel Neff使用异常变量来查看它是否完全失败的例子。这会使我的逻辑中的一些评估变得更简单一些。我可以去任何一个方向。不确定这两种方式是否具有明显优势。但是，在这个时间点，我不会改变我们的做法。重要的是要记录你正在做的事情，以及为什么有些白痴不会在你身后出现并捣乱一切。

Just for kicks though, to get a better idea if the code is any shorter or cleaner one way or the other, I pulled out all the comments. They came out exactly the same number of lines. I went ahead and compiled the two versions and ran them through Reflector Code Metrics and got the following:

但是，为了获得更好的想法，如果代码是更短或更清洁的方式或其他方式，我提出了所有的评论。它们出现的线数完全相同。我继续编译了两个版本并通过Reflector Code Metrics运行它们并获得以下内容：

Metric: Inside-Catch / Outside-For
CodeSize: 197 / 185
CyclomaticComplexity: 3 / 3
Instructions: 79 / 80
Locals: 6 / 7

Final exception logic inside the catch (22 lines):

catch中的最终异常逻辑（22行）：

MyMethodResult result = null;
for (int retryAttempt = 1; retryAttempt <= Config.MaxRetryAttempts; retryAttempt++)
{
    try
    {
        result = SomeWebService.MyMethod(myId);
        break;
    }
    catch (Exception ex)
    {
        if (retryAttempt < Config.MaxRetryAttempts)
        {
            Logger.LogEvent(string.Format("Retry attempt #{0} for SomeWebService.MyMethod({1})", retryAttempt, myId);
            Thread.Sleep(retryAttempt * Config.RetrySleepSeconds * 1000);
        }
        else
        {
            Logger.LogError(string.Format("Max Retry Attempts Exceeded for SomeWebService.MyMethod({0})", MyId), ex);
            throw new Exception(string.Format("Max Retry Attempts Exceeded for SomeWebService.MyMethod({0})", myId), ex);
        }
    }
}

Final exception logic after the for-loop (22 lines):

for-loop之后的最终异常逻辑（22行）：

MyMethodResult result = null;
Exception retryException = null;
for (int retryAttempt = 1; retryAttempt <= Config.MaxRetryAttempts; retryAttempt++)
{
    try
    {
        result = SomeWebService.MyMethod(myId);
        retryException = null;
        break;
    }
    catch (Exception ex)
    {
        retryException = ex;
        Logger.LogEvent(string.Format("Retry attempt #{0} for SomeWebService.MyMethod({1})", retryAttempt, myId);
        Thread.Sleep(retryAttempt * Config.RetrySleepSeconds * 1000);
    }
}
if (retryException != null)
{
    Logger.LogError(string.Format("Max Retry Attempts Exceeded for SomeWebService.MyMethod({0})", MyId), ex);
    throw new Exception(string.Format("Max Retry Attempts Exceeded for SomeWebService.MyMethod({0})", myId), ex);
}

#7

I'm using the following generic method for a retry scenario. I especially want to draw attention to the PreserveStackTrace method which helps to preserve the full call stack trace, because (as I learned the hard way) neither throw or throw ex yields the complete call stack trace information.

我正在使用以下通用方法进行重试方案。我特别想提请注意PreserveStackTrace方法，它有助于保留完整的调用堆栈跟踪，因为（因为我学到了很难的方法），抛出或抛出ex都不会产生完整的调用堆栈跟踪信息。

public static void RetryBeforeThrow<T>(Action action, int retries, int timeout) where T : Exception
{
    int tries = 1;

    do
    {
        try
        {
            action();
            return;
        }
        catch (T ex)
        {
            if (retries <= 0)
            {
                PreserveStackTrace(ex);
                throw;
            }

            Thread.Sleep(timeout);
        }
    }
    while (tries++ < retries);
}

/// <summary>
/// Sets a flag on an <see cref="T:System.Exception"/> so that all the stack trace information is preserved 
/// when the exception is re-thrown.
/// </summary>
/// <remarks>This is useful because "throw" removes information, such as the original stack frame.</remarks>
/// <see href="http://weblogs.asp.net/fmarguerie/archive/2008/01/02/rethrowing-exceptions-and-preserving-the-full-call-stack-trace.aspx"/>
public static void PreserveStackTrace(Exception ex)
{
    MethodInfo preserveStackTrace = typeof(Exception).GetMethod("InternalPreserveStackTrace", BindingFlags.Instance | BindingFlags.NonPublic);
    preserveStackTrace.Invoke(ex, null);
}

#8

As everyone else has pointed out the correct approach is to wrap your try/catch inside some loop with a MAX_RETRY of some sort.

正如其他人都指出的那样，正确的方法是使用某种MAX_RETRY将try / catch包装在某个循环中。

You might also consider adding a timeout between each loop iteration. Otherwise you're likely to burn through your retry counter before the transient issue has had a chance to resolve itself.

您还可以考虑在每次循环迭代之间添加超时。否则，在瞬态问题有机会自行解决之前，您可能会烧掉重试计数器。

#9

It seems you have the answers you need, but I thought I'd post this link, What is an Action Policy?, that I found to provide a much more elegant solution. Lokad has some rather labyrinthine implementations, but the guy's logic is pretty solid, and the end code you'd end up writing is pretty and simple.

看来你有你需要的答案，但我想我会发布这个链接，什么是行动政策？，我发现它提供了一个更优雅的解决方案。 Lokad有一些相当迷宫的实现，但是这个人的逻辑非常可靠，而你最终编写的最终代码非常简单。

#10

int cnt=0;
bool cont = true;
while (cont)
{
     try
     {
         MDO = OperationsWebService.MessageDownload(MI);
         cont = false; 
     }
     catch (Exception ex) 
     { 
         ++cnt;
         if (cnt == 5)
         {
             // 5 retries, ok now log and deal with the error. 
            cont = false;
         }
     } 
}

UPDATED : Fixed code based on comments.

更新：基于评论的固定代码。