Try / Catch / Log / Rethrow - ¿Es Anti Pattern?

19

Puedo ver varias publicaciones donde se enfatizó la importancia de manejar la excepción en la ubicación central o en el límite del proceso como una buena práctica en lugar de ensuciar cada bloque de código alrededor de try / catch. Creo firmemente que la mayoría de nosotros entendemos la importancia de esto, sin embargo, veo que la gente todavía termina con el patrón anti captura-registro-relanzamiento principalmente porque para facilitar la resolución de problemas durante cualquier excepción, desean registrar más información específica del contexto (ejemplo: parámetros del método pasado) y la forma es envolver el método alrededor de try / catch / log / rethrow.

public static bool DoOperation(int num1, int num2)
{
    try
    {
        /* do some work with num1 and num2 */
    }
    catch (Exception ex)
    {
        logger.log("error occured while number 1 = {num1} and number 2 = {num2}"); 
        throw;
    }
}

¿Hay una manera correcta de lograr esto mientras se mantienen las buenas prácticas de manejo de excepciones? Escuché sobre el marco de AOP como PostSharp para esto, pero me gustaría saber si hay algún inconveniente o un costo de rendimiento importante asociado con estos marcos de AOP.

¡Gracias!

rahulaga_dev
fuente
66
Hay una gran diferencia entre envolver cada método en try / catch, registrar la excepción y dejar que el código avance. E intente / atrapar y volver a lanzar la excepción con información adicional. Primero es una práctica terrible. En segundo lugar, está perfectamente bien para mejorar su experiencia de depuración.
Eufórico
Estoy diciendo que intente / atrape cada método y simplemente inicie sesión en el bloque de captura y vuelva a lanzar: ¿está bien?
rahulaga_dev
2
Como lo señaló Amon. Si su idioma tiene rastros de pila, iniciar sesión en cada captura no tiene sentido. Pero envolver la excepción y agregar información adicional es una buena práctica.
Eufórico
1
Ver la respuesta de @ Liath. Cualquier respuesta que diera sería muy similar a la suya: detecte excepciones lo antes posible y si todo lo que puede hacer en esa etapa es registrar alguna información útil, hágalo y vuelva a lanzarla. Ver esto como un antipatrón no tiene sentido en mi opinión.
David Arno
1
Liath: fragmento de código pequeño agregado. Estoy usando c #
rahulaga_dev

Respuestas:

19

El problema no es el bloqueo local, el problema es el registro y la repetición . Maneje la excepción o envuélvala con una nueva excepción que agregue contexto adicional y tírela. De lo contrario, se encontrará con varias entradas de registro duplicadas para la misma excepción.

La idea aquí es mejorar la capacidad de depurar su aplicación.

Ejemplo # 1: manejarlo

try
{
    doSomething();
}
catch (Exception e)
{
    log.Info("Couldn't do something", e);
    doSomethingElse();
}

Si maneja la excepción, puede degradar fácilmente la importancia de la entrada del registro de excepciones y no hay razón para filtrar esa excepción en la cadena. Ya está solucionado.

Manejar una excepción puede incluir informar a los usuarios que ocurrió un problema, registrar el evento o simplemente ignorarlo.

NOTA: si ignora intencionalmente una excepción, le recomiendo que proporcione un comentario en la cláusula catch vacía que indique claramente por qué. Esto les permite a los futuros mantenedores saber que no fue un error o una programación perezosa. Ejemplo:

try
{
    context.DrawLine(x1,y1, x2,y2);
}
catch (OutOfMemoryException)
{
    // WinForms throws OutOfMemory if the figure you are attempting to
    // draw takes up less than one pixel (true story)
}

Ejemplo # 2: Agregar contexto adicional y lanzar

try
{
    doSomething(line);
}
catch (Exception e)
{
    throw new MyApplicationException(filename, line, e);
}

Agregar contexto adicional (como el número de línea y el nombre del archivo en el código de análisis) puede ayudar a mejorar la capacidad de depurar archivos de entrada, suponiendo que el problema esté ahí. Este es un caso especial, por lo que volver a empaquetar una excepción en una "ApplicationException" solo para cambiar el nombre no le ayuda a depurar. Asegúrese de agregar información adicional.

Ejemplo 3: no hagas nada con la excepción

try
{
    doSomething();
}
finally
{
   // cleanup resources but let the exception percolate
}

