Can't remove deprecation warning after updating to 4.0.0

[dillonwelch]

I receive the following error in my app after upgrading to 4.0.0 (from 3.0.8)

DEPRECATION WARNING: 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.

You can opt into the new behavior and remove this warning by setting:

  config.active_record.raise_in_transactional_callbacks = true

The line that it's complaining about is has_paper_trail in my models. I have the config statement set in my config.application.rb, and was only able to remove the warnings by adding ActiveRecord::Base.raise_in_transactional_callbacks = true before my Bundler.require statement in an earlier part of my config.application.rb

According to rails/rails#18084 (comment), it might be an issue somewhere in paper_trail. Any thoughts/suggestions?

[jaredbeck]

Thanks for the bug report, I think you're onto something.

As I'm sure you know, when you call has_paper_trail it calls after_commit (among many other things). When after_commit is called, you get a deprecation warning from rails.

Do you eager load paper_trail in an initializer? We no longer recommend either. We do not recommend using an initializer and we do not recommend an eager load.

However, even without an initializer, I suspect you'll still have this issue, because requireing paper_trail will autoload ::ActiveRecord::Base (on line 4 of paper_trail/frameworks/active_record/models/paper_trail/version.rb). Can you reproduce this issue in a brand new rails app? I suspect you'll be able to.

[dillonwelch]

I don't eager load paper_trail in an initializer.

I use paper_trail in both a Rails project and a non-Rails gem (which gets included in the Rails project) that uses ActiveRecord. The issue happens in both.

For the non-Rails app, I had to update my spec_helper.rb so that the database connection was established before I required paper_trail.

Interestingly, I made a dummy Rails app and was not able to reproduce the issue.

rails new paper_trail_test
cd paper_trail_test
vim Gemfile # Add paper_trail to Gemfile
bundle
bin/rails generate scaffold article title:string body:text
rake db:migrate
vim test/models/article_test.rb # Uncomment the assert true test
bin/rake test test/models/article_test.rb

Note that I use RSpec in my real apps, but was trying to add as little as possible to the dummy example.

[jaredbeck]

I also can't reproduce this in a new rails app (rails 4.2.3, paper_trail 4.0.0). In addition to the steps you tried, I also called has_paper_trail in the model and ran bin/rake g paper_trail:install.

So, what is our test app missing? I still think you're onto something.

[dillonwelch]

For my non-Rails app that I have this issue in, my main file looks something like this:

require 'active_model_serializers'
require 'active_record'
require 'paper_trail'

# Load I18n locales
# Load version file
# Load concern files
# Load model files <----- I get the warnings at this point.
# Bunch of other stuff

The first model that is loaded has the has_paper_trail line in it, and that's when I start seeing the warnings in the console.

[dillonwelch]

A possibly related issue is, again in my non-Rails app, when I include paper_trail before setting up my database connection, I get the following error:

ActiveRecord::ConnectionNotEstablished: No connection pool for PaperTrail::VersionAssociation

[jaredbeck]

Since we can't come up with a minimal reproduction, do you want to share one of your apps that has this problem so I can at least reproduce the problem and label this issue a confirmed bug?

[dillonwelch]

I can't share my app directly, because it's company code under a private GitHub repo.

What I've done instead is taken the app and removed everything not necessary to reproduce the error and put that in a new repo with a dummy model.

https://github.com/oniofchaos/paper_trail_test

[jaredbeck]

Thank you so much for taking the time to prepare a minimum reproduction. However, in your example you're not setting the configuration option. If you apply the following patch the warning goes away.

diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index cff08b9..4eaacbb 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -1,4 +1,7 @@
 require 'active_record'
+
+ActiveRecord::Base.raise_in_transactional_callbacks = true
+
 # Connect to the test database (assumes migrated).
 ActiveRecord::Base.establish_connection(
   YAML.load_file('config/database.yml')['test']

What am I missing? Thanks.

[jaredbeck]

Note to self: See Active Record Configurations by John J. Wang for an explanation of how Rails config is applied to ActiveRecord during rails boot.

[dillonwelch]

Whoops, forgot to add that back in... :)

Try adding it after this line. I think you should get the original deprecation warning. If not, I can take a further look when I get back to work on Monday.

[jaredbeck]

Try adding it after this line.

The latest it can be configured is right before you require 'bear'

diff --git a/lib/main.rb b/lib/main.rb
index 53b355c..0308fa3 100644
--- a/lib/main.rb
+++ b/lib/main.rb
@@ -3,4 +3,5 @@ require 'pry'
 require 'active_record'
 require 'paper_trail'

+ActiveRecord::Base.raise_in_transactional_callbacks = true
 require 'bear'

Note how we can require 'paper_trail' without causing the warning.

I think you just need to finish configuring ActiveRecord before you require any of your models. Please check your app for any models being required before you've finished configuring ActiveRecord.

[dillonwelch]

Yes, this is correct. That's how I ended up solving the error. In the previous version of paper_trail, I was able to setup my configurations after requiring my models. It's possible that ActiveRecord ignored the setting and it just didn't throw any warnings.

[jaredbeck]

Yes, this is correct. That's how I ended up solving the error.

I'm glad to hear there's a solution. I'll close this issue. Please let me know if you find a way to reproduce it in a new rails app.

In the previous version of paper_trail, I was able to setup my configurations after requiring my models.

If by "previous version" you mean 3.0.8, that's because it didn't use transaction callbacks like after_commit.

[dillonwelch]

Ok, I've figured out how the error appeared in my Rails app.

• Create a gem like my paper_trail_test example that uses Ruby + ActiveRecord and has models that use paper_trail.
• Use the gem in a Rails app

In the Rails app, config/application.rb probably looks something like this (from a new Rails project):

require File.expand_path('../boot', __FILE__)

require 'rails/all'

# Require the gems listed in Gemfile, including any gems
# you've limited to :test, :development, or :production.
Bundler.require(*Rails.groups)

module PaperTrailTestRails
  class Application < Rails::Application
    # Settings in config/environments/* take precedence over those specified here.
    # Application configuration should go into files in config/initializers
    # -- all .rb files in that directory are automatically loaded.

    # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone.
    # Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC.
    # config.time_zone = 'Central Time (US & Canada)'

    # 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

    # Do not swallow errors in after_commit/after_rollback callbacks.
    config.active_record.raise_in_transactional_callbacks = true
  end
end

The Ruby gem gets loaded by the Bundler.require(*Rails.groups) line and the config setting has not been set yet.