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

ActiveRecord attempting to autosave empty associated records after failed transactions #17570

Closed
moserrya opened this issue Nov 9, 2014 · 5 comments

Comments

@moserrya
Copy link

moserrya commented Nov 9, 2014

The issue happens when creating an associated record in a transaction and that transaction fails. Models for this example:

class Person < ActiveRecord::Base
  validates_presence_of :name
  has_many :feelings
end

class Feeling < ActiveRecord::Base
  # :person_id, :description
end

Let's create a transaction wherein a person gets a new feeling:

person = Person.find(1)
person.transaction do
  person.feelings.create!(description: 'kaboom', size: 'large')
  person.update!(name: nil)
end
rescue ActiveRecord::AutosaveAssociation
person.name = 'Biglet'
person.save

The feeling is rightly rolled back when person.update! fails. However, if we rescue from the exception in our transaction block and call another save on the same person object later, it attempts (or succeeds) to autosave a new Feeling, but with all columns nulled out. Cannot imagine this is the correct behavior. Wanted to verify this is in fact a bug and discuss the correct behavior before I go about creating a PR.

Reproduced with AR 4.0.5, 4.1.7, 4.2.0.beta4

@rafaelfranca
Copy link
Member

Yeah, agree it doesn't seem correct behaviour. I believe the problem here is that the rollback of the transaction doesn't put the object in the previous state before starting the transaction so the feeling object is inside your person instance, when you save, it tries to save it again.

Do you have any suggestion of what you would expect here?

@moserrya
Copy link
Author

Let's say our person has two persisted feelings then we have the failed transaction listed above. I would expect to see two feelings when I call person.feelings, not three, and person.save would not attempt to autosave the failed feeling from the transaction. In an ideal world this would happen without additional db calls. Resetting the parent object seems pretty reasonable, but I assume this would constitute a breaking change.

@rails-bot
Copy link

This issue has been automatically marked as stale because it has not been commented on for at least
three months.

The resources of the Rails team are limited, and so we are asking for your help.

If you can still reproduce this error on the 4-2-stable, 4-1-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

@yoongkang
Copy link
Contributor

I can confirm that this still occurs on master, but I don't think it's necessarily a bug. A transaction is supposed to rollback the state of the database upon an exception, but not the state of an object's instance data.

From the docs for Transactions:

Exceptions will force a ROLLBACK that returns the database to the state before the transaction began. Be aware, though, that the objects will not have their instance data returned to their pre-transactional state.

It appears that this behaviour has been around for a very long time, possibly since Rails was released.

@rails-bot
Copy link

This issue has been automatically closed because of inactivity.

If you can still reproduce this error on the 4-2-stable, 4-1-stable branches or on master,
please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions.

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

No branches or pull requests

4 participants