En este caso final, solo permite que la excepción se vaya sin tocarla. El controlador de excepciones en la capa más externa puede manejar el registro. La finallycláusula se utiliza para asegurarse de que se limpien los recursos necesarios para su método, pero este no es el lugar para registrar que se produjo la excepción.

Berin Loritsch
fuente
Me gustó " El problema no es el bloqueo local, el problema es el registro y volver a lanzar " Creo que esto tiene mucho sentido para garantizar un registro más limpio. Pero eventualmente también significa estar bien con try / catch disperso en todos los métodos, ¿no es así? Creo que debe haber alguna directriz para garantizar que esta práctica se siga con criterio en lugar de que todos los métodos lo hagan.
rahulaga_dev
Proporcioné una directriz en mi respuesta. ¿Esto no responde a tus preguntas?
Berin Loritsch
@rahulaga_dev No creo que haya una guía / viñeta plateada, así que resuelva este problema, ya que depende en gran medida del contexto. No puede haber una directriz general que le indique dónde manejar una excepción o cuándo volver a lanzarla. En mi opinión, la única guía que veo es diferir el registro / manejo al último tiempo posible y evitar iniciar sesión en el código reutilizable, de modo que no cree dependencias innecesarias. Los usuarios de su código no se divertirían demasiado si registrara cosas (es decir, manejara excepciones) sin darles la oportunidad de manejarlas a su manera. Just my two cents :)
andreee
7

No creo que las capturas locales sean un antipatrón, de hecho, si recuerdo correctamente, ¡en realidad se aplica en Java!

Lo que es clave para mí al implementar el manejo de errores es la estrategia general. Es posible que desee un filtro que capture todas las excepciones en el límite del servicio, puede interceptarlas manualmente; ambas están bien siempre que haya una estrategia general que se ajuste a los estándares de codificación de sus equipos.

Personalmente, me gusta detectar errores dentro de una función cuando puedo hacer uno de los siguientes:

  • Agregue información contextual (como el estado de los objetos o lo que estaba sucediendo)
  • Maneje la excepción de forma segura (como un método TryX)
  • Su sistema está cruzando un límite de servicio y está llamando a una biblioteca externa o API
  • Desea capturar y volver a lanzar un tipo diferente de excepción (quizás con el original como excepción interna)
  • La excepción se lanzó como parte de alguna funcionalidad de fondo de bajo valor

Si no es uno de estos casos, no agrego un try / catch local. Si es así, dependiendo del escenario, puedo manejar la excepción (por ejemplo, un método TryX que devuelve un falso) o volver a lanzar para que la estrategia global maneje la excepción.

Por ejemplo:

public bool TryConnectToDatabase()
{
  try
  {
    this.ConnectToDatabase(_databaseType); // this method will throw if it fails to connect
    return true;
  }
  catch(Exception ex)
  {
     this.Logger.Error(ex, "There was an error connecting to the database, the databaseType was {0}", _databaseType);
    return false;
  }
}

O un ejemplo de repensar:

public IDbConnection ConnectToDatabase()
{
  try
  {
    // connect to the database and return the connection, will throw if the connection cannot be made
  }
  catch(Exception ex)
  {
     this.Logger.Error(ex, "There was an error connecting to the database, the databaseType was {0}", _databaseType);
    throw;
  }
}

Luego, detecta el error en la parte superior de la pila y presenta un mensaje agradable al usuario.

Independientemente del enfoque que adopte, siempre vale la pena crear pruebas unitarias para estos escenarios para que pueda asegurarse de que la funcionalidad no cambie e interrumpa el flujo del proyecto en una fecha posterior.

No ha mencionado en qué idioma está trabajando, pero es un desarrollador de .NET y lo ha visto demasiadas veces sin mencionarlo.

NO ESCRIBA:

catch(Exception ex)
{
  throw ex;
}

Utilizar:

catch(Exception ex)
{
  throw;
}

¡El primero restablece el rastro de la pila y hace que su nivel superior de captura sea completamente inútil!

TLDR

La captura local no es un antipatrón, a menudo puede ser parte de un diseño y puede ayudar a agregar contexto adicional al error.

