Пятого ноября 2021 года, конечно же это была пятница, мы выкатили обновление по нескольким гемам:
минорная версия Ruby on Rails
Ruby Sentry client
http клиенты
Puma
Devise
OmniAuth Ruby client
Mongoid
несколько других гемов по тестам.
Однако, что-то пошло не так. Мы заметили странные ошибки Stripe в Airbrake, затем в нашем аккаунте Stripe мы увидели это:
Менее чем за 1 час было создано 475 новых подписок на общую сумму $73.271.36 долларов. Мы не ожидали такого сильного притока денег.
Наше приложение по какой-то причине создавало новые подписки для старых учетных записей, которые были давно отменены или отключены. Мы понятия не имели, что и почему происходит, но решили отреагировать как можно быстрее.
Первое что мы сразу же сделали, это откатили обновления гемов до их безопасных старых версий, ротировали ключи Stripe API, чтобы заблокировать собственное приложение, и, вернули деньги всем тем, на ком это отразилось:
После возмещения денег всем пользователям на ком это отразилось, все 475 учетные записи были перепроверены вручную на их корректное платежное состояние. Так же мы отправили электронные письма с извинениями каждому клиенту, один за другим.
На тот момент мы не до конца выяснили причину. Большинство обновлений казались безобидными и после более глубокого изучения мы увидели что это действительно так. Мы обдумывали несколько гипотез:
проблемы с кешированием,
странные race conditions,
какие-то проблемы с безопасностью потоков, поскольку Puma изменила свою модель потоков в этом обновлении.
Тем не менее, мы наконец-то нашли проблему в одном из методов системы оплаты:
def renew_early_protected
# Prevent race condition
if user = User.where(
id: self.id
).or(
{:renewing_early_lock_at.lte => 1.hour.ago},
{:renewing_early_lock_at => nil},
).find_one_and_update("$set" => {renewing_early_lock_at: Time.now})
user.renew_early
end
end
Это часть функции кажется хорошей на бумаге: мы предлагаем автоматическое продление вашего плана, если у вас закончились поиски в нашем сервисе.
Цель renew_early_protected
в том, чтобы несколько раз не продлевать подписку и не снимать деньги, когда кто-то отправляет несколько запросов одновременно.
Обратите внимание на or()
метод на 47 строке. В Mongoid (драйвер MongoDB для Ruby) версии 7.0.8 or()
означает фильтровать документы, содержащие любое из условий аргумента. В нашем случае это фильтрует пользователей у которых есть определенный идентификатор, и которые либо никогда не имели renewing_early_lock
, либо их renewing_early_lock
был менее 1 часа назад.
Вычисленный селектор для Mongoid 7.0.8 для определённого пользователя:
computed_selector_in_mongoid_7_0_8 = {
"_id" => BSON::ObjectId('59af54094-----------64'), "$or" => [{
"renewing_early_lock_at" => {
"$lte" => 2022-01-07 20:14:53.44744 UTC
}
}, {
"renewing_early_lock_at" => nil
}]
}
Однако в Mongoid 7.3.3 or()
теперь означает фильтрацию документов, которые содержат любое из условий аргумента ИЛИ любое из предыдущих условий метода! В нашем случае, это фильтрование пользователей по ID ИЛИ отсутсвие renewing_early_lock_at
ИЛИ их renewing_early_lock
был менее 1 часа назад.
Вот как выглядит селектор для того же пользователя:
computed_selector_in_mongoid_7_3_3 = {
"$or" => [{
"_id" => BSON::ObjectId('59af54094-----------64')
}, {
"renewing_early_lock_at" => {
"$lte" => 2022-01-07 19:00:09.571034 UTC
}
}, {
"renewing_early_lock_at" => nil
}]
}
Обратите внимание, что селектор идентификатора пользователя был перемещен в необязательный or()
!
В конечном итоге, мы взимали плату за новые подписки у случайных клиентов, вместо реальных клиентов, которые хотели продлить подписки досрочно:
Нам очень жаль если вас затронула эта проблема. Это неприемлемо.
Даже если бы Mongoid не изменил поведение существующих методов между минорными версиями, наша реализация была дурно пахнущим кодом, поскольку было неясно что код делал. Этот код никогда не должен был быть выпущен в продакшн.
Но также и, что более важно, сама функция была плохой идеей. Использование нашего API для парсинга результатов поиска не должно приводить к продлению подписки по кредитной карте. Мы удалим эту функцию как можно скорее.
Мы приносим глубочайшие извинения всем нашим пользователям. В случае проблем с оплатами - свяжитесь с поддержкой SerpApi.
rinat_crone
Мда... Писать сервис, оперирующий деньгами и не покрывать выборки из БД тестовыми сценариями (как позитивными, так и негативными) — вопиющее дилетантство, как по мне. Смена поведения библиотеки при сохранеии API и мажорной версии это, конечно, тоже не дело, но тесты бы выявили это раньше, чем подобное добралось до прода.
Silverthorne
+1, тесты как DB layer, так и функциональные/black box в таком домене прям необходимы