Skip to content

Do not inline process mailers when calling deliver_all_later #55659

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 11, 2025
+11 −1
Merged

Do not inline process mailers when calling deliver_all_later #55659

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion actionmailer/lib/action_mailer/message_delivery.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def deliver_all_later!(*deliveries, **options)

private
def _deliver_all_later(delivery_method, *deliveries, **options)
deliveries.flatten!
Copy link
Contributor Author

@chaadow chaadow Sep 11, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

after some research, i think what happens is the following:

  • flatten! tries to call respond_to? on each element
  • respond_to? gets delegated to __getobj__
  • which calls @mail_message ||= processed_mailer.message
  • and processed_mailer triggers the inline processing of the mailer

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

correct, flatten! calls Array() which calls respond_to?(:to_ary) and so on

deliveries = deliveries.first if deliveries.first.is_a?(Array)

jobs = deliveries.map do |delivery|
mailer_class = delivery.mailer_class
Expand Down
10 changes: 10 additions & 0 deletions actionmailer/test/message_delivery_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,16 @@ class DummyJob < ActionMailer::MailDeliveryJob; end
end
end

test "deliver_all_later does not inline process the mailers" do
mail1 = DelayedMailer.test_message(1)
mail2 = DelayedMailer.test_message(2)

ActionMailer.deliver_all_later(mail1, mail2)

assert_not mail1.processed?
assert_not mail2.processed?
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for merging @fatkodima . But sorry if you don't mind i'm going to add back at least the test where we pass an array to properly test if deliveries.first.is_a?(Array)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#55660


test "deliver_all_later enqueues multiple deliveries with correct jobs" do
old_delivery_job = BaseMailer.delivery_job
BaseMailer.delivery_job = DummyJob
Expand Down