Liath
fuente
33
¿Cuál es el punto de inicio de sesión en la captura, cuando se utilizaría el mismo registrador en el controlador de excepciones de nivel superior?
Eufórico
Es posible que tenga información adicional (como las variables locales) a la que no tendrá acceso en la parte superior de la pila. Actualizaré el ejemplo para ilustrar.
Liath
2
En ese caso, arroje una nueva excepción con datos adicionales y una excepción interna.
Eufórico
2
@ Sí, también lo he visto, personalmente no soy un fanático, ya que requiere que crees nuevos tipos de excepción para casi todos los métodos / escenarios, lo que creo es una gran carga. Agregar la línea de registro aquí (y presumiblemente otra en la parte superior) ayuda a ilustrar el flujo de código al diagnosticar problemas
Liath
44
Java no te obliga a manejar la excepción, te obliga a ser consciente de ello. puedes atraparlo y hacer lo que sea o simplemente declararlo como algo que la función puede arrojar y no hacer nada con él en la función ... ¡Un pequeño error en una respuesta bastante buena!
Newtopian
4

Esto depende mucho del idioma. Por ejemplo, C ++ no ofrece rastreos de pila en mensajes de error de excepción, por lo que puede ser útil rastrear la excepción a través de capturas, registros y repeticiones frecuentes. Por el contrario, Java y lenguajes similares ofrecen muy buenos seguimientos de pila, aunque el formato de estos seguimientos de pila puede no ser muy configurable. Capturar y volver a generar excepciones en estos lenguajes no tiene ningún sentido a menos que realmente pueda agregar algún contexto importante (por ejemplo, conectar una excepción SQL de bajo nivel con el contexto de una operación de lógica de negocios).

Cualquier estrategia de manejo de errores que se implemente a través de la reflexión es casi necesariamente menos eficiente que la funcionalidad integrada en el lenguaje. Además, el registro generalizado tiene una sobrecarga de rendimiento inevitable. Por lo tanto, debe equilibrar el flujo de información que obtiene con otros requisitos para este software. Dicho esto, las soluciones como PostSharp que se basan en instrumentación a nivel de compilador generalmente funcionarán mucho mejor que la reflexión en tiempo de ejecución.

Personalmente, creo que registrar todo no es útil, porque incluye mucha información irrelevante. Por lo tanto, soy escéptico de las soluciones automatizadas. Dado un buen marco de registro, podría ser suficiente tener una directriz de codificación acordada que discuta qué tipo de información le gustaría registrar y cómo se debe formatear esta información. Luego puede agregar el registro donde sea importante.

Iniciar sesión en la lógica empresarial es mucho más importante que iniciar sesión en las funciones de utilidad. Y la recopilación de trazas de la pila de informes de fallas del mundo real (que solo requiere el registro en el nivel superior de un proceso) le permite ubicar áreas del código donde el registro tendría el mayor valor.

amon
fuente
4

Cuando veo try/catch/logen cada método, surge la preocupación de que los desarrolladores no tenían idea de lo que podría suceder o no en su aplicación, asumieron lo peor y registraron preventivamente todo en todas partes debido a todos los errores que esperaban.

Este es un síntoma de que las pruebas de unidad e integración son insuficientes y los desarrolladores están acostumbrados a pasar por un montón de código en el depurador y esperan que de alguna manera una gran cantidad de registros les permita implementar código defectuoso en un entorno de prueba y encontrar los problemas observando los registros.

El código que arroja excepciones puede ser más útil que el código redundante que captura y registra excepciones. Si lanza una excepción con un mensaje significativo cuando un método recibe un argumento inesperado (y lo registra en el límite del servicio) es mucho más útil que registrar inmediatamente la excepción lanzada como un efecto secundario del argumento no válido y tener que adivinar qué lo causó .

Los nulos son un ejemplo. Si obtiene un valor como argumento o el resultado de una llamada a un método y no debe ser nulo, arroje la excepción. No solo registre el resultado resultante NullReferenceExceptioncinco líneas más tarde debido al valor nulo. De cualquier manera, obtienes una excepción, pero una te dice algo mientras que la otra te hace buscar algo.

Como han dicho otros, es mejor registrar excepciones en el límite del servicio, o cada vez que una excepción no se vuelve a lanzar porque se maneja con gracia. La diferencia más importante es entre algo y nada. Si sus excepciones se registran en un lugar de fácil acceso, encontrará la información que necesita cuando la necesita.

