Yesterday I had a strange problem with my code. The state of objects in the database was changed although it clearly should not. Yes, I was indeed changing those objects, but I was doing so in a transaction block and I knew that further down in the block an exception was raised, leaving the block, forcing the transaction to rollback. Or so I though.
Consider this (very contrived) method
def move_money(from_account, to_account, amount) Bank.transaction do to_account.deposited(amount) raise InsufficientFunds if from_account.balance < amount from_account.withdraw(amount) end true rescue InsufficientFunds false end
You'd reasonably expect that in case the exception is raised the transaction is rolled back. Now consider a further method
def launder_money(amount, *accounts) Bank.transaction do accounts[0..-2].zip(accounts[1..-1]) do |from_account, to_account| moved = move_money(from_account, to_account, amount) unless moved send_collector(from_account) break end end end end
When this method is called and the case occurs where one of the individual move_money calls returns fails, there is a curious effect: The database transaction is not rolled back, instead it is successfully committed. That is, money deposited into an account where there's no corresponding withdrawal.
What went wrong? It's simply that database transactions are only rolled back when the outermost transaction block is left through an exception. With the two methods above this is the case when the first method is called on its own, but when it is called from within the transaction block in the second method, then the database transaction is unaffected.
What I want to have is this:
* Transactional methods that don't have to raise an exception at their caller in order to indicate that the transaction must be rolled back. * Transactional methods composed of other transactional methods.
I think this situation can't be resolved completely automatically. In my opinion, the strategy currently employed by ActiveRecord has it the wrong way around. Whenever a transaction block is left exceptionally, the current database transaction should be marked for rollback. For the (few?) cases where this is not what is wanted, there needs to be a way to indicate this. Maybe it could look like this
Bank.transaction do |tx| ok = true tx.without_rollback_on_failure do ok = recoverable_action(...) end unless ok ... end end
For good measure, Transaction should have query a query method #rollback? (or #aborted?).
What do you think?
Michael