Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle slack resolution note errors consistently #2976

Merged
merged 2 commits into from
Sep 6, 2023
Merged

Handle slack resolution note errors consistently #2976

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cab4de9
Handle slack resolution note errors consistently
matiasb Sep 5, 2023
4cdd9a9
Update changelog
matiasb Sep 5, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
129 changes: 45 additions & 84 deletions engine/apps/slack/scenarios/resolution_note.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,48 @@
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",
Expand Down Expand Up @@ -189,38 +231,7 @@ def remove_resolution_note_slack_message(self, resolution_note: "ResolutionNote"
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)

Expand All @@ -241,26 +252,7 @@ def post_or_update_resolution_note_in_thread(self, resolution_note: "ResolutionN
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(
Expand Down Expand Up @@ -294,38 +286,7 @@ def post_or_update_resolution_note_in_thread(self, resolution_note: "ResolutionN
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"])
Expand Down
9 changes: 8 additions & 1 deletion engine/apps/slack/scenarios/slack_channel_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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:
Expand Down Expand Up @@ -75,7 +77,12 @@ def save_thread_message_for_resolution_note(
# 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"]
Expand Down
48 changes: 48 additions & 0 deletions engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import pytest

from apps.alerts.models import ResolutionNoteSlackMessage
from apps.slack.client import SlackAPIException
from apps.slack.scenarios.slack_channel_integration import SlackChannelMessageEventStep


Expand Down Expand Up @@ -262,6 +263,53 @@ def test_save_thread_message_for_resolution_note_really_long_text(
)
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,
Expand Down