Головна / Статті / П’ять оманливих багів Node.js, які проходять перевірку коду непоміченими

П’ять оманливих багів Node.js, які проходять перевірку коду непоміченими

Розгляньте п’ять реальних помилок у Node.js, пов’язаних із методом forEach, обіцянками з плаваючою точкою та поверхневими копіями, щоб зрозуміти, чому код, який працює без проблем у тестах, все одно може зламатися в реальних умовах.

1485 слів

Кожен із п’яти наведених нижче прикладів коду виконується без помилок, і кожен з них, ймовірно, міг би пройти поверхневу перевірку коду непоміченим. Проте кожен з них вже неодноразово спричиняв справжні перерви у роботі певних продакшн-систем. Перш ніж читати пояснення до кожного фрагмента, спробуйте самостійно з’ясувати, у чому проблема.

Баг 1

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

Зупиніться тут та обміркуйте, що насправді відбувається під час його виконання.

Баг: рядок console.log("All notifications sent!") виконується ще до того, як будь-яке сповіщення фактично надішлеться, а сама зовнішня функція завершується, не чекаючи на завершення окремих операцій надсилання.

Причина полягає у тому, що Array.prototype.forEach не має поняття про обіцянки. Він викликає функцію-колбек для кожного елемента та негайно переходить до наступного, повністю ігноруючи будь-яке значення, яке повертає колбек. Оголошення функції-колбека як async нічого не змінює у поведінці forEach — це просто означає, що кожен виклик тепер створює обіцянку, яку forEach викидає без перевірки. Повідомлення все одно будуть надсилатися з часом, просто асинхронно на фоні, без гарантованого порядку та без механізму для того, щоб викликаючий міг виявити завершення чи невдачу.

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

Перехід на map означає, що обіцянки збираються у масив, а не викидаються, а обгортання цього масиву у Promise.all змушує функцію справді чекати, поки не завершаться усі операції. Тепер запис у журналі відображає правду.

Баг 2

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

Здається, маршрут працює нормально — створюються замовлення, надсилаються електронні листи, і відповідь надходить швидко. То в чому проблема?

Проблема: функція sendConfirmationEmail викликається без await, тому повернена нею обіцянка залишається повністю некерованою. Якщо ця обіцянка відхиляється, ніщо її не ловить. Таку ситуацію називають „плаваючою обіцянкою“, і це не просто стилістична проблема — це ризик для функціонування системи. У поточних версіях Node.js некероване відхилення обіцянки може призвести до зупинки всього процесу, що призведе до збою усіх інших запитів, замість того, щоб просто безшумно зазнати невдачі електронного листа.

Тут також криється справжнє архітектурне питання, окрім відсутності обробки помилок: чи повинен електронний лист із повідомленням про невдачу завадити успішному виконанню замовлення, чи замовлення має пройти незалежно від цього? У більшості випадків бажано другий варіант — замовлення справді має бути успішним, навіть якщо сповіщення не надійшло. Але є різниця між вирішенням не блокувати процес через певну проблему та повною відсутністю обробки її помилок, і цей код випадково зробив друге, хоча, ймовірно, мав на увазі перше.

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

У цій версії електронний лист справді не затримує HTTP-відповідь, але тепер помилка фіксується, замість того щоб безслідно зникнути чи спричинити збій сервера.

Баг 3

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

На перший погляд це нагадує стандартну практику „копіювати замість того, щоб мутувати“, яку можна знайти в будь-якому посібнику з уникнення побічних ефектів. Насправді це ще більш підступна пастка.

Проблема: оператор розповсюдження { ...cart } виконує лише поверхневу копію. Він створює новий об’єкт на верхньому рівні, але updatedCart.items все одно посилається на той самий масив — і ті самі об’єкти елементів — що й cart.items. Тож коли цикл forEach змінює значення item.price, він також непомітно змінює елементи оригінального кошика, адже оператор розповсюдження ніколи не торкається нічого за межами першого рівня структури.

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

