Returns inside transactions and ActiveRecord::Rollback
Asked Answered
F

2

5

create is a method that should return true only when everything went as expected and false otherwise. I'm going for error codes style control flow.

class TransferOperator
  class TransferError < Struct.new(:type, :message); ; end

  attr_reader :transfer, :error

  def initialize(transfer)
    @transfer = transfer
  end

  # Creates the transfer and locks money in the bank
  def create
    return error(:validation_error) if transfer.invalid?

    to_bank = transfer.main_to_bank
    to_bank.with_lock do
      # How does return here behave? Should a raise be issued instead and caught outside?
      return error(:insufficient_buffer) if to_bank.available_balance < transfer.amount
      to_bank.available_balance -= transfer.amount
      to_bank.locked_balance += transfer.amount
      to_bank.save!
      transfer.save!
    end

    # Is it guaranteed here that the above transaction has always been succesful?
    true
  end

  private

  def error(type, message='')
    @error = TransferError.new(type, message)
    false
  end
end

The idea here is to have a such flow for the caller:

def move_money
  @transfer = Transfer.new(params)
  operator = TransferOperator.new(@transfer)
  if operator.create
    redirect_to :root, notice: 'success!'
  else
    if operator.error.type == :validation_error
      render action: 'new'
    elsif operator.error.type == :insufficient_buffer
      redirect_to :root, notice: 'not enough money'
    else
      # Handle other errors here...
    end
  end
end

What happens with the error return inside the transaction?

It it guaranteed that the transaction was successful if true is returned?

From http://api.rubyonrails.org/classes/ActiveRecord/Transactions/ClassMethods.html

One exception is the ActiveRecord::Rollback exception, which will trigger a ROLLBACK when raised, but not be re-raised by the transaction block.

Is it possible that Rails would raise ActiveRecord::Rollback by itself? If it does, then the transaction would silently fail and true returned (which is not what we want).

Femmine answered 6/10, 2014 at 21:35 Comment(0)
C
12

If you want to cause the transaction to rollback, you must raise an error. You have a couple of choices:

  1. Raise ActiveRecord::Rollback and the transaction will be rolled back and no error will be re-raised outside the transaction block. As you said, this will silently roll back the transaction. Probably not what you want.
  2. Raise any other type of error. This will cause the transaction to rollback and your error will be raised. You can rescue that error to redirect the user appropriately.

Returning an error object does nothing. It's just another object being passed around.

Carolann answered 6/10, 2014 at 21:56 Comment(2)
Can we count on Rails not raising ActiveRecord::Rollback itself inside the transaction?Femmine
Rails will never raise an ActiveRecord::Rollback itself. Well, techincally Rails uses this error internally, but it would not raise it in your transaction block.Carolann
W
8

I believe this has changed in Rails 7. At the time of writing the latest version of Rails (7.0.2) will silently rollback the transaction when encountering a return.

See https://github.com/rails/rails/issues/45017 and the deprecation warning in prior versions https://github.com/rails/rails/pull/29333

TLDR; don't use return use next if you need to return from the current context.

If you're nested inside several blocks and can't use next, rather extract the contents of the transaction into its own method and use return there.

Wiltonwiltsey answered 6/9, 2022 at 11:54 Comment(1)
Just spent a bunch of time on this - if you're seeing an unexplained rollback in code that works fine when not in a transaction, there's a very good chance it's this.Phocaea

© 2022 - 2024 — McMap. All rights reserved.