error handling et best practices

Version imprimable

Bonjour,

je considère ce sujet comme important, vu qu'il impacte la conception. Ayant une incertitude, je lance cette discussion. Évidemment, j'ai déjà cherché sur le web qu'est-ce qu'on dit. Mais, tout ce que j'ai trouvé est écrit en anglais, et même si je me débrouille, je ne suis pas un virtuose.

Voici une façon de faire, et j'aimerais avoir votre avis.

J'ai créé la classe ErrorHandling. ErrorHandling.php :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
class ErrorHandling
{
    public string $msg;
 
    public function __construct($msg)
    {
        $this->msg=$msg;
    }
 
    public function disp_msg()
    {
        $page = new PhpEcho([DIR_ROOT, 'view Layout.php']);  //PhpEcho est une classe "moteur de rendu"
        $page->addChildFromCurrent('body', 'block endSession.php',['msg'=>$this->msg]);//en gros,endSession.php va
        // récupérer  $this->msg
        echo $page;
    }
}

endSession.php :

Code:

<?= $this['msg'] ?>

utilisation de la classe ErrorHandling :

Code:

1
2
3
4
5
6
7
8
9
10
11
 $hash = hash_file('md5',   $filepath);
 $pps = new PDOPlusPlus(PDOPlusPlus::MODE_SQL_DIRECT);
 sql = 'SELECT count(*) AS count from `fichier_uploade` where hash="' . $hash . '"';  
 
 $ret = $pps->select($sql);  /*pour vous rassurer, avec PDOPlusPlus, $hash va être échappé lors de l'exécution de la requête */
 if ( $ret[0]['count'] > 0 )
 {
      $exit_hash=new ErrorHandling('Impossible to import the file because already in database');
      $exit_hash->disp_msg();exit;   // ne pas exécuter la suite du script
 
 }

06/10/2020, 09h58
grunk

Salut,
Il y'a une question ? ou c'est juste pour nous présenter ta classe ?
06/10/2020, 10h05
laurentSc

Salut,
oui, présenter la classe mais surtout recueillir un avis !

Citation:

j'aimerais avoir votre avis.
06/10/2020, 15h24
grunk

Dans l'état ta classe ne fait rien de plus qu'un echo un peu compliqué.

A la limite elle gagnerait à être statique et donc appeler directement :

Code:

ErrorHandling::disp_msg($msg);

Tu t'évites une instances qui ne sert à rien , c'est plus rapide et lisible à l'utilisation.

Dans tous les cas quand c'est possible utilise des exceptions , soit les standard déjà fournies soit tu créer les tiennes si c'est nécessaire
06/10/2020, 21h30
laurentSc
Merci pour ta réponse.

OK pour faire une méthode statique. Néanmoins, tu conseilles aussi, si possible, les exceptions. Dans mon cas, ça me semblerait plus propre. Il me semble nécessaire de créer mon propre userException.
Jamais fait, donc j'ai quelques questions. Je vais dire comment je pense m'y prendre et merci de me corriger.

D'abord, je crée la classe userException qui hérite de la classe Exception.

Ensuite, il faut préciser que j'ai une architecture MVC et que donc, toutes les fonctions "contrôleurs" vont être appelées par un fichier index.php
Code:

1 2 $controller = new $routes[$action]; $controller->invoke($data); //toutes les classes "contrôleurs" comportent une méthode invoke()
Donc dans le contrôleur où je dois lancer une exception, je fais throw new userException("message d'erreur") et dans index.php :
Code:

1 2 3 4 5 6 7 try { ...le code } catch(userException $e) { echo 'Une exception a été lancée. Message d\'erreur : ', $e->getMessage(); }
Qu'est-ce que tu en penses ?
06/10/2020, 22h06
laurentSc

Pour info, les 2 classes utilisées (PhpEcho et PDOPlusPlus ) ont été créées par rawsrc : https://www.developpez.net/forums/bl...version-2-0-0/ et https://www.developpez.net/forums/bl...dutiliser-pdo/ et voici ce qu'il disait hier des try catch :

Citation:

C'est quoi cette connerie ? Tu n'as pas besoin d'un try catch. Un fichier déjà traité n'est pas une erreur. Tu fais un hash des fichiers déjà importés et avant d'en importer un autre tu vérifies si son hash n'a pas déjà été pris en compte.

On utilise un try catch pour gérer des erreurs systèmes probables. Ce n'est pas en aucun cas prévu pour gérer les cas particuliers liés à un utilisateur.

Bonjour,
même si je l'ai jamais fait et même si c'est une "connerie" ;) , voici comment je m'y suis pris (et ça marche en plus).

Dans mon contrôleur :

Code:

1
2
3
4
5
6
7
8
9
10
class CSVImport
 
{
 
    public function invoke ()
    {
     ...
     Throw new \Exception("Impossible to import the file because already in database"); /*tous comptes faits, j'utilise la classe Exception */
     ...
     }

et dans index.php :

Code:

1
2
3
4
5
6
7
8
9
10
11
//pas besoin d'inclure la classe car autoloader
use ticket_rawsrc\view\DispMsg;
...
try
{
le code
}
catch (Exception $e)
{
    DispMsg::DispMsg($e->getMessage());
}

classe DispMsg :

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
class  DispMsg
{
    /** This method displays $msg. It's useful to use PhpEcho to display a msg; like this, Layout.php is called
     * and so, the menu is displayed.
     * @param $msg
     */
 public static function DispMsg( $msg)
 {
     $page = new PhpEcho([DIR_ROOT, 'view Layout.php']);
 
     $body = $page->addChildFromCurrent('body', '', ['msg' => $msg]);
     $body->setCode("<p>{$body['msg']}</p>");
     echo $page;
 }

07/10/2020, 11h35
grunk
Les exception ne pas là pour gérer uniquement les erreurs système mais tout ce qui n'est pas une erreur "normale".

Exemple concret :
- Ne pas avoir de résultat suite à une recherche => on remonte une erreur du type "aucun résultat trouvé"
- Ne pas avoir de résultat car on à reçu des paramètre inattendu (genre des caractère qu'on sait pas traiter) => on lève une exception du type InvalidArgumentException.
Dans ton cas du fichier déjà présent , une erreur normale , et donc un simple echo suffit.
En revanche si on t'envoi un fichier qui n'est pas du csv par exemple , là c'est une exception car ton programme ne sait pas gérer celà et ca va crasher si on va plus loin.

Pour choisir, tu peux te dire qu'une erreur qui serait fatale à ton programme , devrait être une exception.

Concernant les try catch , il faut les faire le plus tot possible (le plus proche possible de code qui la génère) et ne pas laisser remonter les exceptions.

Citation:

Envoyé par grunk

Les exception ne pas là pour gérer uniquement les erreurs système mais tout ce qui n'est pas une erreur "normale".

C'est quoi, pour toi, une erreur "normale" ?

Citation:

Envoyé par grunk

Dans ton cas du fichier déjà présent , une erreur normale , et donc un simple echo suffit.
En revanche si on t'envoi un fichier qui n'est pas du csv par exemple , là c'est une exception car ton programme ne sait pas gérer celà et ca va crasher si on va plus loin.

Pourquoi mon erreur est normale ?
Une extension non CSV, c'est prévu dans mon code :
Code:

1 2 3 4 5 6 if ( $CSVFile->checkFileExtension($filepath) === false ) { // DispMsg::dispMsgWithoutMenu('Not conform file'); // exit; Throw new \NotCSVException("Not a CSV extension"); /*un souci m'a poussé à créer mes propres exceptions, mais souci ; j'y viendrai plus tard */ }
Citation:

Envoyé par grunk

Pour choisir, tu peux te dire qu'une erreur qui serait fatale à ton programme , devrait être une exception.

Une erreur est-elle "normale" si elle ne provoque pas d'erreur fatale ?

07/10/2020, 14h49
grunk

Citation:

Envoyé par laurentSc

Une erreur est-elle "normale" si elle ne provoque pas d'erreur fatale ?

Il n'y a que toi qui peut décider ce qui est une erreur acceptable ou une erreur inattendue.

Dans l'exemple que tu donnes , ton programme gère le fait que l'extension du fichier n'est pas bonne => erreur normale.
En revanche que se passe t'il si je me met du XML dans un fichier que je nomme avec une extension ".csv" ? Je vais passer le premier test et au moment de parser le fichier c'est le drame => ici une exception aura tout son sens.

Pour simplifier , tout ce qui résulte d'une action normale de l'utilisateur , doit être traiter en tant qu'erreur normale. Se tromper sur le fichier à envoyer c'est "normal" pour l'utilisateur de base.

Les exception sont à réserver pour les cas qui ne devrait pas arriver.
07/10/2020, 15h23
laurentSc
Ce que je comprends : les erreurs normales sont prévues par le programme et ne sont pas traitées comme des exceptions. Mais si je reviens à mon exemple d'extension non CSV, tu dis que c'est une erreur normale donc faudrait pas lever une exception. Néanmoins, le mécanisme me plaisait car si on lève une exception, le reste du script n'est pas exécuté. Mais si je ne lève pas une exception, pour gérer une erreur normale, je vais alors faire un truc du genre :
Code:

1 2 3 4 if (erreur_normale) { echo "Ceci est une erreur normale"; exit; // pour quitter le script }
OK ?

Si tu préfères faire une exception c'est ton choix , il n'ya pas de règle universelle.
Il faut juste penser que si on met des exception partout , ca veux aussi dire du try/catch/finally partout ce qui ajoute de la lourdeur au code

Citation:

car si on lève une exception, le reste du script n'est pas exécuté

Seul le code de la fonction ayant levé l'exception s'arrete , pas le reste (sauf si elle n'est pas catché , auquel cas on crash).

Citation:

Mais si je ne lève pas une exception, pour gérer une erreur normale, je vais alors faire un truc du genre

Je ferais plutôt :
Code:

1 2 3 4 5 6 7 8 maFonction() : bool { // du code if(erreur) { return false; } // du code return true; }
Et plus loin :
Code:

1 2 3 if(!maFonction()) { echo "Erreur !!!"; }
Et une version qui ajoute une exception :
Code:

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 maFonction() : bool { // du code if(erreur) { return false; } if(grossegalere) { throw new Exception; } // ici rien ne sera exécuté si excetion return true; } try { if(!maFonction()) { echo "ERREUR"; } } catch(Exception $e) { // Usuellement on va ici journaliser l'exception et éventuellement afficher quelque chose à l'utilisateur }
Un bon exemple d'utilisation des exceptions c'est PDO. Si la connexion échoue PDO lève une exception car c'est une erreur critique. Il ne sait pas aller plus loin.
Idem quand on envoi n'importe quoi au SGBD

Bonjour

Perso: exception, exception, exception.

Imagine une fonction qui reçoit en paramètre un nom de fichier, dans ta fonction, tu lis le fichier et tu renvois son contenu et donc, très bêtement :
Code:

1 2 3 4 function doSomething(string $path): string { return file_get_contents($path); ]
Ben, moi, j'y vois deux possibilités de lever des exceptions.
Code:

1 2 3 4 5 6 7 8 9 10 11 12 function doSomething(string $path): string { if ('' == trim($path)) { throw new \InvalidArgumentException('Veuillez spécifier un nom de fichier'); } if (!file_exists($path)) { throw new \Exception("Le fichier n'existe pas"); } return file_get_contents($path); ]
**j'ai tapé le code dans l'éditeur, je ne l'ai pas testé; il y a peut-être des erreurs de syntaxe**

Deux exceptions donc pour 1. m'assurer que la personne qui appelle ma fonction donne bien un nom de fichier 2. m'assurer que le fichier existe.

Perso, je tâche de faire autant d'exceptions que possible (mais raisonnablement bien sûr, faut pas surcharger le code inutilement). Le but : rendre le code solide, éliminer de potentiels bugs, ne pas présumer que la personne qui va appeler mon code sait exactement comme il fonctionne, ...

Depuis que je code ainsi, mon code est plus robuste.

Lire aussi p.ex. https://github.com/thecodingmachine/safe et http://bestpractices.thecodingmachin..._handling.html (et plein d'autres).

Des exceptions, cela permet aussi de stopper net ton code et d'avoir le contexte (le backtrace) et donc simplifie mille fois la recherche du bug. Avec un debugger (je pense à XDebug); c'est aussi très efficace : XDebug s'arrête par défaut sur les exceptions et donc, sur ce que toi, en tant que programmeur, a identifié comme "si le code vient ici, c'est qu'il y a un souci".

Avec des exceptions, tu peux encore programmer des tests unitaires (voire PHPUnit) qui te permet de tester ... que tes exceptions ne sont pas levées.

Et ceatera.

Tu donnais un bout de code dans ton premier post : tu référençais une classe pour afficher un message d'erreur et tu fais ensuite un exit(). Ben, ça, c'est strictement ce que fais une exception ! Mais dans ton cas, ton exit() est silencieux : tu ne sais pas capturer le numéro de la ligne dans ton code qui a provoqué l'erreur ni les appels qui ont amenés à cet affichage ni strictement rien sur le contexte. Ben... avec une exception, tu as le backtrace() (tu sais, tout ce qui s'affiche quand tu rencontres une exception comme p.ex. une erreur fatale). Le backtrace(), c'est hyper utile pour débogger.

Ensuite, toujours un avantage des exceptions, tu peux les lever et les gérer : dans un try catch tu peux décider de ce que tu veux faire : ok, pas grâve, tu ignores et tu continues ou ok, tu enregistres l'erreur dans un fichier, tu l'affiches proprement, tu génères un email vers le développeur, etc.

Tu mentionnes aussi plus haut "je dois créer mes propres exceptions"; ben non. Tu peux utiliser les exceptions standards (comme InvalidArgumentException ou Exception) mais, oui, c'est plus propre de créer les tiennes de telle manière que tu puisses, dans un try catch, identifier ce qu'il convient de faire en fonction de la classe de l'exception.

On peut écrire encore et encore sur les exceptions, je terminerais juste par : plus il y en a, plus le programmeur va produire un code de qualité. Plus il y en a, plus ton code est paré pour le futur. A quoi je pense ici ? Prenons file_get_contents() qui permet de lire un fichier. Que retourne cette fonction ? Ben, tu me diras, cette fonction retourne du texte : le contenu du fichier.

Ben non... Si le fichier n'existe pas, la fonction retourne la valeur False qui n'est pas un string mais un boolean. Et ça, quand tu codes en typé (https://www.php.net/manual/fr/functi...aration.strict) tu casses ton code. Tu reçois un boolean alors que tu t'attends à un string et paf!, ton code ne fonctionne plus.. Et c'est très bien en fait. Tu t'attends à un string => tu dois recevoir un string. Si tu reçois autre chose, c'est qu'il y a eu un souci. "Un souci?" ==> le programmeur doit toujours éviter les soucis; toujours. Un souci, c'est un bug potentiel.

Dans mon exemple avec file_get_contents(), il suffit de d'abord vérifier si le fichier existait ou pas. Si pas, alors retourne une valeur vide; si oui, retourne le contenu du fichier. Dans les deux cas, tu reçois un string et donc, ton programme est bien dans la logique de ce que tu veux faire : traiter une chaîne de caractères.

Exception, exception, exception !

[Edit]Dans tous mes projets, j'utilise https://github.com/webmozart/assert. En très peu de lignes, je valide mes paramètres, je m'assure qu'un dossier / fichier existe, que mon tableau contient bien telle ou telle clef, que mon objet a bien telle méthode, ... Je lève autant d'exceptions à chaque fois.[/Edit]

Bonne soirée.