2010-02-16 15 views
21

Tengo un método de servicio web al que llamo, que es de terceros y está fuera de mi dominio. Por alguna razón, de vez en cuando el servicio web falla con un tiempo de espera de puerta de enlace. Es intermitente y puede invocarse directamente después de un intento fallido.¿Cómo puedo mejorar este escenario de reintento de excepción?

Ahora me queda un dilema de codificación, tengo un código que debería hacer el truco, pero el código parece una hora de aficionados, como verá a continuación.

¿Es este código realmente malo, o aceptable dado el uso? Si no es aceptable, ¿cómo puedo mejorarlo?

Intente por todos los medios mantener la cara seria mirándolo.

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

¡Solo tengo que darte +1 por el impresionante anidamiento! –

+0

@Alastair: Aparentemente, no. (La pregunta no tiene votos) – SLaks

+6

/me try {keep_straight_face} catch {laugh} finally {¡FAIL! } – t0mm13b

Respuesta

21

Puede hacerlo en un bucle.

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

+1 Me gusta el sueño –

+0

+1 yo también^___^ – Plynx

+0

+1 para el sueño y la respuesta aceptada .... –

12

Ésta es otra manera puede probar:

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

Creo que documenta el intento mejor. También es menos código; más fácil de mantener

+0

Eso parece mucho más ordenado, supongo que en la captura, compruebo el valor de MAX_RETRIES? –

+0

No creo que esto funcione. Entraría en el bloque catch en la primera excepción. – Plynx

+1

probablemente quiera un "descanso" al final del intento. :) – Joe

1

Pruebe un bucle, con algún tipo de límite:

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

ACTUALIZADO: Código fijo basado en los comentarios.

11

Todas las respuestas hasta ahora suponen que la reacción a cualquier excepción debería ser volver a intentar la operación. Esta es una buena suposición hasta que es una suposición falsa. Podría estar reintentando fácilmente una operación que está dañando su sistema, todo porque no verificó el tipo de excepción.

Debe casi Nunca utilizar un desnudo "catch", ni "catch (Exception ex) capturar una excepción más específica -.. Uno que sabe que puede recuperar de forma segura desde

+1

Buen punto. Pero, ¿no se levanta una excepción antes de que dañe el sistema? ¿No es ese el punto? – Plynx

+1

@Plynx: Estaba exagerando un poco. La operación que se está reintentando puede haber iniciado un procesamiento costoso, y luego arrojado una excepción, que indica "disco lleno" o algo más que significa que la operación nunca tendrá éxito. Reintentarlo solo desperdiciaría los recursos del sistema y nunca tendría éxito. –

+0

+1 En el escenario del autor, él declara que el problema es de comunicaciones, no se trata del sistema local. Sin embargo, estoy de acuerdo con el razonamiento general. –

1

Debe utilizar la recursividad (o un bucle) , y sólo debe volver a intentar si tienes el error que esperaba

Por ejemplo:.

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

EDITAR: Con una 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; 
     } 
    } 
} 

Usted puede tratar de prevenir futuros errores en retryFilter, tal vez por Thread.Sleep.

Si falla el último intento, arrojará la última excepción.

+0

Como casi todos los demás mencionaron, puedes, y probablemente deberías, usar un ciclo. – SLaks

+0

pero la recursión a veces es divertida ... –

0

Como todos los demás han señalado, el enfoque correcto es ajustar su try/catch dentro de algún ciclo con un MAX_RETRY de algún tipo.

También podría considerar agregar un tiempo de espera entre cada iteración de bucle. De lo contrario, es probable que abras el contador de reintentos antes de que el problema transitorio haya tenido la oportunidad de resolverse.

0

Parece que tiene las respuestas que necesita, pero pensé en publicar este enlace, What is an Action Policy?, que encontré para proporcionar una solución mucho más elegante. Lokad tiene algunas implementaciones bastante laberínticas, pero la lógica del tipo es bastante sólida, y el código final que terminarías escribiendo es bonito y simple.

1

Aquí hay alguna lógica de reintento que estamos usando. No hacemos esto mucho y lo sacaremos y lo documentaremos como nuestro Patrón/Patrón de Reintentos. Tuve que aletear cuando lo escribí por primera vez, así que vine aquí para ver si lo estaba haciendo correctamente. Parece que estaba. La siguiente versión está completamente comentada. Vea a continuación eso para una versión sin comentario.

#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 

Me gusta el ejemplo de Samuel Neff de utilizar una variable de excepción para ver si falló completamente o no. Eso habría hecho algunas de las evaluaciones en mi lógica un poco más simples. Podría ir de cualquier manera. No estoy seguro de que de ninguna manera tenga una ventaja significativa sobre la otra. Sin embargo, en este momento, no voy a cambiar cómo lo hacemos. Lo importante es documentar lo que estás haciendo y por qué para que un idiota no venga detrás de ti y ensucie todo.

Sin embargo, para dar una patada, para obtener una mejor idea si el código es más corto o más limpio en un sentido u otro, saqué todos los comentarios. Salieron exactamente el mismo número de líneas. Seguí adelante y compilado las dos versiones y las pasó por Código Reflector Métrica y obtuve el siguiente:

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

lógica excepción final dentro del retén (22 líneas):

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

lógica excepción final después de la for-loop (22 líneas):

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

Estoy utilizando el siguiente método genérico para un escenario de reintento. Especialmente quiero llamar la atención sobre el método PreserveStackTrace que ayuda a preservar el seguimiento completo de la pila de llamadas, porque (como aprendí por las malas) ni throw ni throw ex arrojan la información completa del rastreo de la pila de llamadas.

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