Scott Hannen
fuente
Gracias Scott El punto que hizo " Si lanza una excepción con un mensaje significativo cuando un método recibe un argumento inesperado (y lo registra en el límite del servicio) " realmente golpeó y me ayudó a visualizar la situación que se cierne a mi alrededor en el contexto de los argumentos del método. Creo que tiene sentido tener una cláusula de seguridad y lanzar ArgumentException en ese caso en lugar de confiar en la captura y el registro de los detalles del argumento
rahulaga_dev
Scott, tengo el mismo sentimiento. Cada vez que veo log y vuelvo a pensar solo para registrar el contexto, siento que los desarrolladores no pueden controlar los invariantes de la clase o no pueden salvaguardar la llamada al método. En cambio, todos los métodos hasta el final están envueltos en un intento / captura / registro / lanzamiento similar. Y es simplemente terrible.
Max
2

Si necesita registrar información de contexto que aún no está en la excepción, envuélvala en una nueva excepción y proporcione la excepción original como InnerException. De esa manera, aún conserva el rastro original de la pila. Entonces:

public static bool DoOperation(int num1, int num2)
{
    try
    {
        /* do some work with num1 and num2 */
    }
    catch (Exception ex)
    {
        throw new Exception("error occured while number 1 = {num1} and number 2 = {num2}", ex);
    }
}

El segundo parámetro para el Exceptionconstructor proporciona una excepción interna. Luego, puede registrar todas las excepciones en un solo lugar y aún así obtener el seguimiento completo de la pila y la información contextual en la misma entrada de registro.

Es posible que desee utilizar una clase de excepción personalizada, pero el punto es el mismo.

try / catch / log / rethrow es un desastre porque conducirá a registros confusos, por ejemplo, ¿qué sucede si ocurre una excepción diferente en otro hilo entre el registro de la información contextual y el registro de la excepción real en el controlador de nivel superior? Sin embargo, try / catch / throw está bien, si la nueva excepción agrega información al original.

JacquesB
fuente
¿Qué pasa con el tipo de excepción original? Si terminamos, se ha ido. ¿Es un problema? Alguien confiaba en la SqlTimeoutException por ejemplo.
Max
@Max: el tipo de excepción original todavía está disponible como excepción interna.
JacquesB
¡Eso es lo que quiero decir! Ahora, todos los que están en la pila de llamadas, que estaban recibiendo la SqlException, nunca lo obtendrán.
Max
1

La excepción en sí misma debe proporcionar toda la información necesaria para un registro adecuado, incluido el mensaje, el código de error y lo que no. Por lo tanto, no debería haber necesidad de detectar una excepción solo para volver a lanzarla o lanzar una excepción diferente.

A menudo verá el patrón de varias excepciones capturadas y relanzadas como una excepción común, como la captura de una excepción DatabaseConnectionException, una InvalidQueryException y una InvalidSQLParameterException y volver a lanzar una DatabaseException. Aunque para esto, diría que todas estas excepciones específicas deberían derivarse de DatabaseException en primer lugar, por lo que no es necesario volver a lanzarlas.

Descubrirá que eliminar las cláusulas innecesarias de prueba de captura (incluso las que tienen fines de registro) en realidad facilitará el trabajo, no lo hará más difícil. Solo los lugares en su programa que manejan la excepción deben registrar la excepción y, si todo lo demás falla, un controlador de excepciones para todo el programa para un intento final de registrar la excepción antes de salir con gracia del programa. La excepción debe tener un seguimiento completo de la pila que indique el punto exacto en el que se produjo la excepción, por lo que a menudo no es necesario proporcionar un registro de "contexto".

Dicho esto, AOP puede ser una solución de solución rápida para usted, aunque generalmente implica una ligera desaceleración en general. Le animo a que, en su lugar, elimine las cláusulas innecesarias de try catch por completo cuando no se agrega nada de valor.

Neil
fuente
1
" La excepción en sí misma debe proporcionar toda la información necesaria para un registro adecuado, incluido el mensaje, el código de error y lo que no ". Deberían, pero en la práctica, no hacen referencia a excepciones nulas como un caso clásico. No conozco ningún lenguaje que le diga la variable que lo causó dentro de una expresión compleja, por ejemplo.
David Arno
1
@DavidArno Es cierto, pero cualquier contexto que pueda proporcionar tampoco podría ser tan específico. De lo contrario lo habría hecho try { tester.test(); } catch (NullPointerException e) { logger.error("Variable tester was null!"); }. El seguimiento de la pila es suficiente en la mayoría de los casos, pero al carecer de eso, el tipo de error suele ser adecuado.
Neil