actionmailer - Best practices for sending many emails at the same time or in a loop in Rails 6?

Question

I have a rake task that loops through bookings and sends an email for each one using .deliver method (which I got from here (which I'm conscious is now 7 years old).

The problem is, sometimes some of the emails don't get sent. Here is my code

# Select bookings starting soon
bookings = Booking.where('start_time < ?', 24.hours.since)

# Email a reminder 
bookings.each do |booking|

  customer = booking.customer

  CustomerMailer.reminder_24h(customer, booking).deliver

end

Since the loop is in a rake task, I don't think there's any value in calling .deliver_later, so I just use .deliver like in the rails cast

I am curious to know if there are best practices that can help using Action Mailer, for example should there be a sleep 2 between each email? Or should I always use .deliver_later to relieve the load on the server? Are there any other rails-related reasons that my code may not work (or, worse, I am using any anti patterns that I should refactor?)

TL;DR why would emails sent in a loop like in the code above occasionally fail to send

question from:https://stackoverflow.com/questions/65867152/best-practices-for-sending-many-emails-at-the-same-time-or-in-a-loop-in-rails-6

Answer 1

No an answer, but some advice from another forum.

Sending emails is a process that is filled with potential failures. It is always a good idea to do it in a background job that can be re-tried in case of intermittent errors like networks etc. and also skipped due to faulty addresses.

Here is a sketch of what may work:

# Reminder process rake task
namespace :bookings do
  desc "Deliver reminders to upcoming bookings"
  task remind_upcoming: :environment do
    EnqueueUpcomingBookingReminders.call(UpcomingBookingRemindersQuery.call)
  end
end
class EnqueueUpcomingBookingReminders
  def self.call(bookings_scope)
    booking_communication_attrs =
      bookings_scope
        .pluck(:id)
        .map { |id| {booking_id: id, type: "reminder"} }
    communications_result = 
      BookingCommunication.insert_all(booking_communication_attrs, unique_by: %i[booking_id type])
      # Email a reminder 
    communications_result.rows.flatten.each do |communication_id|
      DeliverBookingCommunicationJob.perform_later(communication_id)
    end
  end
end
class UpcomingBookingRemindersQuery
  def self.call(scope: Booking)
    Booking
      .upcoming_this_day
      .left_outer_joins(:communications)
      .merge(BookingCommunication.reminder)
      .where(communications: {id: nil})
  end
end
class Booking
  has_many :communications, class_name: "BookingCommunication"
  def self.upcoming_this_day
    where(starts_at:, (Time.current..24.hours.from_now))
  end
end
class BookingCommunication
  belongs_to :booking
  enum step: {confirmation: "confirmation", reminder: "reminder"} # combination of this and the booking id should be unique
  enum status: {pending: "pending", delivered: "delivered", canceled: "canceled", failed: "failed"} # should default to pending at database layer
end
class DeliverBookingCommunicationJob < ApplicationJob
  def perform(communication_id)
    communication = BookingCommunication.find_by(communication_id)
    # Guard against state that invalidates us running this job
    return unless communication
    return unless communication.pending?
    return communication.canceled! if communication.booking.canceled? # This should probably live in the cancel booking process
    booking = communication.booking
    mailer = CustomerMailer.with(customer: booking.customer, booking: booking)
    case communication.step
    when "reminder"
      mailer.reminder_24h.deliver_now
    else
      # log unknown communication step, send to error tracking but dont raise since we do not want job to run again
    end
    communication.delivered!
  rescue SomeEmailRelatedError => err
    communication.failed!
    # deliver err to error tracking service
  end
end