Un bug cache parfois une occasion de mieux découvrir votre framework préféré

Ouvre une nouvelle fenêtreNous aimons utiliser Sylius comme plateforme pour développer des sites e-commerce, des sites de vente, des marketplaces, ou même des ERP !

C’est un excellent outil, très évolutif. Il est possible de modifier son fonctionnement en profondeur tout en conservant la puissance du framework et la possibilité de continuer à le mettre à jour pour bénéficier de nouvelles fonctionnalités. Mais parfois, une modification anodine a des conséquences inattendues…

L’apparition du problème

Lors d’un sprint récent sur un projet basé sur Sylius, Patrick le PO remonte une erreur lors de la modification d’un produit. Il n’arrive pas à valider la modification d’un produit alors que cela marchait auparavant. Étrange, c’est le même formulaire qui permet la création et la mise à jour d’un produit. Il a été modifié il y a peu et des tests automatisés ont été mis en place.

Il n’y a pas à tergiverser, lorsque l’on cherche à modifier certains champs, le site remonte une erreur : “Mise à jour impossible, Product a été modifiée précédemment” ou “Cannot update, the Product was previously modified” selon les préférences de langue.

Les tests automatisés n’ont pas détecté le problème : ils ne couvrent que la création du produit, pas sa modification.

Que nous révèle cette erreur ?

En première approche, j’aime faire un rapide tour dans le code. En restant naïf on a parfois de bonnes surprises, et des réponses rapidement. Le message d’erreur semble suffisamment unique pour commencer par là.

Une recherche dans le code fait remonter que ce texte est associé à la clé de traduction “race_condition_error”. Nouvelle recherche : cette clé n’est utilisée qu’au sein d’une exception spécifique à Sylius (RaceConditionException), elle-même est utilisée dans une seule classe : Sylius\Bundle\CoreBundle\Doctrine\ORM\Handler\ResourceUpdateHandler

Ouch ! Ça parle de Doctrine, qui fait l’interface entre notre code et la base de données. Dans ce contexte, une erreur de “race condition” signifie que le code ne parvient pas à déterminer dans quel ordre traiter une ou plusieurs demandes de mise à jour : la donnée en base peut être plus récente que la modification que l’on envoie.

Cela ne s’annonce pas trivial : il est temps d’identifier la modification qui a fait apparaître l’erreur.

Comment trouver le commit coupable ?

On a identifié un champ qui génère l’erreur lors de la modification d’un produit. J’écris un test simple : créer un produit, puis tenter de modifier ce champ. Le test échoue car l’erreur apparaît.

Je vais remonter dans le temps à l’aide de GIT, l’outil de gestion de version que nous utilisons. Mon objectif : utiliser le test pour déterminer quel commit a fait apparaître l’erreur.

Je pourrais utiliser la commande Ouvre une nouvelle fenêtregit bisect qui remplit parfaitement ce rôle. Mais j’ai ma petite idée du commit “coupable” :

• Je remonte l’historique GIT juste avant le commit : le test passe.
• J’avance d’un commit, le test échoue. Parfait, c’est bien là que tout se joue !

Identifier le problème

Dans ce commit, on a ajouté des champs sur deux entités : Product et ProductVariant.

• Product, vous l’aurez deviné, représente un produit.
• ProductVariant représente un produit particulier, avec ses propres options, sa propre tarification, voire son propre suivi des stocks.

En plus de l’ajout de ces champs, les formulaires ont été modifiés pour les afficher, et au passage intégrer le design du projet.

J’annule les modifications sur ProductVariant, et je retire les champs associés dans les formulaires. L’erreur est encore présente. Mauvaise pioche ?

Je recommence avec les modifications de l’entité Product… L’erreur persiste.

On a toujours la “RaceConditionException”, due à la mise à jour d’une entité en base de données. Or je viens d'annuler tout ce qui a changé dans le code qui concerne les entités Product et ProductVariant. Soit je suis entré dans la quatrième dimension, soit j’ai raté quelque chose.

Que reste t’il dans le commit ? Une innocente modification esthétique : nous n’affichons plus la gestion des stocks lors de l’édition du produit car c’est inutile pour ce projet.

Dans le doute, j’annule cette modification : l’erreur disparaît !

La réponse, et le correctif

En supprimant l’affichage de la gestion des stocks, nous avons cassé l’édition des produits.

Comment est-ce possible ? Le formulaire en question ne présente que deux champs : faut-il suivre les stocks et si oui, quel est le stock actuel…

Et pourtant, un troisième champ est bien présent (mais invisible pour l’utilisateur) : la version. Il n’est pas obligatoire, mais son absence semble poser problème. Je ne sais pas ce qu’est cette version de la variante. Avant de creuser cette piste : testons.

Si je l’affiche (toujours invisible) mais sans rétablir la gestion des stocks, l’erreur ne réapparaît pas. Bingo, voici le correctif : rétablir le champ version dans le formulaire, bien qu’il ne soit pas visible.

Mais, pourquoi ?

Ce système de version sur ProductVariant est présent pour éviter que plusieurs demandes de modifications simultanées ne rentrent en conflit. D’accord, mais pourquoi est-ce que le champ se retrouve enfoui avec la gestion des stocks, s’il est si important ?

Avec le système des stocks, il va y avoir une modification de la variante à chaque fois qu’un produit est vendu. Si deux clients achètent le même produit au même moment, ils vont tous deux demander une modification “stock -1” avec la même version :

• Le système s'appuiera sur la donnée version pour n’autoriser qu’une modification.
• La seconde modification sera rejetée, puis retentée avec la nouvelle version : s’il reste du stock, l’achat sera également validé. Si non, pas de chance, le produit vient de passer en rupture de stock.

Comme le champ de formulaire est caché, il peut être généré n’importe où dans la page. Bien que j’aurais préféré qu’il soit ajouté avec les champs cruciaux de la variante, je peux comprendre comment il s’est retrouvé associé aux champs de formulaire liés aux stocks…

Le(s) mot(s) de la fin

Ce bug aura été l’occasion de creuser un peu plus loin dans notre connaissance du framework Sylius et de ses composants les moins apparents, comme le versionnage des entités.

Le problème peut sembler lunaire : des erreurs liées à Doctrine, la persistance en base, RaceConditionException. Mais l’approche de résolution montre encore une fois sa pertinence : reproduire l’erreur, réduire les modifications jusqu’à trouver l’élément déclencheur, étudier cet élément pour comprendre pourquoi il pose problème et comment corriger.

Comme à chaque fois qu’un bug échappe aux tests automatisés écrit par les développeurs, nous nous posons la question : avons-nous la bonne approche pour la rédaction des tests ? Les tests sont nécessaires mais peuvent être chronophages (écriture, maintenance, exécution). Lorsque l'on travaille avec un framework, on cherche à tester “notre” code, pas celui du framework car il est déjà testé : c’est une perte de temps. Ici, fallait il d’emblée un test automatisé sur la mise à jour, en plus de la création ? Sachant que le même code est concerné dans les deux cas, j’ai tendance à penser que non. Par contre, maintenant que le bug a été rencontré, le test créé pour réussir à l’isoler et à le corriger va rester dans notre suite de tests. On a pu se faire surprendre une fois, mais pas deux !