fix (wallets): improve handling rc upon wallet update (#2669)

## Context This PR improves how we handle race condition scenario upon wallet update
getlago · Oct 10, 2024 · f181636 · f181636
1 parent ed98903
commit f181636
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 35 deletions.
diff --git a/app/jobs/invoices/prepaid_credit_job.rb b/app/jobs/invoices/prepaid_credit_job.rb
@@ -4,6 +4,7 @@ module Invoices
   class PrepaidCreditJob < ApplicationJob
     queue_as 'wallets'
 
+    retry_on ActiveRecord::StaleObjectError, wait: :polynomially_longer, attempts: 6
     unique :until_executed, on_conflict: :log
 
     def perform(invoice)

diff --git a/app/services/wallets/apply_paid_credits_service.rb b/app/services/wallets/apply_paid_credits_service.rb
@@ -11,9 +11,11 @@ def call
       return result unless wallet_transaction
       return result if wallet_transaction.status == 'settled'
 
-      WalletTransactions::SettleService.new(wallet_transaction:).call
-      Wallets::Balance::IncreaseService
-        .new(wallet: wallet_transaction.wallet, credits_amount: wallet_transaction.credit_amount).call
+      ActiveRecord::Base.transaction do
+        WalletTransactions::SettleService.new(wallet_transaction:).call
+        Wallets::Balance::IncreaseService
+          .new(wallet: wallet_transaction.wallet, credits_amount: wallet_transaction.credit_amount).call
+      end
 
       result.wallet_transaction = wallet_transaction
       result

diff --git a/app/services/wallets/balance/increase_service.rb b/app/services/wallets/balance/increase_service.rb
@@ -3,51 +3,34 @@
 module Wallets
   module Balance
     class IncreaseService < BaseService
-      MAX_RETRIES = 5
-
       def initialize(wallet:, credits_amount:, reset_consumed_credits: false)
         super
 
         @wallet = wallet
         @credits_amount = credits_amount
         @reset_consumed_credits = reset_consumed_credits
-        @retries = 0
       end
 
       def call
-        ActiveRecord::Base.transaction do
-          currency = wallet.balance.currency
-          amount_cents = wallet.rate_amount * credits_amount * currency.subunit_to_unit
-
-          update_params = {
-            balance_cents: wallet.balance_cents + amount_cents,
-            credits_balance: wallet.credits_balance + credits_amount,
-            last_balance_sync_at: Time.current
-          }
-
-          if reset_consumed_credits
-            update_params[:consumed_credits] = [0.0, wallet.consumed_credits - credits_amount].max
-            update_params[:consumed_amount_cents] = [0, wallet.consumed_amount_cents - amount_cents].max
-          end
-
-          wallet.update!(update_params)
-          Wallets::Balance::RefreshOngoingService.call(wallet:)
+        currency = wallet.balance.currency
+        amount_cents = wallet.rate_amount * credits_amount * currency.subunit_to_unit
+
+        update_params = {
+          balance_cents: wallet.balance_cents + amount_cents,
+          credits_balance: wallet.credits_balance + credits_amount,
+          last_balance_sync_at: Time.current
+        }
+
+        if reset_consumed_credits
+          update_params[:consumed_credits] = [0.0, wallet.consumed_credits - credits_amount].max
+          update_params[:consumed_amount_cents] = [0, wallet.consumed_amount_cents - amount_cents].max
         end
 
+        wallet.update!(update_params)
+        Wallets::Balance::RefreshOngoingService.call(wallet:)
+
         result.wallet = wallet
         result
-      rescue ActiveRecord::StaleObjectError
-        @retries += 1
-
-        if @retries <= MAX_RETRIES
-          sleep(0.5)
-
-          wallet.reload
-
-          retry
-        end
-
-        result.service_failure!(code: 'race_condition_error', message: '')
       end
 
       private

diff --git a/spec/jobs/invoices/prepaid_credit_job_spec.rb b/spec/jobs/invoices/prepaid_credit_job_spec.rb
@@ -44,4 +44,22 @@
     described_class.perform_now(invoice)
     expect(Invoices::FinalizeOpenCreditService).to have_received(:call).with(invoice:)
   end
+
+  it 'does not retry the job' do
+    expect {
+      described_class.perform_now(invoice)
+    }.not_to have_enqueued_job(described_class)
+  end
+
+  context 'when there is race condition error' do
+    before do
+      allow(Wallets::ApplyPaidCreditsService).to receive(:call).and_raise(ActiveRecord::StaleObjectError.new)
+    end
+
+    it 'retries the job' do
+      expect {
+        described_class.perform_now(invoice)
+      }.to have_enqueued_job(described_class)
+    end
+  end
 end