Essayez / Catch / Log / Rethrow - Est Anti Pattern?

19

Je peux voir plusieurs articles où l'importance de la gestion des exceptions à l'emplacement central ou à la limite du processus a été soulignée comme une bonne pratique plutôt que de joncher chaque bloc de code autour de try / catch. Je crois fermement que la plupart d'entre nous en comprenons l'importance, mais je vois des gens qui se retrouvent toujours avec un modèle anti-capture-journalisation, principalement parce que pour faciliter le dépannage lors de toute exception, ils veulent enregistrer des informations plus spécifiques au contexte (exemple: paramètres de méthode passé) et la façon est d'enrouler la méthode autour 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;
    }
}

Existe-t-il un bon moyen d'y parvenir tout en conservant les bonnes pratiques de gestion des exceptions? J'ai entendu parler du cadre AOP comme PostSharp pour cela, mais j'aimerais savoir s'il y a un inconvénient ou un coût de performance majeur associé à ces cadres AOP.

Merci!

rahulaga_dev
la source
6
Il y a une énorme différence entre encapsuler chaque méthode dans try / catch, journaliser l'exception et laisser le code évoluer. Et essayez / rattrapez et relancez l'exception avec des informations supplémentaires. La première est une pratique terrible. Le deuxième est un moyen parfaitement bien d'améliorer votre expérience de débogage.
Euphoric
Je dis essayez / attrapez chaque méthode et connectez-vous simplement au bloc catch et relancez - est-ce correct?
rahulaga_dev
2
Comme l'a souligné Amon. Si votre langue a des traces de pile, la connexion à chaque capture est inutile. Mais envelopper l'exception et ajouter des informations supplémentaires est une bonne pratique.
Euphoric
1
Voir la réponse de @ Liath. Toute réponse que j'aurais donnée refléterait à peu près la sienne: intercepter les exceptions dès que possible et si tout ce que vous pouvez faire à ce stade est d'enregistrer des informations utiles, faites-le et relancez. À mon avis, considérer cela comme un contre-modèle n'a pas de sens.
David Arno
1
Liath: ajout d'un petit extrait de code. J'utilise c #
rahulaga_dev

Réponses:

19

Le problème n'est pas le bloc de capture local, le problème est le journal et la relance . Gérez l'exception ou encapsulez-la avec une nouvelle exception qui ajoute un contexte supplémentaire et lève cela. Sinon, vous rencontrerez plusieurs entrées de journal en double pour la même exception.

L'idée ici est d' améliorer la capacité de débogage de votre application.

Exemple # 1: manipulez-le

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

Si vous gérez l'exception, vous pouvez facilement rétrograder l'importance de l'entrée du journal des exceptions et il n'y a aucune raison de percoler cette exception dans la chaîne. C'est déjà réglé.

La gestion d'une exception peut consister à informer les utilisateurs qu'un problème s'est produit, à consigner l'événement ou simplement à l'ignorer.

REMARQUE: si vous ignorez intentionnellement une exception, je recommande de fournir un commentaire dans la clause catch vide qui indique clairement pourquoi. Cela permet aux futurs responsables de savoir que ce n'était pas une erreur ou une programmation paresseuse. Exemple:

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

Exemple # 2: ajouter du contexte supplémentaire et lancer

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

L'ajout de contexte supplémentaire (comme le numéro de ligne et le nom de fichier dans le code d'analyse) peut aider à améliorer la capacité de débogage des fichiers d'entrée - en supposant que le problème existe. Ceci est une sorte de cas spécial, donc reconditionner une exception dans une "ApplicationException" juste pour renommer cela ne vous aide pas à déboguer. Assurez-vous d'ajouter des informations supplémentaires.

Exemple # 3: ne faites rien à l'exception

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

Dans ce dernier cas, vous autorisez simplement l'exception à quitter sans la toucher. Le gestionnaire d'exceptions de la couche la plus externe peut gérer la journalisation. La finallyclause est utilisée pour s'assurer que toutes les ressources nécessaires à votre méthode sont nettoyées, mais ce n'est pas l'endroit pour consigner que l'exception a été levée.

