3 files changed, 61 insertions, 13 deletions
diff --git a/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb b/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb
index 2c1badbd113..2bc3cd81b48 100644
--- a/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb
+++ b/spec/lib/gitlab/email/handler/unsubscribe_handler_spec.rb
@@ -10,7 +10,7 @@ RSpec.describe Gitlab::Email::Handler::UnsubscribeHandler do
     stub_config_setting(host: 'localhost')
   end
 
-  let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX}") }
+  let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "#{mail_key}#{Gitlab::Email::Common::UNSUBSCRIBE_SUFFIX}") }
   let(:project)   { create(:project, :public) }
   let(:user)      { create(:user) }
   let(:noteable)  { create(:issue, project: project) }
@@ -21,19 +21,19 @@ RSpec.describe Gitlab::Email::Handler::UnsubscribeHandler do
     let(:mail) { Mail::Message.new(email_raw) }
 
     it "matches the new format" do
-      handler = described_class.new(mail, "#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX}")
+      handler = described_class.new(mail, "#{mail_key}#{Gitlab::Email::Common::UNSUBSCRIBE_SUFFIX}")
 
       expect(handler.can_handle?).to be_truthy
     end
 
     it "matches the legacy format" do
-      handler = described_class.new(mail, "#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_LEGACY}")
+      handler = described_class.new(mail, "#{mail_key}#{Gitlab::Email::Common::UNSUBSCRIBE_SUFFIX_LEGACY}")
 
       expect(handler.can_handle?).to be_truthy
     end
 
     it "doesn't match either format" do
-      handler = described_class.new(mail, "+#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX}")
+      handler = described_class.new(mail, "+#{mail_key}#{Gitlab::Email::Common::UNSUBSCRIBE_SUFFIX}")
 
       expect(handler.can_handle?).to be_falsey
     end
@@ -64,7 +64,7 @@ RSpec.describe Gitlab::Email::Handler::UnsubscribeHandler do
     end
 
     context 'when using old style unsubscribe link' do
-      let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "#{mail_key}#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_LEGACY}") }
+      let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, "#{mail_key}#{Gitlab::Email::Common::UNSUBSCRIBE_SUFFIX_LEGACY}") }
 
       it 'unsubscribes user from notable' do
         expect { receiver.execute }.to change { noteable.subscribed?(user) }.from(true).to(false)
diff --git a/spec/lib/gitlab/email/handler_spec.rb b/spec/lib/gitlab/email/handler_spec.rb
index eff6fb63a5f..d38b7d9c85c 100644
--- a/spec/lib/gitlab/email/handler_spec.rb
+++ b/spec/lib/gitlab/email/handler_spec.rb
@@ -60,7 +60,7 @@ RSpec.describe Gitlab::Email::Handler do
 
   describe 'regexps are set properly' do
     let(:addresses) do
