80 votes

Utilisation de try/catch pour empêcher les crashs de l'application

J'ai travaillé sur une application Android qui utilise fréquemment try/catch pour éviter les plantages même à des endroits où ce n'est pas nécessaire. Par exemple,

Une vue dans layout xml avec id = toolbar est référencée de la manière suivante :

// voir nouvel exemple ci-dessous, celui-ci est juste confus
// on dirait que je parle de try/catch vide
try {
    View view = findViewById(R.id.toolbar);
}
catch(Exception e) {
}

Cette approche est utilisée dans toute l'application. La trace de la pile n'est pas imprimée et il est vraiment difficile de trouver ce qui a mal tourné. L'application se ferme soudainement sans afficher de trace de la pile.

J'ai demandé à mon supérieur de m'expliquer et il a dit,

C'est pour éviter les plantages en production.

Je ne suis pas du tout d'accord avec cela. Pour moi, ce n'est pas la façon d'éviter les plantages d'applications. Cela montre que le développeur ne sait pas ce qu'il fait et est dans le doute.

Est-ce que cette approche est utilisée dans l'industrie pour éviter les plantages des applications d'entreprise?

Si try/catch est vraiment, vraiment nécessaire, est-il possible d'attacher un gestionnaire d'exceptions au thread UI ou à d'autres threads et de tout attraper là-bas? Ce serait une meilleure approche si possible.

Oui, un try/catch vide est mauvais et même si nous imprimons la trace de la pile ou enregistrons l'exception sur le serveur, envelopper des blocs de code dans des try/catch de façon aléatoire à travers toute l'application n'a aucun sens pour moi, par exemple quand chaque fonction est incluse dans un try/catch.

MISE À JOUR

Comme cette question a attiré beaucoup d'attention et que certaines personnes l'ont mal interprétée (peut-être parce que je ne l'ai pas formulée clairement), je vais la reformuler.

Voici ce que font les développeurs ici

  • Une fonction est écrite et testée, cela peut être une petite fonction qui initialise simplement des vues ou une fonction complexe, après les tests elle est enveloppée d'un bloc try/catch. Même pour des fonctions qui ne lèveront jamais d'exception.

  • Cette pratique est utilisée dans toute l'application. Parfois la trace de la pile est imprimée et parfois juste un debug log avec un message d'erreur aléatoire. Ce message d'erreur varie d'un développeur à l'autre.

  • Avec cette approche, l'application ne plante pas mais le comportement de l'application devient indéterminé. Parfois il est même difficile de comprendre ce qui a mal tourné.

  • La vraie question que je pose est ; Est-ce la pratique suivie dans l'industrie pour éviter les plantages des applications d'entreprise? et je ne parle pas de try/catch vide. Est-ce comme si les utilisateurs préféraient une application qui ne plante pas à une application qui se comporte de façon inattendue? Parce que cela revient vraiment à soit planter soit présenter à l'utilisateur un écran vide ou un comportement dont l'utilisateur n'est pas conscient.

  • Je poste ici quelques extraits du vrai code

      private void makeRequestForForgetPassword() {
        try {
            HashMap params = new HashMap<>();
    
            String email= CurrentUserData.msisdn;
            params.put("email", "blabla");
            params.put("new_password", password);
    
            NetworkProcess networkProcessForgetStep = new NetworkProcess(
                serviceCallListenerForgotPasswordStep, ForgotPassword.this);
            networkProcessForgetStep.serviceProcessing(params, 
                Constants.API_FORGOT_PASSWORD);
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
    
     private void languagePopUpDialog(View view) {
        try {
            PopupWindow popupwindow_obj = popupDisplay();
            popupwindow_obj.showAsDropDown(view, -50, 0);
        } catch (Exception e) {
            e.printStackTrace();
        }
    }
    
    void reloadActivity() {
        try {
            onCreateProcess();
        } catch (Exception e) {
        }
    }

Il ne s'agit pas d'un doublon de Android exception handling best practices, là l'OP essaie de gérer les exceptions dans un but différent de cette question.

11 votes

Ne répondez pas à votre question, mais ne passez jamais sous silence les exceptions catch(Exception e){} - ce commentaire avait du sens avant la modification de la question

1 votes

29 votes

Ah, le célèbre ON ERROR RESUME NEXT

81voto

GhostCat Points 83269

Bien sûr, il y a toujours des exceptions aux règles, mais si vous avez besoin d'une règle générale - alors vous avez raison; les blocs catch vides sont "absolument" une mauvaise pratique.

Jetons un coup d'œil de plus près, en commençant d'abord par votre exemple spécifique :

try {
  View view = findViewById(R.id.toolbar);
}
catch(Exception e) { }

Ainsi, une référence à quelque chose est créée; et lorsque cela échoue ... cela n'a pas d'importance; car cette référence n'est pas utilisée en premier lieu! Le code ci-dessus est absolument inutile et encombrant. Ou la personne qui a écrit ce code a-t-elle initialement supposé qu'un deuxième appel similaire ne lancerait plus d'exception magiquement ?!

Peut-être que cela était censé ressembler à :

try {
  View view = findViewById(R.id.toolbar);
  ... et maintenant faites quelque chose avec cette variable view ...
}
catch(Exception e) { }

Mais encore une fois, en quoi cela aide-t-il ?! Les exceptions servent à communiquer respectivement à propager des situations d'erreur à l'intérieur de votre code. Ignorer les erreurs est rarement une bonne idée. En fait, une exception peut être traitée de différentes manières comme :

  • Vous donnez un retour à l'utilisateur; (par exemple : "la valeur que vous avez entrée n'est pas une chaîne, réessayez"); ou pour s'engager dans une gestion d'erreurs plus complexe
  • Peut-être que le problème est d'une manière ou d'une autre prévisible et peut être atténué (par exemple en donnant une réponse "par défaut" lorsque quelque chose a échoué dans une "recherche à distance")
  • ...

En résumé, la chose minimale à faire avec une exception est de la journaliser / tracer; afin que lorsque vous revenez plus tard pour déboguer un problème, vous compreniez "OK, à ce moment-là, cette exception s'est produite".

Et comme l'ont souligné d'autres : vous évitez également de capturer l'exception Exception en général (selon le niveau : il peut y avoir de bonnes raisons d'avoir certaines captures pour Exception, et même certains types d'erreurs au plus haut niveau, pour vous assurer que rien ne se perd; jamais).

Enfin, citons également Ward Cunningham :

Vous savez que vous travaillez avec un code propre lorsque chaque routine que vous lisez est à peu près ce à quoi vous vous attendiez. Vous pouvez appeler cela du code magnifique lorsque le code donne également l'impression que le langage a été conçu pour le problème.

Laissez cela pénétrer et méditez-dessus. Un code propre ne vous surprend pas. L'exemple que vous nous montrez surprend tout le monde qui le regarde.

Mise à jour, en ce qui concerne la mise à jour que l'OP demande :

try {
  faire quelque chose
}
catch(Exception e) { 
  imprimer la pile d'appels
}

Même réponse : faire cela "tout le temps" est également une mauvaise pratique. Parce que ce code surprend également le lecteur.

Le code ci-dessus :

  • Imprime des informations d'erreur quelque part. Il n'est en aucun cas garanti que ce "quelque part" soit une destination raisonnable. Au contraire. Par exemple : dans l'application avec laquelle je travaille, de tels appels apparaîtraient mystérieusement dans nos tampons de trace. Selon le contexte, notre application peut parfois injecter des tonnes et des tonnes de données dans ces tampons; faisant en sorte que ces tampons se vident toutes les quelques secondes. Donc "simplement imprimer des erreurs" se traduit souvent par : "perdre simplement toutes ces informations d'erreur".
  • Ensuite : vous n'utilisez pas le try/catch parce que vous le pouvez. Vous le faites parce que vous comprenez ce que fait votre code ; et vous savez : j'ai plutôt intérêt à avoir un try/catch ici pour faire ce qu'il faut (voir à nouveau les premières parties de ma réponse).

Ainsi, utiliser le try/catch comme un "modèle" comme vous le montrez est, comme je l'ai dit : encore une idée pas très bonne. Et oui, cela empêche les plantages; mais entraîne toutes sortes de comportements "indéfinis". Vous savez, lorsque vous attrapez simplement une exception au lieu de la gérer correctement; vous ouvrez une boîte de Pandore; car vous pourriez rencontrer d'innombrables erreurs secondaires par la suite que vous ne comprenez plus. Parce que vous avez consommé l'événement "cause première" plus tôt; l'avez imprimé quelque part; et ce quelque part a disparu maintenant.

1 votes

Et si j'imprime la pile d'exécution? Je ne pense toujours pas que j'ai besoin de try/catch ici.

3 votes

Je dois admettre que je ne comprends pas votre édition. Pas vraiment une idée de ce que vous faites là; ou où vous en êtes avec ça.

0 votes

@mallaoudin dans un cas général, tout ce qui se trouve après la ligne qui provoque le crash du programme ne sera pas exécuté (évidemment), donc l'impression n'a aucun sens. Dans ce cas spécifique, c'est-à-dire try+except, au moins quelque chose s'afficherait et l'exception ne serait pas étouffée.

15voto

Paresh P. Points 4465

De la documentation Android:

Appelons cela -

Ne pas attraper d'exception générique

Il peut être tentant d'être paresseux lors de la capture des exceptions et de faire quelque chose comme ceci:

try {
    someComplicatedIOFunction();        // peut générer une IOException
    someComplicatedParsingFunction();   // peut générer une ParsingException
    someComplicatedSecurityFunction();  // peut générer une SecurityException
    // ouf, c'est passé
} catch (Exception e) {                 // je vais juste attraper toutes les exceptions
    handleError();                      // avec un gestionnaire générique !
}

Dans presque tous les cas, il est inapproprié de capturer une exception générique Exception ou Throwable (de préférence pas Throwable car cela inclut les exceptions Error). C'est très dangereux car cela signifie que des exceptions que vous n'avez jamais envisagées (y compris des RuntimeExceptions comme ClassCastException) sont attrapées dans la gestion des erreurs au niveau de l'application.

Cela masque les propriétés de gestion des erreurs de votre code, ce qui signifie que si quelqu'un ajoute un nouveau type d'exception dans le code que vous appelez, le compilateur ne vous aidera pas à réaliser que vous devez gérer l'erreur différemment.

Alternatives à la capture d'Exceptions génériques:

  • Capturez chaque exception séparément comme des blocs catch séparés après un seul try. Cela peut être maladroit mais est toujours préférable à la capture de toutes les exceptions.
    Edité par l'auteur: Celui-ci est mon choix. Attention à ne pas répéter trop de code dans les blocs catch. Si vous utilisez Java 7 ou une version ultérieure, utilisez le multi-catch pour éviter de répéter le même bloc catch.
  • Refactorez votre code pour avoir une gestion des erreurs plus fine, avec plusieurs blocs try. Séparez l'IO de l'analyse, gérez les erreurs séparément dans chaque cas.
  • Re-lancer l'exception. Souvent, vous n'avez pas besoin de capturer l'exception à ce niveau, laissez simplement la méthode la lancer.

Dans la plupart des cas, vous ne devriez pas traiter différents types d'exceptions de la même manière.

Mise en forme / paragraphe légèrement modifié depuis la source pour cette réponse.

P.S. N'ayez pas peur des Exceptions !! Ce sont des amis !!!

1 votes

Si vous attrapez chaque exception individuellement, assurez-vous d'éviter de poursuivre l'exécution dans un état incertain (évitez des choses comme Object a; try { a = thing(); } catch (Exception e) {...} try { a.action(); } catch (Exception e) {...}))

0 votes

Pour être juste, attraper une Exception générique pourrait être utile dans le cadre d'un système de "journal avant de planter", bien qu'elle devrait être relancée dans ce cas.

9voto

Syzygy Points 287

Je mettrais cela en commentaire à une autre réponse, mais je n'ai pas encore la réputation pour le faire.

Vous avez raison en disant que c'est une mauvaise pratique, en fait ce que vous avez posté montre différents types de mauvaises pratiques concernant les exceptions.

  1. Manque de gestion d'erreurs
  2. Capture générique
  3. Aucune exception intentionnelle
  4. Try/catch généralisé

Je vais essayer d'expliquer tout ça à travers cet exemple.

try {
   User user = loadUserFromWeb();     
   if(user.getCountry().equals("us")) {  
       enableExtraFields();
   }
   fillFields(user);   
} catch (Exception e) { 
}

Cela peut échouer de plusieurs manières qui devraient être gérées différemment.

  1. Les champs ne seront pas remplis, donc l'utilisateur se voit présenter un écran vide et puis... quoi ? Rien - manque de gestion d'erreurs.
  2. Il n'y a pas de distinction entre différents types d'erreurs, par exemple des problèmes de connexion Internet ou des problèmes avec le serveur lui-même (panne, requête erronée, transmission corrompue, ...) - Capture générique.
  3. Vous ne pouvez pas utiliser les exceptions à des fins personnelles car le système actuel interfère avec cela. - Aucune exception intentionnelle
  4. Les erreurs non essentielles et inattendues (par exemple null.equals(...)) peuvent empêcher l'exécution de code essentiel. - Try/catch généralisé

Solutions

(1) Tout d'abord, échouer silencieusement n'est pas une bonne chose. En cas d'échec, l'application ne fonctionnera pas. Au lieu de cela, il devrait y avoir une tentative pour résoudre le problème ou afficher un avertissement, par exemple "Impossible de charger les données de l'utilisateur, peut-être que vous n'êtes pas connecté à Internet ?". Si l'application ne fait pas ce qu'elle est censée faire, c'est bien plus frustrant pour un utilisateur que si elle se ferme simplement.

(4) Si l'utilisateur est incomplet, par exemple si le pays n'est pas connu et renvoie null. La méthode equals créera une NullPointerException. Si cette NPE est simplement lancée et capturée comme ci-dessus, la méthode fillFields(user) ne sera pas appelée, même si elle pourrait être exécutée sans problèmes. Vous pourriez éviter cela en incluant des vérifications de null, en changeant l'ordre d'exécution ou en ajustant la portée du try/catch. (Ou vous pourriez coder de manière sécurisée comme ceci : "us".equals(user.getCountry()), mais je devais fournir un exemple). Bien sûr, toute autre exception empêchera également fillFields() d'être exécutée, mais s'il n'y a pas d'utilisateur, vous ne voulez probablement pas qu'elle soit exécutée de toute façon.

(1, 2, 3)Charger depuis le web lance souvent une variété d'exceptions, de IOException à HttpMessageNotReadable exception ou même un simple retour. Il peut arriver que l'utilisateur ne soit pas connecté à Internet, que quelque chose ait changé côté serveur ou qu'il soit en panne, mais vous ne le savez pas car vous attrapez(Exception) - au lieu de cela, vous devriez attraper des exceptions spécifiques. Vous pouvez même en attraper plusieurs comme ceci

try{
   User user = loadUserFromWeb(); //renvoie NoInternetException, ServerNotAvailableException ou null si l'utilisateur n'existe pas
   if(user == null) { 
       throw new UserDoesNotExistException(); //il pourrait y avoir de meilleures options pour résoudre cela, mais cela montre comment les exceptions peuvent être utilisées.
   }
   fillFields(user);
   if("us".equals(user.getCountry()) {
       enableExtraFields();
   }
} catch(NoInternetException e){
    displayWarning("Votre connexion internet est interrompue :(");
} catch(ServerNotAvailableException e){
    displayWarning("Il semble que notre serveur a des problèmes, réessayez plus tard.");
} catch(UserDoesNotExistException e){
    startCreateUserActivity();
}

J'espère que cela explique les choses.

Au moins en solution temporaire, ce que vous pourriez faire est envoyer un événement à votre backend avec l'exception. Par exemple via Firebase ou Crashlytics. De cette façon, vous pouvez au moins voir des choses comme (hé, l'activité principale ne se charge pas pour 80% de nos utilisateurs en raison d'un problème comme (4).

9voto

Raman Sahasi Points 14959

C'est définitivement une mauvaise pratique de programmation.

Dans le scénario actuel, s'il y a des centaines de try catch comme celui-ci, alors vous ne saurez même pas où se produit l'exception sans déboguer l'application, ce qui est un cauchemar si votre application est en environnement de production.

Mais vous pouvez inclure un journalisateur pour savoir quand une exception est levée (et pourquoi). Cela ne changera pas votre flux de travail normal.

...
try {
    View view = findViewById(R.id.toolbar);
}catch(Exception e){
    logger.log(Level.SEVERE, "une exception a été levée", e);
}
...

7voto

JoshG79 Points 1073

Ce n'est pas une bonne pratique. D'autres réponses l'ont dit mais je pense qu'il est important de prendre du recul et de comprendre pourquoi nous avons des exceptions en premier lieu.

Chaque fonction a une post-condition – un ensemble de choses qui doivent toutes être vraies après l'exécution de cette fonction. Par exemple, une fonction qui lit à partir d'un fichier a pour post-condition que les données du fichier seront lues depuis le disque et retournées. Une exception est donc lancée lorsqu'une fonction n'a pas été en mesure de satisfaire l'une de ses post-conditions.

En ignorant une exception d'une fonction (ou même en l'ignorant effectivement en simplement enregistrant l'exception), vous dites que vous acceptez que cette fonction ne fasse en réalité pas tout le travail auquel elle s'est engagée à faire. Cela semble improbable – si une fonction ne s'exécute pas correctement, il n'y a aucune garantie que ce qui suit s'exécutera du tout. Et si le reste de votre code s'exécute correctement que la fonction concernée s'exécute ou non, on se demande pourquoi vous avez cette fonction en premier lieu.

[Maintenant, il y a certains cas où les blocs catch vides sont acceptables. Par exemple, le journalisation est quelque chose que vous pourriez justifier en l'enveloppant dans un bloc catch vide. Votre application fonctionnera probablement bien même si une partie de la journalisation ne peut pas être écrite. Mais ce sont des cas spéciaux que vous devez rechercher vraiment dur dans une application normale.]

Donc le point c'est que ce n'est pas une bonne pratique car cela ne permet pas vraiment de maintenir votre application en cours d'exécution (la justification présumée de ce style). Peut-être que techniquement le système d'exploitation ne l'a pas arrêté. Mais il est peu probable que l'application fonctionne toujours correctement après avoir simplement ignoré une exception. Et dans le pire des cas, cela pourrait effectivement causer des dommages (par exemple, la corruption des fichiers des utilisateurs, etc.).

Prograide.com

Prograide est une communauté de développeurs qui cherche à élargir la connaissance de la programmation au-delà de l'anglais.
Pour cela nous avons les plus grands doutes résolus en français et vous pouvez aussi poser vos propres questions ou résoudre celles des autres.

Powered by:

X