Commit d1b9c810 authored by Sean McGivern's avatar Sean McGivern

Set default retries for mailers to 3

We changed the default retries for Sidekiq jobs from 3 to the upstream
default of 25. When we did this, we manually set all existing workers to
have 3 retries, to avoid changing existing behaviour.

However, mailer jobs aren't run through a worker we define: they go via
ActiveJob. This isn't a huge deal, but typically if a mailer fails
twice, it will fail forever. Also, 25 retries take a few weeks: it's not
very useful to get an email several weeks after it was scheduled to
send.

In Sidekiq 6, ActiveJob will have the ability to use `sidekiq_options`,
but in Sidekiq 5 we'll just get this error:

    You cannot include Sidekiq::Worker in an ActiveJob

Instead, here we manually replace the `#enqueue` and `#enqueue_at`
methods for the Sidekiq adapter to ActiveJob, and special-case those to
give 3 retries for mailer jobs again.

Changelog: fixed
parent 9728d78a
# frozen_string_literal: true
class ActiveJob::QueueAdapters::SidekiqAdapter
# With Sidekiq 6, we can do something like:
# class ActionMailer::MailDeliveryJob
# sidekiq_options retry: 3
# end
#
# See https://gitlab.com/gitlab-org/gitlab/-/issues/329430
raise "Update this monkey patch: #{__FILE__}" unless Sidekiq::VERSION == '5.2.9'
def enqueue(job) #:nodoc:
# Sidekiq::Client does not support symbols as keys
job.provider_job_id = Sidekiq::Client.push \
"class" => JobWrapper,
"wrapped" => job.class.to_s,
"queue" => job.queue_name,
"args" => [job.serialize],
"retry" => retry_for(job)
end
def enqueue_at(job, timestamp) #:nodoc:
job.provider_job_id = Sidekiq::Client.push \
"class" => JobWrapper,
"wrapped" => job.class.to_s,
"queue" => job.queue_name,
"args" => [job.serialize],
"at" => timestamp,
"retry" => retry_for(job)
end
private
def retry_for(job)
if job.queue_name == 'mailers'
3
else
true
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Mailer retries' do
# We need to ensure that this runs through Sidekiq to take
# advantage of the middleware. There is a Rails bug that means we
# have to do some extra steps to make this happen:
# https://github.com/rails/rails/issues/37270#issuecomment-553927324
around do |example|
descendants = ActiveJob::Base.descendants + [ActiveJob::Base]
descendants.each(&:disable_test_adapter)
ActiveJob::Base.queue_adapter = :sidekiq
example.run
descendants.each { |a| a.queue_adapter = :test }
end
it 'sets retries for mailers to 3' do
DeviseMailer.user_admin_approval(create(:user)).deliver_later
expect(Sidekiq::Queues['mailers'].first).to include('retry' => 3)
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment