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

Registering mail interceptor causes warning in Rails 6.1 #42170

Closed
ryanb opened this issue May 7, 2021 · 4 comments
Closed

Registering mail interceptor causes warning in Rails 6.1 #42170

ryanb opened this issue May 7, 2021 · 4 comments

Comments

@ryanb
Copy link
Contributor

ryanb commented May 7, 2021

Steps to reproduce

In a new Rails 6.1.3.2 app, create a mail interceptor as documented in the guide.

# config/initializers/my_mail_interceptor.rb
class MyMailInterceptor
  def self.delivering_email(message)
    # ...
  end
end
ActionMailer::Base.register_interceptor(MyMailInterceptor)

And then run the test suite.

rails test

Expected behavior

No deprecation warning.

Actual behavior

Running the tests shows this warning.

DEPRECATION WARNING: Initialization autoloaded the constants ActionText::ContentHelper and ActionText::TagHelper.

Being able to do this is deprecated. Autoloading during initialization is going
to be an error condition in future versions of Rails.

Reloading does not reboot the application, and therefore code executed during
initialization does not run again. So, if you reload ActionText::ContentHelper, for example,
the expected changes won't be reflected in that stale Module object.

These autoloaded constants have been unloaded.

In order to autoload safely at boot time, please wrap your code in a reloader
callback this way:

    Rails.application.reloader.to_prepare do
      # Autoload classes and modules needed at boot time here.
    end

That block runs when the application boots, and every time there is a reload.
For historical reasons, it may run twice, so it has to be idempotent.

Check the "Autoloading and Reloading Constants" guide to learn more about how

Wrapping the ActionMailer::Base call in the recommended block appears to resolve this.

Rails.application.reloader.to_prepare do
  ActionMailer::Base.register_interceptor(MyMailInterceptor)
end

However, I have a couple questions.

  1. Will this cause the interceptor to be added multiple times in some cases? I'm uncertain if that call is idempotent.

  2. Should the guide be updated to include this block? Or is there another preferred method for registering an interceptor?

System configuration

Rails version: 6.1.3.2

Ruby version: 2.5.5

@pixeltrix
Copy link
Contributor

  1. Will this cause the interceptor to be added multiple times in some cases? I'm uncertain if that call is idempotent.

Ultimately the code that's responsible for this is in the mail gem and it appears to be idempotent.

  1. Should the guide be updated to include this block? Or is there another preferred method for registering an interceptor?

The cause of the warning could be a missing require or something in Action Text - needs investigating before we update the docs.

@pixeltrix
Copy link
Contributor

@ryanb yep, that guide is wrong - the Configuring Action Mailer section of the configuration guide has the correct format:

Rails.application.config.action_mailer.interceptors = ["MyMailInterceptor"]

Similarly, observers are:

Rails.application.config.action_mailer.observers = ["MyMailObserver"]

The reason it's a string and not the class is so that you can save the interceptor/observer in somewhere like app/lib and it doesn't get loaded in the initializer.

The underlying cause of the message you're seeing is that ActionMailer::Base is loaded using autoload and referring to it in the initializer file triggers it. Since it includes the AbstractController::Helpers module, it also triggers the loading of helpers - hence the warnings about ActionText::ContentHelper and ActionText::TagHelper. The other engine components, Action Mailbox and ActiveStorage, don't have helpers and therefore don't display warnings.

This holds true for other configuration as well - for example setting ActionController::Base.default_protect_from_forgery in an initializer will trigger the same warning. The general rule is that config setting in intializers should be mediated through the Rails.application.config object - these values are then copied to the base class in initializer blocks like the one for Action Mailer

If for some reason you need to access a base class in an initializer but there isn't a configuration available or it's not configuration related then you can use Active Support on_load hooks to avoid the error there, e.g. this would also work for your situation:

ActiveSupport.on_load(:action_mailer) do
  ActionMailer::Base.register_interceptor(MyMailInterceptor)
end

That warning could probably be more helpful about this situation but it's already quite long - sorry for the confusion.

@ryanb
Copy link
Contributor Author

ryanb commented May 11, 2021

@pixeltrix Thank you for taking the time to write such a thorough response. Very informative!

@mountriv99
Copy link

Thanks @pixeltrix !
I've recently upgraded to Rails 7.1 and the last solution is the only way that could make my interceptors run. The official way of adding it to Rails.application.config.action_mailer doesn't work for me and it took me a looooong time trying to figure out what went wrong. Glad I found this post!

pboling added a commit to pboling/sanitize_email that referenced this issue Jun 18, 2024
pboling added a commit to pboling/sanitize_email that referenced this issue Jul 10, 2024
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

3 participants