Berin Loritsch
la source
J'ai aimé " Le problème n'est pas le bloc de capture local, le problème est le journal et la relance " Je pense que cela est parfaitement logique pour assurer une journalisation plus propre. Mais finalement, cela signifie également être OK avec try / catch dispersé dans toutes les méthodes, n'est-ce pas? Je pense qu'il doit y avoir des lignes directrices pour s'assurer que cette pratique est suivie judicieusement plutôt que d'avoir toutes les méthodes le faire.
rahulaga_dev
J'ai fourni une ligne directrice dans ma réponse. Cela ne répond-il pas à vos questions?
Berin Loritsch
@rahulaga_dev Je ne pense pas qu'il existe une ligne directrice / solution miracle, alors résolvez ce problème, car cela dépend beaucoup du contexte. Il ne peut pas y avoir de directive générale qui vous indique où gérer une exception ou quand la renvoyer. OMI, la seule directive que je vois est de reporter la journalisation / la manipulation au dernier moment possible et d'éviter la journalisation dans du code réutilisable, de sorte que vous ne créiez pas de dépendances inutiles. Les utilisateurs de votre code ne seraient pas trop amusés si vous enregistriez des choses (c'est-à-dire que vous gériez des exceptions) sans leur donner la possibilité de les gérer à leur manière. Juste mes deux cents :)
andreee
7

Je ne crois pas que les captures locales soient un anti-modèle, en fait, si je me souviens bien, elles sont en fait appliquées en Java!

Ce qui est essentiel pour moi lors de la mise en œuvre de la gestion des erreurs, c'est la stratégie globale. Vous voudrez peut-être un filtre qui capture toutes les exceptions à la limite du service, vous pouvez les intercepter manuellement - les deux sont très bien tant qu'il existe une stratégie globale, qui tombera dans les normes de codage de vos équipes.

Personnellement, j'aime détecter les erreurs dans une fonction lorsque je peux effectuer l'une des opérations suivantes:

  • Ajouter des informations contextuelles (telles que l'état des objets ou ce qui se passait)
  • Gérez l'exception en toute sécurité (comme une méthode TryX)
  • Votre système franchit une frontière de service et appelle une bibliothèque externe ou une API
  • Vous souhaitez intercepter et renvoyer un autre type d'exception (peut-être avec l'original comme exception interne)
  • L'exception a été levée dans le cadre d'une fonctionnalité d'arrière-plan de faible valeur

Si ce n'est pas un de ces cas, je n'ajoute pas de try / catch local. Si c'est le cas, selon le scénario, je peux gérer l'exception (par exemple une méthode TryX qui retourne un faux) ou relancer afin que l'exception soit gérée par la stratégie globale.

Par exemple:

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 un exemple de retour:

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

Ensuite, vous attrapez l'erreur en haut de la pile et présentez un bon message convivial à l'utilisateur.

Quelle que soit l'approche que vous adoptez, il est toujours utile de créer des tests unitaires pour ces scénarios afin de vous assurer que la fonctionnalité ne change pas et ne perturbe pas le flux du projet à une date ultérieure.

Vous n'avez pas mentionné la langue dans laquelle vous travaillez, mais vous êtes un développeur .NET et vous l'avez vu trop de fois pour ne pas le mentionner.

N'ÉCRIS PAS:

catch(Exception ex)
{
  throw ex;
}

Utilisation:

catch(Exception ex)
{
  throw;
}

Le premier réinitialise la trace de la pile et rend votre capture de niveau supérieur totalement inutile!

TLDR

La capture locale n'est pas un anti-modèle, elle peut souvent faire partie d'une conception et peut aider à ajouter un contexte supplémentaire à l'erreur.

Liath
la source
3
Quel est l'intérêt de se connecter à la capture, alors que le même enregistreur serait utilisé dans le gestionnaire d'exceptions de niveau supérieur?
Euphoric
Vous pouvez avoir des informations supplémentaires (telles que les variables locales) auxquelles vous n'aurez pas accès en haut de la pile. Je vais mettre à jour l'exemple pour illustrer.
Liath
2
Dans ce cas, lancez une nouvelle exception avec des données supplémentaires et une exception interne.
Euphoric
2
@ Euphoric yep, j'ai vu cela aussi, personnellement je ne suis pas fan cependant car cela vous oblige à créer de nouveaux types d'exception pour presque chaque méthode / scénario unique qui me semble être beaucoup de frais généraux. L'ajout de la ligne de journal ici (et probablement une autre en haut) permet d'illustrer le flux de code lors du diagnostic des problèmes
Liath
4
Java ne vous oblige pas à gérer l'exception, il vous oblige à en être conscient. vous pouvez soit l'attraper et faire quoi que ce soit ou simplement le déclarer comme quelque chose que la fonction peut lancer et ne rien faire avec lui dans la fonction ...
Newtopian
4

Cela dépend beaucoup de la langue. Par exemple, C ++ n'offre pas de traces de pile dans les messages d'erreur d'exception, il peut donc être utile de suivre l'exception par le biais de captures-journaux-relances fréquentes. En revanche, Java et les langages similaires offrent de très bonnes traces de pile, bien que le format de ces traces de pile ne soit pas très configurable. La capture et la réexception d'exceptions dans ces langages sont totalement inutiles, sauf si vous pouvez vraiment ajouter un contexte important (par exemple, la connexion d'une exception SQL de bas niveau avec le contexte d'une opération de logique métier).

Toute stratégie de gestion des erreurs implémentée par réflexion est presque nécessairement moins efficace que les fonctionnalités intégrées au langage. En outre, la journalisation omniprésente a des coûts inévitables de performances. Vous devez donc équilibrer le flux d'informations que vous obtenez par rapport aux autres exigences de ce logiciel. Cela dit, les solutions comme PostSharp qui sont construites sur une instrumentation au niveau du compilateur feront généralement beaucoup mieux que la réflexion au moment de l'exécution.

Personnellement, je pense que tout enregistrer n'est pas utile, car il comprend des tonnes d'informations non pertinentes. Je suis donc sceptique vis-à-vis des solutions automatisées. Étant donné un bon cadre de journalisation, il pourrait être suffisant d'avoir une directive de codage convenue qui discute du type d'informations que vous souhaitez enregistrer et de la façon dont ces informations doivent être formatées. Vous pouvez ensuite ajouter la journalisation là où cela est important.

La connexion à la logique métier est beaucoup plus importante que la connexion aux fonctions utilitaires. De plus, la collecte de traces de pile de rapports de plantage réels (qui ne nécessite que la journalisation au niveau supérieur d'un processus) vous permet de localiser les zones du code où la journalisation aurait le plus de valeur.

amon
la source
4

Quand je vois try/catch/logdans chaque méthode, cela soulève des inquiétudes que les développeurs n'avaient aucune idée de ce qui pourrait ou ne pourrait pas se produire dans leur application, ont supposé le pire et ont tout enregistré de manière préventive partout en raison de tous les bogues qu'ils attendaient.

C'est un symptôme que les tests unitaires et d'intégration sont insuffisants et que les développeurs sont habitués à parcourir beaucoup de code dans le débogueur et espèrent que beaucoup de journalisation leur permettront de déployer du code bogué dans un environnement de test et de trouver les problèmes en regardant les journaux.

Le code qui lève des exceptions peut être plus utile que le code redondant qui intercepte et enregistre les exceptions. Si vous lancez une exception avec un message significatif lorsqu'une méthode reçoit un argument inattendu (et l'enregistrez à la limite du service), il est beaucoup plus utile que de consigner immédiatement l'exception levée comme effet secondaire de l'argument non valide et d'avoir à deviner ce qui l'a provoqué .

Les nuls en sont un exemple. Si vous obtenez une valeur en tant qu'argument ou le résultat d'un appel de méthode et qu'elle ne devrait pas être nulle, lancez l'exception alors. Ne vous contentez pas de consigner les NullReferenceExceptioncinq lignes générées par la suite en raison de la valeur nulle. Quoi qu'il en soit, vous obtenez une exception, mais l'un vous dit quelque chose tandis que l'autre vous fait rechercher quelque chose.

Comme d'autres l'ont dit, il est préférable de consigner les exceptions à la limite du service, ou chaque fois qu'une exception n'est pas renvoyée car elle est gérée avec élégance. La différence la plus importante est entre quelque chose et rien. Si vos exceptions sont enregistrées dans un seul endroit facile d'accès, vous trouverez les informations dont vous avez besoin quand vous en avez besoin.

Scott Hannen
la source
Merci Scott. Le point que vous avez fait " Si vous lâchez une exception avec un message significatif lorsqu'une méthode reçoit un argument inattendu (et l'enregistrez à la limite du service) " a vraiment frappé et m'a aidé à visualiser la situation planant autour de moi dans le contexte des arguments de méthode. Je pense qu'il est logique d'avoir une clause de sauvegarde et de lancer ArgumentException dans ce cas plutôt que de s'appuyer sur les détails des arguments de capture et de journalisation
rahulaga_dev
Scott, j'ai le même sentiment. Chaque fois que je vois log et rethow juste pour enregistrer le contexte, j'ai l'impression que les développeurs ne peuvent pas contrôler les invariants de la classe ou ne peuvent pas sauvegarder l'appel de méthode. Au lieu de cela, chaque méthode tout au long est enveloppée dans un essai / capture / journal / lancer similaire. Et c'est tout simplement terrible.
Max
2

Si vous devez enregistrer des informations de contexte qui ne figurent pas déjà dans l'exception, vous les encapsulez dans une nouvelle exception et fournissez l'exception d'origine sous la forme InnerException. De cette façon, vous avez toujours la trace de pile d'origine préservée. Donc:

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

Le deuxième paramètre du Exceptionconstructeur fournit une exception interne. Ensuite, vous pouvez consigner toutes les exceptions en un seul endroit, et vous obtenez toujours la trace complète de la pile et les informations contextuelles dans la même entrée de journal.

Vous voudrez peut-être utiliser une classe d'exception personnalisée, mais le point est le même.

try / catch / log / rethrow est un gâchis car cela conduira à des journaux confus - par exemple, si une exception différente se produit dans un autre thread entre la journalisation des informations contextuelles et la journalisation de l'exception réelle dans le gestionnaire de niveau supérieur? try / catch / throw est bien si la nouvelle exception ajoute des informations à l'original.

JacquesB
la source
Qu'en est-il du type d'exception d'origine? Si nous terminons, c'est parti. C'est un problème? Quelqu'un s'appuyait sur la SqlTimeoutException par exemple.
Max
@Max: le type d'exception d'origine est toujours disponible en tant qu'exception interne.
JacquesB
C'est ce que je veux dire! Désormais, tout le monde dans la pile d'appels, qui détectait l'exception SqlException, ne l'obtiendra jamais.
Max
1

L'exception elle-même devrait fournir toutes les informations nécessaires à une bonne journalisation, y compris le message, le code d'erreur, etc. Par conséquent, il ne devrait pas être nécessaire d'attraper une exception uniquement pour la relancer ou lever une exception différente.

Souvent, vous verrez le modèle de plusieurs exceptions interceptées et renvoyées comme une exception commune, comme intercepter une DatabaseConnectionException, une InvalidQueryException et une InvalidSQLParameterException et renvoyer une DatabaseException. Bien qu'à cela, je dirais que toutes ces exceptions spécifiques devraient dériver de DatabaseException en premier lieu, donc une nouvelle tentative n'est pas nécessaire.

Vous constaterez que la suppression des clauses try catch inutiles (même celles à des fins de journalisation uniquement) rendra le travail plus facile, pas plus difficile. Seuls les emplacements de votre programme qui gèrent l'exception doivent consigner l'exception et, en cas d'échec, un gestionnaire d'exceptions à l'échelle du programme pour une dernière tentative de consignation de l'exception avant de quitter le programme en douceur. L'exception doit avoir une trace de pile complète indiquant le point exact où l'exception a été levée, il n'est donc souvent pas nécessaire de fournir une journalisation «contextuelle».

Cela dit, l'AOP peut être une solution miracle pour vous, bien qu'elle entraîne généralement un léger ralentissement global. Je vous encourage à supprimer à la place les clauses try catch inutiles où rien de valeur n'est ajouté.

Neil
la source
1
" L'exception elle-même devrait fournir toutes les informations nécessaires à une bonne journalisation, y compris le message, le code d'erreur, etc." Ils le devraient, mais en pratique, ils ne font pas référence aux exceptions Null étant un cas classique. Je ne connais aucun langage qui vous dira la variable qui l'a provoqué dans une expression complexe, par exemple.
David Arno
1
@DavidArno Vrai, mais tout contexte que vous pourriez fournir ne pourrait pas non plus être aussi spécifique. Sinon, vous auriez try { tester.test(); } catch (NullPointerException e) { logger.error("Variable tester was null!"); }. La trace de la pile est suffisante dans la plupart des cas, mais à défaut, le type d'erreur est généralement adéquat.
Neil