Skip to content

Wallet#add_credits uses return inside a with_lock block — credit grants can silently roll back depending on Rails version #31

Description

@TalGruperFolloze

Summary

lib/usage_credits/models/wallet.rb (~line 235) does return transaction from inside a with_lock do ... end block. with_lock opens a transaction, so this is a non-local return out of a transaction block — and Rails' behavior for that has changed several times across the version range this gem supports (rails >= 6.1).

Why it matters

Depending on the host app's Rails version and load_defaults, the same add_credits call either commits or silently rolls back:

Rails Behavior on return out of a transaction
6.1 – 7.0 Emits DEPRECATION WARNING: Using return, break or throw to exit a transaction block is deprecated ... This results in the transaction being committed, but in the next release of Rails it will rollback. Silent rollback on 7.0.x was reported in rails/rails#45017.
7.1 Controlled by config.active_record.commit_transaction_on_non_local_return — true for apps on load_defaults 7.1, but apps still on older defaults get the rollback.
7.2+ Historical commit behavior restored; the config is deprecated.

So an app on 7.0, or on 7.1 without load_defaults 7.1, gets a silent rollback of every credit grant. There's no exception — add_credits still returns a Transaction object, so callers and callbacks see success — but the ledger row is gone. Nothing appears in the logs.

Every grant path routes through this: start_credit_period!, apply_credits_config_change!, add_credits!, and per-user give_credits. Wallets would read zero with no indication anything failed.

This is also exactly the case rubocop-rails' Rails/TransactionExitStatement cop flags, including its explicit note that with_lock implicitly opens a transaction.

Suggested fix

Delete the return keyword. transaction is already the last expression in the block, so the return value of the method is unchanged — it's a one-word change that makes the behavior identical and version-independent:

with_lock do
  # ...
  transaction   # was: return transaction
end

Two related suggestions:

  1. Audit the rest of the codebase for other non-local exits (return / break / throw) inside with_lock or transaction blocks.
  2. Consider enabling Rails/TransactionExitStatement in the gem's RuboCop config to catch regressions.

Definition of done

A spec that grants credits and then asserts the balance actually persists after the call — reloading the wallet and checking the row exists. Because the failure mode is silent and the method still returns a truthy Transaction, only a persistence assertion catches it.

Environment

  • usage_credits 0.5.0 (latest published as of Aug 2026)
  • Currently on Rails 6.1, preparing a Rails 7 upgrade

Happy to open a PR for the one-word change if that's helpful.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions