From 42a22e6aaa00dd4fb702c5531b9d3a48a5326ddc Mon Sep 17 00:00:00 2001 From: PieterCK Date: Mon, 1 Jul 2024 10:22:21 +0700 Subject: [PATCH] slack-integration: Block requests from Slack retries. A Slack fail condition occurs when we don't respond with HTTP 200 within 3 seconds after Slack calls our endpoint. If this happens, Slack will retry sending the same payload. This is often triggered because we need to perform callbacks when converting messages. To avoid sending the same message multiple times, we block subsequent retry calls from Slack. This commit returns early HTTP 200 response as soon as we get any retry calls from Slack. Part of #30465. --- zerver/webhooks/slack/tests.py | 12 ++++++++++++ zerver/webhooks/slack/view.py | 13 +++++++++++++ 2 files changed, 25 insertions(+) diff --git a/zerver/webhooks/slack/tests.py b/zerver/webhooks/slack/tests.py index 2767839fc6..172e758f5b 100644 --- a/zerver/webhooks/slack/tests.py +++ b/zerver/webhooks/slack/tests.py @@ -299,6 +299,18 @@ class SlackWebhookTests(WebhookTestCase): content_type="application/json", ) + def test_block_slack_retries(self) -> None: + payload = self.get_body("message_with_normal_text") + with patch("zerver.webhooks.slack.view.check_send_webhook_message") as m: + result = self.client_post( + self.url, + payload, + headers={"X-Slack-Retry-Num": 1}, + content_type="application/json", + ) + self.assertFalse(m.called) + self.assert_json_success(result) + class SlackLegacyWebhookTests(WebhookTestCase): CHANNEL_NAME = "slack" diff --git a/zerver/webhooks/slack/view.py b/zerver/webhooks/slack/view.py index 8b1c4b4a9d..54105b8c1b 100644 --- a/zerver/webhooks/slack/view.py +++ b/zerver/webhooks/slack/view.py @@ -165,6 +165,10 @@ def handle_slack_webhook_message( raise JsonableError(_("Error: channels_map_to_topics parameter other than 0 or 1")) +def is_retry_call_from_slack(request: HttpRequest) -> bool: + return "X-Slack-Retry-Num" in request.headers + + @webhook_view("Slack", notify_bot_owner_on_invalid_json=False) @typed_endpoint def api_slack_webhook( @@ -215,6 +219,15 @@ def api_slack_webhook( ) return json_success(request=request, data={"challenge": challenge}) + # A Slack fail condition occurs when we don't respond with HTTP 200 + # within 3 seconds after Slack calls our endpoint. If this happens, + # Slack will retry sending the same payload. This is often triggered + # because of we have to do two callbacks for each call. To avoid + # sending the same message multiple times, we block subsequent retry + # calls from Slack. + if is_retry_call_from_slack(request): + return json_success(request) + # Prevent any Zulip messages sent through the Slack Bridge from looping # back here. if is_zulip_slack_bridge_bot_message(payload):