-      %W(sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX} sent_notification_key#{Gitlab::IncomingEmail::UNSUBSCRIBE_SUFFIX_LEGACY}) +
+      %W(sent_notification_key#{Gitlab::Email::Common::UNSUBSCRIBE_SUFFIX} sent_notification_key#{Gitlab::Email::Common::UNSUBSCRIBE_SUFFIX_LEGACY}) +
         %w(sent_notification_key path-to-project-123-user_email_token-merge-request) +
         %w(path-to-project-123-user_email_token-issue path-to-project-123-user_email_token-issue-123) +
         %w(path/to/project+user_email_token path/to/project+merge-request+user_email_token some/project)
diff --git a/spec/lib/gitlab/email/receiver_spec.rb b/spec/lib/gitlab/email/receiver_spec.rb
index 79476c63e66..9240d07fd59 100644
--- a/spec/lib/gitlab/email/receiver_spec.rb
+++ b/spec/lib/gitlab/email/receiver_spec.rb
@@ -5,11 +5,10 @@ require 'spec_helper'
 RSpec.describe Gitlab::Email::Receiver do
   include_context :email_shared_context
 
+  let_it_be(:project) { create(:project) }
   let(:metric_transaction) { instance_double(Gitlab::Metrics::WebTransaction) }
 
   shared_examples 'successful receive' do
-    let_it_be(:project) { create(:project) }
-
     let(:handler) { double(:handler, project: project, execute: true, metrics_event: nil, metrics_params: nil) }
     let(:client_id) { 'email/jake@example.com' }
 
@@ -39,7 +38,7 @@ RSpec.describe Gitlab::Email::Receiver do
     end
   end
 
-  shared_examples 'failed receive' do
+  shared_examples 'failed receive with event' do
     it 'adds metric event' do
       expect(::Gitlab::Metrics::BackgroundTransaction).to receive(:current).and_return(metric_transaction)
       expect(metric_transaction).to receive(:add_event).with('email_receiver_error', { error: expected_error.name })
@@ -48,6 +47,14 @@ RSpec.describe Gitlab::Email::Receiver do
     end
   end
 
+  shared_examples 'failed receive without event' do
+    it 'adds metric event' do
+      expect(::Gitlab::Metrics::BackgroundTransaction).not_to receive(:current)
+
+      expect { receiver.execute }.to raise_error(expected_error)
+    end
+  end
+
   context 'when the email contains a valid email address in a header' do
     before do
       stub_incoming_email_setting(enabled: true, address: "incoming+%{key}@appmail.example.com")
@@ -100,21 +107,21 @@ RSpec.describe Gitlab::Email::Receiver do
     let(:email_raw) { fixture_file('emails/valid_reply.eml').gsub(mail_key, '!!!') }
     let(:expected_error) { Gitlab::Email::UnknownIncomingEmail }
 
-    it_behaves_like 'failed receive'
+    it_behaves_like 'failed receive with event'
   end
 
   context 'when the email is blank' do
     let(:email_raw) { '' }
     let(:expected_error) { Gitlab::Email::EmptyEmailError }
 
-    it_behaves_like 'failed receive'
+    it_behaves_like 'failed receive without event'
   end
 
   context 'when the email was auto generated with Auto-Submitted header' do
     let(:email_raw) { fixture_file('emails/auto_submitted.eml') }
     let(:expected_error) { Gitlab::Email::AutoGeneratedEmailError }
 
-    it_behaves_like 'failed receive'
+    it_behaves_like 'failed receive without event'
   end
 
   context "when the email's To field is blank" do
@@ -164,7 +171,48 @@ RSpec.describe Gitlab::Email::Receiver do
     let(:email_raw) { fixture_file('emails/auto_reply.eml') }
     let(:expected_error) { Gitlab::Email::AutoGeneratedEmailError }
 
-    it_behaves_like 'failed receive'
+    it_behaves_like 'failed receive without event'
+  end
+
+  describe 'event raising via errors' do
+    let(:handler) { double(:handler, project: project, execute: true, metrics_event: nil, metrics_params: nil) }
+    let(:email_raw) { "arbitrary text. could be anything really. we're going to raise an error anyway." }
+
+    before do
+      allow(receiver).to receive(:handler).and_return(handler)
+      allow(handler).to receive(:execute).and_raise(expected_error)
+    end
+
+    describe 'handling errors which do not raise events' do
+      where(:expected_error) do
+        [
+          Gitlab::Email::AutoGeneratedEmailError,
+          Gitlab::Email::ProjectNotFound,
+          Gitlab::Email::EmptyEmailError,
+          Gitlab::Email::UserNotFoundError,
+          Gitlab::Email::UserBlockedError,
+          Gitlab::Email::UserNotAuthorizedError,
+          Gitlab::Email::NoteableNotFoundError,
+          Gitlab::Email::InvalidAttachment,
+          Gitlab::Email::InvalidRecordError,
+          Gitlab::Email::EmailTooLarge
+        ]
+      end
+
+      with_them do
+        it_behaves_like 'failed receive without event'
+      end
+    end
+
+    describe 'handling errors which do raise events' do
+      where(:expected_error) do
+        [Gitlab::Email::EmailUnparsableError, Gitlab::Email::UnknownIncomingEmail, ArgumentError, StandardError]
+      end
+
+      with_them do
+        it_behaves_like 'failed receive with event'
+      end
+    end
   end
 
   it 'requires all handlers to have a unique metric_event' do