son - propiedades de las excepciones en c#

¿Es esta una mala práctica detectar una excepción no específica como System.Exception? ¿Por qué?

Question

Actualmente estoy haciendo una revisión del código y el siguiente código me hizo saltar. Veo varios problemas con este código. ¿Estas de acuerdo conmigo? Si es así, ¿cómo le explico a mi colega que esto es incorrecto (tipo testarudo ...)?

• Captura una excepción genérica (Excepción ex)
• El uso de "if (ex is something)" en lugar de tener otro bloque catch
• Comemos SoapException, HttpException y WebException. Pero si el servicio web falló, no hay mucho que hacer.

Código:

try
{
    // Call to a WebService
}
catch (Exception ex)
{
    if (ex is SoapException || ex is HttpException || ex is WebException)
    {
        // Log Error and eat it.
    }
    else
    {
        throw;
    }
}

Answer 1

El mantra es:

• Solo debe detectar excepciones si puede manejarlas adecuadamente

Así:

• No deberías atrapar excepciones generales.

En su caso, sí, debería simplemente tomar esas excepciones y hacer algo útil (probablemente no solo comerlas, podría throw después de que las haya registrado).

Tu codificador está usando throw (no throw ex ) que es bueno .

Así es como puedes atrapar múltiples excepciones específicas:

try
{
    // Call to a WebService
}
catch (SoapException ex)
{
    // Log Error and eat it
}
catch (HttpException ex)
{
    // Log Error and eat it
}
catch (WebException ex)
{
    // Log Error and eat it
}

Esto es más o menos equivalente a lo que hace tu código. Su desarrollador probablemente lo hizo de esa manera para evitar la duplicación de los bloques de "error de registro y cómelo".

Answer 2

El problema con atrapar y volver a lanzar la misma excepción es que, aunque .NET hace todo lo posible para mantener intacto el registro de la pila, siempre termina siendo modificado, lo que puede dificultar el seguimiento de la excepción en realidad (por ejemplo, la excepción). el número de línea probablemente parezca ser la línea de la declaración de reenvío en lugar de la línea donde originalmente se generó la excepción). Hay toda una carga de más información sobre la diferencia entre catch / rethrow, filtrado y no captura aquí.

Cuando hay una lógica duplicada como esta, lo que realmente necesita es un filtro de excepción, por lo que solo detecta los tipos de excepción que le interesan. VB.NET tiene esta funcionalidad, pero desafortunadamente C # no. Una sintaxis hipotética podría ser similar a:

try
{
    // Call to a WebService
}
catch (Exception ex) if (ex is SoapException || ex is HttpException || /* etc. */)
{
    // Log Error and eat it
}

