summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2015-08-25 09:23:43 +0000
committerDmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>2015-08-25 09:23:43 +0000
commitc1b490d6ef995c9d0baf0133a96f13e4757214b6 (patch)
tree0167619da83fa70212f1dd6d264e852e667968c9
parentc06cf2bde6fcb08f476d6cf9180c545b757ce829 (diff)
parent001c8cd0ee6e70cc96727cc37eedf263b916d24b (diff)
downloadgitlab-ce-c1b490d6ef995c9d0baf0133a96f13e4757214b6.tar.gz
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
-rw-r--r--CHANGELOG1
-rw-r--r--app/workers/emails_on_push_worker.rb33
-rw-r--r--spec/workers/emails_on_push_worker_spec.rb34
3 files changed, 54 insertions, 14 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 9fddcf82783..35b5803d341 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -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)
diff --git a/app/workers/emails_on_push_worker.rb b/app/workers/emails_on_push_worker.rb
index 1d21addece6..916a99bb273 100644
--- a/app/workers/emails_on_push_worker.rb
+++ b/app/workers/emails_on_push_worker.rb
@@ -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
diff --git a/spec/workers/emails_on_push_worker_spec.rb b/spec/workers/emails_on_push_worker_spec.rb
new file mode 100644
index 00000000000..3600c771075
--- /dev/null
+++ b/spec/workers/emails_on_push_worker_spec.rb
@@ -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