La liste de contrôle de révision de code que j'ai construite après 2 000 demandes de tirage

J'ai catégorisé 2 147 commentaires de PR laissés sur 18 mois. 34 % concernaient la gestion des erreurs. 22 % concernaient le nommage. Seulement 8 % concernaient les performances. Ce n'est pas ce à quoi je m'attendais lorsque j'ai commencé à suivre mes révisions. Je pensais que j'allais repérer des problèmes architecturaux et des bugs complexes. Au lieu de cela, je signalais constamment les mêmes problèmes fondamentaux : des vérifications de null manquantes, des noms de variables vagues et des messages d'erreur qui ne disaient rien d'utile aux utilisateurs. Après avoir vu les mêmes problèmes surgir lors d'incidents en production, j'ai réalisé qu'il ne s'agissait pas de simples critiques — ils faisaient la différence entre des systèmes qui échouent en douceur et des systèmes qui vous réveillent à 3 heures du matin.

Pourquoi la plupart des listes de contrôle de révision de code passent à côté de l'essentiel

Tout le monde vous dit de vérifier le style de code, la couverture des tests et la documentation. C'est bien, mais ce n'est pas là où la production tombe en panne. Les listes de contrôle que je vois partagées sur les blogs d'ingénierie se concentrent sur ce qui est facile à mesurer plutôt que sur ce qui est réellement important. Ils vous diront de vérifier que les fonctions mesurent moins de 50 lignes, mais ils ne vous diront pas de vérifier si la gestion des erreurs aide réellement quelqu'un à déboguer le problème à 2 heures du matin quand les journaux sont votre seul ami.

Les meilleures révisions de code ne prévent pas seulement des bogues — elles préviennent le genre de bogues qui se transforment en incidents. Un contrôle de null manquant n'est pas seulement un crash potentiel; c'est un potentiel événement de corruption de données lorsque ce crash se produit en milieu de transaction.

J'ai commencé à suivre mes commentaires de PR parce que j'étais frustré. Je révisais le code, l'approuvais, puis voyais le même développeur faire la même erreur dans la prochaine PR. Le retour d'information ne restait pas. J'ai donc construit une feuille de calcul. Chaque commentaire que j'ai laissé a été catégorisé : gestion des erreurs, nommage, tests, sécurité, performances, architecture ou autre. Après six mois, j'avais assez de données pour voir des modèles. Après dix-huit mois, ces modèles s'étaient transformés en une liste de contrôle qui fonctionnait réellement.

La partie surprenante n'était pas seulement ce qui était en tête de liste — c'était ce qui n'y était pas. Les commentaires sur l'optimisation des performances représentaient seulement 8 % de mes révisions. Les problèmes de sécurité étaient de 6 %. Ce sont les choses sur lesquelles nous nous obsédons lors des discussions architecturales, mais dans les PR du jour au lendemain, elles sont rares. Qu'est-ce qui est commun ? Les développeurs assumant le chemin heureux, des noms de choses mal choisis et écrivant des messages d'erreur pour eux-mêmes au lieu de pour la personne qui déboguera à minuit.

Les modèles de gestion des erreurs qui continuent de casser la production

Voici ma liste numérotée de problèmes de gestion des erreurs, classée par leur fréquence de cause d'incidents réels :

