From d4ba57b68b7ebcd0bf21c95d3a635c2c3fbab446 Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Thu, 9 May 2024 13:16:53 -0300 Subject: [PATCH] Avoid retrying to update Slack log message if cant_update_message (#4329) Do not retry updating a message if Slack returns `cant_update_message` API [error](https://api.slack.com/methods/chat.update#errors) (meaning bot user has no permission to update the message). --- engine/apps/slack/errors.py | 4 ++ .../apps/slack/scenarios/distribute_alerts.py | 2 + .../test_distribute_alerts.py | 39 ++++++++++++++++++- engine/apps/slack/tests/test_slack_client.py | 2 + 4 files changed, 45 insertions(+), 2 deletions(-) diff --git a/engine/apps/slack/errors.py b/engine/apps/slack/errors.py index ac750ac6..bf7fa819 100644 --- a/engine/apps/slack/errors.py +++ b/engine/apps/slack/errors.py @@ -68,6 +68,10 @@ class SlackAPIMessageNotFoundError(SlackAPIError): errors = ("message_not_found",) +class SlackAPICantUpdateMessageError(SlackAPIError): + errors = ("cant_update_message",) + + class SlackAPIUserNotFoundError(SlackAPIError): errors = ("user_not_found",) diff --git a/engine/apps/slack/scenarios/distribute_alerts.py b/engine/apps/slack/scenarios/distribute_alerts.py index c125b1ba..a67c1a5f 100644 --- a/engine/apps/slack/scenarios/distribute_alerts.py +++ b/engine/apps/slack/scenarios/distribute_alerts.py @@ -13,6 +13,7 @@ from apps.alerts.models import Alert, AlertGroup, AlertGroupLogRecord, AlertRece from apps.api.permissions import RBACPermission from apps.slack.constants import CACHE_UPDATE_INCIDENT_SLACK_MESSAGE_LIFETIME from apps.slack.errors import ( + SlackAPICantUpdateMessageError, SlackAPIChannelArchivedError, SlackAPIChannelInactiveError, SlackAPIChannelNotFoundError, @@ -947,6 +948,7 @@ class UpdateLogReportMessageStep(scenario_step.ScenarioStep): SlackAPIChannelArchivedError, SlackAPIChannelInactiveError, SlackAPIInvalidAuthError, + SlackAPICantUpdateMessageError, ): pass else: diff --git a/engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py b/engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py index 1eba2b9a..75b32aa8 100644 --- a/engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py +++ b/engine/apps/slack/tests/test_scenario_steps/test_distribute_alerts.py @@ -1,11 +1,12 @@ from unittest.mock import patch import pytest +from django.utils import timezone from apps.alerts.models import AlertGroup -from apps.slack.errors import SlackAPIRestrictedActionError +from apps.slack.errors import SlackAPICantUpdateMessageError, SlackAPIRestrictedActionError from apps.slack.models import SlackMessage -from apps.slack.scenarios.distribute_alerts import AlertShootingStep +from apps.slack.scenarios.distribute_alerts import AlertShootingStep, UpdateLogReportMessageStep from apps.slack.scenarios.scenario_step import ScenarioStep from apps.slack.tests.conftest import build_slack_response @@ -64,3 +65,37 @@ def test_alert_shooting_no_channel_filter( mock_post_alert_group_to_slack.assert_called_once() assert mock_post_alert_group_to_slack.call_args[1]["channel_id"] == "DEFAULT_CHANNEL_ID" + + +@pytest.mark.django_db +def test_update_log_report_cant_update( + make_slack_team_identity, + make_organization, + make_alert_receive_channel, + make_alert_group, + make_alert, + make_slack_message, +): + slack_team_identity = make_slack_team_identity() + organization = make_organization( + slack_team_identity=slack_team_identity, general_log_channel_id="DEFAULT_CHANNEL_ID" + ) + alert_receive_channel = make_alert_receive_channel(organization) + + alert_group = make_alert_group(alert_receive_channel, channel_filter=None) + # alert = make_alert(alert_group, raw_request_data={}) + log_message = make_slack_message( + alert_group=alert_group, + channel_id="RANDOM_CHANNEL_ID", + slack_id="RANDOM_MESSAGE_ID", + last_updated=timezone.now() - timezone.timedelta(minutes=5), + ) + alert_group.slack_log_message = log_message + + step = UpdateLogReportMessageStep(slack_team_identity, organization) + with patch.object(step._slack_client, "api_call") as mock_slack_api_call: + mock_slack_api_call.side_effect = SlackAPICantUpdateMessageError( + response=build_slack_response({"error": "cant_update_message"}) + ) + # not raising error, will not retry + step.update_log_message(alert_group) diff --git a/engine/apps/slack/tests/test_slack_client.py b/engine/apps/slack/tests/test_slack_client.py index 7214aa96..e5b3def4 100644 --- a/engine/apps/slack/tests/test_slack_client.py +++ b/engine/apps/slack/tests/test_slack_client.py @@ -9,6 +9,7 @@ from slack_sdk.web import SlackResponse from apps.slack.client import SlackClient, server_error_retry_handler from apps.slack.errors import ( SlackAPICannotDMBotError, + SlackAPICantUpdateMessageError, SlackAPIChannelArchivedError, SlackAPIChannelInactiveError, SlackAPIChannelNotFoundError, @@ -116,6 +117,7 @@ def test_slack_client_generic_error(mock_request, monkeypatch, make_organization [ ("account_inactive", SlackAPITokenError), ("cannot_dm_bot", SlackAPICannotDMBotError), + ("cant_update_message", SlackAPICantUpdateMessageError), ("channel_not_found", SlackAPIChannelNotFoundError), ("fatal_error", SlackAPIServerError), ("fetch_members_failed", SlackAPIFetchMembersFailedError),