webhooks: Use 200 status code for unknown events.

Because the third party might not be expecting a 400 from our
webhooks, we now instead use 200 status code for unknown events,
while sending back the error to Sentry. Because it is no longer an error
response, the response type should now be "success".

Fixes #24721.

(cherry picked from commit 84723654c8)

zerver/decorator.py

@@ -33,6 +33,7 @@ from django.utils.timezone import now as timezone_now
 from django.utils.translation import gettext as _
 from django.views.decorators.csrf import csrf_exempt
 from django_otp import user_has_device
+from sentry_sdk import capture_exception
 from two_factor.utils import default_device
 from typing_extensions import Concatenate, ParamSpec
 
@@ -368,6 +369,8 @@ def webhook_view(
                 else:
                     if isinstance(err, WebhookError):
                         err.webhook_name = webhook_client_name
+                    if isinstance(err, UnsupportedWebhookEventTypeError):
+                        capture_exception(err)
                     log_exception_to_webhook_logger(request, err)
                 raise err
 
@@ -777,6 +780,8 @@ def authenticated_rest_api_view(
 
                 if isinstance(err, WebhookError):
                     err.webhook_name = webhook_client_name
+                if isinstance(err, UnsupportedWebhookEventTypeError):
+                    capture_exception(err)
                 log_exception_to_webhook_logger(request, err)
                 raise err
 

zerver/lib/exceptions.py

@@ -370,6 +370,7 @@ class UnsupportedWebhookEventTypeError(WebhookError):
     """
 
     code = ErrorCode.UNSUPPORTED_WEBHOOK_EVENT_TYPE
+    http_status_code = 200
     data_fields = ["webhook_name", "event_type"]
 
     def __init__(self, event_type: Optional[str]) -> None:
@@ -378,7 +379,9 @@ class UnsupportedWebhookEventTypeError(WebhookError):
 
     @staticmethod
     def msg_format() -> str:
-        return _("The '{event_type}' event isn't currently supported by the {webhook_name} webhook")
+        return _(
+            "The '{event_type}' event isn't currently supported by the {webhook_name} webhook; ignoring"
+        )
 
 
 class AnomalousWebhookPayloadError(WebhookError):

zerver/lib/response.py

@@ -117,8 +117,11 @@ def json_response_from_error(exception: JsonableError) -> MutableJsonResponse:
     middleware takes care of transforming it into a response by
     calling this function.
     """
+    response_type = "error"
+    if 200 <= exception.http_status_code < 300:
+        response_type = "success"
     response = json_response(
-        "error", msg=exception.msg, data=exception.data, status=exception.http_status_code
+        response_type, msg=exception.msg, data=exception.data, status=exception.http_status_code
     )
 
     for header, value in exception.extra_headers.items():

zerver/tests/test_decorators.py

@@ -406,7 +406,9 @@ class DecoratorTestCase(ZulipTestCase):
         request = HostRequestMock()
         request.host = "zulip.testserver"
         request.POST["api_key"] = webhook_bot_api_key
-        exception_msg = "The 'test_event' event isn't currently supported by the ClientName webhook"
+        exception_msg = (
+            "The 'test_event' event isn't currently supported by the ClientName webhook; ignoring"
+        )
         with self.assertLogs("zulip.zerver.webhooks.unsupported", level="ERROR") as log:
             with self.assertRaisesRegex(UnsupportedWebhookEventTypeError, exception_msg):
                 request.body = b"invalidjson"
@@ -577,9 +579,7 @@ class DecoratorLoggingTestCase(ZulipTestCase):
         with mock.patch(
             "zerver.decorator.webhook_unsupported_events_logger.exception"
         ) as mock_exception:
-            exception_msg = (
+            exception_msg = "The 'test_event' event isn't currently supported by the ClientName webhook; ignoring"
-                "The 'test_event' event isn't currently supported by the ClientName webhook"
-            )
             with self.assertRaisesRegex(UnsupportedWebhookEventTypeError, exception_msg):
                 my_webhook_raises_exception(request)
 

zerver/webhooks/github/tests.py

@@ -525,7 +525,7 @@ A temporary team so that I can get some webhook fixtures!
         stack_info = m.call_args[1]["stack_info"]
 
         self.assertIn(
-            "The 'team/edited (changes: bogus_key1/bogus_key2)' event isn't currently supported by the GitHub webhook",
+            "The 'team/edited (changes: bogus_key1/bogus_key2)' event isn't currently supported by the GitHub webhook; ignoring",
             msg,
         )
         self.assertTrue(stack_info)

zerver/webhooks/github/view.py

@@ -55,7 +55,7 @@ class Helper:
         self.include_title = include_title
 
     def log_unsupported(self, event: str) -> None:
-        summary = f"The '{event}' event isn't currently supported by the GitHub webhook"
+        summary = f"The '{event}' event isn't currently supported by the GitHub webhook; ignoring"
         log_unsupported_webhook_event(request=self.request, summary=summary)
 
 

zerver/webhooks/pagerduty/tests.py

@@ -80,7 +80,8 @@ class PagerDutyHookTests(WebhookTestCase):
         for version in range(1, 4):
             payload = self.get_body(f"unsupported_v{version}")
             result = self.client_post(self.url, payload, content_type="application/json")
-            self.assert_json_error(
+            self.assert_json_success(result)
+            self.assert_in_response(
+                "The 'incident.unsupported' event isn't currently supported by the PagerDuty webhook; ignoring",
                 result,
-                "The 'incident.unsupported' event isn't currently supported by the PagerDuty webhook",
             )

zerver/webhooks/updown/fixtures/unknown_event.json

@@ -0,0 +1,27 @@
+[{
+  "event": "unknown",
+  "check": {
+    "token": "ngg8",
+    "url": "https://updown.io",
+    "alias": "",
+    "last_status": 200,
+    "uptime": 99.954,
+    "down": false,
+    "down_since": null,
+    "error": null,
+    "period": 30,
+    "apdex_t": 0.25,
+    "string_match": "",
+    "enabled": true,
+    "published": true,
+    "last_check_at": "2016-02-07T13:16:07Z",
+    "next_check_at": "2016-02-07T13:16:37Z",
+    "favicon_url": "https://updown.io/favicon.png"
+  },
+  "downtime": {
+    "error": null,
+    "started_at": null,
+    "ended_at": null,
+    "duration": null
+  }
+}]

zerver/webhooks/updown/tests.py

@@ -52,3 +52,6 @@ class UpdownHookTests(WebhookTestCase):
             topic_name=topic_name,
             content=up_content,
         )
+
+    def test_unknown_event(self) -> None:
+        self.check_webhook("unknown_event", expect_noop=True)