aboutsummaryrefslogtreecommitdiffstats
path: root/actionmailer
diff options
context:
space:
mode:
Diffstat (limited to 'actionmailer')
-rw-r--r--actionmailer/CHANGELOG.md4
-rw-r--r--actionmailer/lib/action_mailer/base.rb2
-rw-r--r--actionmailer/lib/action_mailer/delivery_job.rb21
-rw-r--r--actionmailer/lib/action_mailer/message_delivery.rb59
-rw-r--r--actionmailer/lib/action_mailer/rescuable.rb27
-rw-r--r--actionmailer/test/mailers/delayed_mailer.rb20
-rw-r--r--actionmailer/test/message_delivery_test.rb51
7 files changed, 161 insertions, 23 deletions
diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md
index d45e74133a..3b9f503a0b 100644
--- a/actionmailer/CHANGELOG.md
+++ b/actionmailer/CHANGELOG.md
@@ -1,2 +1,6 @@
+* Exception handling: use `rescue_from` to handle exceptions raised by
+ mailer actions, by message delivery, and by deferred delivery jobs.
+
+ *Jeremy Daer*
Please check [5-0-stable](https://github.com/rails/rails/blob/5-0-stable/actionmailer/CHANGELOG.md) for previous changes.
diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb
index 10ee5490c3..ea5af9e4f2 100644
--- a/actionmailer/lib/action_mailer/base.rb
+++ b/actionmailer/lib/action_mailer/base.rb
@@ -5,6 +5,7 @@ require 'active_support/core_ext/hash/except'
require 'active_support/core_ext/module/anonymous'
require 'action_mailer/log_subscriber'
+require 'action_mailer/rescuable'
module ActionMailer
# Action Mailer allows you to send email from your application using a mailer model and views.
@@ -420,6 +421,7 @@ module ActionMailer
# * <tt>deliver_later_queue_name</tt> - The name of the queue used with <tt>deliver_later</tt>.
class Base < AbstractController::Base
include DeliveryMethods
+ include Rescuable
include Previews
abstract!
diff --git a/actionmailer/lib/action_mailer/delivery_job.rb b/actionmailer/lib/action_mailer/delivery_job.rb
index 52772af2d3..d371c1b61a 100644
--- a/actionmailer/lib/action_mailer/delivery_job.rb
+++ b/actionmailer/lib/action_mailer/delivery_job.rb
@@ -3,11 +3,32 @@ require 'active_job'
module ActionMailer
# The <tt>ActionMailer::DeliveryJob</tt> class is used when you
# want to send emails outside of the request-response cycle.
+ #
+ # Exceptions are rescued and handled by the mailer class.
class DeliveryJob < ActiveJob::Base # :nodoc:
queue_as { ActionMailer::Base.deliver_later_queue_name }
+ rescue_from StandardError, with: :handle_exception_with_mailer_class
+
def perform(mailer, mail_method, delivery_method, *args) #:nodoc:
mailer.constantize.public_send(mail_method, *args).send(delivery_method)
end
+
+ private
+ # "Deserialize" the mailer class name by hand in case another argument
+ # (like a Global ID reference) raised DeserializationError.
+ def mailer_class
+ if mailer = Array(@serialized_arguments).first || Array(arguments).first
+ mailer.constantize
+ end
+ end
+
+ def handle_exception_with_mailer_class(exception)
+ if klass = mailer_class
+ klass.handle_exception exception
+ else
+ raise exception
+ end
+ end
end
end
diff --git a/actionmailer/lib/action_mailer/message_delivery.rb b/actionmailer/lib/action_mailer/message_delivery.rb
index d638057d72..c5ba5f9f1d 100644
--- a/actionmailer/lib/action_mailer/message_delivery.rb
+++ b/actionmailer/lib/action_mailer/message_delivery.rb
@@ -1,7 +1,6 @@
require 'delegate'
module ActionMailer
-
# The <tt>ActionMailer::MessageDelivery</tt> class is used by
# <tt>ActionMailer::Base</tt> when creating a new mailer.
# <tt>MessageDelivery</tt> is a wrapper (+Delegator+ subclass) around a lazy
@@ -14,30 +13,35 @@ module ActionMailer
# Notifier.welcome(User.first).deliver_later # enqueue email delivery as a job through Active Job
# Notifier.welcome(User.first).message # a Mail::Message object
class MessageDelivery < Delegator
- def initialize(mailer, mail_method, *args) #:nodoc:
- @mailer = mailer
- @mail_method = mail_method
- @args = args
- @obj = nil
+ def initialize(mailer_class, action, *args) #:nodoc:
+ @mailer_class, @action, @args = mailer_class, action, args
+
+ # The mail is only processed if we try to call any methods on it.
+ # Typical usage will leave it unloaded and call deliver_later.
+ @processed_mailer = nil
+ @mail_message = nil
end
+ # Method calls are delegated to the Mail::Message that's ready to deliver.
def __getobj__ #:nodoc:
- @obj ||= begin
- mailer = @mailer.new
- mailer.process @mail_method, *@args
- mailer.message
- end
+ @mail_message ||= processed_mailer.message
end
- def __setobj__(obj) #:nodoc:
- @obj = obj
+ # Unused except for delegator internals (dup, marshaling).
+ def __setobj__(mail_message) #:nodoc:
+ @mail_message = mail_message
end
- # Returns the Mail::Message object
+ # Returns the resulting Mail::Message
def message
__getobj__
end
+ # Was the delegate loaded, causing the mailer action to be processed?
+ def processed?
+ @processed_mailer || @mail_message
+ end
+
# Enqueues the email to be delivered through Active Job. When the
# job runs it will send the email using +deliver_now!+. That means
# that the message will be sent bypassing checking +perform_deliveries+
@@ -78,7 +82,9 @@ module ActionMailer
# Notifier.welcome(User.first).deliver_now!
#
def deliver_now!
- message.deliver!
+ processed_mailer.handle_exceptions do
+ message.deliver!
+ end
end
# Delivers an email:
@@ -86,24 +92,33 @@ module ActionMailer
# Notifier.welcome(User.first).deliver_now
#
def deliver_now
- message.deliver
+ processed_mailer.handle_exceptions do
+ message.deliver
+ end
end
private
+ # Returns the processed Mailer instance. We keep this instance
+ # on hand so we can delegate exception handling to it.
+ def processed_mailer
+ @processed_mailer ||= @mailer_class.new.tap do |mailer|
+ mailer.process @action, *@args
+ end
+ end
def enqueue_delivery(delivery_method, options={})
- if @obj
- raise "You've accessed the message before asking to deliver it " \
- "later, so you may have made local changes that would be " \
- "silently lost if we enqueued a job to deliver it. Why? Only " \
+ if processed?
+ ::Kernel.raise "You've accessed the message before asking to " \
+ "deliver it later, so you may have made local changes that would " \
+ "be silently lost if we enqueued a job to deliver it. Why? Only " \
"the mailer method *arguments* are passed with the delivery job! " \
"Do not access the message in any way if you mean to deliver it " \
"later. Workarounds: 1. don't touch the message before calling " \
"#deliver_later, 2. only touch the message *within your mailer " \
"method*, or 3. use a custom Active Job instead of #deliver_later."
else
- args = @mailer.name, @mail_method.to_s, delivery_method.to_s, *@args
- ActionMailer::DeliveryJob.set(options).perform_later(*args)
+ args = @mailer_class.name, @action.to_s, delivery_method.to_s, *@args
+ ::ActionMailer::DeliveryJob.set(options).perform_later(*args)
end
end
end
diff --git a/actionmailer/lib/action_mailer/rescuable.rb b/actionmailer/lib/action_mailer/rescuable.rb
new file mode 100644
index 0000000000..f2eabfa057
--- /dev/null
+++ b/actionmailer/lib/action_mailer/rescuable.rb
@@ -0,0 +1,27 @@
+module ActionMailer #:nodoc:
+ # Provides `rescue_from` for mailers. Wraps mailer action processing,
+ # mail job processing, and mail delivery.
+ module Rescuable
+ extend ActiveSupport::Concern
+ include ActiveSupport::Rescuable
+
+ class_methods do
+ def handle_exception(exception) #:nodoc:
+ rescue_with_handler(exception) || raise(exception)
+ end
+ end
+
+ def handle_exceptions #:nodoc:
+ yield
+ rescue => exception
+ rescue_with_handler(exception) || raise
+ end
+
+ private
+ def process(*)
+ handle_exceptions do
+ super
+ end
+ end
+ end
+end
diff --git a/actionmailer/test/mailers/delayed_mailer.rb b/actionmailer/test/mailers/delayed_mailer.rb
index 62d4baa434..e6211ef028 100644
--- a/actionmailer/test/mailers/delayed_mailer.rb
+++ b/actionmailer/test/mailers/delayed_mailer.rb
@@ -1,6 +1,26 @@
+require 'active_job/arguments'
+
+class DelayedMailerError < StandardError; end
+
class DelayedMailer < ActionMailer::Base
+ cattr_accessor :last_error
+ cattr_accessor :last_rescue_from_instance
+
+ rescue_from DelayedMailerError do |error|
+ @@last_error = error
+ @@last_rescue_from_instance = self
+ end
+
+ rescue_from ActiveJob::DeserializationError do |error|
+ @@last_error = error
+ @@last_rescue_from_instance = self
+ end
def test_message(*)
mail(from: 'test-sender@test.com', to: 'test-receiver@test.com', subject: 'Test Subject', body: 'Test Body')
end
+
+ def test_raise(klass_name)
+ raise klass_name.constantize, 'boom'
+ end
end
diff --git a/actionmailer/test/message_delivery_test.rb b/actionmailer/test/message_delivery_test.rb
index f06d69369f..aaed94d519 100644
--- a/actionmailer/test/message_delivery_test.rb
+++ b/actionmailer/test/message_delivery_test.rb
@@ -12,16 +12,23 @@ class MessageDeliveryTest < ActiveSupport::TestCase
ActionMailer::Base.deliver_later_queue_name = :test_queue
ActionMailer::Base.delivery_method = :test
ActiveJob::Base.logger = Logger.new(nil)
- @mail = DelayedMailer.test_message(1, 2, 3)
ActionMailer::Base.deliveries.clear
ActiveJob::Base.queue_adapter.perform_enqueued_at_jobs = true
ActiveJob::Base.queue_adapter.perform_enqueued_jobs = true
+
+ DelayedMailer.last_error = nil
+ DelayedMailer.last_rescue_from_instance = nil
+
+ @mail = DelayedMailer.test_message(1, 2, 3)
end
teardown do
ActiveJob::Base.logger = @previous_logger
ActionMailer::Base.delivery_method = @previous_delivery_method
ActionMailer::Base.deliver_later_queue_name = @previous_deliver_later_queue_name
+
+ DelayedMailer.last_error = nil
+ DelayedMailer.last_rescue_from_instance = nil
end
test 'should have a message' do
@@ -101,4 +108,46 @@ class MessageDeliveryTest < ActiveSupport::TestCase
@mail.deliver_later
end
end
+
+ test 'job delegates error handling to mailer' do
+ # Superclass not rescued by mailer's rescue_from RuntimeError
+ message = DelayedMailer.test_raise('StandardError')
+ assert_raise(StandardError) { message.deliver_later }
+ assert_nil DelayedMailer.last_error
+ assert_nil DelayedMailer.last_rescue_from_instance
+
+ # Rescued by mailer's rescue_from RuntimeError
+ message = DelayedMailer.test_raise('DelayedMailerError')
+ assert_nothing_raised { message.deliver_later }
+ assert_equal 'boom', DelayedMailer.last_error.message
+ assert_kind_of DelayedMailer, DelayedMailer.last_rescue_from_instance
+ end
+
+ class DeserializationErrorFixture
+ include GlobalID::Identification
+
+ def self.find(id)
+ raise 'boom, missing find'
+ end
+
+ attr_reader :id
+ def initialize(id = 1)
+ @id = id
+ end
+
+ def to_global_id(options = {})
+ super app: 'foo'
+ end
+ end
+
+ test 'job delegates deserialization errors to mailer class' do
+ # Inject an argument that can't be deserialized.
+ message = DelayedMailer.test_message(DeserializationErrorFixture.new)
+
+ # DeserializationError is raised, rescued, and delegated to the handler
+ # on the mailer class.
+ assert_nothing_raised { message.deliver_later }
+ assert_equal DelayedMailer, DelayedMailer.last_rescue_from_instance
+ assert_equal 'Error while trying to deserialize arguments: boom, missing find', DelayedMailer.last_error.message
+ end
end