Sin embargo, como no puede hacer esto, lo que suelo hacer es usar una expresión lambda para el código común (podría usar un delegate en C # 2.0), por ejemplo

Action<Exception> logAndEat = ex => 
    {  
        // Log Error and eat it
    };

try
{
    // Call to a WebService
}
catch (SoapException ex)
{
    logAndEat(ex);
}
catch (HttpException ex)
{
    logAndEat(ex);
}
catch (WebException ex)
{
    logAndEat(ex);
}

Answer 3

Por lo general, aún debe detectar excepciones genéricas en un controlador global (que es el lugar perfecto para registrar Excepciones inesperadas), pero, como se dijo anteriormente, solo debería detectar tipos de excepciones específicos en otros lugares si planea hacer algo con ellos. Los bloques catch deben buscar esos tipos de excepciones explícitamente, no como lo hace el código que publicaste.

Answer 4

Actualmente estoy haciendo una revisión del código y el siguiente código me hizo saltar. Veo varios problemas con este código. ¿Estas de acuerdo conmigo?

No totalmente, mira abajo.

• Captura una excepción genérica (Excepción ex)

En general, capturar una excepción genérica está realmente bien siempre y cuando la vuelvas a lanzar (con throw;) cuando llegues a la conclusión de que no puedes manejarla. El código hace eso, entonces no hay problema inmediato aquí.

• El uso de "if (ex is something)" en lugar de tener otro bloque catch

El efecto neto del bloque catch es que solo SoapException, HttpException, etc. se manejan realmente y todas las demás excepciones se propagan por la pila de llamadas. Supongo que, en lo que respecta a la funcionalidad, esto es lo que se supone que debe hacer el código, por lo que tampoco hay problema aquí.

Sin embargo, desde un punto de vista de la estética y la legibilidad preferiría múltiples bloques catch al "if (ex es SoapException || ..)". Una vez que refactorice el código de manejo común en un método, los múltiples bloques de captura son solo un poco más tipados y son más fáciles de leer para la mayoría de los desarrolladores. Además, el lanzamiento final se pasa por alto fácilmente.

• Comemos SoapException, HttpException y WebException. Pero si el servicio web falló, no hay mucho que hacer.

Aquí posiblemente aceche el mayor problema del código, pero es difícil dar consejos sin saber más sobre la naturaleza de la aplicación. Si la llamada al servicio web está haciendo algo de lo que usted depende más adelante, entonces probablemente sea incorrecto simplemente registrar y comer las excepciones. Por lo general, permite que la excepción se propague a la persona que llama (generalmente después de envolverla, por ejemplo, en una XyzWebServiceDownException), tal vez incluso después de volver a intentar la llamada al servicio web varias veces (para ser más robusto cuando hay problemas de red espurios).

Answer 5

A veces es la única manera de manejar situaciones de "atrapar todas las excepciones", sin detectar realmente todas las excepciones.

Creo que a veces, por ejemplo, el marco / código de tiempo de ejecución de bajo nivel necesita asegurarse de que nunca se escape ninguna excepción. Desafortunadamente, tampoco hay forma de que el código de la infraestructura pueda saber qué excepciones surgen por el código ejecutado por la secuencia.

En ese caso, un posible bloqueo catch podría verse así:

try
{
   // User code called here
}
catch (Exception ex)
{
   if (ExceptionIsFatal(ex))
     throw;

   Log(ex);
}

Sin embargo, hay tres puntos importantes aquí:

1. Esto no es algo para cada situación. En las revisiones de código, somos muy exigentes con los lugares donde esto es realmente necesario y, por lo tanto, permitido.
2. El método ExceptionIsFatal () asegura que no comemos excepciones que nunca se deben tragar (ExecutionEngineException, OutOfMemoryException, ThreadAbortException, etc.)
3. Lo que se traga se registra cuidadosamente (event-log, log4net, YMMV)

Por lo general, estoy dispuesto a permitir que las excepciones no detectadas simplemente "bloqueen" la aplicación terminando el CLR. Sin embargo, especialmente en aplicaciones de servidor, esto a veces no es sensato. Si un hilo encuentra un problema que se considera no fatal, no hay razón para desmantelar todo el proceso, eliminando todas las demás solicitudes en ejecución (WCF, por ejemplo, maneja algunos casos de esta manera).

Answer 6

Me gustaría agregar aquí, porque el manejo de excepciones en casi todos los códigos java / C # que he visto es simplemente incorrecto. Es decir, termina con un error muy difícil de depurar para Excepciones ignoradas o, igualmente malo, obtiene una excepción oscura que no le dice nada, porque seguir ciegamente la "captura (Excepción) es malo" y las cosas simplemente son peores.

Primero, entienda que una excepción es una forma de facilitar la devolución de información de error a través de las capas de código. Ahora, error 1: una capa no es solo un marco de pila, una capa es un código que tiene una responsabilidad bien definida. Si acaba de codificar interfaces e impls solo porque, bueno, tiene mejores cosas que arreglar.

Si las capas están bien diseñadas y tienen responsabilidades específicas, entonces la información del error tiene un significado diferente a medida que surge. <-esta es la clave sobre qué hacer, no hay una regla universal.

Entonces, esto significa que cuando se produce una excepción, tienes 2 opciones, pero necesitas entender en qué parte de la capa estás:

A) Si estás en el medio de una capa, y eres solo una función de ayuda interna, normalmente privada, y algo va mal: NO TE PREOCUPES, deja que la persona que llama reciba la excepción. Está perfectamente bien porque no tiene contexto comercial y 1) No está ignorando el error y 2) La persona que llama es parte de su capa y debería haber sabido que esto puede suceder, pero puede que ahora no tenga contexto para manejarlo abajo.

o ...

B) Usted es el límite superior de la capa, la fachada hacia el interior. Luego, si obtiene una excepción, el valor predeterminado será CAPTURAR TODO y evitar que cualquier excepción específica cruce a la capa superior, lo que no tendrá sentido para la persona que llama, o peor aún, puede cambiar y la persona que llama tendrá una dependencia a una implementación detalle y ambos se romperán.

La fuerza de una aplicación es el nivel de desacoplamiento entre las capas. Aquí va a detener todo como una regla general y volver a lanzar el error con una excepción genérica traduciendo la información a un error más significativo para la capa superior.

