Startseite / Artikel / Fünf betrügerische Node.js-Bugs, die unbemerkt die Code-Review überstehen

Node.js JavaScript Async/Await Debugging Backend Development

Fünf betrügerische Node.js-Bugs, die unbemerkt die Code-Review überstehen

Erkunden Sie fünf reale Node.js-Bugs aus der Praxis, die forEach-, floating-Promises- und flache Kopien betreffen, um zu verstehen, warum Code, der unter Entwicklung einwandfrei funktioniert, in der Produktion dennoch fehlschlagen kann.

1485 Wörter

Jedes der fünf untenstehenden Codebeispiele wird ohne Fehler ausgeführt, und jedes würde vermutlich bei einer schnellen Codeprüfung unbemerkt durchgehen. Dennoch hat jedes von ihnen in irgendeinem Produktivsystem mehrfach zu einem echten Ausfall geführt. Versuchen Sie, bevor Sie die Erklärung unter jedem Codeausschnitt lesen, selbst herauszufinden, was falsch ist.

Fehler 1

async function notifyAllUsers(userIds) {
  userIds.forEach(async (id) => {
    const user = await getUser(id);
    await sendNotification(user);
  });
  console.log("All notifications sent!");
}

Machen Sie hier eine Pause und überlegen Sie, was tatsächlich passiert, wenn dieser Code ausgeführt wird.

Der Fehler: Die Zeile console.log("All notifications sent!") wird ausgeführt, bevor überhaupt eine Benachrichtigung gesendet wurde, und die äußere Funktion beendet sich, ohne jemals auf die Abschluss der einzelnen Sendvorgänge zu warten.

Der Grund ist, dass Array.prototype.forEach kein Konzept für Promises kennt. Es ruft den Callback für jedes Element auf und wechselt sofort zum nächsten, wobei es den von dem Callback zurückgegebenen Wert völlig ignoriert. Die Deklaration des Callbacks als async ändert nichts an dem Verhalten von forEach – es bedeutet lediglich, dass jede Aufrufung nun eine Promise erzeugt, die forEach ohne Prüfung verwirft. Die Benachrichtigungen werden dennoch irgendwann gesendet, nur asynchron im Hintergrund, ohne garantierte Reihenfolge und ohne Mechanismus, mit dem der Aufrufer Abschluss oder Fehler erkennen kann.

async function notifyAllUsers(userIds) {
  await Promise.all(userIds.map(async (id) => {
    const user = await getUser(id);
    await sendNotification(user);
  }));
  console.log("All notifications sent!");
}

Durch den Wechsel zu map werden die Promises in ein Array gesammelt anstatt weggeworfen, und das Umhüllen dieses Arrays mit Promise.all zwingt die Funktion dazu, tatsächlich zu warten, bis alle Sendvorgänge abgeschlossen sind. Jetzt sagt die Log-Ausgabe die Wahrheit.

Bug 2

app.post("/orders", async (req, res) => {
  const order = await createOrder(req.body);
  sendConfirmationEmail(order.customerEmail);
  res.status(201).json(order);
});

Die Route scheint einwandfrei zu funktionieren – Bestellungen werden erstellt, E-Mails gesendet und die Antwort kommt schnell zurück. Was ist also das Problem?

Das Problem: sendConfirmationEmail wird ohne await aufgerufen, wodurch die zurückgegebene Promise völlig unverwaltet bleibt. Wenn diese Promise abgelehnt wird, fängt nichts sie auf. Dieses Muster wird als „schwimmende Promise“ bezeichnet und ist nicht nur ein stilistisches Problem – es stellt vielmehr eine Betriebsgefahr dar. In aktuellen Versionen von Node.js kann eine unverarbeitete Ablehnung einer Promise den gesamten Prozess zum Absturz bringen und alle anderen laufenden Anfragen mit sich reißen, anstatt lediglich die E-Mail stillschweigend fehlschlagen zu lassen.

Hier verbirgt sich auch eine echte architektonische Frage, abgesehen von der fehlenden Fehlerrichtlinie: Soll ein fehlerhafter Bestätigungs-E-Mail-Verkehr verhindern, dass die Bestellung erfolgreich abgewickelt wird, oder soll die Bestellung dennoch durchgehen? In den meisten Fällen wäre man mit Letzterem zufrieden – die Bestellung war tatsächlich erfolgreich, auch wenn die Benachrichtigung fehlte. Doch es gibt einen Unterschied zwischen der Entscheidung, aufgrund eines Problems nicht abzubrechen, und dem völligen Versäumnis, dessen Fehler zu behandeln; dieser Code hat versehentlich Letzteres getan, obwohl vermutlich Ersteres beabsichtigt war.

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);
});

In dieser Version hält die E-Mail tatsächlich nicht die HTTP-Antwort auf, doch ein Fehler wird nun protokolliert, anstatt stillschweigend zu verschwinden oder den Server zum Absturz zu bringen.

Bug 3

function applyDiscount(cart) {
  const updatedCart = { ...cart };
  updatedCart.items.forEach((item) => {
    item.price = item.price * 0.9;
  });
  return updatedCart;
}

Auf den ersten Blick ähnelt dies dem üblichen Muster „kopieren anstelle von verändern“, das in jedem Leitfaden zur Vermeidung von Nebeneffekten vorkommt. Tatsächlich handelt es sich dabei um eine subtilere Falle.

