inconsistent model state after transaction rollback #5527

Closed
kmeehl opened this Issue Mar 20, 2012 · 19 comments

Projects

None yet
@kmeehl
kmeehl commented Mar 20, 2012

After a rollback is initiated from a callback during a save action, the object being saved appears in an inconsistent state.

  1. the call to :save returns nil (as expected)
  2. :persisted? returns true when it is expected to return false
  3. the object has a non-nil id when id is expected to be nil
  4. an attempt to reload the object yields a RecordNotFound (as expected)
class FooObserver < ActiveRecord::Observer
  def after_create(foo)
    raise ActiveRecord::Rollback.new
  end
end

class Foo < ActiveRecord::Base
end

> Foo.count
=> 0
> f = Foo.new
=> #<Foo id: nil>
> f.save
=> nil
> f.persisted?
=> true
> f
=> #<Foo id: 1>
> Foo.count
=> 0
> f.reload
ActiveRecord::RecordNotFound: Couldn't find Foo with ID=1
@markmcspadden
Contributor

Confirmed that the code included does cause the situation described.

Would guess it has something to do with:

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

https://github.com/rails/rails/blob/master/activerecord/lib/active_record/transactions.rb#L98-99

Raising any other exception seems to both ROLLBACK the transaction and return the object to the expected state.

@markmcspadden
Contributor

Digging deeper on this.... foo.save! does not cause this issue.

@kmeehl
kmeehl commented Mar 21, 2012

Raising any other exception seems to both ROLLBACK the transaction and return the object to the expected state.

This is true, but the documentation states that any other exception raised will bubble all the way up. In the case of a call to save (not save!), that exception is quite unexpected. So in this case, raising a Rollback exception is the only way to roll back the transaction while having save perform as expected.

@markmcspadden
Contributor

Makes sense to me....

Mind applying the PR locally to ensure it solves the issue?

@markmcspadden
Contributor

Hey @kmeehl - Wanted to bump....see if you still have the issue...see if the PR #5535 fixes it...thanks

@rafaelfranca
Member

Closing since #5535 was merged.

@MMore
MMore commented Nov 1, 2012

I am using persisted? often in tests. So I had a test that had a wrong behavior because it returns true although the records was not saved. So this method must not return true when a rollback has been performed.

What do you think?

@carlosantoniodasilva

@MMore If it's a new record and it's not saved because of a rollback or something else, it should not be persisted?. Can you show the related code? Thanks.

@MMore
MMore commented Nov 1, 2012

I am using RSpec for tests. This is an extract:

@record.persisted?.should be_false

# prompts a rollback in the after_callback
AnotherRecord.any_instance.should_receive(:save!).and_raise
expect { @record.save }.to raise_error

@record.persisted?.should be_false # BUT THIS IS TRUE! There is even an ID in this record.

Record.count.should == 0 # THIS IS TRUE, so do not trust "persisted?"!
@mwalsher

Hate to dig up an old issue, but I'm having a problem with my model indicating that it's persisted, despite being rolled back (rails 4.0.0):

BillingProfile.transaction do
  if @billing_profile.save
    unless SomeService.do_something # returns false and rollback occurs
      raise ActiveRecord::Rollback
    end
  end
end

@billing_profile.persisted? # Still return true, despite rollback
@billing_profile.id # Is set, despite rollback

Am I missing something or is this a bug?

@robin850
Member

@mwalsher : Sorry if I'm wrong, but isn't your problem related with #13751 ?

@chancancode
Member

Yep, it's covered in #13744

@webit-dev

Sorry, but I think this problem is not really solved.
I got strange behaviour with rollbacks, tested in rails 4.0.3 and rails 4.1.0.rc1

Example:

We have a class Foo with an attribute bar as string:

rails g model foo bar:string
class Foo < ActiveRecord::Base
end

Now we can experiment a little in the rails console:

f=Foo.new; begin; Foo.transaction { f.bar = 'something'; f.save; raise 'some_exception' }; rescue;  end

in rails 4.0.3 we have the described bug above:

f.persisted?
=> true

this is fixed in rails 4.1.0.rc1

f.persisted?
=> false

but now the strange things happen. (rails 4.1.0.rc1)
I used ruby 2.1.1.
Consider this (full example):

f=Foo.new; begin; Foo.transaction { f.bar = 'something'; f.save; raise 'some_exception' }; rescue;  end
=> nil
f.persisted?
=> false
f.changes
=> {"id"=>[8, nil]}
f.bar
=> "something"
f.bar_changed?
=> false
f.save
=> true

You may think it is now saved to database with all attributes, right?
Now look at this:

f.reload
=> #<Foo id: 9, bar: nil, created_at: nil, updated_at: nil>

Where are our "something" and the datetime fields gone?

But it gets even worse. It is important if you call persisted? or not.

Checkout this example without calling persisted?:

f=Foo.new; begin; Foo.transaction { f.bar = 'something'; f.save; raise 'some_exception' }; rescue;  end
=> nil
f.changes
=> {}
f.bar
=> "something"
f.bar_changed?
=> false
f.save
=> true

Ok now the changes are empty, but this is not the only difference. In fact the save does nothing now, because we didn't call the persisted? method. You can check it by calling

f.reload
=> ActiveRecord::RecordNotFound

BTW:
In rails 4.0.3 it tries to do an update-query instead of an insert-query at the second save,
because persisted? is true there I think.
The Update does nothing, because it doesn't match a record.

Something is really strange here. Shouldn't this ticket be reopened?

@lowski
lowski commented May 11, 2014

In Rails 4.1.1 (which is supposed to fix this problem) I found this issue:
Calling persisted? after update_attributes! and save! in transaction block returns true and id attribute is not nil. It looks like rollback after update_attributes! is losing some information.

Example:

ActiveRecord::Base.transaction do
    invitation.save!
    raise ActiveRecord::Rollback
end

invitation.persisted? # false
invitation.new_record? # false
invitation.id # nil

With update_attributes:

ActiveRecord::Base.transaction do
    invitation.save!
    invitation.update_attributes!({ token: "123456" })
    raise ActiveRecord::Rollback
end

invitation.persisted? # true
invitation.new_record? # false
invitation.id # some id

I checked this issue with pg and mysql2 gems and both of them seem to be affected.

@icatcher-at

I can confirm the example by @lowski. Just happened to me as well on pg.

@kubaw
kubaw commented Oct 28, 2015

I also encounter this problem. rails 4.1.2, postgres.

@aghuddleston

Also experiencing the issue described by @lowski on rails 4.2.5 with postgres.

@trak3r
Contributor
trak3r commented Apr 28, 2016

FYI this was working fine with Rails 4.2.2 and PostgreSQL (pg 0.18.4).
We have test coverage for this exact case.
But it began failing again while testing 5.0.0.beta3 so the issue seems to have crept back in.

@arthurnn
Member

@trak3r can you open a new ticket with a reproducible steps to simulate this on rails 5 ? thanks a lot. ❤️

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