Чтение диффа на корректность — Примеры

Пример 1 — Дифф, глотающий свои сбои

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

+ async function chargeWithRetry(order) {
+   for (let attempt = 0; attempt < 3; attempt++) {
+     try {
+       return await gateway.charge(order);
+     } catch (e) {
+       log.warn("попытка списания не удалась");
+     }
+   }
+   return { status: "pending" };
+ }

Решение

БЛОКЕР — после трёх неудач он возвращает "pending", и вызывающий считает
заказ оплаченным: неудавшееся списание выдаётся за успех.
  вопрос: что должно произойти, когда все попытки провалились?

БЛОКЕР — пойманная ошибка логируется без причины, поэтому причина
теряется. вопрос: может ли лог нести ошибку и номер попытки?

ВОПРОС — повторять окончательный отказ бессмысленно.
  вопрос: это повторяемо или только тайм-аут?

Первые две находки и есть ревью: изменение превращает сбой в правдоподобно выглядящее значение и уничтожает то, что нужно для диагностики, а всё остальное вторично — сказать, что есть что, тоже часть работы. Ловушка в том, чтобы ревьюить имена и стиль цикла первыми: к моменту, когда внимание доходит до пути отказа, оно уже потрачено, а риск этого диффа целиком живёт там.

Пример 2 — Дифф, который читает баланс, а затем записывает его

Дифф ниже проверяет баланс и затем списывает его. Скажите, какому классу дефектов он подвержен, затем напишите находку.

+ async function debit(accountId, amount) {
+   const account = await accounts.get(accountId);
+   if (account.balance < amount) throw new InsufficientFunds();
+   account.balance -= amount;
+   await accounts.save(account);
+   return account.balance;
+ }

Решение

БЛОКЕР — чтение и запись не атомарны. Два одновременных списания могут
оба прочитать один баланс, оба пройти проверку и оба сохранить, поэтому
счёт уходит в минус.
  вопрос: можно ли сделать это одним условным обновлением или транзакцией,
  держащей строку, пока она читается и пишется?

ЗАМЕЧАНИЕ — сравнение безопасно, только если balance — целое число
минимальных единиц; стоит назвать единицу в типе.

Это тот вид изменения, который ревью должно искать прицельно — чтение, решение, затем запись, — а не читать каждую строку с равным вниманием. Отсутствующий тест тоже стоит поднять: тест, который запустит два списания сразу, упал бы. Ловушка в том, что код верен, когда запросы приходят по одному, — именно так его и будут тестировать и почти никогда так он не работает.