Das Problem: Der Spread-Operator { ...cart } führt nur eine flache Kopie durch. Er erstellt zwar ein neues Objekt auf der obersten Ebene, doch updatedCart.items verweist weiterhin auf denselben Array – und dieselben Elementobjekte – wie cart.items. Wenn daher die forEach-Schleife item.price verändert, werden dadurch auch die ursprünglichen Artikel des Warenkorbs unsichtbar verändert, da der Spread-Operator niemals etwas jenseits der ersten Ebene der Struktur berührt hat.

console.log(cart.items[0].price);        // already discounted, unintentionally
console.log(updatedCart.items[0].price); // same object, same value

Jeder, der annahm, der ursprüngliche cart würde unverändert bleiben – eine vernünftige Erwartung, da der Name der Funktion darauf hindeutet, dass etwas Neues zurückgegeben wird – arbeitet letztendlich mit stillschweigend beschädigten Daten.

function applyDiscount(cart) {
  return {
    ...cart,
    items: cart.items.map((item) => ({ ...item, price: item.price * 0.9 })),
  };
}

Jede verschachtelte Ebene, die geändert werden muss, muss an dieser Ebene explizit kopiert werden. Eine oberflächliche Verbreitung schützt nur die Ebene, auf der sie direkt angewendet wird, nicht das darunterliegende Verschachtelte.

Fehler 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);
});

Das sieht wie eine gewöhnliche Routine zur „Verschmelzung eines Aktualisierungsobjekts mit einem bereits vorhandenen“ aus. Wo versteckt sich also die Gefahr?

Der Fehler: req.body kommt direkt vom Client, und es gibt hier nichts, was beschränkt, welche Schlüssel in das Benutzerobjekt übernommen werden dürfen. Wenn der Anfragekörper beispielsweise "role": "admin" oder "isVerified": true enthält, werden diese Eigenschaften genauso leicht übernommen wie jedes legitime Einstellungsfeld, da Object.assign kein Konzept dafür hat, welche Schlüssel schreibbar sein sollten – es übernimmt einfach alles, was ihm gegeben wird.

Dies fällt unter eine bekannte Kategorie von Schwachstellen namens Massenzuweisung und tritt häufig in APIs auf, die Anfragedaten direkt in Datenbankmodelle schreiben, ohne sie zunächst über eine Allowlist zu filtern. Das Risiko nimmt tatsächlich zu, je allgemeiner und wiederverwendbarer die Fusionsfunktion wird – genau das macht diesen Code ursprünglich ja zuverlässig erscheinen lassen.

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 };
}

Durch die Definition einer expliziten Allowlist garantieren Sie, dass eine Anfrage niemals ein Feld beeinflussen kann, das nicht ausdrücklich zur Modifikation freigegeben wurde – unabhängig davon, welche zusätzlichen Schlüssel jemand in den Payload einfügt.

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 };
}

An diesem Code ist technisch gesehen nichts falsch. Aber was kostet Sie es, ihn auf diese Weise zu schreiben?

Das Problem – oder genauer gesagt, die verpasste Chance – besteht darin, dass diese beiden Abfragen überhaupt nicht voneinander abhängen. Die zweite Abfrage benötigt kein Ergebnis der ersten, um ausgeführt werden zu können. Durch das Verknüpfen beider mit aufeinanderfolgenden await-Aufrufen wird die Gesamtwartezeit zur Summe der Dauer beider Abfragen, wobei eine die andere ohne wirklichen Grund blockiert.

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 };
}

Durch die Verwendung von Promise.all können beide Abfragen gleichzeitig ausgeführt werden anstelle nacheinander, wodurch die Gesamtzeit auf etwa die Zeit der langsameren Abfrage zusammenschrumpft, anstatt auf die Summe beider. Dies ist kein Fehler im Sinne von falschen Ergebnissen – die sequenzielle Version liefert vollkommen genaue Daten. Es handelt sich jedoch um einen Fehler, da dadurch leistungsstarkes Potenzial stillschweigend verschwendet wird. Dieses Vorgehen entsteht meist aus Gewohnheit und wird selten in Frage gestellt, da der Code auf den ersten Blick nicht fehlerhaft erscheint.

Was alle fünf gemeinsam haben

Jeder dieser Codeausschnitte lief fehlerfrei, erzeugte einen plausiblen Ausgabeinhalt und würde bei einer schnellen Überprüfung bestehen. Keiner von ihnen würde auffallen, wenn der einzige Test darin besteht zu prüfen, „ob es funktioniert, wenn ich es einmal ausprobiere“. Um sie aufzudecken, ist ein bestimmter Skeptizismus erforderlich: Wartet dieser asynchrone Code tatsächlich auf alles, wofür er scheinbar wartet? Kopiert diese Operation wirklich jede Ebene, die sie benötigt? Vertraut diese Verschmelzungslogik auf Eingaben, denen sie eigentlich nicht vertrauen sollte? Solch ein Instinkt entsteht nicht durch das Auswendiglernen weiterer Syntaxregeln, sondern dadurch, dass man bereits mindestens einmal von jedem dieser fünf genauen Muster betroffen war.