Méfiez-vous des dangers des exceptions génériques

En travaillant sur un projet récent, j'ai trouvé un morceau de code qui effectuait le nettoyage des ressources. Parce qu'il avait de nombreux appels divers, il pourrait potentiellement lancer six exceptions différentes. Le programmeur d'origine, dans une tentative de simplifier le code (ou simplement d'enregistrer la saisie), a déclaré que la méthode lève Exceptionplutôt que les six exceptions différentes qui pourraient être levées. Cela a forcé le code appelant à être enveloppé dans un bloc try / catch qui a intercepté Exception. Le programmeur a décidé que parce que le code était à des fins de nettoyage, les cas d'échec n'étaient pas importants, donc le bloc catch restait vide pendant que le système s'arrêtait de toute façon.

De toute évidence, ce ne sont pas les meilleures pratiques de programmation, mais rien ne semble être terriblement faux ... sauf pour un petit problème de logique dans la troisième ligne du code d'origine:

Liste 1. Code de nettoyage d'origine

private void cleanupConnections () lance ExceptionOne, ExceptionTwo {for (int i = 0; i <connections.length; i ++) {connection [i] .release (); // Lance la connexion ExceptionOne, ExceptionTwo [i] = null; } connexions = null; } résumé protégé void cleanupFiles () lève ExceptionThree, ExceptionFour; protected abstract void removeListeners () jette ExceptionFive, ExceptionSix; public void cleanupEverything () lève Exception {cleanupConnections (); cleanupFiles (); removeListeners (); } public void done () {try {doStuff (); cleanupEverything (); doMoreStuff (); } catch (Exception e) {}}

Dans une autre partie du code, le connectionstableau n'est initialisé que lorsque la première connexion est créée. Mais si une connexion n'est jamais créée, le tableau de connexions est nul. Ainsi, dans certains cas, l'appel à connections[i].release()aboutit à un fichier NullPointerException. C'est un problème relativement facile à résoudre. Ajoutez simplement un chèque pour connections != null.

Cependant, l'exception n'est jamais signalée. Il est jeté cleanupConnections(), jeté à nouveau cleanupEverything()et finalement pris done(). La done()méthode ne fait rien à l'exception, elle ne l'enregistre même pas. Et comme il cleanupEverything()n'est appelé que via done(), l'exception n'est jamais vue. Le code n'est donc jamais corrigé.

Ainsi, dans le scénario d'échec, les méthodes cleanupFiles()et removeListeners()ne sont jamais appelées (donc leurs ressources ne sont jamais libérées) et doMoreStuff()ne sont jamais appelées, par conséquent, le traitement final done()ne se termine jamais. Pour aggraver les choses, done()n'est pas appelé lorsque le système s'arrête; à la place, il est appelé pour terminer chaque transaction. Les ressources fuient donc à chaque transaction.

Ce problème est clairement un problème majeur: les erreurs ne sont pas signalées et les ressources fuient. Mais le code lui-même semble assez innocent, et, d'après la manière dont le code a été écrit, ce problème s'avère difficile à retracer. Cependant, en appliquant quelques directives simples, le problème peut être trouvé et résolu:

  • N'ignorez pas les exceptions
  • N'attrapez pas les Exceptions génériques
  • Ne lancez pas de Exceptions génériques

N'ignorez pas les exceptions

Le problème le plus évident avec le code du Listing 1 est qu'une erreur dans le programme est complètement ignorée. Une exception inattendue (les exceptions, de par leur nature, sont inattendues) est levée, et le code n'est pas prêt à traiter cette exception. L'exception n'est même pas signalée car le code suppose que les exceptions attendues n'auront aucune conséquence.

Dans la plupart des cas, une exception doit, à tout le moins, être enregistrée. Plusieurs packages de journalisation (voir la barre latérale «Exceptions de journalisation») peuvent enregistrer les erreurs et les exceptions du système sans affecter de manière significative les performances du système. La plupart des systèmes de journalisation permettent également l'impression des traces de pile, fournissant ainsi des informations précieuses sur le lieu et la raison de l'exception. Enfin, comme les journaux sont généralement écrits dans des fichiers, un enregistrement des exceptions peut être examiné et analysé. Voir le listing 11 dans la barre latérale pour un exemple de journalisation des traces de pile.

La journalisation des exceptions n'est pas critique dans quelques situations spécifiques. L'une d'elles consiste à nettoyer les ressources dans une clause finally.

Exceptions enfin

Dans le listing 2, certaines données sont lues à partir d'un fichier. Le fichier doit se fermer indépendamment du fait qu'une exception lit les données, de sorte que la close()méthode est encapsulée dans une clause finally. Mais si une erreur ferme le fichier, on ne peut pas faire grand-chose à ce sujet:

Liste 2

public void loadFile (String fileName) lève IOException {InputStream in = null; try {in = new FileInputStream (fileName); readSomeData (dans); } enfin {if (in! = null) {try {in.close (); } catch (IOException ioe) {// Ignoré}}}}

Notez que loadFile()signale toujours un IOExceptionà la méthode appelante si le chargement des données réel échoue en raison d'un problème d'E / S (entrée / sortie). Notez également que même si une exception de close()est ignorée, le code l'indique explicitement dans un commentaire pour le rendre clair à toute personne travaillant sur le code. Vous pouvez appliquer cette même procédure pour nettoyer tous les flux d'E / S, fermer les sockets et les connexions JDBC, etc.

La chose importante à propos de l'ignorance des exceptions est de s'assurer qu'une seule méthode est encapsulée dans le bloc try / catch ignorant (ainsi les autres méthodes du bloc englobant sont toujours appelées) et qu'une exception spécifique est interceptée. Cette circonstance particulière diffère nettement de la capture d'un générique Exception. Dans tous les autres cas, l'exception doit être (au minimum) journalisée, de préférence avec une trace de pile.

N'attrapez pas les exceptions génériques

Souvent, dans les logiciels complexes, un bloc de code donné exécute des méthodes qui lancent diverses exceptions. Chargement et une classe Dynamiquement instanciation d' un objet peut lancer plusieurs exceptions différentes, y compris ClassNotFoundException, InstantiationException, IllegalAccessExceptionet ClassCastException.

Au lieu d'ajouter les quatre blocs catch différents au bloc try, un programmeur occupé peut simplement envelopper les appels de méthode dans un bloc try / catch qui intercepte les Exceptions génériques (voir le Listing 3 ci-dessous). Bien que cela semble inoffensif, des effets secondaires involontaires pourraient en résulter. Par exemple, si className()est nul, Class.forName()lancera un NullPointerException, qui sera intercepté dans la méthode.

Dans ce cas, le bloc catch intercepte les exceptions qu'il n'a jamais eu l'intention d'intercepter car a NullPointerExceptionest une sous-classe de RuntimeException, qui, à son tour, est une sous-classe de Exception. Ainsi , les génériques catch (Exception e)prises toutes les sous - classes de RuntimeException, y compris NullPointerException, IndexOutOfBoundsExceptionet ArrayStoreException. En règle générale, un programmeur n'a pas l'intention d'attraper ces exceptions.

Dans le Listing 3, les null classNamerésultats dans a NullPointerException, qui indique à la méthode appelante que le nom de classe n'est pas valide:

Liste 3

public SomeInterface buildInstance (String className) {SomeInterface impl = null; essayez {Class clazz = Class.forName (className); impl = (SomeInterface) clazz.newInstance (); } catch (Exception e) {log.error ("Erreur lors de la création de la classe:" + className); } return impl; }

Une autre conséquence de la clause catch générique est que la journalisation est limitée car catchelle ne connaît pas l'exception spécifique interceptée. Certains programmeurs, confrontés à ce problème, ont recours à l'ajout d'une vérification pour voir le type d'exception (voir le Listing 4), ce qui contredit l'objectif de l'utilisation des blocs catch:

Liste 4

catch (Exception e) {if (e instanceof ClassNotFoundException) {log.error ("Nom de classe non valide:" + className + "," + e.toString ()); } else {log.error ("Impossible de créer la classe:" + className + "," + e.toString ()); }}

Le Listing 5 fournit un exemple complet de capture d'exceptions spécifiques qui pourraient intéresser un programmeur. L' instanceofopérateur n'est pas requis car les exceptions spécifiques sont interceptées. Chacune des exceptions vérifiées ( ClassNotFoundException, InstantiationException, IllegalAccessException) est pris et traitée. Le cas particulier qui produirait un ClassCastException(la classe se charge correctement, mais n'implémente pas l' SomeInterfaceinterface) est également vérifié en vérifiant cette exception:

Liste 5

public SomeInterface buildInstance(String className) { SomeInterface impl = null; try { Class clazz = Class.forName(className); impl = (SomeInterface)clazz.newInstance(); } catch (ClassNotFoundException e) { log.error("Invalid class name: " + className + ", " + e.toString()); } catch (InstantiationException e) { log.error("Cannot create class: " + className + ", " + e.toString()); } catch (IllegalAccessException e) { log.error("Cannot create class: " + className + ", " + e.toString()); } catch (ClassCastException e) { log.error("Invalid class type, " + className + " does not implement " + SomeInterface.class.getName()); } return impl; } 

In some cases, it is preferable to rethrow a known exception (or perhaps create a new exception) than try to deal with it in the method. This allows the calling method to handle the error condition by putting the exception into a known context.

Listing 6 below provides an alternate version of the buildInterface() method, which throws a ClassNotFoundException if a problem occurs while loading and instantiating the class. In this example, the calling method is assured to receive either a properly instantiated object or an exception. Thus, the calling method does not need to check if the returned object is null.

Note that this example uses the Java 1.4 method of creating a new exception wrapped around another exception to preserve the original stack trace information. Otherwise, the stack trace would indicate the method buildInstance() as the method where the exception originated, instead of the underlying exception thrown by newInstance():

Listing 6

public SomeInterface buildInstance(String className) throws ClassNotFoundException { try { Class clazz = Class.forName(className); return (SomeInterface)clazz.newInstance(); } catch (ClassNotFoundException e) { log.error("Invalid class name: " + className + ", " + e.toString()); throw e; } catch (InstantiationException e) { throw new ClassNotFoundException("Cannot create class: " + className, e); } catch (IllegalAccessException e) { throw new ClassNotFoundException("Cannot create class: " + className, e); } catch (ClassCastException e) { throw new ClassNotFoundException(className + " does not implement " + SomeInterface.class.getName(), e); } } 

In some cases, the code may be able to recover from certain error conditions. In these cases, catching specific exceptions is important so the code can figure out whether a condition is recoverable. Look at the class instantiation example in Listing 6 with this in mind.

In Listing 7, the code returns a default object for an invalid className, but throws an exception for illegal operations, like an invalid cast or a security violation.

Note:IllegalClassException is a domain exception class mentioned here for demonstration purposes.

Listing 7

public SomeInterface buildInstance(String className) throws IllegalClassException { SomeInterface impl = null; try { Class clazz = Class.forName(className); return (SomeInterface)clazz.newInstance(); } catch (ClassNotFoundException e) { log.warn("Invalid class name: " + className + ", using default"); } catch (InstantiationException e) { log.warn("Invalid class name: " + className + ", using default"); } catch (IllegalAccessException e) { throw new IllegalClassException("Cannot create class: " + className, e); } catch (ClassCastException e) { throw new IllegalClassException(className + " does not implement " + SomeInterface.class.getName(), e); } if (impl == null) { impl = new DefaultImplemantation(); } return impl; } 

When generic Exceptions should be caught

Certain cases justify when it is handy, and required, to catch generic Exceptions. These cases are very specific, but important to large, failure-tolerant systems. In Listing 8, requests are read from a queue of requests and processed in order. But if any exceptions occur while the request is being processed (either a BadRequestException or any subclass of RuntimeException, including NullPointerException), then that exception will be caught outside the processing while loop. So any error causes the processing loop to stop, and any remaining requests will not be processed. That represents a poor way of handling an error during request processing:

Listing 8

public void processAllRequests () {Request req = null; essayez {while (true) {req = getNextRequest (); if (req! = null) {processRequest (req); // lance BadRequestException} else {// La file d'attente de la requête est vide, doit être effectuée break; }}} catch (BadRequestException e) {log.error ("Requête invalide:" + req, e); }}