Try / Catch / Log / Rethrow - É anti-padrão?

19

Eu posso ver várias postagens em que a importância de lidar com exceção no local central ou no limite do processo foi enfatizada como uma boa prática, em vez de desarrumar todos os blocos de código em torno de try / catch. Acredito firmemente que a maioria de nós entende a importância disso, no entanto, vejo pessoas ainda terminando com anti-padrão catch-log-rehrow principalmente porque, para facilitar a solução de problemas durante qualquer exceção, elas desejam registrar mais informações específicas do contexto (exemplo: parâmetros de método passado) ea maneira é envolver o método em torno de try / catch / log / rehrow.

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

Existe maneira correta de conseguir isso, mantendo as boas práticas de manipulação de exceções? Ouvi falar da estrutura da AOP, como o PostSharp, mas gostaria de saber se há alguma desvantagem ou custo de desempenho importante associado a essas estruturas da AOP.

Obrigado!

rahulaga_dev
fonte
6
Há uma enorme diferença entre agrupar cada método em try / catch, registrar a exceção e deixar o código seguir. E tente / capture e repita a exceção com informações adicionais. Primeiro é uma prática terrível. O segundo é a maneira perfeita de melhorar sua experiência de depuração.
eufórico
Estou dizendo tentar / pegar cada método e simplesmente logar pegar bloco e relançar - isso está bem para fazer?
Rahulaga_dev 6/02
2
Como apontado por Amon. Se o seu idioma tiver rastreamentos de pilha, o login em cada captura não fará sentido. Mas quebrar a exceção e adicionar informações adicionais é uma boa prática.
eufórico
1
Veja a resposta de @ Liath. Qualquer resposta que eu desse refletiria muito bem a dele: capte exceções o mais cedo possível e se tudo o que você puder fazer nesse estágio for registrar algumas informações úteis, faça-o e repita o processo. Vendo isso como um antipadrão não faz sentido na minha opinião.
David Arno
1
Liath: adicionado pequeno trecho de código. Eu estou usando c # #
rahulaga_dev

Respostas:

19

O problema não é o bloco de captura local, o problema é o log e a repetição . Manipule a exceção ou envolva-a com uma nova exceção que adicione contexto adicional e ative isso. Caso contrário, você encontrará várias entradas de log duplicadas para a mesma exceção.

A idéia aqui é aprimorar a capacidade de depurar seu aplicativo.

Exemplo 1: lidar com isso

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

Se você manipular a exceção, poderá reduzir facilmente a importância da entrada do log de exceções e não há motivo para percorrer essa exceção na cadeia. Já está resolvido.

O tratamento de uma exceção pode incluir informar aos usuários que ocorreu um problema, registrar o evento ou simplesmente ignorá-lo.

NOTA: se você intencionalmente ignorar uma exceção, recomendo fornecer um comentário na cláusula de captura vazia que indique claramente o motivo. Isso permite que futuros mantenedores saibam que não foi um erro ou uma programação lenta. Exemplo:

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

Exemplo 2: Adicione contexto adicional e jogue

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

Adicionar contexto adicional (como o número da linha e o nome do arquivo no código de análise) pode ajudar a aprimorar a capacidade de depurar arquivos de entrada - supondo que o problema esteja lá. Esse é um caso especial, portanto, reorganizar uma exceção em uma "ApplicationException" apenas para mudar a marca, não ajuda a depurar. Certifique-se de adicionar informações adicionais.

Exemplo # 3: Não faça nada com a exceção

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

Neste caso final, você apenas permite que a exceção saia sem tocá-la. O manipulador de exceção na camada mais externa pode lidar com o log. A finallycláusula é usada para garantir que todos os recursos necessários ao seu método sejam limpos, mas este não é o local para registrar que a exceção foi lançada.

Berin Loritsch
fonte
Gostei de " O problema não é o bloco de captura local, o problema é o registro e a repetição " Acho que isso faz todo o sentido para garantir um registro mais limpo. Mas, eventualmente, também significa estar bem com o try / catch sendo espalhado por todos os métodos, não é? Eu acho que deve haver alguma orientação para garantir que essa prática seja seguida criteriosamente, em vez de ter todos os métodos.
Rahulaga_dev 7/02
Eu forneci uma diretriz na minha resposta. Isso não responde às suas perguntas?
Berin Loritsch
@rahulaga_dev Eu não acho que exista uma diretriz / bala de prata, então resolva esse problema, pois depende muito do contexto. Não pode haver uma diretriz geral que indique onde lidar com uma exceção ou quando repeti-la novamente. Na IMO, a única diretriz que vejo é adiar o log / manuseio para o tempo mais recente possível e evitar o log em código reutilizável, para que você não crie dependências desnecessárias. Os usuários do seu código não se divertiam muito se você registrasse coisas (por exemplo, exceções tratadas) sem dar a eles a oportunidade de lidar com elas do seu jeito. Apenas meus dois centavos :)
andreee
7

Não acredito que as capturas locais sejam um antipadrão, na verdade, se bem me lembro, ele é realmente aplicado em Java!

O que é essencial para mim ao implementar o tratamento de erros é a estratégia geral. Você pode querer um filtro que capte todas as exceções no limite do serviço, interceptá-las manualmente - ambas são boas, desde que exista uma estratégia geral que se enquadre nos padrões de codificação de suas equipes.

Pessoalmente, gosto de detectar erros em uma função quando posso executar um dos seguintes procedimentos:

  • Adicione informações contextuais (como o estado dos objetos ou o que estava acontecendo)
  • Manipule a exceção com segurança (como um método TryX)
  • Seu sistema está ultrapassando um limite de serviço e chamando para uma biblioteca ou API externa
  • Você deseja capturar e relançar um tipo diferente de exceção (talvez com o original como uma exceção interna)
  • A exceção foi lançada como parte de alguma funcionalidade de fundo de baixo valor

Se não for um desses casos, não adiciono uma tentativa / captura local. Se for, dependendo do cenário, eu posso lidar com a exceção (por exemplo, um método TryX que retorna um false) ou repetir novamente, para que a exceção seja tratada pela estratégia global.

Por exemplo:

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

Ou um exemplo de repetição:

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

Então você captura o erro no topo da pilha e apresenta uma boa mensagem amigável para o usuário.

Seja qual for a abordagem adotada, sempre vale a pena criar testes de unidade para esses cenários, para garantir que a funcionalidade não mude e interrompa o fluxo do projeto posteriormente.

Você não mencionou em qual idioma está trabalhando, mas como desenvolvedor do .NET e já viu isso muitas vezes para não mencionar.

NÃO ESCREVA:

catch(Exception ex)
{
  throw ex;
}

Usar:

catch(Exception ex)
{
  throw;
}

O primeiro redefine o rastreamento de pilha e torna seu nível superior capturado totalmente inútil!

TLDR

Capturar localmente não é um antipadrão, geralmente pode fazer parte de um design e pode ajudar a adicionar contexto adicional ao erro.

Liath
fonte
3
Qual é o ponto do log na captura, quando o mesmo criador de log seria usado no manipulador de exceções de nível superior?
eufórico
Você pode ter informações adicionais (como as variáveis ​​locais) às quais não terá acesso no topo da pilha. Vou atualizar o exemplo para ilustrar.
Liath
2
Nesse caso, lance uma nova exceção com dados adicionais e exceção interna.
eufórico
2
@Euphoric sim, eu já vi isso também, pessoalmente eu não sou fã, no entanto, pois exige que você crie novos tipos de exceção para quase todos os métodos / cenários que considero muito trabalhosos. Adicionar a linha de registro aqui (e provavelmente outra na parte superior) ajuda a ilustrar o fluxo de código ao diagnosticar problemas
Liath
4
Java não o força a lidar com a exceção, obriga a estar ciente disso. você pode pegá-lo e fazer o que quer ou apenas declará-lo como algo que a função pode lançar e não fazer nada com ela na função ... Pouco detalhando uma resposta muito boa!
Newtopian
4

