Пятого ноября 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.

Комментарии (7)


  1. rinat_crone
    30.01.2022 18:40
    +17

    Мда... Писать сервис, оперирующий деньгами и не покрывать выборки из БД тестовыми сценариями (как позитивными, так и негативными) — вопиющее дилетантство, как по мне. Смена поведения библиотеки при сохранеии API и мажорной версии это, конечно, тоже не дело, но тесты бы выявили это раньше, чем подобное добралось до прода.


    1. Silverthorne
      30.01.2022 21:57

      +1, тесты как DB layer, так и функциональные/black box в таком домене прям необходимы


  1. z0ic
    30.01.2022 18:45
    +1

    Проблема разделения семантики языка запросов и семантики языка программирования. Теперь я понимаю для чего был создан более универсальный LINQ.


    1. stitrace
      30.01.2022 23:03
      +3

      Чтобы продолжать не писать тесты?


  1. DrAndyHunter
    31.01.2022 07:52
    +3

    Оперировать деньгами и подписками через Mongodb? Ну такое...


    1. spacediver
      31.01.2022 12:41

      «Так исторически сложилось»))


  1. diogen4212
    31.01.2022 17:07

    показалось, что новость о донатах в Геншине