From de41a5a50eb33194a23235766a253cbd3d33c3d4 Mon Sep 17 00:00:00 2001
From: Markus Koller <mkoller@gitlab.com>
Date: Thu, 28 Apr 2022 17:24:56 +0200
Subject: [PATCH] Log error responses from Slack/Mattermost

This should help us troubleshoot problems with the Slack and Mattermost
integrations, when notifications are apparently sent by GitLab but
don't arrive in the chat app.
---
 .../integrations/slack_mattermost_notifier.rb | 12 ++++++++-
 ...ack_mattermost_notifier_shared_examples.rb | 27 ++++++++++++++++++-
 2 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/app/models/concerns/integrations/slack_mattermost_notifier.rb b/app/models/concerns/integrations/slack_mattermost_notifier.rb
index be13701289adc6..3bdaa852ddfb03 100644
--- a/app/models/concerns/integrations/slack_mattermost_notifier.rb
+++ b/app/models/concerns/integrations/slack_mattermost_notifier.rb
@@ -14,11 +14,21 @@ def notify(message, opts)
       # - https://gitlab.com/gitlab-org/slack-notifier#middleware
       # - https://gitlab.com/gitlab-org/gitlab/-/issues/347048
       notifier = ::Slack::Messenger.new(webhook, opts.merge(http_client: HTTPClient))
-      notifier.ping(
+      responses = notifier.ping(
         message.pretext,
         attachments: message.attachments,
         fallback: message.fallback
       )
+
+      responses.each do |response|
+        unless response.success?
+          log_error('SlackMattermostNotifier HTTP error response',
+            request_host: response.request.uri.host,
+            response_code: response.code,
+            response_body: response.body
+          )
+        end
+      end
     end
 
     class HTTPClient
diff --git a/spec/support/shared_examples/models/concerns/integrations/slack_mattermost_notifier_shared_examples.rb b/spec/support/shared_examples/models/concerns/integrations/slack_mattermost_notifier_shared_examples.rb
index da5c35c970abfe..2e062cda4e9070 100644
--- a/spec/support/shared_examples/models/concerns/integrations/slack_mattermost_notifier_shared_examples.rb
+++ b/spec/support/shared_examples/models/concerns/integrations/slack_mattermost_notifier_shared_examples.rb
@@ -45,9 +45,33 @@ def execute_with_options(options)
     end
 
     it "notifies about #{event_type} events" do
+      expect(chat_integration).not_to receive(:log_error)
+
       chat_integration.execute(data)
+
       expect(WebMock).to have_requested(:post, stubbed_resolved_hostname)
     end
+
+    context 'when the response is not successful' do
+      let!(:stubbed_resolved_hostname) do
+        stub_full_request(webhook_url, method: :post)
+          .to_return(status: 409, body: 'error message')
+          .request_pattern.uri_pattern.to_s
+      end
+
+      it 'logs an error' do
+        expect(chat_integration).to receive(:log_error).with(
+          'SlackMattermostNotifier HTTP error response',
+          request_host: 'example.gitlab.com',
+          response_code: 409,
+          response_body: 'error message'
+        )
+
+        chat_integration.execute(data)
+
+        expect(WebMock).to have_requested(:post, stubbed_resolved_hostname)
+      end
+    end
   end
 
   shared_examples "untriggered #{integration_name} integration" do |event_type: nil, branches_to_be_notified: nil|
@@ -59,8 +83,9 @@ def execute_with_options(options)
       stub_full_request(webhook_url, method: :post).request_pattern.uri_pattern.to_s
     end
 
-    it "notifies about #{event_type} events" do
+    it "does not notify about #{event_type} events" do
       chat_integration.execute(data)
+
       expect(WebMock).not_to have_requested(:post, stubbed_resolved_hostname)
     end
   end
-- 
GitLab