Isso depende muito do idioma. Por exemplo, o C ++ não oferece rastreamentos de pilha em mensagens de erro de exceção, portanto, rastrear a exceção por meio de captura-log-repetição frequente pode ser útil. Por outro lado, Java e linguagens similares oferecem rastreamentos de pilha muito bons, embora o formato desses rastreamentos de pilha possa não ser muito configurável. Capturar e repetir exceções nessas linguagens é totalmente inútil, a menos que você possa realmente adicionar algum contexto importante (por exemplo, conectar uma exceção SQL de baixo nível ao contexto de uma operação de lógica de negócios).

Qualquer estratégia de tratamento de erros implementada por meio da reflexão é quase necessariamente menos eficiente do que a funcionalidade incorporada à linguagem. Além disso, o registro generalizado possui sobrecarga de desempenho inevitável. Portanto, você precisa equilibrar o fluxo de informações que você obtém com outros requisitos para este software. Dito isto, soluções como o PostSharp, construídas com instrumentação no nível do compilador, geralmente se saem muito melhor do que a reflexão em tempo de execução.

Pessoalmente, acredito que registrar tudo não é útil, pois inclui muitas informações irrelevantes. Portanto, sou cético em relação a soluções automatizadas. Dada uma boa estrutura de registro, pode ser suficiente ter uma diretriz de codificação acordada que discuta que tipo de informação você gostaria de registrar e como essas informações devem ser formatadas. Você pode adicionar o registro onde for necessário.

O logon na lógica de negócios é muito mais importante do que o logon nas funções do utilitário. A coleta de rastreamentos de pilha de relatórios de falhas do mundo real (que exigem apenas o registro no nível superior de um processo) permite localizar áreas do código em que o registro teria mais valor.

amon
fonte
4

Quando vejo try/catch/logem todos os métodos, levanta preocupações de que os desenvolvedores não tinham idéia do que poderia ou não acontecer em seu aplicativo, assumiram o pior e registraram preventivamente tudo em todos os lugares por causa de todos os erros que estavam esperando.

Isso é um sintoma de que o teste de unidade e integração é insuficiente e os desenvolvedores estão acostumados a percorrer muitos códigos no depurador e esperam que, de alguma forma, muitos logs permitam implantar código de buggy em um ambiente de teste e encontrar os problemas observando os logs.

Código que lança exceções pode ser mais útil que o código redundante que captura e registra exceções. Se você lançar uma exceção com uma mensagem significativa quando um método recebe um argumento inesperado (e registra-o no limite do serviço), é muito mais útil do que registrar imediatamente a exceção lançada como efeito colateral do argumento inválido e ter que adivinhar o que o causou. .

Nulos são um exemplo. Se você obtiver um valor como argumento ou como resultado de uma chamada de método e não deve ser nulo, lance a exceção. Não basta registrar o resultado NullReferenceExceptionlançado cinco linhas depois, devido ao valor nulo. De qualquer maneira, você obtém uma exceção, mas uma diz alguma coisa enquanto a outra faz com que você procure por alguma coisa.

Como dito por outras pessoas, é melhor registrar exceções no limite do serviço ou sempre que uma exceção não for retrocedida, pois é tratada normalmente. A diferença mais importante é entre algo e nada. Se as suas exceções estiverem registradas em um local fácil de acessar, você encontrará as informações necessárias quando precisar.

