Browse Source

fix(slack): Ignore 404s from webhook (#26741)

* fix(slack): Ignore 404s from webhook

Slack Webhook 404s are an expected response and we should not be raising an exception
everytime we get a 404. This change catches 404 errors while letting others through

Fixes SENTRY-MEN
Aniket Das "Tekky 3 years ago
parent
commit
71da4ae36f
2 changed files with 36 additions and 3 deletions
  1. 9 3
      src/sentry_plugins/slack/client.py
  2. 27 0
      tests/sentry_plugins/slack/test_plugin.py

+ 9 - 3
src/sentry_plugins/slack/client.py

@@ -1,3 +1,4 @@
+from sentry.shared_integrations.exceptions import ApiError
 from sentry_plugins.client import ApiClient
 
 
@@ -13,6 +14,11 @@ class SlackApiClient(ApiClient):
         super().__init__()
 
     def request(self, data):
-        return self._request(
-            path=self.webhook, method="post", data=data, json=False, allow_text=True
-        )
+        try:
+            return self._request(
+                path=self.webhook, method="post", data=data, json=False, allow_text=True
+            )
+        except ApiError as e:
+            # Ignore 404 from slack webhooks
+            if e.code != 404:
+                raise e

+ 27 - 0
tests/sentry_plugins/slack/test_plugin.py

@@ -1,11 +1,13 @@
 from urllib.parse import parse_qs
 
+import pytest
 import responses
 from exam import fixture
 
 from sentry.integrations.slack.message_builder import LEVEL_TO_COLOR
 from sentry.models import Rule
 from sentry.plugins.base import Notification
+from sentry.shared_integrations.exceptions import ApiError
 from sentry.testutils import PluginTestCase
 from sentry.utils import json
 from sentry_plugins.slack.plugin import SlackPlugin
@@ -127,3 +129,28 @@ class SlackPluginTest(PluginTestCase):
                 }
             ],
         }
+
+    @responses.activate
+    def test_no_error_on_404(self):
+        responses.add("POST", "http://example.com/slack", status=404)
+        self.plugin.set_option("webhook", "http://example.com/slack", self.project)
+
+        event = self.store_event(
+            data={"message": "Hello world", "level": "warning", "culprit": "foo.bar"},
+            project_id=self.project.id,
+        )
+
+        rule = Rule.objects.create(project=self.project, label="my rule")
+
+        notification = Notification(event=event, rule=rule)
+
+        # No exception since 404s are supposed to be ignored
+        with self.options({"system.url-prefix": "http://example.com"}):
+            self.plugin.notify(notification)
+
+        responses.replace("POST", "http://example.com/slack", status=400)
+
+        # Other exceptions should not be ignored
+        with self.options({"system.url-prefix": "http://example.com"}):
+            with pytest.raises(ApiError):
+                self.plugin.notify(notification)