Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Transitions don't require a new transaction #205

Closed
MSch opened this issue Mar 10, 2016 · 2 comments
Closed

Transitions don't require a new transaction #205

MSch opened this issue Mar 10, 2016 · 2 comments

Comments

@MSch
Copy link

MSch commented Mar 10, 2016

https://github.com/gocardless/statesman/blob/master/lib/statesman/adapters/active_record.rb#L73

If ActiveRecord::Base.transaction is called while already inside a transaction it does not create a "nested" transaction (actually a savepoint) c.f. Nested Transactions in the Rails documentation

This means the following code will lead to an incorrect state in the DB:

MyModel.transaction do
  begin do
    my_model.transition_to(:something_that_fails)
  rescue Error
    # handle it somehow, expecting the transition to have been rolled back
    # but because we were already in a tx nothing happened, the db has been modified
    # and all guarantees are now void
  end
end

I actually ran into the opposite issue with aasm a while back, so the ideal solution would be to make the requires_new setting on the AR transaction configurable, c.f. aasm/aasm#107

@hmac
Copy link
Contributor

hmac commented Feb 21, 2017

@MSch are you able to provide steps to reproduce or (even better) a failing test case for this? What you've described makes sense in the abstract (you could get into a state where you think a transition has rolled back but it hasn't) but I'm struggling to make that happen in reality.

@hmac
Copy link
Contributor

hmac commented May 26, 2017

I think it makes sense to have requires_new: true on all transactions - Rails' fake nested transactions are pretty much never what you actually want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants