diff options
6 files changed, 25 insertions, 80 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 521d5fdb39..3a05a9b020 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,16 @@ +* Deprecate `ActiveRecord::Base.errors_in_transactional_callbacks=`. + + *Rafael Mendonça França* + +* Change transaction callbacks to not swallowing errors. + + Before this change any error raised inside a transaction callback are + rescued and printed in the logs. + + Now these errors are not rescue anymore and just bubble up, as the other callbacks. + + *Rafael Mendonça França* + * Remove deprecated `sanitize_sql_hash_for_conditions`. *Rafael Mendonça França* diff --git a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb index fd666c8c39..f6ef3b0675 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/transaction.rb @@ -69,12 +69,7 @@ module ActiveRecord def rollback_records ite = records.uniq while record = ite.shift - begin - record.rolledback! full_rollback? - rescue => e - raise if ActiveRecord::Base.raise_in_transactional_callbacks - record.logger.error(e) if record.respond_to?(:logger) && record.logger - end + record.rolledback! full_rollback? end ensure ite.each do |i| @@ -89,12 +84,7 @@ module ActiveRecord def commit_records ite = records.uniq while record = ite.shift - begin - record.committed! - rescue => e - raise if ActiveRecord::Base.raise_in_transactional_callbacks - record.logger.error(e) if record.respond_to?(:logger) && record.logger - end + record.committed! end ensure ite.each do |i| diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 31ca90fb58..1c48196762 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -4,23 +4,10 @@ module ActiveRecord extend ActiveSupport::Concern #:nodoc: ACTIONS = [:create, :destroy, :update] - #:nodoc: - CALLBACK_WARN_MESSAGE = "Currently, Active Record suppresses errors raised " \ - "within `after_rollback`/`after_commit` callbacks and only print them to " \ - "the logs. In the next version, these errors will no longer be suppressed. " \ - "Instead, the errors will propagate normally just like in other Active " \ - "Record callbacks.\n" \ - "\n" \ - "You can opt into the new behavior and remove this warning by setting:\n" \ - "\n" \ - " config.active_record.raise_in_transactional_callbacks = true\n\n" included do define_callbacks :commit, :rollback, scope: [:kind, :name] - - mattr_accessor :raise_in_transactional_callbacks, instance_writer: false - self.raise_in_transactional_callbacks = false end # = Active Record Transactions @@ -236,9 +223,6 @@ module ActiveRecord def after_commit(*args, &block) set_options_for_callbacks!(args) set_callback(:commit, :after, *args, &block) - unless ActiveRecord::Base.raise_in_transactional_callbacks - ActiveSupport::Deprecation.warn(CALLBACK_WARN_MESSAGE) - end end # This callback is called after a create, update, or destroy are rolled back. @@ -247,9 +231,16 @@ module ActiveRecord def after_rollback(*args, &block) set_options_for_callbacks!(args) set_callback(:rollback, :after, *args, &block) - unless ActiveRecord::Base.raise_in_transactional_callbacks - ActiveSupport::Deprecation.warn(CALLBACK_WARN_MESSAGE) - end + end + + def raise_in_transactional_callbacks + ActiveSupport::Deprecation.warn('ActiveRecord::Base.raise_in_transactional_callbacks is deprecated and will be removed without replacement.') + true + end + + def raise_in_transactional_callbacks=(value) + ActiveSupport::Deprecation.warn('ActiveRecord::Base.raise_in_transactional_callbacks= is deprecated, has no effect and will be removed without replacement.') + value end private diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 80ac57ec7c..925491acbd 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -24,9 +24,6 @@ ActiveSupport::Deprecation.debug = true # Disable available locale checks to avoid warnings running the test suite. I18n.enforce_available_locales = false -# Enable raise errors in after_commit and after_rollback. -ActiveRecord::Base.raise_in_transactional_callbacks = true - # Connect to the database ARTest.connect diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index 0f5caa52e3..185fc22e98 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -266,47 +266,6 @@ class TransactionCallbacksTest < ActiveRecord::TestCase assert_equal 2, @first.rollbacks end - def test_after_transaction_callbacks_should_prevent_callbacks_from_being_called - old_transaction_config = ActiveRecord::Base.raise_in_transactional_callbacks - ActiveRecord::Base.raise_in_transactional_callbacks = false - - def @first.last_after_transaction_error=(e); @last_transaction_error = e; end - def @first.last_after_transaction_error; @last_transaction_error; end - @first.after_commit_block{|r| r.last_after_transaction_error = :commit; raise "fail!";} - @first.after_rollback_block{|r| r.last_after_transaction_error = :rollback; raise "fail!";} - - second = TopicWithCallbacks.find(3) - second.after_commit_block{|r| r.history << :after_commit} - second.after_rollback_block{|r| r.history << :after_rollback} - - Topic.transaction do - @first.save! - second.save! - end - assert_equal :commit, @first.last_after_transaction_error - assert_equal [:after_commit], second.history - - second.history.clear - Topic.transaction do - @first.save! - second.save! - raise ActiveRecord::Rollback - end - assert_equal :rollback, @first.last_after_transaction_error - assert_equal [:after_rollback], second.history - ensure - ActiveRecord::Base.raise_in_transactional_callbacks = old_transaction_config - end - - def test_after_commit_should_not_raise_when_raise_in_transactional_callbacks_false - old_transaction_config = ActiveRecord::Base.raise_in_transactional_callbacks - ActiveRecord::Base.raise_in_transactional_callbacks = false - @first.after_commit_block{ fail "boom" } - Topic.transaction { @first.save! } - ensure - ActiveRecord::Base.raise_in_transactional_callbacks = old_transaction_config - end - def test_after_commit_callback_should_not_swallow_errors @first.after_commit_block{ fail "boom" } assert_raises(RuntimeError) do diff --git a/railties/lib/rails/generators/rails/app/templates/config/application.rb b/railties/lib/rails/generators/rails/app/templates/config/application.rb index dc0cc07888..c59ffb3491 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/application.rb +++ b/railties/lib/rails/generators/rails/app/templates/config/application.rb @@ -31,10 +31,5 @@ module <%= app_const_base %> # The default locale is :en and all translations from config/locales/*.rb,yml are auto loaded. # config.i18n.load_path += Dir[Rails.root.join('my', 'locales', '*.{rb,yml}').to_s] # config.i18n.default_locale = :de - <%- unless options.skip_active_record? -%> - - # Do not swallow errors in after_commit/after_rollback callbacks. - config.active_record.raise_in_transactional_callbacks = true - <%- end -%> end end |