nested_attributes doesn't rollback parent when before_save/after_save callbacks fail

When saving an object and nested models validation errors on the nested models cause the entire save to be aborted. However, if a nested model returns false in a before_save callback, or raises an exception in an after_save callback only the nested model remains unsaved while the parent is still saved.

The related documentation for AutosaveAssociation (http://api.rubyonrails.org/classes/ActiveRecord/AutosaveAssociation...) only mentions that the autosaved associations will fail on validation, and doesn't mention whether they should fail on callback failures.

This seems inconsistent, at least, with the documentation for ActiveRecord::Callbacks which state that they will rollback the entire transaction.

I've tested by starting a new project, freezing in edge Rails and then setting this up:

class CreateUsersAndWidgets < ActiveRecord::Migration
  def self.up
    create_table :users do |t|
      t.string :name
    end

    create_table :widgets do |t|
      t.integer :user_id
      t.string :name
    end
  end

  def self.down
  end
end

app/models/user.rb

class User < ActiveRecord::Base
  has_many :widgets

  accepts_nested_attributes_for :widgets
end

app/models/widget.rb

class Widget < ActiveRecord::Base  
  attr_accessor :fail
  belongs_to :user

  before_save :cause_before_save_problems
  after_save :cause_after_save_problems
  validate :cause_validation_problems

  def cause_validation_problems
    errors.add(:fail, 'in validation') if fail == :in_validation
  end

  def cause_before_save_problems
    return false if fail == :before_save
  end

  def cause_after_save_problems
    raise ActiveRecord::Rollback if fail == :after_save
  end
end

test/unit/user_test.rb

require File.join(File.dirname(__FILE__), '..', 'test_helper')

class UserTest < ActiveSupport::TestCase
  self.use_transactional_fixtures = false 

  def test_user_rolls_back_when_widget_validation_fails
    @user = User.create :name => 'Foo', :widgets_attributes => { '0' => { :name => 'Bar', :fail => :in_validation } }

    assert @user.widgets.first.new_record?
    assert @user.new_record?
  end

  def test_user_rolls_back_when_widget_before_save_fails
    @user = User.create :name => 'Foo', :widgets_attributes => { '0' => { :name => 'Bar', :fail => :before_save } }

    assert @user.widgets.first.new_record?
    assert @user.new_record?
  end

  def test_user_rolls_back_when_widget_after_save_fails
    @user = User.create :name => 'Foo', :widgets_attributes => { '0' => { :name => 'Bar', :fail => :after_save } }

    assert @user.widgets.first.new_record?
    assert @user.new_record?
  end
end

Here is the test run output:

Loaded suite test/unit/user_test
Started
FF.
Finished in 0.044073 seconds.

  1) Failure:
test_user_rolls_back_when_widget_after_save_fails(UserTest) [/test/unit/user_test.rb:23]:
<false> is not true.

  2) Failure:
test_user_rolls_back_when_widget_before_save_fails(UserTest) [/test/unit/user_test.rb:17]:
<false> is not true.

3 tests, 5 assertions, 2 failures, 0 errors

And the related log for all 3 tests (running against PostgreSQL):

SQL (0.1ms)   SET client_min_messages TO 'panic'
SQL (0.1ms)   SET client_min_messages TO 'notice'
SQL (0.1ms)   BEGIN
SQL (0.5ms)   INSERT INTO "users" ("name") VALUES(E'Foo') RETURNING "id"
SQL (0.3ms)   INSERT INTO "widgets" ("name", "user_id") VALUES(E'Bar', 5) RETURNING "id"
SQL (1.3ms)   COMMIT
SQL (0.1ms)   BEGIN
SQL (0.2ms)   INSERT INTO "users" ("name") VALUES(E'Foo') RETURNING "id"
SQL (0.8ms)   COMMIT
SQL (0.1ms)   BEGIN
SQL (0.1ms)   ROLLBACK

With some digging around I've found that in database_statement.rb when the ActiveRecord::Rollback exception is raised by the after_save or before_save callbacks in the child, the value of transaction_open is false so no rollback is triggered. With the significant chain of events that happen before this it's hard to tell if that behavior is intended or not.

I'd dig more but the task seems daunting and I thought I would check if anybody knows of a specific reason for this being the intended behavior?

If this is the intended behavior it seems counter to what you'd expect from the documentation for ActiveRecord::Callbacks so it probably deserves a note in the docs at least.