1. Échecs silencieux dans les tâches en arrière-plan. Le code attrape une exception, l'enregistre et continue. Cela semble raisonnable jusqu'à ce que vous réalisiez que cette tâche était censée envoyer un e-mail de confirmation de paiement. Maintenant, votre client pense qu'il n'a pas été facturé, alors qu'il l'a été. Je vois ce modèle dans 40 % des PR de tâches en arrière-plan. La solution est simple : si l'opération est critique, ne capturez pas l'exception — laissez-la remonter et déclencher votre alerte. Si ce n'est pas critique, documentez pourquoi il est acceptable d'échouer silencieusement.
2. Messages d'erreur génériques qui cachent le contexte. "Une erreur s'est produite" ne me dit rien. "Échec du traitement du paiement" est mieux mais reste inutile. "Échec de la facturation de la carte se terminant par 4242 : fonds insuffisants (code d'erreur : carte_refusée)" aide réellement. Je signale cela dans environ 30 % des PR. Le test que j'utilise : si vous voyiez cette erreur dans les journaux de production à 3 heures du matin, pourriez-vous la diagnostiquer sans ajouter plus de journalisation et redéployer ?
3. Capturer Exception au lieu d'exceptions spécifiques. C'est controversé parce que certains guides de style le recommandent, mais je l'ai vu cacher des bogues trop souvent. Lorsque vous capturerez Exception, vous attraperez également NullPointerException, IllegalStateException, et toutes les autres exceptions qui indiquent une erreur de programmeur plutôt que des conditions d'exécution. Capturez ce que vous vous attendez à gérer. Laissez les erreurs de programmeur échouer — c'est ainsi que vous les trouvez.
4. Ne pas valider les données externes à la frontière. Les réponses API, les entrées utilisateur, le contenu des fichiers—si cela provient de l'extérieur de votre processus, validez-le immédiatement. Je vois des développeurs valider dans la couche de logique métier, ce qui signifie que des données non valides se sont déjà propagées à travers plusieurs fonctions. À ce moment-là, vous déboguez pourquoi une valeur null est entrée dans votre base de données. Validez à la frontière, échouez rapidement, renvoyez des erreurs claires.
5. Logique de réessai sans attente exponentielle. Le service est hors ligne, donc vous réessayez immédiatement. Il est toujours hors ligne, donc vous réessayez immédiatement encore. Félicitations, vous venez de transformer une dégradation de service en une panne complète en l'inondant de réessaies. Je vois cela dans 15 % des PR qui ajoutent une logique de réessai. Utilisez toujours le retour exponentiel avec du jitter. Ayez toujours un nombre maximal de réessais. Considérez toujours si le fait de réessayer a même un sens pour cette opération.
6. Ne pas gérer les échecs partiels dans les opérations par lot. Vous traitez 1 000 enregistrements. L'enregistrement 500 échoue. Que se passe-t-il avec les enregistrements 501-1000 ? Dans la plupart des codes que je révise, ils ne sont jamais traités. Le lot échoue, est réessayé, échoue à nouveau à l'enregistrement 500, et vous êtes bloqué. Gérez explicitement les échecs partiels : suivez ce qui a réussi, ce qui a échoué et pourquoi. Rendez vos opérations par lot récupérables.
7. Supposer que les transactions de base de données réussiront toujours. Vous démarrez une transaction, effectuez un travail et validez. Mais que se passe-t-il si la validation échoue ? Que se passe-t-il si vous perdez la connexion à la base de données en plein milieu de la transaction ? Je vois du code qui ne gère pas cela dans 25 % des PR touchant le code de base de données. Le résultat : l'état de votre application et l'état de votre base de données divergent, et vous passez des heures à déboguer pourquoi les données ne correspondent pas à ce que les journaux disent qu'il s'est passé.

Le jour où un nom de variable a causé un incident de 50 000 $

C'était un mardi matin quand j'ai reçu le message Slack : "Nous venons de rembourser 50 000 $ aux mauvais clients." J'ai ouvert le canal d'incidents et commencé à lire. Un développeur avait poussé un correctif la nuit précédente pour un bogue dans notre système de traitement des remboursements. Le correctif était d'une ligne. La PR avait été approuvée par deux ingénieurs seniors. Les tests ont réussi. Tout avait l'air en ordre.

Le bogue était dans un nom de variable. Le code d'origine avait une variable appelée `refundAmount` qui représentait le montant à rembourser en centimes. Le développeur a ajouté une nouvelle variable appelée `refundAmount` qui représentait le montant en dollars. Ils ont oublié de renommer la variable originale. Le code s'est bien compilé - ce n'étaient que des entiers. Les tests ont réussi parce que les données de test utilisaient des montants où les centimes et les dollars étaient suffisamment proches pour que les assertions ne l'aient pas détecté.

En production, nous avons traité 200 remboursements ce matin-là. La moitié d'entre eux étaient pour le mauvais montant. Un remboursement de 10,00 $ est devenu un remboursement de 1 000 $. Un remboursement de 5,00 $ est devenu un remboursement de 500 $. Au moment où quelqu'un a remarqué, nous avions trop payé de 50 000 $. Nous avons dû examiner manuellement chaque remboursement, contacter les clients et, dans certains cas, demander un remboursement. Cela a pris trois jours pour faire le ménage.

La cause profonde n'était pas l'erreur du développeur — tout le monde fait des erreurs. C'était que nous avions deux variables avec le même nom représentant différentes unités dans la même portée. La révision de code ne l'a pas détecté parce que les réviseurs se concentraient sur la logique, pas sur le nommage. Après cet incident, j'ai ajouté une règle à ma liste de contrôle : si une variable représente une quantité avec des unités, l'unité doit être dans le nom. Pas `amount`, mais `amountCents` ou `amountDollars`. Pas `duration`, mais `durationSeconds` ou `durationMilliseconds`.

Cela peut sembler pédant...