Handle slack resolution note errors consistently (#2976)

CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Add option to create new contact point for existing integrations ([#2909](https://github.com/grafana/oncall/issues/2909))
 
+### Changed
+
+- Handle slack resolution note errors consistently ([#2976](https://github.com/grafana/oncall/pull/2976))
+
 ## v1.3.35 (2023-09-05)
 
 ### Fixed

engine/apps/slack/scenarios/resolution_note.py

@@ -30,6 +30,48 @@ logger = logging.getLogger(__name__)
 logger.setLevel(logging.DEBUG)
 
 
+def handle_resolution_note_message_exception(step, action_name, exc):
+    """Check common API errors when updating a resolution note message."""
+    if exc.response["error"] == "channel_not_found":
+        logger.warning(
+            f"Unable to {action_name} resolution note message in slack. "
+            f"Slack team identity pk: {step.slack_team_identity.pk}.\n"
+            f"Reason: 'channel_not_found'"
+        )
+    elif exc.response["error"] == "message_not_found":
+        logger.warning(
+            f"Unable to {action_name} resolution note message in slack. "
+            f"Slack team identity pk: {step.slack_team_identity.pk}.\n"
+            f"Reason: 'message_not_found'"
+        )
+    elif exc.response["error"] == "is_archived":
+        logger.warning(
+            f"Unable to {action_name} resolution note message in slack. "
+            f"Slack team identity pk: {step.slack_team_identity.pk}.\n"
+            f"Reason: 'is_archived'"
+        )
+    elif exc.response["error"] == "invalid_auth":
+        logger.warning(
+            f"Unable to {action_name} resolution note message in slack. "
+            f"Slack team identity pk: {step.slack_team_identity.pk}.\n"
+            f"Reason: 'invalid_auth'"
+        )
+    elif exc.response["error"] == "account_inactive":
+        logger.warning(
+            f"Unable to {action_name} resolution note message in slack. "
+            f"Slack team identity pk: {step.slack_team_identity.pk}.\n"
+            f"Reason: 'account_inactive'"
+        )
+    elif exc.response["error"] == "is_inactive":
+        logger.warning(
+            f"Unable to {action_name} resolution note message in slack. "
+            f"Slack team identity pk: {step.slack_team_identity.pk}.\n"
+            f"Reason: 'is_inactive'"
+        )
+    else:
+        raise exc
+
+
 class AddToResolutionNoteStep(scenario_step.ScenarioStep):
     callback_id = [
         "add_resolution_note",
@@ -189,38 +231,7 @@ class UpdateResolutionNoteStep(scenario_step.ScenarioStep):
                         ts=resolution_note_slack_message.ts,
                     )
                 except SlackAPIException as e:
-                    if e.response["error"] == "channel_not_found":
-                        logger.warning(
-                            f"Unable to delete resolution note message in slack. "
-                            f"Slack team identity pk: {self.slack_team_identity.pk}.\n"
-                            f"Reason: 'channel_not_found'"
-                        )
-                    elif e.response["error"] == "message_not_found":
-                        logger.warning(
-                            f"Unable to delete resolution note message in slack. "
-                            f"Slack team identity pk: {self.slack_team_identity.pk}.\n"
-                            f"Reason: 'message_not_found'"
-                        )
-                    elif e.response["error"] == "is_archived":
-                        logger.warning(
-                            f"Unable to delete resolution note message in slack. "
-                            f"Slack team identity pk: {self.slack_team_identity.pk}.\n"
-                            f"Reason: 'is_archived'"
-                        )
-                    elif e.response["error"] == "invalid_auth":
-                        logger.warning(
-                            f"Unable to delete resolution note message in slack. "
-                            f"Slack team identity pk: {self.slack_team_identity.pk}.\n"
-                            f"Reason: 'invalid_auth'"
-                        )
-                    elif e.response["error"] == "is_inactive":
-                        logger.warning(
-                            f"Unable to delete resolution note message in slack. "
-                            f"Slack team identity pk: {self.slack_team_identity.pk}.\n"
-                            f"Reason: 'is_inactive'"
-                        )
-                    else:
-                        raise e
+                    handle_resolution_note_message_exception(self, "delete", e)
             else:
                 self.remove_resolution_note_reaction(resolution_note_slack_message)
 
@@ -241,26 +252,7 @@ class UpdateResolutionNoteStep(scenario_step.ScenarioStep):
                     blocks=blocks,
                 )
             except SlackAPIException as e:
-                if e.response["error"] == "channel_not_found":
-                    logger.warning(
-                        f"Unable to post resolution note message to slack. "
-                        f"Slack team identity pk: {self.slack_team_identity.pk}.\n"
-                        f"Reason: 'channel_not_found'"
-                    )
-                elif e.response["error"] == "is_archived":
-                    logger.warning(
-                        f"Unable to post resolution note message to slack. "
-                        f"Slack team identity pk: {self.slack_team_identity.pk}.\n"
-                        f"Reason: 'is_archived'"
-                    )
-                elif e.response["error"] == "invalid_auth":
-                    logger.warning(
-                        f"Unable to post resolution note message to slack. "
-                        f"Slack team identity pk: {self.slack_team_identity.pk}.\n"
-                        f"Reason: 'invalid_auth'"
-                    )
-                else:
-                    raise e
+                handle_resolution_note_message_exception(self, "post", e)
             else:
                 message_ts = result["message"]["ts"]
                 result_permalink = self._slack_client.chat_getPermalink(
@@ -294,38 +286,7 @@ class UpdateResolutionNoteStep(scenario_step.ScenarioStep):
                     blocks=blocks,
                 )
             except SlackAPIException as e:
-                if e.response["error"] == "channel_not_found":
-                    logger.warning(
-                        f"Unable to update resolution note message in slack. "
-                        f"Slack team identity pk: {self.slack_team_identity.pk}.\n"
-                        f"Reason: 'channel_not_found'"
-                    )
-                elif e.response["error"] == "message_not_found":
-                    logger.warning(
-                        f"Unable to update resolution note message in slack. "
-                        f"Slack team identity pk: {self.slack_team_identity.pk}.\n"
-                        f"Reason: 'message_not_found'"
-                    )
-                elif e.response["error"] == "invalid_auth":
-                    logger.warning(
-                        f"Unable to update resolution note message in slack. "
-                        f"Slack team identity pk: {self.slack_team_identity.pk}.\n"
-                        f"Reason: 'invalid_auth'"
-                    )
-                elif e.response["error"] == "is_inactive":
-                    logger.warning(
-                        f"Unable to update resolution note message in slack. "
-                        f"Slack team identity pk: {self.slack_team_identity.pk}.\n"
-                        f"Reason: 'is_inactive'"
-                    )
-                elif e.response["error"] == "account_inactive":
-                    logger.warning(
-                        f"Unable to update resolution note message in slack. "
-                        f"Slack team identity pk: {self.slack_team_identity.pk}.\n"
-                        f"Reason: 'account_inactive'"
-                    )
-                else:
-                    raise e
+                handle_resolution_note_message_exception(self, "update", e)
             else:
                 resolution_note_slack_message.text = resolution_note.text
                 resolution_note_slack_message.save(update_fields=["text"])

engine/apps/slack/scenarios/slack_channel_integration.py

@@ -1,7 +1,9 @@
 import logging
 import typing
 
+from apps.slack.client import SlackAPIException
 from apps.slack.scenarios import scenario_step
+from apps.slack.scenarios.resolution_note import handle_resolution_note_message_exception
 from apps.slack.types import EventPayload, EventType, MessageEventSubtype, PayloadType, ScenarioRoute
 
 if typing.TYPE_CHECKING:
@@ -75,7 +77,12 @@ class SlackChannelMessageEventStep(scenario_step.ScenarioStep):
             # SlackMessage instances without alert_group set (e.g., SSR Slack messages)
             return
 
-        result = self._slack_client.chat_getPermalink(channel=channel, message_ts=message_ts)
+        try:
+            result = self._slack_client.chat_getPermalink(channel=channel, message_ts=message_ts)
+        except SlackAPIException as e:
+            handle_resolution_note_message_exception(self, "save thread message", e)
+            return
+
         permalink = None
         if result["permalink"] is not None:
             permalink = result["permalink"]

engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py

@@ -3,6 +3,7 @@ from unittest.mock import Mock, patch
 import pytest
 
 from apps.alerts.models import ResolutionNoteSlackMessage
+from apps.slack.client import SlackAPIException
 from apps.slack.scenarios.slack_channel_integration import SlackChannelMessageEventStep
 
 
@@ -262,6 +263,53 @@ class TestSlackChannelMessageEventStep:
         )
         MockResolutionNoteSlackMessage.objects.get_or_create.assert_not_called()
 
+    @patch("apps.alerts.models.ResolutionNoteSlackMessage")
+    def test_save_thread_message_for_resolution_note_api_errors(
+        self,
+        MockResolutionNoteSlackMessage,
+        make_organization_and_user_with_slack_identities,
+        make_alert_receive_channel,
+        make_alert_group,
+        make_slack_message,
+    ) -> None:
+        (
+            organization,
+            user,
+            slack_team_identity,
+            slack_user_identity,
+        ) = make_organization_and_user_with_slack_identities()
+        integration = make_alert_receive_channel(organization)
+        alert_group = make_alert_group(integration)
+
+        channel = "potato"
+        ts = 88945.4849
+        thread_ts = 16789.123
+
+        make_slack_message(alert_group, slack_id=thread_ts, channel_id=channel)
+
+        step = SlackChannelMessageEventStep(slack_team_identity, organization, user)
+        step._slack_client = Mock()
+        step._slack_client.chat_getPermalink.side_effect = [
+            SlackAPIException("error!", response={"ok": False, "error": "message_not_found"})
+        ]
+
+        payload = {
+            "event": {
+                "channel": channel,
+                "ts": ts,
+                "thread_ts": thread_ts,
+                "text": "h" * 2901,
+            },
+        }
+
+        step.save_thread_message_for_resolution_note(slack_user_identity, payload)
+
+        step._slack_client.chat_getPermalink.assert_called_once_with(
+            channel=payload["event"]["channel"],
+            message_ts=payload["event"]["ts"],
+        )
+        MockResolutionNoteSlackMessage.objects.get_or_create.assert_not_called()
+
     @pytest.mark.parametrize("resolution_note_slack_message_already_exists", [True, False])
     def test_save_thread_message_for_resolution_note(
         self,