Merge branch 'handle-smtp-input-errors' into 'master'
Gracefully handle SMTP user input errors (e.g. incorrect email addresses) to prevent Sidekiq retries ### What does this MR do? This MR gracefully handles SMTP input errors (e.g. incorrect or invalid e-mail addresses) to prevent these types of exceptions from causing Sidekiq to retry the task. If these specific exceptions occur, they will be logged, and the e-mail will be dropped from the queue. ### Why was this MR needed? If you include an author that has a misspelled e-mail address, Sidekiq will keep sending e-mail to all the recipients even if they have already received the e-mail. The only way to recover is to clear the Sidekiq queue. Note that other exceptions can still be thrown (e.g. `IOError`, `Net::SMTPAuthenticationError`, `Net::SMTPServerBusy`, `Net::SMTPUnknownError`, and `TimeoutError`). If the worker encounters these, Sidekiq should retry the task. ### What are the relevant issue numbers? Closes https://github.com/gitlabhq/gitlabhq/issues/9560 See merge request !1163
This commit is contained in:
commit
c1b490d6ef
|
@ -13,6 +13,7 @@ v 8.0.0 (unreleased)
|
|||
- Search for comments should be case insensetive
|
||||
- Create cross-reference for closing references on commits pushed to non-default branches (Maël Valais)
|
||||
- Ability to search milestones
|
||||
- Gracefully handle SMTP user input errors (e.g. incorrect email addresses) to prevent Sidekiq retries (Stan Hu)
|
||||
|
||||
v 7.14.1 (unreleased)
|
||||
- Only include base URL in OmniAuth full_host parameter (Stan Hu)
|
||||
|
|
|
@ -4,7 +4,7 @@ class EmailsOnPushWorker
|
|||
def perform(project_id, recipients, push_data, options = {})
|
||||
options.symbolize_keys!
|
||||
options.reverse_merge!(
|
||||
send_from_committer_email: false,
|
||||
send_from_committer_email: false,
|
||||
disable_diffs: false
|
||||
)
|
||||
send_from_committer_email = options[:send_from_committer_email]
|
||||
|
@ -16,9 +16,9 @@ class EmailsOnPushWorker
|
|||
ref = push_data["ref"]
|
||||
author_id = push_data["user_id"]
|
||||
|
||||
action =
|
||||
action =
|
||||
if Gitlab::Git.blank_ref?(before_sha)
|
||||
:create
|
||||
:create
|
||||
elsif Gitlab::Git.blank_ref?(after_sha)
|
||||
:delete
|
||||
else
|
||||
|
@ -42,17 +42,22 @@ class EmailsOnPushWorker
|
|||
end
|
||||
|
||||
recipients.split(" ").each do |recipient|
|
||||
Notify.repository_push_email(
|
||||
project_id,
|
||||
recipient,
|
||||
author_id: author_id,
|
||||
ref: ref,
|
||||
action: action,
|
||||
compare: compare,
|
||||
reverse_compare: reverse_compare,
|
||||
send_from_committer_email: send_from_committer_email,
|
||||
disable_diffs: disable_diffs
|
||||
).deliver
|
||||
begin
|
||||
Notify.repository_push_email(
|
||||
project_id,
|
||||
recipient,
|
||||
author_id: author_id,
|
||||
ref: ref,
|
||||
action: action,
|
||||
compare: compare,
|
||||
reverse_compare: reverse_compare,
|
||||
send_from_committer_email: send_from_committer_email,
|
||||
disable_diffs: disable_diffs
|
||||
).deliver
|
||||
# These are input errors and won't be corrected even if Sidekiq retries
|
||||
rescue Net::SMTPFatalError, Net::SMTPSyntaxError => e
|
||||
logger.info("Failed to send e-mail for project '#{project.name_with_namespace}' to #{recipient}: #{e}")
|
||||
end
|
||||
end
|
||||
ensure
|
||||
compare = nil
|
||||
|
|
|
@ -0,0 +1,34 @@
|
|||
require 'spec_helper'
|
||||
|
||||
describe EmailsOnPushWorker do
|
||||
include RepoHelpers
|
||||
|
||||
let(:project) { create(:project) }
|
||||
let(:user) { create(:user) }
|
||||
let(:data) { Gitlab::PushDataBuilder.build_sample(project, user) }
|
||||
|
||||
subject { EmailsOnPushWorker.new }
|
||||
|
||||
before do
|
||||
allow(Project).to receive(:find).and_return(project)
|
||||
end
|
||||
|
||||
describe "#perform" do
|
||||
it "sends mail" do
|
||||
subject.perform(project.id, user.email, data.stringify_keys)
|
||||
|
||||
email = ActionMailer::Base.deliveries.last
|
||||
expect(email.subject).to include('Change some files')
|
||||
expect(email.to).to eq([user.email])
|
||||
end
|
||||
|
||||
it "gracefully handles an input SMTP error" do
|
||||
ActionMailer::Base.deliveries.clear
|
||||
allow(Notify).to receive(:repository_push_email).and_raise(Net::SMTPFatalError)
|
||||
|
||||
subject.perform(project.id, user.email, data.stringify_keys)
|
||||
|
||||
expect(ActionMailer::Base.deliveries.count).to eq(0)
|
||||
end
|
||||
end
|
||||
end
|
Loading…
Reference in New Issue