Strona główna / Artykuły / Pięć oszukańczych błędów w Node.js, które przechodzą przez przegląd kodu niezauważone

Pięć oszukańczych błędów w Node.js, które przechodzą przez przegląd kodu niezauważone

Przejrzyj pięć rzeczywistych błędów w Node.js związanych z funkcją forEach, obietnicami typu floating oraz płytkimi kopiiami, aby zrozumieć, dlaczego kod, który działa poprawnie w środowisku testowym, może nadal zawieść w produkcji.

1485 słów

Każdy z pięciu poniższych przykładów kodu działa bez błędów i prawdopodobnie prześlizgnąłby się niezauważony podczas szybkiego przeglądu kodu. Mimo to każdy z nich spowodował prawdziwy awarię w jakimś systemie produkcyjnym, i to niejednokrotnie. Zanim przeczytasz wyjaśnienie pod każdym fragmencem, spróbuj samodzielnie ustalić, co jest nie tak.

Błąd 1

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

Zatrzymaj się tutaj i przemyśl, co tak naprawdę dzieje się podczas jego uruchomienia.

Błąd: linia console.log("Wszystkie powiadomienia wysłane!") jest wykonywana zanim jakiekolwiek powiadomienie faktycznie zostanie wysłane, a sama funkcja zewnętrzna kończy swoją pracę, nie czekając na zakończenie poszczególnych wysyłek.

Powodem jest to, że Array.prototype.forEach nie rozumie pojęcia obietnic. Wywołuje funkcję zwrotną dla każdego elementu i natychmiast przechodzi do następnego, całkowicie ignorując wartość, którą ta funkcja zwraca. Deklarowanie funkcji zwrotnej jako async nic nie zmienia w sposobie działania forEach — oznacza to jedynie, że każde wywołanie teraz tworzy obietnicę, którą forEach odrzuca bez jej sprawdzenia. Powiadomienia i tak zostaną w końcu wysłane, tylko asynchronicznie w tle, bez gwarantowanej kolejności i bez mechanizmu umożliwiającego wywołującemu stwierdzenie, czy operacja zakończyła się pomyślnie, czy nie.

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

Zmiana na map oznacza, że obietnice są zbierane do tablicy zamiast byłyby odrzucane, a umieszczenie tej tablicy wewnątrz Promise.all zmusza funkcję do rzeczywistego czekania, aż wszystkie operacje się zakończą. Teraz informacja z logu jest dokładna.

Błąd 2

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

Trasa wydaje się działać poprawnie — zamówienia są tworzone, wysyłane są e-maile, a odpowiedź przychodzi szybko. Więc w czym problem?

Błąd: funkcja sendConfirmationEmail jest wywoływana bez użycia await, więc jej zwracane obietnicy pozostają całkowicie niezarządzane. Jeśli takie obietnice zostaną odrzucone, nic ich nie przechwyci. Taki wzorzec nazywa się „pływającą obietnicą” i nie jest to tylko kwestia stylu – stanowi zagrożenie operacyjne. W obecnych wersjach Node.js nierozwiązane odrzucenie obietnicy może doprowadzić do upadku całego procesu, powodując awarię wszystkich innych bieżących zapytań, zamiast po prostu cicho pominąć wysyłkę e-maila.

Kryje się tu również poważne pytanie architektoniczne, poza brakiem obsługi błędów: czy e-mail z potwierdzeniem w przypadku niepowodzenia powinien uniemożliwić zakończenie zamówienia pomyślnie, czy też zamówienie powinno zostać zrealizowane mimo to? W większości przypadków chciałoby się tego drugiego – zamówienie faktycznie powinno zostać zrealizowane, nawet jeśli powiadomienie nie dotrze. Istnieje jednak różnica pomiędzy decyzją o nieroz blokowaniu procesu a całkowitym brakiem obsługi błędów, a ten kod przypadkowo zrobił to drugie, mimo że prawdopodobnie zamierzał pierwsze.

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

W tej wersji e-mail rzeczywiście nie blokuje odpowiedzi HTTP, ale teraz każde niepowodzenie jest rejestrowane, zamiast znikać bez śladu lub powodować awarię serwera.

Błąd 3

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

Na pierwszy rzut oka przypomina to standardową zasadę „kopiowanie zamiast modyfikacji”, którą można znaleźć we wszystkich przewodnikach dotyczących unikania efektów ubocznych. W rzeczywistości jest to jednak bardziej subtelna pułapka.

Błąd: operator rozszerzania { ...cart } wykonywa jedynie powierzchowną kopię. Tworzy nowy obiekt na najwyższym poziomie, ale updatedCart.items nadal wskazuje na ten sam tablicę — i te same obiekty elementów — co cart.items. Dlatego gdy pętla forEach modyfikuje item.price, w rzeczywistości zmienia również oryginalne elementy koszyka, choć niewidocznie, ponieważ operator rozszerzania nigdy nie dotykał niczego poza pierwszym poziomem struktury.

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

Każdy, kto zakładał, że oryginalny cart pozostanie nietknięty — co jest rozsądnym oczekiwaniem, biorąc pod uwagę, że nazwa funkcji sugeruje zwracanie czegoś nowego — ostatecznie pracuje z danych w tajemnicy uszkodzonymi.

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

Każda warstwa nawiasów, która musi ulec zmianie, musi zostać wyraźnie skopiowana na tej warstwie. Płytkie rozszerzenie chroni jedynie poziom, na którym bezpośrednio działa, a nie żadne elementy znajdujące się poniżej.

Błąd 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);
});

To wygląda jak zwykła procedura „połączenia obiektu aktualizacji z istniejącym”. Więc gdzie kryje się niebezpieczeństwo?

Błąd: req.body pochodzi bezpośrednio od klienta, a nic tutaj nie ogranicza, które klucze mogą trafić do obiektu użytkownika. Jeśli ciało żądania zawiera coś w rodzaju "role": "admin" lub "isVerified": true, te właściwości są łączone równie łatwo jak każde prawidłowe pole ustawień, ponieważ Object.assign nie ma pojęcia o tym, które klucze powinny być edytowalne — łączy wszystko, co otrzymuje.

To należy do dobrze znanej kategorii wrażliwości zwanej masowym przypisywaniem i często występuje w API, które bezpośrednio wprowadzają treść żądania do modeli bazy danych, nie filtrowując jej najpierw za pomocą listy dozwolonych wartości. Ryzyko wzrasta wraz z tym, jak funkcja łączenia staje się bardziej uniwersalna i wielokrotnie użyteczna — co właśnie sprawia, że tego typu kod wydaje się na początku godny zaufania.

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

Definiując wyraźną listę dozwolonych wartości, gwarantujesz, że żądanie nigdy nie będzie mogło wpłynąć na pole, którego modyfikacji nie zostało specjalnie upoważnione, bez względu na to, jakie dodatkowe klucze ktoś doda do treści żądania.

Błąd 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 };
}

W tym kodzie technicznie nie ma nic złego. Ale ile to kosztuje, gdy piszesz go w ten sposób?

Błąd – a właściwie przegapiona szansa – polega na tym, że te dwa zapytania w ogóle nie są od siebie zależne. Drugie zapytanie nie potrzebuje żadnego wyniku z pierwszego, aby mogło zostać wykonywane. Poprzez łączenie ich sekwencyjnymi wywołaniami await, całkowity czas oczekiwania staje się sumą czasu trwania obu zapytań, przy czym jedno blokuje drugie bez żadnego rzeczywistego powodu.

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

Użycie Promise.all umożliwia wykonywanie obu zapytań równocześnie, a nie po kolei, dzięki czemu całkowity czas wykonywania spada mniej więcej do czasu potrzebnego na wolniejsze z nich, zamiast być sumą czasów obu. Nie jest to błąd w sensie generowania niewłaściwych wyników — wersja sekwencyjna zwraca idealnie dokładne dane. Jest to jednak błąd, ponieważ w tajemnicy marnuje dostępną wydajność. Jest to wzorzec stosowany z przyzwyczajenia i rzadko kwestionowany, ponieważ na pierwszy rzut oka kod nie wygląda na uszkodzony.

Co łączy te pięć przypadków

Każdy z tych fragmentów działał bez błędów, generował rozsądne wyniki i sprawiał wrażenie poprawnego po szybkim przejrzeniu. Żaden z nich nie zostałby wykryty, gdyby jedynym testem było sprawdzenie, „czy działa, gdy spróbuję raz”. Wykrycie takich problemów wymaga specyficznego rodzaju sceptycyzmu: czy ten asynchroniczny kod rzeczywiście czeka na wszystko, na co zdaje się czekać; czy ta operacja kopiowania faktycznie tworzy kopię każdej warstwy, której potrzebuje; czy ta logika łączenia ufa danym, którym nie powinien ufać. Taki instynkt nie wynika z zapamiętywania większej ilości składni – pochodzi z doświadczenia, gdy każdy z tych pięciu konkretnych wzorców już raz sprawił problemy.

Literatura pokrewna

  • Powszechne błędne wyobrażenia o async/await, które powodują błędy w produkcji — Wyjaśnia dziewięć subtelnych nieporozumień związanych z async/await – od warunków konkurencyjnych po nierozpatrzone odrzucenia – które potajemnie psują aplikacje JavaScript w rzeczywistych warunkach.
  • Powszechne błędne wyobrażenia o Node.js i bazach danych, które powodują błędy w produkcji — Dowiedz się, dlaczego async/await, poolowanie połączeń oraz ORM-y nie zapobiegają automatycznie warunkom konkurencyjnym, wyczerpaniu połączeń czy iniekcjom SQL w aplikacjach Node.js.