Пяць хітрых багоў у Node.js, якія праходзяць перагляд коду непазначаныя
Разберымся з пяцьма рэальнымі багамі у Node.js, якія стосуюцца функцыі forEach, об’ектаў з типам promise і паверхневых копій, ўбачымчы, чаму код, які працюе нормальна ў тэставанні, можа застацца нефункцыональным у рэальных умовах.
Кожны з пяці прыкладаў коду, якія даўно нижэй, выконваюцца без якоўых-леба прычын, і кожны з іх, верагодна, засталіся бы непазначаным пад час швайнае перагляду коду. Аднак кожны з іх вельмі часта спрычыняў справжню перャбою ў якой-небудзь прымэтной системе. Перш чым чытаць адказ пад кожным фрагментам, спробуйце самі з’ясаваць, у чым проблема.
Баг 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-адказ, але тепер памылка фіксуецца, у працоўнасці не зникае таямніча і не выклікае крах сервера.
Bug 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 дазваляе абам запыткам выкананыя паралельна, а не адна за іншай, таму загальны час выканання складаеся прыблізна з часу выканання медленейшай запыткі, а не з сумы часу обох. Гэта не ўломка у сенсе таго, што гэта дае некоректныя рынкі — последовальная версія вярна вяртае даны. Цэла ўломка у сенсе таго, што гэта бесшумна марнуе доступную продуктывасць, і гэта той тип падходу, які ствараецца з прыzwычкі і рэдка калі паддаецца сумневам, адколі на першы погляд у кодзе няма нічага, што бы выглядала не так.
Шта ўсе пяць элементаў маюць спакойна
Кожны з эых фрагментаў запрацавалі без адхылэнняяў, стварылі результаты, якія здаваліся разумнымі, і ўсе было гаразд пад час кароткага перагляду. Жадны з іх не быў б выявлены, якбы вашым ежынственным тэстам было «чы робіць гэта, калі спробаваць раз». Ёх выявленне трэбуе спецыяльнага роду скептыцизму: чакаець кі ўсё гэта асінхронны код практычна на всё, на што, як здаецца, чакае; чы справды копіюванне дадзеных воспамінае кожны слой, які ёму трэба; чы логіка з’еднання дазволяе сабе паверыць у данні, у якія ёй не трэба паверыць. Такі інстынкт не ствараецца праз запам’ятовыванне большай колькасці сынтаксу. Ён вырастае праз тое, што кожны з гэтых пяці конкретных патэранаў вядома прынаймні раз вадзіў да проблем.
Спадневаная літэратура
- Пашчэрэдзіны JavaScript і TypeScript, якія тыха разбиваюць код — Апаведамляе пра тонкія пашчэрэдзіны JavaScript і TypeScript — ад порэвання NaN да асінхронных таймінгаў і прымусовага пераканвання типаў — якія вызываюць багі, нават калі код выглядае правільна.
- Пяць пасткі Temporal API, якія тыха зноў вызываюць багі з датамі — Дазволяе дазнацца, як Temporal API ў JavaScript все ж можа вызываць багі з часовымя регіонамі, серыялізацыёй, тачнасцю і трываласцю, якщо не выправіць пяць распашчэтных спосабаў ўжыцца.