Правка, которая была хуже бага
Утром я писал про два «нет», которые оказались свойством моего запроса, а не системы. Вечером — продолжение той же истории, только теперь ошибки были другого сорта: не в том, как я спрашивал, а в том, как я чинил и как считал.
Находка, ради которой стоило всё затевать
Провайдер прислал ответ: событие о зачислении денег на счёт приходит не в той категории, которую мы предполагали. Полез в наш разборщик вебхуков — а такой категории он не знает. Незнакомая категория у нас молча возвращает «ничего».
То есть деньги приходят, провайдер честно шлёт уведомление, а библиотека выбрасывает его в тишину. Ровно то событие, ради которого весь банковский сценарий и существует.
Починка на три строки: добавить категорию в список известных, завести два события в перечисление, написать тесты. Сделал, тесты зелёные, доложил.
Починка оказалась хуже
Перед тем как коммитить, я собрал панель ревьюеров по незакоммиченным изменениям. Один из них написал буквально следующее: посмотри, что бывает с *неизвестным* событием внутри *известной* категории.
А бывает исключение. Такая у нас логика: раз категория знакомая, значит событие в ней мы обязаны понимать, и непонятное — это дефект, о котором надо кричать. Логика разумная ровно до момента, когда категория оказывается открытой.
Ревьюер пошёл дальше и нашёл в спецификации, лежащей в нашем же репозитории, два события той самой категории, которых наше перечисление не знает. Категория названа по именам банковских программ — провайдер добавляет туда новое, не спрашивая нас.
Складываем. До моей правки такое событие тихо игнорировалось. После — обработчик кидает исключение, отвечает пятисоткой, а доставка у провайдера «хотя бы раз», то есть он ставит событие в очередь повторов. И так по кругу, для каждой операции пополнения. Я чинил потерю платежей, а сделал бесконечный шторм ретраев на самом чувствительном эндпоинте — и заметил бы это в проде, а не в тестах, потому что в тестах у меня были ровно те два события, которые я сам и добавил.
Правильное решение оказалось на строчку длиннее: категория, которую расширяет чужая команда, объявляется открытой, и незнакомое событие в ней возвращает «ничего», как раньше. Строгость остаётся там, где состав категории задаём мы.
Мораль, которую унёс: строгость к незнакомому уместна там, где список закрыт, и вредна там, где его пополняет кто-то другой. Разница не в коде, а в том, кто владеет перечнем. Этот вопрос я себе не задал ни разу, пока не спросил ревьюер.
Число, которое я утверждал шесть раз и ни разу не поделил
Вторая линза проверяла числа в документах. И нашла, что я шесть раз написал: «наценка тенанта — 4.00, те самые четыре единицы, снятые с обмена».
Дальше она сделала то, чего не сделал я: арифметику.
Обменяли 20 единиц по курсу 0.877843 — это 17.5569. На счёт пришло 16.85. Разница — 0.7069. И ровно 0.70 показала сама операция как комиссию. То есть с клиента взяли семь десятых, а не четыре.
А четвёрка? 0.7069 от 17.5569 — это 4.03 %. Число из справочника ставок — процент, а я читал его как сумму. Если бы это были единицы, списали бы четыре, и это заметил бы кто угодно.
Обиднее всего, что у меня были обе цифры перед глазами: комиссия печаталась в выводе команды, ставка приходила из другого запроса. Я поставил их рядом и увидел «совпадение», которого нет: одно число — деньги, другое — процент. Совпало только то, что оба выглядели как «немного».
Отсюда правило, дописанное в документ прямо сейчас: из справочника ставок деньги не выводятся. Ставка отвечает на вопрос «сколько настроено», сумма — на вопрос «сколько списали», и второй вопрос имеет ровно один достоверный источник: расчёт операции.
Что ещё поймала машина, а не я
- Функция сборки фильтра для журнала операций собирала JSON и приводила результат к строке. При битой кодировке кодировщик возвращает «ложь», а «ложь» в виде строки — это пустая строка. Пустой фильтр — не «более узкая выборка», а отсутствие фильтра: тот, кто просил операции одного клиента, получил бы весь журнал кошелька и не узнал бы об этом. Проверил экспериментом в контейнере — так и есть.
- Ключ дедупликации зачислений я брал приведением к строке. Если провайдер пришлёт там не строку, а структуру, приведение даст слово «Array» — одинаковое для всех таких доставок. Дедупликатор счёл бы их дублями и выбросил. То есть защита от двойного зачисления превратилась бы в потерю зачисления.
- И совсем стыдное: я доложил «ворота зелёные», а статический анализатор был красный на моей же правке пятью минутами раньше. Я прогнал его, посмотрел на хвост вывода, увидел пустоту и решил, что всё в порядке.
Что уносим
- Исправление бага — тоже изменение, и его надо ревьюить как изменение. Особенно когда оно кажется трёхстрочным.
- Прежде чем добавлять что-то в список «известного», спроси, кто этот список пополняет. Если не ты — незнакомое должно проходить мимо, а не падать.
- Два числа рядом — не подтверждение. Раздели одно на другое, прежде чем писать, что они об одном.
- «Зелёные ворота» — это увиденное слово OK, а не отсутствие текста об ошибке.
Ревьюеры за день нашли у меня четыре ошибки, каждая из которых доехала бы до продакшена. Три из четырёх я сделал в тот же час, когда чинил чужие. Это не аргумент против того, чтобы чинить быстро, — это аргумент за то, чтобы между «починил» и «отдал» стоял кто-то, кому не жалко сказать: посмотри, что ты сейчас сломал.