REGLA: Todos los puntos de entrada a una capa deben estar protegidos con CATCH ALL y todos los errores traducidos o manejados. Ahora bien, este ''manejado'' ocurre solo el 1% del tiempo, la mayoría de las veces solo necesitas (o puedes) devolver el error en una abstracción correcta.

No, estoy seguro de que esto es muy difícil de entender. Ejemplo real ->

Tengo un paquete que ejecuta algunas simulaciones. Estas simulaciones están en scripts de texto. hay un paquete que compila estos scripts y hay un paquete utils genérico que solo lee archivos de texto y, por supuesto, el RTL base java. La dependencia de UML es->

Simulador-> Compilador-> utilsTextLoader-> Archivo Java

1) Si algo se rompe en el cargador utils dentro de un privado y obtengo un FileNotFound, Permisos o lo que sea, déjalo pasar. No hay nada más que puedas hacer.

2) En el límite, en la función utilsTextLoader inicialmente llamada, usted seguirá la regla anterior y CATCH_ALL. El compilador no se preocupa por lo que sucede, solo necesita ahora si el archivo se cargó o no. Entonces, en la captura, vuelva a lanzar una nueva excepción y traduzca FileNotFound o lo que sea a "No se pudo leer el archivo XXXX".

3) El compilador ahora sabrá que la fuente no se cargó. Eso es TODO lo que necesita saber. Entonces, si más tarde cambio utilsTestLoader para cargar desde la red, el compilador no cambiará. Si suelta FileNotFound y luego cambia, impactará en el compilador por nada.

4) El ciclo se repite: la función real que llamó a la capa inferior del archivo no hará nada al obtener la excepción. Entonces lo deja subir.

5) Cuando la excepción llega a la capa entre el simulador y el compilador, el compilador vuelve a CATCHES_ALL, ocultando cualquier detalle y arrojando un error más específico: "No se pudo compilar el script XXX".

6) Finalmente, repita el ciclo una vez más, la función del simulador que llamó al compilador simplemente lo suelta.

7) El límite final es para el usuario. El usuario es una CAPA y todo se aplica. El principal tiene un intento que atrapa_ALL y finalmente solo hace un buen cuadro de diálogo o página y "arroja" un error traducido al usuario.

Entonces el usuario ve.

Simulador: el error fatal no pudo iniciar el simulador

-Compiler: no se pudo compilar el script FOO1

--TextLoader: no se pudo leer el archivo foo1.scp

--- trl: FileNotFound

Comparar con:

a) Compilador: NullPointer Exception <- caso común y una noche perdida que elimina errores de nombre de archivo

b) Loader: archivo no encontrado <- ¿He mencionado que el cargador carga cientos de scripts?

o

c) ¡No pasa nada porque todo fue ignorado!

Por supuesto, esto supone que en cada nuevo lanzamiento no olvidó establecer la excepción de causa.

Bueno, mis 2cts. Estas simples reglas me han salvado la vida muchas veces ...

-Cerveza inglesa

Answer 7

la princeple es solo para atrapar la excepción que puedes manejar. por ejemplo, si sabes cómo tratar con findnotfound, tomas el archivo no de la excepción de otro exponente, no lo tomas y lo lanzas a la capa superior.

Answer 8

No creo que esto sea tan malo en este caso, pero también hago algo similar en mi código con excepciones que se pueden ignorar de forma segura y que el resto se vuelven a lanzar. Como notó la respuesta de Michael , el hecho de que cada captura sea un bloque separado podría causar algunos problemas de legibilidad que se evitan yendo por esta ruta.

En lo que respecta a señalar esto a su colega, creo que le sería difícil convencerlos de que esta es la forma incorrecta de hacer las cosas, más aún si son obstinadas, debido a los posibles problemas de legibilidad para hacer las cosas al revés. . Dado que esta versión todavía arroja las excepciones genéricas que no pueden manejarse, está en consonancia con el espíritu de la práctica. Sin embargo, si el código estaba en línea con lo siguiente:

try
{
  // Do some work
}
catch (Exception ex)
{
  if (ex is SoapException)
  {
    // SoapException specific recovery actions
  }
  else if (ex is HttpException)
  {
    // HttpException specific recovery actions
  }
  else if (ex is WebException)
  {
    // WebException specific recoery actions
  }
  else
  {
    throw;
  }
}

Entonces creo que tendrías un poco más de razón para preocuparte ya que no tiene sentido hacer un trabajo para una excepción específica al buscarlo en un bloque de excepción general.