Будь-хто, хто припускав, що оригінальний cart залишиться недоторканим — що є цілком логічним, адже назва функції свідчить про те, що вона повертає щось нове — у підсумку працює з даними, які були тихо пошкоджені.

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

Кожен рівень вкладеності, який потребує змін, має бути явно скопійований на цьому рівні. Поверхневе поширення захищає лише той рівень, на якому воно працює безпосередньо, а не щось, що знаходиться під ним.

Баг 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);
});

Це схоже на звичайну процедуру „злиття об’єкта оновлення з існуючим“. То де ж ховається небезпека?

Баг: req.body надходить безпосередньо від клієнта, і тут немає жодних обмежень щодо ключів, які можуть потрапити до об’єкта користувача. Якщо тіло запиту містить щось на кшталт "role": "admin" або "isVerified": true, ці властивості зливаються так само легко, як і будь-які законні поля налаштувань, оскільки Object.assign не має уявлення про те, які ключі мають бути записуваними — він зливає все, що йому дають.

Це належить до добре відомої категорії вразливостей під назвою масове присвоєння, і вона часто зустрічається в API, які без попереднього фільтрування через список дозволених значень вставляють тіла запитів безпосередньо до моделей бази даних. Ризик насправді зростає, коли функція об’єднання стає більш універсальною та повторно використовуваною — саме це спочатку і створює враження надійності такого коду.

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

Визначивши чіткий список дозволених значень, ви гарантуєте, що запит ніколи не зможе вплинути на поле, яке йому конкретно не було дозволено модифікувати, незалежно від того, які додаткові ключі хтось додасть у вантаж запиту.

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

У цьому коді технічно немає жодних помилок. Але яку ціну ви платите за те, що пишете його таким чином?

Проблема — а точніше, втрачена можливість — полягає у тому, що ці два запити зовсім не залежать один від одного. Другому запиту не потрібні жодні результати від першого, щоб він міг бути виконаний. Шляхом поєднання їх за допомогою послідовних викликів await загальний час очікування стає сумою тривалостей обох запитів, причому один блокує інший без реальної причини.

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

Використання Promise.all дозволяє обом запитам виконуватися одночасно, а не по черзі, тож загальний час виконання скорочується приблизно до часу виконання повільнішого запиту, замість суми часів обох. Це не є багом у сенсі отримання неправильних результатів — послідовна версія повертає абсолютно точні дані. Це є багом у сенсі тихого марнування доступної продуктивності, і це саме той підхід, який використовується з звички та рідко піддається сумніву, оскільки при поверхневому перегляді код виглядає бездоганно.

Що є спільним у всіх п’яти

Кожен із цих фрагментів працював без помилок, генерував прийнятний результат та легко проходив швидкий огляд. Жоден з них не виявиться під час тестування, яке обмежується лише запитанням «Чи працює це, якщо спробувати один раз?». Щоб їх виявити, потрібен певний тип скептицизму: чи справді цей асинхронний код чекає на все, на що, здається, чекає; чи справді ця операція копіювання дублює кожен необхідний рівень; чи довіряє ця логіка об’єднання даних інформації, якій не слід довіряти. Такий інстинкт не формується шляхом запам’ятовування додаткової синтаксису. Він виникає через те, що кожен із цих п’яти конкретних шаблонів вже завдав проблем принаймні один раз.

Пов’язана література

  • Поширені хибні уявлення про async/await, які спричиняють проблеми в продакшені — пояснює дев’ять тонких непорозумінь щодо async/await — від ситуацій змагання до некерованих відмов — які тихо руйнують реальні JavaScript-додатки.
  • Поширені хибні уявлення про Node.js та бази даних, які спричиняють проблеми в продакшені — дізнайтеся, чому async/await, пулінг з’єднань та ORM не запобігають автоматично ситуаціям змагання, виснаженню з’єднань чи втручанню SQL у додатках Node.js.