Пять обманчивых ошибок в Node.js, которые проходят проверку кода незамеченными
Рассмотрим пять реальных ошибок в Node.js, связанных с методом forEach, «плавающими» обещаниями и поверхностными копиями, чтобы понять, почему код, который работает нормально в тестах, может всё равно выдать ошибки в производственной среде.
Каждый из пяти приведённых ниже примеров кода выполняется без ошибок, и каждый из них, скорее всего, мог бы пройти незамеченным при кратком анализе кода. Однако каждый из них уже неоднократно вызывал реальные сбои в какой-либо производственной системе. Прежде чем читать объяснение к каждому фрагменту, попробуйте сами выяснить, в чём проблема.
Ошибка 1
async function notifyAllUsers(userIds) {
userIds.forEach(async (id) => {
const user = await getUser(id);
await sendNotification(user);
});
console.log("All notifications sent!");
}
Остановитесь здесь и подумайте, что на самом деле происходит при его выполнении.
Ошибка: строка console.log("Все уведомления отправлены!") выполняется до того, как хотя бы одно уведомление действительно будет отправлено, а сама внешняя функция завершается, так и не дождавшись окончания отправки отдельных уведомлений.
Причина в том, что у 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 };
}
Определив явный список разрешенных значений, вы гарантируете, что запрос никогда не сможет повлиять на поле, для изменения которого ему конкретно не было дано разрешение, независимо от того, какие дополнительные ключи кто-то вставит в тело запроса.
Ошибка 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 позволяет выполнять оба запроса одновременно, а не поочередно, в результате чего общее время выполнения сокращается примерно до времени выполнения более медленного запроса, а не к сумме времени обоих. Это не является багом в смысле получения некорректных результатов — последовательная версия возвращает совершенно точные данные. Это баг потому, что он тихо тратит доступную производительность, и это тип паттерна, который формируется из привычки и редко ставится под сомнение, поскольку при поверхностном просмотре код кажется непроблемным.
Что у всех пяти общего
Каждый из этих фрагментов выполнился без ошибок, выдал разумный результат и легко проходит поверхностную проверку. Ни один из них не будет обнаружен, если единственным критерием тестирования будет вопрос «работает ли это при одной попытке использования». Чтобы их обнаружить, необходим особый вид скептицизма: действительно ли этот асинхронный код ждёт всего того, на что, кажется, ожидает; действительно ли операция копирования создаёт все необходимые копии; действительно ли логика объединения доверяет входным данным, которым ей не следует доверять. Такой инстинкт формируется не за счёт запоминания дополнительной синтаксиса, а благодаря тому, что человек уже сталкивался с каждым из этих пяти конкретных шаблонов хотя бы один раз ранее.
Связанные материалы
- Распространённые ошибки JavaScript и TypeScript, которые тайно ломают код — Рассматриваются тонкие проблемы JavaScript и TypeScript — от сравнений с NaN до асинхронных задержек и принудительной конвертации типов — которые вызывают баги, несмотря на кажущуюся корректность кода.
- Пять ловушек API Temporal, которые тайно вновь вызывают проблемы с датами — Узнайте, как API Temporal в JavaScript всё ещё может приводить к ошибкам с часовыми поясами, сериализацией, точностью и продолжительностью, если не устранить пять распространённых способов неправильного использования.