Cinq bugs trompeurs de Node.js qui passent inaperçus lors des revues de code
Découvrez cinq erreurs réelles de Node.js impliquant forEach, des promesses flottantes et des copies superficielles pour comprendre pourquoi du code qui fonctionne bien peut encore échouer en production.
Chacun des cinq exemples de code ci-dessous s’exécute sans erreur, et chacun pourrait facilement passer inaperçu lors d’une revue rapide du code. Pourtant, chacun d’eux a provoqué une panne réelle dans un système de production, à plusieurs reprises. Avant de lire l’explication sous chaque extrait, essayez de déterminer par vous-même ce qui ne va pas.
Bug 1
async function notifyAllUsers(userIds) {
userIds.forEach(async (id) => {
const user = await getUser(id);
await sendNotification(user);
});
console.log("All notifications sent!");
}
Arrêtez-vous ici et réfléchissez à ce qui se passe réellement lorsque ce code s’exécute.
Le bug : la ligne console.log("Toutes les notifications ont été envoyées !") s’exécute avant même que toute notification ne soit réellement envoyée, et la fonction principale se termine sans jamais attendre que les envois individuels soient achevés.
La raison en est que Array.prototype.forEach ne prend pas en compte les promesses. Il exécute la fonction de rappel pour chaque élément puis passe immédiatement au suivant, sans tenir compte de la valeur retournée par cette fonction. Déclarer la fonction de rappel comme async ne change rien au comportement de forEach — cela signifie simplement que chaque appel génère désormais une promesse que forEach jette sans l’examiner. Les notifications sont néanmoins envoyées à un moment donné, mais de manière asynchrone en arrière-plan, sans ordre garanti et sans mécanisme permettant à l’appelant de détecter la fin ou l’échec.
async function notifyAllUsers(userIds) {
await Promise.all(userIds.map(async (id) => {
const user = await getUser(id);
await sendNotification(user);
}));
console.log("All notifications sent!");
}
Passer à map signifie que les promesses sont collectées dans un tableau au lieu d’être rejetées, et en enveloppant ce tableau dans Promise.all, on force la fonction à attendre réellement que toutes les opérations soient terminées. Désormais, l’affichage dans le journal affiche bien la vérité.
Bug 2
app.post("/orders", async (req, res) => {
const order = await createOrder(req.body);
sendConfirmationEmail(order.customerEmail);
res.status(201).json(order);
});
La route semble fonctionner correctement — les commandes sont créées, les e-mails sont envoyés, et la réponse revient rapidement. Alors quel est le problème ?
Le bug : la fonction sendConfirmationEmail est appelée sans utilisation de await, ce qui fait que la promesse qu’elle renvoie reste complètement non gérée. Si cette promesse échoue, rien ne la capture. Ce comportement est connu sous le nom de promesse flottante, et il ne s’agit pas simplement d’un problème stylistique — c’est un risque opérationnel. Dans les versions actuelles de Node.js, un échec non géré d’une promesse peut faire planter l’ensemble du processus, entraînant l’échec de toutes les autres requêtes en cours, au lieu que l’envoi d’e-mail échoue simplement de manière discrète.
Il y a également une véritable question d’architecture qui se cache ici, en dehors du manque de gestion des erreurs : un e-mail de confirmation échoué devrait-il empêcher la validation de la commande, ou celle-ci devrait-elle être acceptée malgré tout ? Dans la plupart des cas, on préférerait la seconde option — la commande est bel et bien validée même si la notification échoue. Cependant, il y a une différence entre choisir de ne pas bloquer quelque chose et échouer complètement à gérer ses erreurs, et ce code a accidentellement opté pour la seconde solution alors qu’il visait probablement la première.
app.post("/orders", async (req, res) => {
const order = await createOrder(req.body);
sendConfirmationEmail(order.customerEmail).catch((err) => {
logger.error({ orderId: order.id, err }, "Failed to send confirmation email");
});
res.status(201).json(order);
});
Avec cette version, l’e-mail ne retient vraiment pas la réponse HTTP, mais une erreur est désormais enregistrée au lieu de disparaître silencieusement ou de faire planter le serveur.
Bug 3
function applyDiscount(cart) {
const updatedCart = { ...cart };
updatedCart.items.forEach((item) => {
item.price = item.price * 0.9;
});
return updatedCart;
}
À première vue, cela ressemble à l’idiome standard « copier plutôt que modifier » que l’on trouve dans tous les guides sur la manière d’éviter les effets secondaires. En réalité, il s’agit d’un piège plus subtil.
Le bug : l’opérateur de propagation { ...cart } ne réalise qu’une copie superficielle. Il crée un nouvel objet au niveau le plus élevé, mais updatedCart.items fait toujours référence au même tableau — et aux mêmes objets d’éléments — que cart.items. Ainsi, lorsque la boucle forEach modifie item.price, elle modifie également, de manière invisible, les éléments du panier d’origine, car l’opérateur de propagation n’a jamais touché quoi que ce soit au-delà du premier niveau de la structure.
console.log(cart.items[0].price); // already discounted, unintentionally
console.log(updatedCart.items[0].price); // same object, same value
Tout ceux qui pensaient que le cart d’origine resterait inchangé — attente raisonnable étant donné que le nom de la fonction suggère qu’elle renvoie quelque chose de nouveau — se retrouvent à travailler avec des données corrompues sans s’en rendre compte.
function applyDiscount(cart) {
return {
...cart,
items: cart.items.map((item) => ({ ...item, price: item.price * 0.9 })),
};
}
Chaque niveau de nesting qui doit être modifié doit être copié explicitement à ce niveau. Un spread superficiel ne protège que le niveau sur lequel il agit directement, et non ce qui est niché en dessous.
Bug 4
function updateUserSettings(user, updates) {
return Object.assign({}, user, updates);
}
app.patch("/settings", (req, res) => {
const updated = updateUserSettings(req.user, req.body);
saveUser(updated);
res.json(updated);
});
Cela ressemble à une routine ordinaire de « fusion d’un objet de mise à jour avec un objet existant ». Alors où se cache le danger ?
Le bug : req.body provient directement du client, et rien ici ne limite les clés autorisées à être intégrées dans l’objet utilisateur. Si le corps d’une requête contient quelque chose comme "role": "admin" ou "isVerified": true, ces propriétés sont intégrées tout aussi facilement que n’importe quel champ de configuration légitime, car Object.assign ne distingue pas quelles clés doivent être modifiables — il fusionne tout ce qui lui est donné.
Cela relève d’une catégorie bien connue de vulnérabilité appelée affectation massive, et elle apparaît fréquemment dans des API qui insèrent directement les corps de requête dans des modèles de base de données sans les filtrer au préalable à l’aide d’une liste d’autorisation. Le risque augmente même lorsque la fonction de fusion devient plus générique et réutilisable — ce qui est justement ce qui fait que ce type de code semble d’abord fiable.
function updateUserSettings(user, updates) {
const allowedFields = ["displayName", "timezone", "emailNotifications"];
const safeUpdates = {};
for (const field of allowedFields) {
if (field in updates) safeUpdates[field] = updates[field];
}
return { ...user, ...safeUpdates };
}
En définissant une liste d’autorisation explicite, vous garantissez qu’une requête ne pourra jamais affecter un champ pour lequel elle n’a pas été spécifiquement autorisée à le modifier, quelles que soient les clés supplémentaires que quelqu’un pourrait insérer dans le payload.
Bug 5
async function getUserWithPosts(userId) {
const user = await db.query("SELECT * FROM users WHERE id = $1", [userId]);
const posts = await db.query("SELECT * FROM posts WHERE user_id = $1", [userId]);
return { ...user, posts };
}
Rien dans ce code n’est techniquement incorrect. Mais quel est le coût pour vous d’écrire ainsi ?
Le problème — ou plutôt, l’opportunité manquée — est que ces deux requêtes ne dépendent absolument pas l’une de l’autre. La seconde requête n’a besoin d’aucun résultat provenant de la première pour pouvoir s’exécuter. En les enchaînant avec des appels séquentiels await, le temps d’attente total devient la somme des durées des deux requêtes, l’une bloquant l’autre sans raison réelle.
async function getUserWithPosts(userId) {
const [user, posts] = await Promise.all([
db.query("SELECT * FROM users WHERE id = $1", [userId]),
db.query("SELECT * FROM posts WHERE user_id = $1", [userId]),
]);
return { ...user, posts };
}
L’utilisation de Promise.all permet à les deux requêtes d’être exécutées en parallèle plutôt qu’une après l’autre, de sorte que le temps total correspond approximativement à celui de la requête la plus lente, et non à la somme des deux. Ce n’est pas un bug au sens où il produirait des résultats incorrects — la version séquentielle renvoie en effet des données parfaitement exactes. Il s’agit plutôt d’un bug car il gaspille silencieusement les ressources de performance disponibles, et c’est un schéma que l’on adopte par habitude sans vraiment le remettre en question, puisque rien dans le code ne semble anormal à une lecture superficielle.
Ce que les cinq ont en commun
Chacun de ces extraits s’est exécuté sans erreur, a produit des résultats plausibles et passerait facilement un examen rapide. Aucun d’eux ne serait détecté si votre seul critère de test était « fonctionne-t-il lorsque je l’essaie une fois ? ». Pour les repérer, il faut un type particulier de scepticisme : ce code asynchrone attend-il réellement tout ce pour quoi il semble attendre ; cette opération de copie duplique-t-elle vraiment chaque élément nécessaire ; cette logique de fusion fait-elle confiance à des données auxquelles elle n’a aucune raison de faire confiance ? Ce genre d’intuition ne provient pas de la mémorisation de plus de syntaxe, mais du fait d’avoir déjà été confronté au moins une fois à chacun de ces cinq schémas précis.
Lectures complémentaires
- Dangers communs de JavaScript et TypeScript qui détruisent le code en secret — Explique les pièges subtils de JavaScript et TypeScript, allant des comparaisons avec NaN aux problèmes de synchronisation asynchrone et de coercition de type, qui provoquent des erreurs bien que le code paraisse correct.
- Cinq pièges de l’API Temporal qui réintroduisent en secret des problèmes de date — Découvrez comment l’API Temporal de JavaScript peut encore causer des erreurs liées aux fuseaux horaires, à la sérialisation, à la précision et à la durée, à moins que cinq modes d’usage erronés courants ne soient corrigés.