Scott Hannen
fonte
Obrigado Scott. O ponto que você fez " Se você lança uma exceção com uma mensagem significativa quando um método recebe um argumento inesperado (e registra-o no limite do serviço) " realmente ocorreu e me ajudou a visualizar a situação pairando ao meu redor no contexto dos argumentos do método. Eu acho que faz sentido ter uma cláusula de guarda segura e lançar ArgumentException nesse caso, em vez de confiar nos detalhes do argumento de captura e registro em log
#
Scott, tenho o mesmo sentimento. Sempre que vejo log e refiz como apenas para registrar o contexto, sinto que os desenvolvedores não podem controlar os invariantes da classe ou não podem proteger a chamada do método. Em vez disso, todo método é envolvido em uma tentativa / captura / log / lançamento semelhante. E é simplesmente terrível.
Max
2

Se você precisar registrar informações de contexto que ainda não estão na exceção, envolva-as em uma nova exceção e forneça a exceção original como InnerException. Dessa forma, você ainda tem o rastreamento de pilha original preservado. Então:

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

O segundo parâmetro para o Exceptionconstrutor fornece uma exceção interna. Em seguida, você pode registrar todas as exceções em um único local e ainda obter o rastreamento completo da pilha e as informações contextuais na mesma entrada de log.

Você pode querer usar uma classe de exceção personalizada, mas o ponto é o mesmo.

try / catch / log / rehrow é uma bagunça, pois levará a logs confusos - por exemplo, e se uma exceção diferente acontecer em outro encadeamento entre o registro das informações contextuais e a exceção real no manipulador de nível superior? try / catch / throw é bom, se a nova exceção adiciona informações ao original.

JacquesB
fonte
E o tipo de exceção original? Se embrulharmos, ele se foi. Isso é um problema? Alguém estava contando com o SqlTimeoutException, por exemplo.
Max
@ Max: O tipo de exceção original ainda está disponível como exceção interna.
JacquesB
É o que eu quero dizer! Agora, todos os que estão na pilha de chamadas que estavam capturando a SqlException nunca a receberão.
Max
1

A exceção em si deve fornecer todas as informações necessárias para o registro adequado, incluindo mensagem, código de erro e outros. Portanto, não deve haver necessidade de capturar uma exceção apenas para reproduzi-la novamente ou lançar uma exceção diferente.

Freqüentemente, você verá o padrão de várias exceções capturadas e relançadas como uma exceção comum, como capturar uma DatabaseConnectionException, uma InvalidQueryException e uma InvalidSQLParameterException e relançar uma DatabaseException. Apesar disso, eu argumentaria que todas essas exceções específicas devem derivar de DatabaseException em primeiro lugar, portanto, não é necessário reler novamente.

Você descobrirá que a remoção de cláusulas de tentativa e captura desnecessárias (mesmo para fins puramente de registro) facilitará o trabalho, não mais. Somente os locais em seu programa que manipulam a exceção devem registrar a exceção e, se tudo isso falhar, um manipulador de exceção para todo o programa para uma tentativa final de registrar a exceção antes de sair normalmente do programa. A exceção deve ter um rastreamento de pilha completo, indicando o ponto exato em que a exceção foi lançada; portanto, muitas vezes não é necessário fornecer o log de "contexto".

Dito isto, a AOP pode ser uma solução rápida para você, embora geralmente implique uma ligeira desaceleração geral. Recomendamos que você remova inteiramente as cláusulas try catch desnecessárias, onde nada de valor é adicionado.

Neil
fonte
1
" A exceção em si deve fornecer todas as informações necessárias para o registro adequado, incluindo mensagem, código de erro e outras coisas ". Deveriam, mas, na prática, não referenciam as exceções nulas como um caso clássico. Não conheço nenhuma linguagem que diga a variável que a causou dentro de uma expressão complexa, por exemplo.
David Arno
1
@DavidArno True, mas qualquer contexto que você poderia fornecer também não poderia ser tão específico. Caso contrário, você teria try { tester.test(); } catch (NullPointerException e) { logger.error("Variable tester was null!"); }. O rastreamento de pilha é suficiente na maioria dos casos, mas, na falta disso, o tipo de erro geralmente é adequado.
611 Neil