diff --git a/CHANGELOG.md b/CHANGELOG.md index 70d691ac..0681a413 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Notify user via Slack/mobile push-notification when their shift swap request is taken by @joeyorlando ([#2992](https://github.com/grafana/oncall/pull/2992)) +### Changed + +- Improve Slack error handling by @vadimkerr ([#3000](https://github.com/grafana/oncall/pull/3000)) + ### Fixed - Avoid task retries because of missing AlertGroupLogRecord on send_alert_group_signal ([#3001](https://github.com/grafana/oncall/pull/3001)) diff --git a/engine/apps/alerts/tasks/notify_ical_schedule_shift.py b/engine/apps/alerts/tasks/notify_ical_schedule_shift.py index 96f3331a..d06e7878 100644 --- a/engine/apps/alerts/tasks/notify_ical_schedule_shift.py +++ b/engine/apps/alerts/tasks/notify_ical_schedule_shift.py @@ -6,7 +6,13 @@ from typing import TYPE_CHECKING from django.utils import timezone from apps.schedules.ical_utils import calculate_shift_diff, parse_event_uid -from apps.slack.client import SlackAPIException, SlackAPITokenException, SlackClientWithErrorHandling +from apps.slack.client import SlackClient +from apps.slack.errors import ( + SlackAPIChannelArchivedError, + SlackAPIChannelNotFoundError, + SlackAPIInvalidAuthError, + SlackAPITokenError, +) from apps.slack.scenarios import scenario_step from common.custom_celery_tasks import shared_dedicated_queue_retry_task @@ -146,7 +152,7 @@ def notify_ical_schedule_shift(schedule_pk): if len(new_shifts) > 0 or schedule.empty_oncall: task_logger.info(f"new_shifts: {new_shifts}") if schedule.notify_oncall_shift_freq != OnCallSchedule.NotifyOnCallShiftFreq.NEVER: - slack_client = SlackClientWithErrorHandling(schedule.organization.slack_team_identity.bot_access_token) + slack_client = SlackClient(schedule.organization.slack_team_identity) step = scenario_step.ScenarioStep.get_step("schedules", "EditScheduleShiftNotifyStep") report_blocks = step.get_report_blocks_ical(new_shifts, upcoming_shifts, schedule, schedule.empty_oncall) @@ -156,11 +162,10 @@ def notify_ical_schedule_shift(schedule_pk): blocks=report_blocks, text=f"On-call shift for schedule {schedule.name} has changed", ) - except SlackAPITokenException: + except ( + SlackAPITokenError, + SlackAPIChannelNotFoundError, + SlackAPIChannelArchivedError, + SlackAPIInvalidAuthError, + ): pass - except SlackAPIException as e: - expected_exceptions = ["channel_not_found", "is_archived", "invalid_auth"] - if e.response["error"] in expected_exceptions: - print(e) - else: - raise e diff --git a/engine/apps/alerts/tests/test_notify_ical_schedule_shift.py b/engine/apps/alerts/tests/test_notify_ical_schedule_shift.py index bce4da6d..e309111b 100644 --- a/engine/apps/alerts/tests/test_notify_ical_schedule_shift.py +++ b/engine/apps/alerts/tests/test_notify_ical_schedule_shift.py @@ -100,7 +100,7 @@ def test_next_shift_notification_long_shifts( with patch("apps.alerts.tasks.notify_ical_schedule_shift.datetime", Mock(wraps=datetime)) as mock_datetime: mock_datetime.datetime.now.return_value = datetime.datetime(2021, 9, 29, 12, 0, tzinfo=pytz.UTC) - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_ical_schedule_shift(ical_schedule.pk) slack_blocks = mock_slack_api_call.call_args_list[0][1]["blocks"] @@ -204,7 +204,7 @@ def test_overrides_changes_no_current_no_triggering_notification( schedule.prev_ical_file_overrides = ical_before schedule.save() - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_ical_schedule_shift(schedule.pk) assert not mock_slack_api_call.called @@ -252,7 +252,7 @@ def test_no_changes_no_triggering_notification( schedule.empty_oncall = False schedule.save() - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_ical_schedule_shift(schedule.pk) assert not mock_slack_api_call.called @@ -300,7 +300,7 @@ def test_current_shift_changes_trigger_notification( schedule.empty_oncall = False schedule.save() - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_ical_schedule_shift(schedule.pk) assert mock_slack_api_call.called @@ -364,7 +364,7 @@ def test_current_shift_changes_swap_split( schedule.empty_oncall = False schedule.save() - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_ical_schedule_shift(schedule.pk) text_block = mock_slack_api_call.call_args_list[0][1]["blocks"][1]["text"]["text"] @@ -433,7 +433,7 @@ def test_next_shift_changes_no_triggering_notification( on_call_shift_2.add_rolling_users([[user2]]) schedule.refresh_ical_file() - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_ical_schedule_shift(schedule.pk) assert not mock_slack_api_call.called @@ -500,7 +500,7 @@ def test_lower_priority_changes_no_triggering_notification( on_call_shift_2.add_rolling_users([[user2]]) schedule.refresh_ical_file() - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_ical_schedule_shift(schedule.pk) assert not mock_slack_api_call.called @@ -630,7 +630,7 @@ def test_vtimezone_changes_no_triggering_notification( schedule.cached_ical_file_primary = ical_after schedule.save() - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_ical_schedule_shift(schedule.pk) assert not mock_slack_api_call.called @@ -687,7 +687,7 @@ def test_no_changes_no_triggering_notification_from_old_to_new_task_version( schedule.empty_oncall = False schedule.save() - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_ical_schedule_shift(schedule.pk) assert not mock_slack_api_call.called @@ -749,7 +749,7 @@ def test_current_shift_changes_trigger_notification_from_old_to_new_task_version on_call_shift.add_rolling_users([[user2]]) schedule.refresh_ical_file() - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_ical_schedule_shift(schedule.pk) assert mock_slack_api_call.called @@ -814,7 +814,7 @@ def test_next_shift_notification_long_and_short_shifts( schedule.empty_oncall = False schedule.save() - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_ical_schedule_shift(schedule.pk) assert mock_slack_api_call.called diff --git a/engine/apps/api/tests/conftest.py b/engine/apps/api/tests/conftest.py index 3c3783b0..8c67999b 100644 --- a/engine/apps/api/tests/conftest.py +++ b/engine/apps/api/tests/conftest.py @@ -3,7 +3,7 @@ from datetime import timedelta import pytest from django.utils import timezone -from apps.slack.client import SlackClientWithErrorHandling +from apps.slack.client import SlackClient from apps.slack.scenarios.distribute_alerts import AlertShootingStep @@ -20,7 +20,7 @@ def mock_slack_api_call(monkeypatch): "team": {"name": "TEST_SLACK_TEAM_NAME"}, } - monkeypatch.setattr(SlackClientWithErrorHandling, "api_call", _mock_api_call) + monkeypatch.setattr(SlackClient, "api_call", _mock_api_call) @pytest.fixture() diff --git a/engine/apps/integrations/tasks.py b/engine/apps/integrations/tasks.py index 1e788f55..2f03c263 100644 --- a/engine/apps/integrations/tasks.py +++ b/engine/apps/integrations/tasks.py @@ -8,7 +8,8 @@ from django.core.cache import cache from apps.alerts.models.alert_group_counter import ConcurrentUpdateError from apps.alerts.tasks import resolve_alert_group_by_source_if_needed -from apps.slack.client import SlackAPIException, SlackClientWithErrorHandling +from apps.slack.client import SlackClient +from apps.slack.errors import SlackAPIError from common.custom_celery_tasks import shared_dedicated_queue_retry_task from common.custom_celery_tasks.create_alert_base_task import CreateAlertBaseTask @@ -157,7 +158,7 @@ def notify_about_integration_ratelimit_in_slack(organization_id, text, **kwargs) slack_team_identity = organization.slack_team_identity if slack_team_identity is not None: try: - sc = SlackClientWithErrorHandling(slack_team_identity.bot_access_token) + sc = SlackClient(slack_team_identity) sc.chat_postMessage(channel=organization.general_log_channel_id, text=text, team=slack_team_identity) - except SlackAPIException as e: + except SlackAPIError as e: logger.warning(f"Slack exception {e} while sending message for organization {organization_id}") diff --git a/engine/apps/public_api/helpers.py b/engine/apps/public_api/helpers.py index dec78ec6..ec8ecd60 100644 --- a/engine/apps/public_api/helpers.py +++ b/engine/apps/public_api/helpers.py @@ -1,13 +1,14 @@ from apps.public_api.constants import VALID_DATE_FOR_DELETE_INCIDENT -from apps.slack.client import SlackAPITokenException, SlackClientWithErrorHandling +from apps.slack.client import SlackClient +from apps.slack.errors import SlackAPITokenError def team_has_slack_token_for_deleting(alert_group): if alert_group.slack_message and alert_group.slack_message.slack_team_identity: - sc = SlackClientWithErrorHandling(alert_group.slack_message.slack_team_identity.bot_access_token) + sc = SlackClient(alert_group.slack_message.slack_team_identity) try: sc.auth_test() - except SlackAPITokenException: + except SlackAPITokenError: return False return True diff --git a/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py b/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py index a39cf6f7..54c8b4b4 100644 --- a/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py +++ b/engine/apps/schedules/tests/tasks/shift_swaps/test_slack_followups.py @@ -154,7 +154,7 @@ def test_followup_offsets(): assert ShiftSwapRequest.FOLLOWUP_OFFSETS[idx] > FOLLOWUP_WINDOW -@patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") +@patch("apps.slack.client.SlackClient.chat_postMessage") @pytest.mark.django_db def test_send_shift_swap_request_followup(mock_slack_chat_post_message, shift_swap_request_test_setup): shift_swap_request = shift_swap_request_test_setup() diff --git a/engine/apps/schedules/tests/test_notify_about_empty_shifts_in_schedule.py b/engine/apps/schedules/tests/test_notify_about_empty_shifts_in_schedule.py index 69178698..adf0bd9d 100644 --- a/engine/apps/schedules/tests/test_notify_about_empty_shifts_in_schedule.py +++ b/engine/apps/schedules/tests/test_notify_about_empty_shifts_in_schedule.py @@ -49,7 +49,7 @@ def test_no_empty_shifts_no_triggering_notification( empty_shifts_report_sent_at = schedule.empty_shifts_report_sent_at - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_about_empty_shifts_in_schedule(schedule.pk) assert not mock_slack_api_call.called @@ -97,7 +97,7 @@ def test_empty_shifts_trigger_notification( empty_shifts_report_sent_at = schedule.empty_shifts_report_sent_at - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_about_empty_shifts_in_schedule(schedule.pk) assert mock_slack_api_call.called @@ -160,7 +160,7 @@ def test_empty_non_empty_shifts_trigger_notification( empty_shifts_report_sent_at = schedule.empty_shifts_report_sent_at - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_about_empty_shifts_in_schedule(schedule.pk) assert mock_slack_api_call.called diff --git a/engine/apps/schedules/tests/test_notify_about_gaps_in_schedule.py b/engine/apps/schedules/tests/test_notify_about_gaps_in_schedule.py index 432b59e0..912d31b9 100644 --- a/engine/apps/schedules/tests/test_notify_about_gaps_in_schedule.py +++ b/engine/apps/schedules/tests/test_notify_about_gaps_in_schedule.py @@ -48,7 +48,7 @@ def test_no_gaps_no_triggering_notification( gaps_report_sent_at = schedule.gaps_report_sent_at - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_about_gaps_in_schedule(schedule.pk) assert not mock_slack_api_call.called @@ -113,7 +113,7 @@ def test_gaps_in_the_past_no_triggering_notification( gaps_report_sent_at = schedule.gaps_report_sent_at - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_about_gaps_in_schedule(schedule.pk) assert not mock_slack_api_call.called @@ -165,7 +165,7 @@ def test_gaps_now_trigger_notification( assert schedule.has_gaps is False - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_about_gaps_in_schedule(schedule.pk) assert mock_slack_api_call.called @@ -219,7 +219,7 @@ def test_gaps_near_future_trigger_notification( assert schedule.has_gaps is False - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_about_gaps_in_schedule(schedule.pk) assert mock_slack_api_call.called @@ -271,7 +271,7 @@ def test_gaps_later_than_7_days_no_triggering_notification( gaps_report_sent_at = schedule.gaps_report_sent_at - with patch("apps.slack.client.SlackClientWithErrorHandling.chat_postMessage") as mock_slack_api_call: + with patch("apps.slack.client.SlackClient.chat_postMessage") as mock_slack_api_call: notify_about_gaps_in_schedule(schedule.pk) assert not mock_slack_api_call.called diff --git a/engine/apps/slack/alert_group_slack_service.py b/engine/apps/slack/alert_group_slack_service.py index 259513e2..43e02524 100644 --- a/engine/apps/slack/alert_group_slack_service.py +++ b/engine/apps/slack/alert_group_slack_service.py @@ -1,12 +1,15 @@ import logging import typing -from apps.slack.client import ( - SlackAPIChannelArchivedException, - SlackAPIException, - SlackAPIRateLimitException, - SlackAPITokenException, - SlackClientWithErrorHandling, +from apps.slack.client import SlackClient +from apps.slack.errors import ( + SlackAPIChannelArchivedError, + SlackAPIChannelInactiveError, + SlackAPIChannelNotFoundError, + SlackAPIInvalidAuthError, + SlackAPIMessageNotFoundError, + SlackAPIRatelimitError, + SlackAPITokenError, ) if typing.TYPE_CHECKING: @@ -17,18 +20,18 @@ logger = logging.getLogger(__name__) class AlertGroupSlackService: - _slack_client: SlackClientWithErrorHandling + _slack_client: SlackClient def __init__( self, slack_team_identity: "SlackTeamIdentity", - slack_client: typing.Optional[SlackClientWithErrorHandling] = None, + slack_client: typing.Optional[SlackClient] = None, ): self.slack_team_identity = slack_team_identity if slack_client is not None: self._slack_client = slack_client else: - self._slack_client = SlackClientWithErrorHandling(slack_team_identity.bot_access_token) + self._slack_client = SlackClient(slack_team_identity) def update_alert_group_slack_message(self, alert_group: "AlertGroup") -> None: from apps.alerts.models import AlertReceiveChannel @@ -42,7 +45,7 @@ class AlertGroupSlackService: blocks=alert_group.render_slack_blocks(), ) logger.info(f"Message has been updated for alert_group {alert_group.pk}") - except SlackAPIRateLimitException as e: + except SlackAPIRatelimitError as e: if alert_group.channel.integration != AlertReceiveChannel.INTEGRATION_MAINTENANCE: if not alert_group.channel.is_rate_limited_in_slack: alert_group.channel.start_send_rate_limit_message_task(e.retry_after) @@ -50,20 +53,14 @@ class AlertGroupSlackService: f"Message has not been updated for alert_group {alert_group.pk} due to slack rate limit." ) else: - raise e - - except SlackAPIException as e: - if e.response["error"] == "message_not_found": # message deleted from channel - logger.info(f"Skip updating slack message for alert_group {alert_group.pk} due message_not_found") - elif e.response["error"] == "is_inactive": # deleted channel error - logger.info(f"Skip updating slack message for alert_group {alert_group.pk} due to is_inactive") - elif e.response["error"] == "account_inactive": - logger.info(f"Skip updating slack message for alert_group {alert_group.pk} due to account_inactive") - elif e.response["error"] == "channel_not_found": - logger.info(f"Skip updating slack message for alert_group {alert_group.pk} due to channel_not_found") - else: - raise e - logger.info(f"Finished _update_slack_message for alert_group {alert_group.pk}") + raise + except ( + SlackAPIMessageNotFoundError, + SlackAPIChannelInactiveError, + SlackAPITokenError, + SlackAPIChannelNotFoundError, + ): + pass def publish_message_to_alert_group_thread( self, alert_group: "AlertGroup", attachments=[], mrkdwn=True, unfurl_links=True, text=None @@ -82,37 +79,17 @@ class AlertGroupSlackService: mrkdwn=mrkdwn, unfurl_links=unfurl_links, ) - except SlackAPITokenException as e: - logger.warning( - f"Unable to post message to thread in slack. " - f"Slack team identity pk: {self.slack_team_identity.pk}.\n" - f"{e}" - ) - except SlackAPIChannelArchivedException: - logger.warning( - f"Unable to post message to thread in slack. " - f"Slack team identity pk: {self.slack_team_identity.pk}.\n" - f"Reason: 'is_archived'" - ) - except SlackAPIException as e: - if e.response["error"] == "channel_not_found": # channel was deleted - logger.warning( - f"Unable to post message to thread in slack. " - f"Slack team identity pk: {self.slack_team_identity.pk}.\n" - f"Reason: 'channel_not_found'" - ) - elif e.response["error"] == "invalid_auth": - logger.warning( - f"Unable to post message to thread in slack. " - f"Slack team identity pk: {self.slack_team_identity.pk}.\n" - f"Reason: 'invalid_auth'" - ) - else: - raise e - else: - alert_group.slack_messages.create( - slack_id=result["ts"], - organization=alert_group.channel.organization, - _slack_team_identity=self.slack_team_identity, - channel_id=alert_group.slack_message.channel_id, - ) + except ( + SlackAPITokenError, + SlackAPIChannelArchivedError, + SlackAPIChannelNotFoundError, + SlackAPIInvalidAuthError, + ): + return + + alert_group.slack_messages.create( + slack_id=result["ts"], + organization=alert_group.channel.organization, + _slack_team_identity=self.slack_team_identity, + channel_id=alert_group.slack_message.channel_id, + ) diff --git a/engine/apps/slack/client.py b/engine/apps/slack/client.py index 81fe4493..a9ef14ef 100644 --- a/engine/apps/slack/client.py +++ b/engine/apps/slack/client.py @@ -1,36 +1,59 @@ import logging +import typing from typing import Optional, Tuple from django.utils import timezone -from slack_sdk.errors import SlackApiError +from rest_framework import status +from slack_sdk.errors import SlackApiError as SlackSDKApiError +from slack_sdk.http_retry import HttpRequest, HttpResponse, RetryHandler, RetryState, default_retry_handlers from slack_sdk.web import SlackResponse, WebClient -from apps.slack.constants import SLACK_RATE_LIMIT_DELAY +from apps.slack.errors import SlackAPIRatelimitError, SlackAPIServerError, SlackAPITokenError, get_error_class + +if typing.TYPE_CHECKING: + from apps.slack.models import SlackTeamIdentity logger = logging.getLogger(__name__) -class SlackAPIException(Exception): - def __init__(self, msg: str, response: SlackResponse): - super().__init__(msg) - self.response = response +class SlackServerErrorRetryHandler(RetryHandler): + """Retry failed Slack API calls on Slack server errors""" + + def _can_retry( + self, + *, + state: RetryState, + request: HttpRequest, + response: Optional[HttpResponse] = None, + error: Optional[Exception] = None, + ) -> bool: + # Retry Slack API call on 5xx errors + if response and response.status_code in [ + status.HTTP_500_INTERNAL_SERVER_ERROR, + status.HTTP_503_SERVICE_UNAVAILABLE, + status.HTTP_504_GATEWAY_TIMEOUT, + ]: + return True + + # Retry Slack API call on "internal_error" and "fatal_error" errors + if response and response.body and response.body.get("error") in SlackAPIServerError.errors: + return True + + return False -class SlackAPITokenException(SlackAPIException): - pass +server_error_retry_handler = SlackServerErrorRetryHandler(max_retry_count=2) -class SlackAPIChannelArchivedException(SlackAPIException): - pass +class SlackClient(WebClient): + def __init__(self, slack_team_identity: "SlackTeamIdentity", timeout: int = 30) -> None: + super().__init__( + token=slack_team_identity.bot_access_token, + timeout=timeout, + retry_handlers=default_retry_handlers() + [server_error_retry_handler], + ) + self.slack_team_identity = slack_team_identity - -class SlackAPIRateLimitException(SlackAPIException): - def __init__(self, msg: str, response: SlackResponse, retry_after: int): - super().__init__(msg, response) - self.retry_after = retry_after - - -class SlackClientWithErrorHandling(WebClient): def paginated_api_call(self, method: str, paginated_key: str, **kwargs): """ `paginated_key` represents a key from the response which is paginated. For example "users" or "channels" @@ -84,49 +107,48 @@ class SlackClientWithErrorHandling(WebClient): cumulative_response[paginated_key] += response[paginated_key] cursor = next_cursor - except SlackAPIRateLimitException: + except SlackAPIRatelimitError: rate_limited = True return cumulative_response, cursor, rate_limited - def api_call(self, *args, **kwargs): - try: - response = super(SlackClientWithErrorHandling, self).api_call(*args, **kwargs) - except SlackApiError as err: - response = err.response + def api_call(self, *args, **kwargs) -> SlackResponse: + """Wrap Slack SDK api_call with more granular error handling and logging""" - if not response["ok"]: - exception_text = "Slack API Call Error: {} \nArgs: {} \nKwargs: {} \nResponse: {}".format( - response["error"], args, kwargs, response + try: + response = super().api_call(*args, **kwargs) + self._unmark_token_revoked() # unmark token as revoked if the API call was successful + return response + except SlackSDKApiError as e: + logger.error( + "Slack API call error! slack_team_identity={} args={} kwargs={} status={} error={} response={}".format( + self.slack_team_identity.pk, + args, + kwargs, + e.response["status"] if isinstance(e.response, dict) else e.response.status_code, + e.response.get("error"), + e.response, + ) ) - if response["error"] == "is_archived": - raise SlackAPIChannelArchivedException(exception_text, response=response) + # narrow down the error + error_class = get_error_class(e.response) - if response["error"] in ("rate_limited", "ratelimited", "message_limit_exceeded"): - # "message_limit_exceeded" is related to the limit on post messages for free Slack workspace - retry_after = int(response.headers.get("Retry-After", SLACK_RATE_LIMIT_DELAY)) - raise SlackAPIRateLimitException(exception_text, response, retry_after) - - if response["error"] == "code_already_used": - return response - - # Optionally detect account_inactive - if response["error"] == "account_inactive" or response["error"] == "token_revoked": - if "team" in kwargs: - team_identity = kwargs["team"] - del kwargs["team"] - team_identity.detected_token_revoked = timezone.now() - team_identity.is_profile_populated = False - team_identity.save(update_fields=["detected_token_revoked", "is_profile_populated"]) - raise SlackAPITokenException(exception_text, response=response) + # mark / unmark token as revoked + if error_class is SlackAPITokenError: + self._mark_token_revoked() else: - if "team" in kwargs: - slack_team_identity = kwargs["team"] - if slack_team_identity.detected_token_revoked: - slack_team_identity.detected_token_revoked = None - slack_team_identity.save(update_fields=["detected_token_revoked"]) + self._unmark_token_revoked() - raise SlackAPIException(exception_text, response=response) + # raise the narrowed down error class + raise error_class(e.response) from e - return response + def _mark_token_revoked(self) -> None: + if not self.slack_team_identity.detected_token_revoked: + self.slack_team_identity.detected_token_revoked = timezone.now() + self.slack_team_identity.save(update_fields=["detected_token_revoked"]) + + def _unmark_token_revoked(self) -> None: + if self.slack_team_identity.detected_token_revoked: + self.slack_team_identity.detected_token_revoked = None + self.slack_team_identity.save(update_fields=["detected_token_revoked"]) diff --git a/engine/apps/slack/errors.py b/engine/apps/slack/errors.py new file mode 100644 index 00000000..844c8b7f --- /dev/null +++ b/engine/apps/slack/errors.py @@ -0,0 +1,110 @@ +import typing + +from slack_sdk.web import SlackResponse + +from apps.slack.constants import SLACK_RATE_LIMIT_DELAY + + +class UnexpectedResponse(typing.TypedDict): + status: int + headers: dict[str, typing.Any] + body: str + + +class SlackAPIError(Exception): + """ + Base class for Slack API errors. To add a new error class, add a new subclass of SlackAPIError in this file. + See get_error_class at the end of this file for more details on how these are raised. + """ + + errors: tuple[str, ...] + + def __init__(self, response: UnexpectedResponse | SlackResponse): + super().__init__(f"Slack API error! Response: {response}") + self.response = response + + +class SlackAPIServerError(SlackAPIError): + errors = ("internal_error", "fatal_error") + + +class SlackAPITokenError(SlackAPIError): + errors = ("account_inactive", "token_revoked") + + +class SlackAPIChannelArchivedError(SlackAPIError): + errors = ("is_archived",) + + +class SlackAPIRatelimitError(SlackAPIError): + errors = ("ratelimited", "rate_limited", "message_limit_exceeded") + + def __init__(self, response: SlackResponse): + super().__init__(response) + self.retry_after = int(response.headers.get("Retry-After", SLACK_RATE_LIMIT_DELAY)) + + +class SlackAPIPlanUpgradeRequiredError(SlackAPIError): + errors = ("plan_upgrade_required",) + + +class SlackAPIInvalidAuthError(SlackAPIError): + errors = ("invalid_auth",) + + +class SlackAPIUsergroupNotFoundError(SlackAPIError): + errors = ("no_such_subteam",) + + +class SlackAPIChannelNotFoundError(SlackAPIError): + errors = ("channel_not_found",) + + +class SlackAPIMessageNotFoundError(SlackAPIError): + errors = ("message_not_found",) + + +class SlackAPIUserNotFoundError(SlackAPIError): + errors = ("user_not_found",) + + +class SlackAPIChannelInactiveError(SlackAPIError): + errors = ("is_inactive",) + + +class SlackAPIRestrictedActionError(SlackAPIError): + errors = ("restricted_action",) + + +class SlackAPIPermissionDeniedError(SlackAPIError): + errors = ("permission_denied",) + + +class SlackAPIFetchMembersFailedError(SlackAPIError): + errors = ("fetch_members_failed",) + + +class SlackAPIViewNotFoundError(SlackAPIError): + errors = ("not_found",) + + +class SlackAPICannotDMBotError(SlackAPIError): + errors = ("cannot_dm_bot",) + + +class SlackAPIMethodNotSupportedForChannelTypeError(SlackAPIError): + errors = ("method_not_supported_for_channel_type",) + + +_error_to_error_class = { + error: error_class for error_class in SlackAPIError.__subclasses__() for error in error_class.errors +} + + +def get_error_class(response: UnexpectedResponse | SlackResponse) -> typing.Type[SlackAPIError]: + """Get an appropriate error class for the response""" + + if isinstance(response, dict): # UnexpectedResponse + return SlackAPIServerError + + return _error_to_error_class.get(response["error"], SlackAPIError) diff --git a/engine/apps/slack/models/slack_message.py b/engine/apps/slack/models/slack_message.py index adec5272..a3ae3647 100644 --- a/engine/apps/slack/models/slack_message.py +++ b/engine/apps/slack/models/slack_message.py @@ -5,11 +5,13 @@ import uuid from django.db import models -from apps.slack.client import ( - SlackAPIChannelArchivedException, - SlackAPIException, - SlackAPITokenException, - SlackClientWithErrorHandling, +from apps.slack.client import SlackClient +from apps.slack.errors import ( + SlackAPIChannelArchivedError, + SlackAPIError, + SlackAPIFetchMembersFailedError, + SlackAPIMethodNotSupportedForChannelTypeError, + SlackAPITokenError, ) if typing.TYPE_CHECKING: @@ -78,28 +80,22 @@ class SlackMessage(models.Model): return self._slack_team_identity @property - def permalink(self): - if self.slack_team_identity is not None and self.cached_permalink is None: - sc = SlackClientWithErrorHandling(self.slack_team_identity.bot_access_token) - result = None - try: - result = sc.chat_getPermalink(channel=self.channel_id, message_ts=self.slack_id) - except SlackAPIException as e: - if e.response["error"] == "message_not_found": - return "https://slack.com/resources/using-slack/page/404" - elif e.response["error"] == "channel_not_found": - return "https://slack.com/resources/using-slack/page/404" - - if result is not None and result["permalink"] is not None: - # Reconnect to DB in case we use read-only DB here. - _self = SlackMessage.objects.get(pk=self.pk) - _self.cached_permalink = result["permalink"] - _self.save() - self.cached_permalink = _self.cached_permalink - - if self.cached_permalink is not None: + def permalink(self) -> typing.Optional[str]: + if self.cached_permalink or not self.slack_team_identity: return self.cached_permalink + try: + result = SlackClient(self.slack_team_identity).chat_getPermalink( + channel=self.channel_id, message_ts=self.slack_id + ) + except SlackAPIError: + return None + + self.cached_permalink = result["permalink"] + self.save(update_fields=["cached_permalink"]) + + return self.cached_permalink + def send_slack_notification(self, user, alert_group, notification_policy): from apps.base.models import UserNotificationPolicyLogRecord @@ -135,7 +131,7 @@ class SlackMessage(models.Model): }, } ] - sc = SlackClientWithErrorHandling(self.slack_team_identity.bot_access_token) + sc = SlackClient(self.slack_team_identity) channel_id = slack_message.channel_id try: @@ -146,8 +142,7 @@ class SlackMessage(models.Model): thread_ts=slack_message.slack_id, unfurl_links=True, ) - except SlackAPITokenException as e: - print(e) + except SlackAPITokenError: UserNotificationPolicyLogRecord( author=user, type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED, @@ -159,8 +154,7 @@ class SlackMessage(models.Model): notification_error_code=UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_IN_SLACK_TOKEN_ERROR, ).save() return - except SlackAPIChannelArchivedException as e: - print(e) + except SlackAPIChannelArchivedError: UserNotificationPolicyLogRecord( author=user, type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED, @@ -186,24 +180,11 @@ class SlackMessage(models.Model): channel_members = [] try: channel_members = sc.conversations_members(channel=channel_id)["members"] - except SlackAPIException as e: - if e.response["error"] == "fetch_members_failed": - logger.warning( - f"Unable to get members from slack conversation: 'fetch_members_failed'. " - f"Slack team identity pk: {self.slack_team_identity.pk}.\n" - f"{e}" - ) - else: - raise e + except SlackAPIFetchMembersFailedError: + pass if slack_user_identity.slack_id not in channel_members: time.sleep(5) # 2 messages in the same moment are ratelimited by Slack. Dirty hack. slack_user_identity.send_link_to_slack_message(slack_message) - except SlackAPITokenException as e: - print(e) - except SlackAPIException as e: - if e.response["error"] == "method_not_supported_for_channel_type": - # It's ok, just a private channel. Passing - pass - else: - raise e + except (SlackAPITokenError, SlackAPIMethodNotSupportedForChannelTypeError): + pass diff --git a/engine/apps/slack/models/slack_team_identity.py b/engine/apps/slack/models/slack_team_identity.py index c80cd456..60d8b19a 100644 --- a/engine/apps/slack/models/slack_team_identity.py +++ b/engine/apps/slack/models/slack_team_identity.py @@ -5,8 +5,14 @@ from django.db import models from django.db.models import JSONField from apps.api.permissions import RBACPermission -from apps.slack.client import SlackAPIException, SlackAPITokenException, SlackClientWithErrorHandling +from apps.slack.client import SlackClient from apps.slack.constants import SLACK_INVALID_AUTH_RESPONSE, SLACK_WRONG_TEAM_NAMES +from apps.slack.errors import ( + SlackAPIChannelNotFoundError, + SlackAPIFetchMembersFailedError, + SlackAPIInvalidAuthError, + SlackAPITokenError, +) from apps.user_management.models.user import User from common.insight_log.chatops_insight_logs import ChatOpsEvent, ChatOpsTypePlug, write_chatops_insight_log @@ -85,7 +91,7 @@ class SlackTeamIdentity(models.Model): @property def bot_id(self): if self.cached_bot_id is None: - sc = SlackClientWithErrorHandling(self.bot_access_token) + sc = SlackClient(self) auth = sc.auth_test() self.cached_bot_id = auth.get("bot_id") self.save(update_fields=["cached_bot_id"]) @@ -93,7 +99,7 @@ class SlackTeamIdentity(models.Model): @property def members(self): - sc = SlackClientWithErrorHandling(self.bot_access_token) + sc = SlackClient(self) next_cursor = None members = [] @@ -108,22 +114,19 @@ class SlackTeamIdentity(models.Model): def name(self): if self.cached_name is None or self.cached_name in SLACK_WRONG_TEAM_NAMES: try: - sc = SlackClientWithErrorHandling(self.bot_access_token) + sc = SlackClient(self) result = sc.team_info() self.cached_name = result["team"]["name"] self.save() - except SlackAPIException as e: - if e.response["error"] == "invalid_auth": - self.cached_name = SLACK_INVALID_AUTH_RESPONSE - self.save() - else: - raise e + except SlackAPIInvalidAuthError: + self.cached_name = SLACK_INVALID_AUTH_RESPONSE + self.save() return self.cached_name @property def app_id(self): if not self.cached_app_id: - sc = SlackClientWithErrorHandling(self.bot_access_token) + sc = SlackClient(self) result = sc.bots_info(bot=self.bot_id) app_id = result["bot"]["app_id"] self.cached_app_id = app_id @@ -131,7 +134,7 @@ class SlackTeamIdentity(models.Model): return self.cached_app_id def get_users_from_slack_conversation_for_organization(self, channel_id, organization): - sc = SlackClientWithErrorHandling(self.bot_access_token) + sc = SlackClient(self) members = self.get_conversation_members(sc, channel_id) return organization.users.filter( @@ -139,32 +142,10 @@ class SlackTeamIdentity(models.Model): **User.build_permissions_query(RBACPermission.Permissions.CHATOPS_WRITE, organization), ) - def get_conversation_members(self, slack_client: SlackClientWithErrorHandling, channel_id: str): + def get_conversation_members(self, slack_client: SlackClient, channel_id: str): try: - members = slack_client.paginated_api_call( + return slack_client.paginated_api_call( "conversations_members", paginated_key="members", channel=channel_id )["members"] - except SlackAPITokenException as e: - logger.warning( - f"Unable to get members from slack conversation for Slack team identity pk: {self.pk}.\n" f"{e}" - ) - members = [] - except SlackAPIException as e: - if e.response["error"] == "fetch_members_failed": - logger.warning( - f"Unable to get members from slack conversation: 'fetch_members_failed'. " - f"Slack team identity pk: {self.pk}.\n" - f"{e}" - ) - members = [] - elif e.response["error"] == "channel_not_found": - logger.warning( - f"Unable to get members from slack conversation: 'channel_not_found'. " - f"Slack team identity pk: {self.pk}.\n" - f"{e}" - ) - members = [] - else: - raise e - - return members + except (SlackAPITokenError, SlackAPIFetchMembersFailedError, SlackAPIChannelNotFoundError): + return [] diff --git a/engine/apps/slack/models/slack_user_identity.py b/engine/apps/slack/models/slack_user_identity.py index d016c7b9..ee76b32f 100644 --- a/engine/apps/slack/models/slack_user_identity.py +++ b/engine/apps/slack/models/slack_user_identity.py @@ -4,8 +4,14 @@ import typing import requests from django.db import models -from apps.slack.client import SlackAPIException, SlackAPITokenException, SlackClientWithErrorHandling +from apps.slack.client import SlackClient from apps.slack.constants import SLACK_BOT_ID +from apps.slack.errors import ( + SlackAPICannotDMBotError, + SlackAPIInvalidAuthError, + SlackAPITokenError, + SlackAPIUserNotFoundError, +) from apps.slack.scenarios.notified_user_not_in_channel import NotifiedUserNotInChannelStep from apps.user_management.models import Organization, User @@ -133,7 +139,7 @@ class SlackUserIdentity(models.Model): }, ] - sc = SlackClientWithErrorHandling(self.slack_team_identity.bot_access_token) + sc = SlackClient(self.slack_team_identity) return sc.chat_postMessage( channel=self.im_channel_id, text="You are invited to look at an alert group!", @@ -154,32 +160,27 @@ class SlackUserIdentity(models.Model): @property def slack_login(self): if self.cached_slack_login is None or self.cached_slack_login == "slack_token_revoked_unable_to_cache_login": - sc = SlackClientWithErrorHandling(self.slack_team_identity.bot_access_token) + sc = SlackClient(self.slack_team_identity) try: result = sc.users_info(user=self.slack_id, team=self.slack_team_identity) self.cached_slack_login = result["user"]["name"] self.save() - except SlackAPITokenException as e: - logger.warning("Unable to get slack login: token revoked\n" + str(e)) + except SlackAPITokenError: self.cached_slack_login = "slack_token_revoked_unable_to_cache_login" self.save() return "slack_token_revoked_unable_to_cache_login" - except SlackAPIException as e: - if e.response["error"] == "user_not_found": - logger.warning("user_not_found " + str(e)) - self.cached_slack_login = "user_not_found" - self.save() - elif e.response["error"] == "invalid_auth": - return "no_enough_permissions_to_retrieve" - else: - raise e + except SlackAPIUserNotFoundError: + self.cached_slack_login = "user_not_found" + self.save() + except SlackAPIInvalidAuthError: + return "no_enough_permissions_to_retrieve" return str(self.cached_slack_login) @property def timezone(self): if self.cached_timezone is None or self.cached_timezone == "None": - sc = SlackClientWithErrorHandling(self.slack_team_identity.bot_access_token) + sc = SlackClient(self.slack_team_identity) try: result = sc.users_info(user=self.slack_id) tz_from_slack = result["user"].get("tz", "UTC") @@ -187,8 +188,8 @@ class SlackUserIdentity(models.Model): tz_from_slack = "UTC" self.cached_timezone = tz_from_slack self.save(update_fields=["cached_timezone"]) - except SlackAPITokenException as e: - print("Token revoked: " + str(e)) + except SlackAPITokenError: + pass except requests.exceptions.Timeout: # Do not save tz in case of timeout to try to load it later again return "UTC" @@ -198,25 +199,22 @@ class SlackUserIdentity(models.Model): @property def im_channel_id(self): if self.cached_im_channel_id is None: - sc = SlackClientWithErrorHandling(self.slack_team_identity.bot_access_token) + sc = SlackClient(self.slack_team_identity) try: result = sc.conversations_open(users=self.slack_id, return_im=True) self.cached_im_channel_id = result["channel"]["id"] self.save() - except SlackAPIException as e: - if e.response["error"] == "cannot_dm_bot": - logger.warning("Trying to DM bot " + str(e)) - else: - raise e + except SlackAPICannotDMBotError: + pass return self.cached_im_channel_id def update_profile_info(self): - sc = SlackClientWithErrorHandling(self.slack_team_identity.bot_access_token) + sc = SlackClient(self.slack_team_identity) logger.info("Update user profile info") try: result = sc.users_info(user=self.slack_id, team=self.slack_team_identity) - except SlackAPITokenException as e: + except SlackAPITokenError as e: logger.warning(f"Unable to get user info due token revoked or account inactive: {e}") result = None else: diff --git a/engine/apps/slack/models/slack_usergroup.py b/engine/apps/slack/models/slack_usergroup.py index 9af48d94..0bd45f27 100644 --- a/engine/apps/slack/models/slack_usergroup.py +++ b/engine/apps/slack/models/slack_usergroup.py @@ -9,7 +9,9 @@ from django.db.models import JSONField from django.utils import timezone from apps.api.permissions import RBACPermission -from apps.slack.client import SlackAPIException, SlackClientWithErrorHandling +from apps.slack.client import SlackClient +from apps.slack.errors import SlackAPIError, SlackAPIPermissionDeniedError +from apps.slack.models import SlackTeamIdentity from apps.user_management.models.user import User from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length @@ -68,12 +70,12 @@ class SlackUserGroup(models.Model): @property def can_be_updated(self) -> bool: - sc = SlackClientWithErrorHandling(self.slack_team_identity.bot_access_token, timeout=5) + sc = SlackClient(self.slack_team_identity, timeout=5) try: sc.usergroups_update(usergroup=self.slack_id) return True - except (SlackAPIException, requests.exceptions.Timeout): + except (SlackAPIError, requests.exceptions.Timeout): return False @property @@ -104,12 +106,11 @@ class SlackUserGroup(models.Model): try: self.update_members(slack_ids) - except SlackAPIException as e: - if e.response["error"] == "permission_denied": - logger.warning(f"Could not update usergroup {self.slack_id} due to permission_denied") + except SlackAPIPermissionDeniedError: + pass def update_members(self, slack_ids): - sc = SlackClientWithErrorHandling(self.slack_team_identity.bot_access_token) + sc = SlackClient(self.slack_team_identity) sc.usergroups_users_update(usergroup=self.slack_id, users=slack_ids) @@ -123,59 +124,24 @@ class SlackUserGroup(models.Model): ) @classmethod - def update_or_create_slack_usergroup_from_slack(cls, slack_id, slack_team_identity): - sc = SlackClientWithErrorHandling(slack_team_identity.bot_access_token) - bot_access_token_accepted = True + def update_or_create_slack_usergroup_from_slack(cls, slack_id: str, slack_team_identity: SlackTeamIdentity) -> None: + sc = SlackClient(slack_team_identity) + usergroups = sc.usergroups_list()["usergroups"] + + usergroup = [ug for ug in usergroups if ug["id"] == slack_id][0] try: - usergroups_list = sc.usergroups_list() - except SlackAPIException as e: - if e.response["error"] == "not_allowed_token_type": - # Trying same request with access token. It is required due to migration to granular permissions - # and can be removed after clients reinstall their bots - try: - sc_with_access_token = SlackClientWithErrorHandling(slack_team_identity.access_token) - usergroups_list = sc_with_access_token.usergroups_list() - bot_access_token_accepted = False - except SlackAPIException as err: - if err.response["error"] == "missing_scope": - return None, False - else: - raise err - elif e.response["error"] == "missing_scope": - return None, False - else: - raise e + members = sc.usergroups_users_list(usergroup=usergroup["id"])["users"] + except SlackAPIError: + return - for usergroup in usergroups_list["usergroups"]: - if usergroup["id"] == slack_id: - try: - if bot_access_token_accepted: - usergroups_users = sc.usergroups_users_list(usergroup=usergroup["id"]) - else: - sc_with_access_token = SlackClientWithErrorHandling(slack_team_identity.access_token) - usergroups_users = sc_with_access_token.usergroups_users_list(usergroup=usergroup["id"]) - except SlackAPIException as e: - if e.response["error"] == "no_such_subteam": - logger.info("User group does not exist") - else: - logger.error( - f"'usergroups.users.list' slack api error. " - f"SlackTeamIdentity pk: {slack_team_identity.pk}\n{e}" - ) - else: - usergroup_name = usergroup["name"] - usergroup_handle = usergroup["handle"] - usergroup_members = usergroups_users["users"] - usergroup_is_active = usergroup["date_delete"] == 0 - - return SlackUserGroup.objects.update_or_create( - slack_id=usergroup["id"], - slack_team_identity=slack_team_identity, - defaults={ - "name": usergroup_name, - "handle": usergroup_handle, - "members": usergroup_members, - "is_active": usergroup_is_active, - "last_populated": timezone.now().date(), - }, - ) + SlackUserGroup.objects.update_or_create( + slack_id=usergroup["id"], + slack_team_identity=slack_team_identity, + defaults={ + "name": usergroup["name"], + "handle": usergroup["handle"], + "members": members, + "is_active": usergroup["date_delete"] == 0, + "last_populated": timezone.now().date(), + }, + ) diff --git a/engine/apps/slack/scenarios/distribute_alerts.py b/engine/apps/slack/scenarios/distribute_alerts.py index 6b0e3575..1718ce43 100644 --- a/engine/apps/slack/scenarios/distribute_alerts.py +++ b/engine/apps/slack/scenarios/distribute_alerts.py @@ -15,14 +15,18 @@ from apps.alerts.models import Alert, AlertGroup, AlertGroupLogRecord, AlertRece from apps.alerts.tasks import custom_button_result from apps.alerts.utils import render_curl_command from apps.api.permissions import RBACPermission -from apps.slack.client import ( - SlackAPIChannelArchivedException, - SlackAPIException, - SlackAPIRateLimitException, - SlackAPITokenException, - SlackClientWithErrorHandling, -) from apps.slack.constants import CACHE_UPDATE_INCIDENT_SLACK_MESSAGE_LIFETIME +from apps.slack.errors import ( + SlackAPIChannelArchivedError, + SlackAPIChannelInactiveError, + SlackAPIChannelNotFoundError, + SlackAPIError, + SlackAPIInvalidAuthError, + SlackAPIMessageNotFoundError, + SlackAPIRatelimitError, + SlackAPIRestrictedActionError, + SlackAPITokenError, +) from apps.slack.scenarios import scenario_step from apps.slack.scenarios.slack_renderer import AlertGroupLogSlackRenderer from apps.slack.slack_formatter import SlackFormatter @@ -74,9 +78,9 @@ class AlertShootingStep(scenario_step.ScenarioStep): try: channel_id = alert.group.channel_filter.slack_channel_id_or_general_log_id self._send_first_alert(alert, channel_id) - except SlackAPIException as e: + except SlackAPIError: AlertGroup.objects.filter(pk=alert.group.pk).update(slack_message_sent=False) - raise e + raise if alert.group.channel.maintenance_mode == AlertReceiveChannel.DEBUG_MAINTENANCE: self._send_debug_mode_notice(alert.group, channel_id) @@ -159,15 +163,15 @@ class AlertShootingStep(scenario_step.ScenarioStep): ) alert.delivered = True - except SlackAPITokenException: + except SlackAPITokenError: alert_group.reason_to_skip_escalation = AlertGroup.ACCOUNT_INACTIVE alert_group.save(update_fields=["reason_to_skip_escalation"]) logger.info("Not delivering alert due to account_inactive.") - except SlackAPIChannelArchivedException: + except SlackAPIChannelArchivedError: alert_group.reason_to_skip_escalation = AlertGroup.CHANNEL_ARCHIVED alert_group.save(update_fields=["reason_to_skip_escalation"]) logger.info("Not delivering alert due to channel is archived.") - except SlackAPIRateLimitException as e: + except SlackAPIRatelimitError as e: # don't rate limit maintenance alert if alert_group.channel.integration != AlertReceiveChannel.INTEGRATION_MAINTENANCE: alert_group.reason_to_skip_escalation = AlertGroup.RATE_LIMITED @@ -176,19 +180,14 @@ class AlertShootingStep(scenario_step.ScenarioStep): logger.info("Not delivering alert due to slack rate limit.") else: raise e - except SlackAPIException as e: - # TODO: slack-onprem check exceptions - if e.response["error"] == "channel_not_found": - alert_group.reason_to_skip_escalation = AlertGroup.CHANNEL_ARCHIVED - alert_group.save(update_fields=["reason_to_skip_escalation"]) - logger.info("Not delivering alert due to channel is archived.") - elif e.response["error"] == "restricted_action": - # workspace settings prevent bot to post message (eg. bot is not a full member) - alert_group.reason_to_skip_escalation = AlertGroup.RESTRICTED_ACTION - alert_group.save(update_fields=["reason_to_skip_escalation"]) - logger.info("Not delivering alert due to workspace restricted action.") - else: - raise e + except SlackAPIChannelNotFoundError: + alert_group.reason_to_skip_escalation = AlertGroup.CHANNEL_ARCHIVED + alert_group.save(update_fields=["reason_to_skip_escalation"]) + logger.info("Not delivering alert due to channel is archived.") + except SlackAPIRestrictedActionError: + alert_group.reason_to_skip_escalation = AlertGroup.RESTRICTED_ACTION + alert_group.save(update_fields=["reason_to_skip_escalation"]) + logger.info("Not delivering alert due to workspace restricted action.") finally: alert.save() @@ -753,18 +752,13 @@ class UnAcknowledgeGroupStep(AlertGroupActionsMixin, scenario_step.ScenarioStep) text=text, attachments=message_attachments, ) - except SlackAPIException as e: + except SlackAPIMessageNotFoundError: # post to thread if ack reminder message was deleted in Slack - if e.response["error"] == "message_not_found": - self.alert_group_slack_service.publish_message_to_alert_group_thread( - alert_group, attachments=message_attachments, text=text - ) - elif e.response["error"] == "account_inactive": - logger.info( - f"Skip unacknowledge slack message for alert_group {alert_group.pk} due to account_inactive" - ) - else: - raise + self.alert_group_slack_service.publish_message_to_alert_group_thread( + alert_group, attachments=message_attachments, text=text + ) + except SlackAPITokenError: + pass else: self.alert_group_slack_service.publish_message_to_alert_group_thread( alert_group, attachments=message_attachments, text=text @@ -856,27 +850,8 @@ class AcknowledgeConfirmationStep(AcknowledgeGroupStep): attachments=attachments, thread_ts=alert_group.slack_message.slack_id, ) - except SlackAPITokenException as e: - logger.warning( - f"Unable to post acknowledge reminder in slack. " - f"Slack team identity pk: {self.slack_team_identity.pk}.\n" - f"{e}" - ) - except SlackAPIChannelArchivedException: - logger.warning( - f"Unable to post acknowledge reminder in slack. " - f"Slack team identity pk: {self.slack_team_identity.pk}.\n" - f"Reason: 'is_archived'" - ) - except SlackAPIException as e: - if e.response["error"] == "channel_not_found": - logger.warning( - f"Unable to post acknowledge reminder in slack. " - f"Slack team identity pk: {self.slack_team_identity.pk}.\n" - f"Reason: 'channel_not_found'" - ) - else: - raise e + except (SlackAPITokenError, SlackAPIChannelArchivedError, SlackAPIChannelNotFoundError): + pass else: alert_group.slack_messages.create( slack_id=response["ts"], @@ -917,41 +892,13 @@ class DeleteGroupStep(scenario_step.ScenarioStep): for message_ts in bot_messages_ts: try: self._slack_client.chat_delete(channel=channel_id, ts=message_ts) - except SlackAPITokenException as e: - logger.error( - f"Unable to delete messages in slack. Message ts: {message_ts}" - f"Slack team identity pk: {self.slack_team_identity.pk}.\n" - f"{e}" - ) - except SlackAPIException as e: - if e.response["error"] == "channel_not_found": - logger.warning( - f"Unable to delete messages in slack. Message ts: {message_ts}" - f"Slack team identity pk: {self.slack_team_identity.pk}.\n" - f"Reason: 'channel_not_found'" - f"{e}" - ) - elif e.response["error"] == "message_not_found": - logger.warning( - f"Unable to delete messages in slack. Message ts: {message_ts}" - f"Slack team identity pk: {self.slack_team_identity.pk}.\n" - f"Reason: 'message_not_found'" - f"{e}" - ) - elif e.response["error"] == "is_archived": - logger.warning( - f"Unable to delete messages in slack. Message ts: {message_ts}" - f"Slack team identity pk: {self.slack_team_identity.pk}.\n" - f"Reason: 'is_archived'" - f"{e}" - ) - elif e.response["error"] == "cant_delete_message": - sc_with_access_token = SlackClientWithErrorHandling( - self.slack_team_identity.access_token - ) # used access_token instead of bot_access_token - sc_with_access_token.chat_delete(channel=channel_id, ts=message_ts) - else: - raise e + except ( + SlackAPITokenError, + SlackAPIChannelNotFoundError, + SlackAPIMessageNotFoundError, + SlackAPIChannelArchivedError, + ): + pass def remove_resolution_note_reaction(self, alert_group: AlertGroup) -> None: for message in alert_group.resolution_note_slack_messages.filter(added_to_resolution_note=True): @@ -959,14 +906,8 @@ class DeleteGroupStep(scenario_step.ScenarioStep): message.save(update_fields=["added_to_resolution_note"]) try: self._slack_client.reactions_remove(channel=message.slack_channel_id, name="memo", timestamp=message.ts) - except SlackAPITokenException as e: - logger.warning( - f"Unable to delete resolution note reaction in slack. " - f"Slack team identity pk: {self.slack_team_identity.pk}.\n" - f"{e}" - ) - except SlackAPIException as e: - logger.warning(f"Unable to delete resolution note reaction in slack.\n" f"{e}") + except SlackAPIError: + pass class UpdateLogReportMessageStep(scenario_step.ScenarioStep): @@ -992,23 +933,19 @@ class UpdateLogReportMessageStep(scenario_step.ScenarioStep): thread_ts=slack_message.slack_id, text="Building escalation plan... :thinking_face:", ) - except SlackAPITokenException as e: - print(e) - except SlackAPIRateLimitException as e: + except SlackAPIRatelimitError as e: if not alert_group.channel.is_rate_limited_in_slack: alert_group.channel.start_send_rate_limit_message_task(e.retry_after) logger.info( f"Log message has not been posted for alert_group {alert_group.pk} due to slack rate limit." ) - except SlackAPIException as e: - if e.response["error"] == "channel_not_found": - pass - elif e.response["error"] == "invalid_auth": - pass - elif e.response["error"] == "is_archived": - pass - else: - raise e + except ( + SlackAPITokenError, + SlackAPIChannelNotFoundError, + SlackAPIInvalidAuthError, + SlackAPIChannelArchivedError, + ): + pass else: logger.debug(f"Create new slack_log_message for alert_group {alert_group.pk}") slack_log_message = alert_group.slack_messages.create( @@ -1054,30 +991,20 @@ class UpdateLogReportMessageStep(scenario_step.ScenarioStep): ts=slack_log_message.slack_id, attachments=attachments, ) - except SlackAPITokenException as e: - print(e) - except SlackAPIRateLimitException as e: + except SlackAPIRatelimitError as e: if not alert_group.channel.is_rate_limited_in_slack: alert_group.channel.start_send_rate_limit_message_task(e.retry_after) - logger.info( - f"Log message has not been updated for alert_group {alert_group.pk} due to slack rate limit." - ) - except SlackAPIException as e: - if e.response["error"] == "message_not_found": - alert_group.slack_log_message = None - alert_group.save(update_fields=["slack_log_message"]) - elif e.response["error"] == "channel_not_found": - pass - elif e.response["error"] == "is_archived": - pass - elif e.response["error"] == "is_inactive": - pass - elif e.response["error"] == "account_inactive": - pass - elif e.response["error"] == "invalid_auth": - pass - else: - raise e + except SlackAPIMessageNotFoundError: + alert_group.slack_log_message = None + alert_group.save(update_fields=["slack_log_message"]) + except ( + SlackAPITokenError, + SlackAPIChannelNotFoundError, + SlackAPIChannelArchivedError, + SlackAPIChannelInactiveError, + SlackAPIInvalidAuthError, + ): + pass else: slack_log_message.last_updated = timezone.now() slack_log_message.save(update_fields=["last_updated"]) diff --git a/engine/apps/slack/scenarios/invited_to_channel.py b/engine/apps/slack/scenarios/invited_to_channel.py index 1ec5a7a1..e56ab9f4 100644 --- a/engine/apps/slack/scenarios/invited_to_channel.py +++ b/engine/apps/slack/scenarios/invited_to_channel.py @@ -3,7 +3,7 @@ import typing from django.utils import timezone -from apps.slack.client import SlackClientWithErrorHandling +from apps.slack.client import SlackClient from apps.slack.scenarios import scenario_step from apps.slack.types import EventPayload, EventType, PayloadType, ScenarioRoute @@ -23,7 +23,7 @@ class InvitedToChannelStep(scenario_step.ScenarioStep): ) -> None: if payload["event"]["user"] == slack_team_identity.bot_user_id: channel_id = payload["event"]["channel"] - slack_client = SlackClientWithErrorHandling(slack_team_identity.bot_access_token) + slack_client = SlackClient(slack_team_identity) channel = slack_client.conversations_info(channel=channel_id)["channel"] slack_team_identity.cached_channels.update_or_create( diff --git a/engine/apps/slack/scenarios/manual_incident.py b/engine/apps/slack/scenarios/manual_incident.py index eb7af94e..7979a980 100644 --- a/engine/apps/slack/scenarios/manual_incident.py +++ b/engine/apps/slack/scenarios/manual_incident.py @@ -5,8 +5,8 @@ from uuid import uuid4 from django.conf import settings from apps.alerts.models import AlertReceiveChannel, ChannelFilter -from apps.slack.client import SlackAPIException from apps.slack.constants import DIVIDER +from apps.slack.errors import SlackAPIChannelNotFoundError from apps.slack.scenarios import scenario_step from apps.slack.types import ( Block, @@ -116,15 +116,12 @@ class FinishCreateIncidentFromSlashCommand(scenario_step.ScenarioStep): user=slack_user_identity.slack_id, text=":white_check_mark: Alert *{}* successfully submitted".format(title), ) - except SlackAPIException as e: - if e.response["error"] == "channel_not_found": - self._slack_client.chat_postEphemeral( - channel=slack_user_identity.im_channel_id, - user=slack_user_identity.slack_id, - text=":white_check_mark: Alert *{}* successfully submitted".format(title), - ) - else: - raise e + except SlackAPIChannelNotFoundError: + self._slack_client.chat_postEphemeral( + channel=slack_user_identity.im_channel_id, + user=slack_user_identity.slack_id, + text=":white_check_mark: Alert *{}* successfully submitted".format(title), + ) # Deprecated, use custom oncall property instead. # Update private metadata to use it in rendering: diff --git a/engine/apps/slack/scenarios/notification_delivery.py b/engine/apps/slack/scenarios/notification_delivery.py index f711b3c6..d1815b38 100644 --- a/engine/apps/slack/scenarios/notification_delivery.py +++ b/engine/apps/slack/scenarios/notification_delivery.py @@ -1,6 +1,11 @@ import typing -from apps.slack.client import SlackAPIException, SlackAPITokenException +from apps.slack.errors import ( + SlackAPIChannelArchivedError, + SlackAPIChannelNotFoundError, + SlackAPIInvalidAuthError, + SlackAPITokenError, +) from apps.slack.scenarios import scenario_step from apps.slack.types import Block @@ -70,22 +75,18 @@ class NotificationDeliveryStep(scenario_step.ScenarioStep): }, }, ] + try: - # TODO: slack-onprem, check exceptions self._slack_client.chat_postMessage( channel=channel, text=text, blocks=blocks, unfurl_links=True, ) - except SlackAPITokenException as e: - print(e) - except SlackAPIException as e: - if e.response["error"] == "channel_not_found": - pass - elif e.response["error"] == "is_archived": - pass - elif e.response["error"] == "invalid_auth": - print(e) - else: - raise e + except ( + SlackAPITokenError, + SlackAPIChannelNotFoundError, + SlackAPIChannelArchivedError, + SlackAPIInvalidAuthError, + ): + pass diff --git a/engine/apps/slack/scenarios/paging.py b/engine/apps/slack/scenarios/paging.py index 499d0ecd..0ad4c7fe 100644 --- a/engine/apps/slack/scenarios/paging.py +++ b/engine/apps/slack/scenarios/paging.py @@ -15,8 +15,8 @@ from apps.alerts.paging import ( check_user_availability, direct_paging, ) -from apps.slack.client import SlackAPIException from apps.slack.constants import DIVIDER, PRIVATE_METADATA_MAX_LENGTH +from apps.slack.errors import SlackAPIChannelNotFoundError from apps.slack.scenarios import scenario_step from apps.slack.types import ( Block, @@ -207,15 +207,12 @@ class FinishDirectPaging(scenario_step.ScenarioStep): user=slack_user_identity.slack_id, text=text, ) - except SlackAPIException as e: - if e.response["error"] == "channel_not_found": - self._slack_client.chat_postEphemeral( - channel=slack_user_identity.im_channel_id, - user=slack_user_identity.slack_id, - text=text, - ) - else: - raise e + except SlackAPIChannelNotFoundError: + self._slack_client.chat_postEphemeral( + channel=slack_user_identity.im_channel_id, + user=slack_user_identity.slack_id, + text=text, + ) # OnChange steps, responsible for rerendering form on changed values diff --git a/engine/apps/slack/scenarios/resolution_note.py b/engine/apps/slack/scenarios/resolution_note.py index f6c640d6..09eea274 100644 --- a/engine/apps/slack/scenarios/resolution_note.py +++ b/engine/apps/slack/scenarios/resolution_note.py @@ -6,8 +6,17 @@ import typing from django.db.models import Q from apps.api.permissions import RBACPermission -from apps.slack.client import SlackAPIException from apps.slack.constants import DIVIDER +from apps.slack.errors import ( + SlackAPIChannelArchivedError, + SlackAPIChannelInactiveError, + SlackAPIChannelNotFoundError, + SlackAPIError, + SlackAPIInvalidAuthError, + SlackAPIMessageNotFoundError, + SlackAPITokenError, + SlackAPIViewNotFoundError, +) from apps.slack.scenarios import scenario_step from apps.slack.types import ( Block, @@ -30,46 +39,14 @@ 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 +RESOLUTION_NOTE_EXCEPTIONS = ( + SlackAPIChannelNotFoundError, + SlackAPIMessageNotFoundError, + SlackAPIChannelArchivedError, + SlackAPIInvalidAuthError, + SlackAPITokenError, + SlackAPIChannelInactiveError, +) class AddToResolutionNoteStep(scenario_step.ScenarioStep): @@ -198,7 +175,7 @@ class AddToResolutionNoteStep(scenario_step.ScenarioStep): name="memo", timestamp=resolution_note_slack_message.ts, ) - except SlackAPIException: + except SlackAPIError: pass self.alert_group_slack_service.update_alert_group_slack_message(alert_group) @@ -230,8 +207,8 @@ class UpdateResolutionNoteStep(scenario_step.ScenarioStep): channel=resolution_note_slack_message.slack_channel_id, ts=resolution_note_slack_message.ts, ) - except SlackAPIException as e: - handle_resolution_note_message_exception(self, "delete", e) + except RESOLUTION_NOTE_EXCEPTIONS: + pass else: self.remove_resolution_note_reaction(resolution_note_slack_message) @@ -251,8 +228,8 @@ class UpdateResolutionNoteStep(scenario_step.ScenarioStep): text=resolution_note.text, blocks=blocks, ) - except SlackAPIException as e: - handle_resolution_note_message_exception(self, "post", e) + except RESOLUTION_NOTE_EXCEPTIONS: + pass else: message_ts = result["message"]["ts"] result_permalink = self._slack_client.chat_getPermalink( @@ -285,8 +262,8 @@ class UpdateResolutionNoteStep(scenario_step.ScenarioStep): text=resolution_note_slack_message.text, blocks=blocks, ) - except SlackAPIException as e: - handle_resolution_note_message_exception(self, "update", e) + except RESOLUTION_NOTE_EXCEPTIONS: + pass else: resolution_note_slack_message.text = resolution_note.text resolution_note_slack_message.save(update_fields=["text"]) @@ -302,8 +279,8 @@ class UpdateResolutionNoteStep(scenario_step.ScenarioStep): name="memo", timestamp=slack_thread_message.ts, ) - except SlackAPIException as e: - logger.exception(e) + except SlackAPIError: + pass def remove_resolution_note_reaction(self, slack_thread_message: "ResolutionNoteSlackMessage") -> None: try: @@ -312,8 +289,8 @@ class UpdateResolutionNoteStep(scenario_step.ScenarioStep): name="memo", timestamp=slack_thread_message.ts, ) - except SlackAPIException as e: - logger.exception(e) + except SlackAPIError: + pass def get_resolution_note_blocks(self, resolution_note: "ResolutionNote") -> Block.AnyBlocks: blocks: Block.AnyBlocks = [] @@ -408,16 +385,8 @@ class ResolutionNoteModalStep(AlertGroupActionsMixin, scenario_step.ScenarioStep view=view, view_id=payload["view"]["id"], ) - except SlackAPIException as e: - if e.response["error"] == "not_found": - # Ignore "not_found" error, it means that the view was closed by user before the update request. - # It doesn't disrupt the user experience. - logger.debug( - f"API call to views.update failed for alert group {alert_group.pk}, error: not_found. " - f"Most likely the view was closed by user before the request was processed. " - ) - else: - raise + except SlackAPIViewNotFoundError: + pass else: self._slack_client.views_open(trigger_id=payload["trigger_id"], view=view) diff --git a/engine/apps/slack/scenarios/scenario_step.py b/engine/apps/slack/scenarios/scenario_step.py index a8a38c1a..f6201e3c 100644 --- a/engine/apps/slack/scenarios/scenario_step.py +++ b/engine/apps/slack/scenarios/scenario_step.py @@ -3,7 +3,7 @@ import logging import typing from apps.slack.alert_group_slack_service import AlertGroupSlackService -from apps.slack.client import SlackClientWithErrorHandling +from apps.slack.client import SlackClient if typing.TYPE_CHECKING: from apps.slack.models import SlackTeamIdentity, SlackUserIdentity @@ -20,7 +20,7 @@ class ScenarioStep(object): organization: typing.Optional["Organization"] = None, user: typing.Optional["User"] = None, ): - self._slack_client = SlackClientWithErrorHandling(slack_team_identity.bot_access_token) + self._slack_client = SlackClient(slack_team_identity) self.slack_team_identity = slack_team_identity self.organization = organization self.user = user diff --git a/engine/apps/slack/scenarios/slack_channel_integration.py b/engine/apps/slack/scenarios/slack_channel_integration.py index b24f8040..234ea53c 100644 --- a/engine/apps/slack/scenarios/slack_channel_integration.py +++ b/engine/apps/slack/scenarios/slack_channel_integration.py @@ -1,9 +1,8 @@ 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.scenarios.resolution_note import RESOLUTION_NOTE_EXCEPTIONS from apps.slack.types import EventPayload, EventType, MessageEventSubtype, PayloadType, ScenarioRoute if typing.TYPE_CHECKING: @@ -79,8 +78,7 @@ class SlackChannelMessageEventStep(scenario_step.ScenarioStep): 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) + except RESOLUTION_NOTE_EXCEPTIONS: return permalink = None diff --git a/engine/apps/slack/tasks.py b/engine/apps/slack/tasks.py index 4addb985..f4a04a57 100644 --- a/engine/apps/slack/tasks.py +++ b/engine/apps/slack/tasks.py @@ -10,8 +10,15 @@ from django.utils import timezone from apps.alerts.tasks.compare_escalations import compare_escalations from apps.slack.alert_group_slack_service import AlertGroupSlackService -from apps.slack.client import SlackAPIException, SlackAPITokenException, SlackClientWithErrorHandling +from apps.slack.client import SlackClient from apps.slack.constants import CACHE_UPDATE_INCIDENT_SLACK_MESSAGE_LIFETIME, SLACK_BOT_ID +from apps.slack.errors import ( + SlackAPIInvalidAuthError, + SlackAPIPlanUpgradeRequiredError, + SlackAPIRatelimitError, + SlackAPITokenError, + SlackAPIUsergroupNotFoundError, +) from apps.slack.scenarios.scenario_step import ScenarioStep from apps.slack.utils import ( get_cache_key_update_incident_slack_message, @@ -147,7 +154,7 @@ def send_message_to_thread_if_bot_not_in_channel(alert_group_pk, slack_team_iden slack_team_identity = SlackTeamIdentity.objects.get(pk=slack_team_identity_pk) alert_group = AlertGroup.objects.get(pk=alert_group_pk) - sc = SlackClientWithErrorHandling(slack_team_identity.bot_access_token) + sc = SlackClient(slack_team_identity) bot_user_id = slack_team_identity.bot_user_id members = slack_team_identity.get_conversation_members(sc, channel_id) @@ -313,9 +320,7 @@ def post_slack_rate_limit_message(integration_id): def populate_slack_usergroups(): from apps.slack.models import SlackTeamIdentity - slack_team_identities = SlackTeamIdentity.objects.filter( - detected_token_revoked__isnull=True, - ) + slack_team_identities = SlackTeamIdentity.objects.filter(detected_token_revoked__isnull=True) delay = 0 counter = 0 @@ -336,111 +341,46 @@ def populate_slack_usergroups_for_team(slack_team_identity_id): from apps.slack.models import SlackTeamIdentity, SlackUserGroup slack_team_identity = SlackTeamIdentity.objects.get(pk=slack_team_identity_id) - sc = SlackClientWithErrorHandling(slack_team_identity.bot_access_token) + sc = SlackClient(slack_team_identity) - def handle_usergroups_list_slack_api_exception(exception): - if exception.response["error"] == "plan_upgrade_required": - logger.info(f"SlackTeamIdentity with pk {slack_team_identity.pk} does not have access to User Groups") - elif exception.response["error"] == "invalid_auth": - logger.warning(f"invalid_auth, SlackTeamIdentity pk: {slack_team_identity.pk}") - # in some cases slack rate limit error looks like 'rate_limited', in some - 'ratelimited', be aware - elif exception.response["error"] == "rate_limited" or exception.response["error"] == "ratelimited": - delay = random.randint(5, 25) * 60 - logger.warning( - f"'usergroups.list' slack api error: rate_limited. SlackTeamIdentity pk: {slack_team_identity.pk}." - f"Delay populate_slack_usergroups_for_team task by {delay // 60} min." - ) - return populate_slack_usergroups_for_team.apply_async((slack_team_identity_id,), countdown=delay) - elif exception.response["error"] == "missing_scope": - logger.warning( - f"'usergroups.users.list' slack api error: missing_scope. " - f"SlackTeamIdentity pk: {slack_team_identity.pk}.\n{exception}" - ) - return - else: - logger.error( - f"'usergroups.list' slack api error. SlackTeamIdentity pk: {slack_team_identity.pk}\n{exception}" - ) - raise exception - - usergroups_list = None - bot_access_token_accepted = True try: - usergroups_list = sc.usergroups_list() - except SlackAPITokenException as e: - logger.info(f"token revoked\n{e}") - except SlackAPIException as e: - if e.response["error"] == "not_allowed_token_type": - try: - # Trying same request with access token. It is required due to migration to granular permissions - # and can be removed after clients reinstall their bots - sc_with_access_token = SlackClientWithErrorHandling(slack_team_identity.access_token) - usergroups_list = sc_with_access_token.usergroups_list() - bot_access_token_accepted = False - except SlackAPIException as err: - handle_usergroups_list_slack_api_exception(err) - else: - handle_usergroups_list_slack_api_exception(e) - if usergroups_list is not None: - today = timezone.now().date() - populated_user_groups_ids = slack_team_identity.usergroups.filter(last_populated=today).values_list( - "slack_id", flat=True + usergroups = sc.usergroups_list()["usergroups"] + except SlackAPIRatelimitError as e: + populate_slack_usergroups_for_team.apply_async((slack_team_identity_id,), countdown=e.retry_after) + return + except (SlackAPITokenError, SlackAPIInvalidAuthError, SlackAPIPlanUpgradeRequiredError): + return + + today = timezone.now().date() + populated_user_groups_ids = slack_team_identity.usergroups.filter(last_populated=today).values_list( + "slack_id", flat=True + ) + + for usergroup in usergroups: + # skip groups that were recently populated + if usergroup["id"] in populated_user_groups_ids: + continue + + try: + members = sc.usergroups_users_list(usergroup=usergroup["id"])["users"] + except SlackAPIRatelimitError as e: + populate_slack_usergroups_for_team.apply_async((slack_team_identity_id,), countdown=e.retry_after) + return + except (SlackAPIUsergroupNotFoundError, SlackAPIInvalidAuthError): + return + + SlackUserGroup.objects.update_or_create( + slack_id=usergroup["id"], + slack_team_identity=slack_team_identity, + defaults={ + "name": usergroup["name"], + "handle": usergroup["handle"], + "members": members, + "is_active": usergroup["date_delete"] == 0, + "last_populated": today, + }, ) - for usergroup in usergroups_list["usergroups"]: - # skip groups that were recently populated - if usergroup["id"] in populated_user_groups_ids: - continue - try: - if bot_access_token_accepted: - usergroups_users = sc.usergroups_users_list(usergroup=usergroup["id"]) - else: - sc_with_access_token = SlackClientWithErrorHandling(slack_team_identity.access_token) - usergroups_users = sc_with_access_token.usergroups_users_list(usergroup=usergroup["id"]) - except SlackAPIException as e: - if e.response["error"] == "no_such_subteam": - logger.info("User group does not exist") - elif e.response["error"] == "missing_scope": - logger.warning( - f"'usergroups.users.list' slack api error: missing_scope. " - f"SlackTeamIdentity pk: {slack_team_identity.pk}.\n{e}" - ) - return - elif e.response["error"] == "invalid_auth": - logger.warning(f"invalid_auth, SlackTeamIdentity pk: {slack_team_identity.pk}") - # in some cases slack rate limit error looks like 'rate_limited', in some - 'ratelimited', be aware - elif e.response["error"] == "rate_limited" or e.response["error"] == "ratelimited": - delay = random.randint(5, 25) * 60 - logger.warning( - f"'usergroups.users.list' slack api error: rate_limited. " - f"SlackTeamIdentity pk: {slack_team_identity.pk}." - f"Delay populate_slack_usergroups_for_team task by {delay // 60} min." - ) - return populate_slack_usergroups_for_team.apply_async((slack_team_identity_id,), countdown=delay) - else: - logger.error( - f"'usergroups.users.list' slack api error. " - f"SlackTeamIdentity pk: {slack_team_identity.pk}\n{e}" - ) - raise e - else: - usergroup_name = usergroup["name"] - usergroup_handle = usergroup["handle"] - usergroup_members = usergroups_users["users"] - usergroup_is_active = usergroup["date_delete"] == 0 - - SlackUserGroup.objects.update_or_create( - slack_id=usergroup["id"], - slack_team_identity=slack_team_identity, - defaults={ - "name": usergroup_name, - "handle": usergroup_handle, - "members": usergroup_members, - "is_active": usergroup_is_active, - "last_populated": today, - }, - ) - @shared_dedicated_queue_retry_task() def start_update_slack_user_group_for_schedules(): @@ -478,9 +418,7 @@ def update_slack_user_group_for_schedules(user_group_pk): def populate_slack_channels(): from apps.slack.models import SlackTeamIdentity - slack_team_identities = SlackTeamIdentity.objects.filter( - detected_token_revoked__isnull=True, - ) + slack_team_identities = SlackTeamIdentity.objects.filter(detected_token_revoked__isnull=True) delay = 0 counter = 0 @@ -516,7 +454,7 @@ def populate_slack_channels_for_team(slack_team_identity_id: int, cursor: Option from apps.slack.models import SlackChannel, SlackTeamIdentity slack_team_identity = SlackTeamIdentity.objects.get(pk=slack_team_identity_id) - sc = SlackClientWithErrorHandling(slack_team_identity.bot_access_token) + sc = SlackClient(slack_team_identity) active_task_id_key = get_populate_slack_channel_task_id_key(slack_team_identity_id) active_task_id = cache.get(active_task_id_key) @@ -545,21 +483,8 @@ def populate_slack_channels_for_team(slack_team_identity_id: int, cursor: Option limit=1000, cursor=cursor, ) - except SlackAPITokenException as e: - logger.info(f"token revoked\n{e}") - except SlackAPIException as e: - if e.response["error"] == "invalid_auth": - logger.warning( - f"invalid_auth while populating slack channels, SlackTeamIdentity pk: {slack_team_identity.pk}" - ) - elif e.response["error"] == "missing_scope": - logger.warning( - f"conversations.list' slack api error: missing_scope. " - f"SlackTeamIdentity pk: {slack_team_identity.pk}.\n{e}" - ) - else: - logger.error(f"'conversations.list' slack api error. SlackTeamIdentity pk: {slack_team_identity.pk}\n{e}") - raise e + except (SlackAPITokenError, SlackAPIInvalidAuthError): + return else: today = timezone.now().date() diff --git a/engine/apps/slack/test_slack_message.py b/engine/apps/slack/test_slack_message.py new file mode 100644 index 00000000..7067e60f --- /dev/null +++ b/engine/apps/slack/test_slack_message.py @@ -0,0 +1,62 @@ +from unittest.mock import patch + +import pytest + +from apps.slack.client import SlackClient +from apps.slack.errors import SlackAPIError +from apps.slack.tests.conftest import build_slack_response + + +@pytest.fixture +def slack_message_setup( + make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group, make_slack_message +): + def _slack_message_setup(cached_permalink): + ( + 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) + + return make_slack_message(alert_group, cached_permalink=cached_permalink) + + return _slack_message_setup + + +@patch.object( + SlackClient, + "chat_getPermalink", + return_value=build_slack_response({"ok": True, "permalink": "test_permalink"}), +) +@pytest.mark.django_db +def test_slack_message_permalink(mock_slack_api_call, slack_message_setup): + slack_message = slack_message_setup(cached_permalink=None) + assert slack_message.permalink == "test_permalink" + mock_slack_api_call.assert_called_once() + + +@patch.object( + SlackClient, + "chat_getPermalink", + side_effect=SlackAPIError(response=build_slack_response({"ok": False, "error": "message_not_found"})), +) +@pytest.mark.django_db +def test_slack_message_permalink_error(mock_slack_api_call, slack_message_setup): + slack_message = slack_message_setup(cached_permalink=None) + assert slack_message.permalink is None + mock_slack_api_call.assert_called_once() + + +@patch.object( + SlackClient, + "chat_getPermalink", + return_value=build_slack_response({"ok": True, "permalink": "test_permalink"}), +) +@pytest.mark.django_db +def test_slack_message_permalink_cache(mock_slack_api_call, slack_message_setup): + slack_message = slack_message_setup(cached_permalink="cached_permalink") + assert slack_message.permalink == "cached_permalink" + mock_slack_api_call.assert_not_called() diff --git a/engine/apps/slack/tests/test_populate_slack_channels.py b/engine/apps/slack/tests/test_populate_slack_channels.py index f8a2784b..32ee88b9 100644 --- a/engine/apps/slack/tests/test_populate_slack_channels.py +++ b/engine/apps/slack/tests/test_populate_slack_channels.py @@ -3,7 +3,7 @@ from unittest.mock import patch import pytest from django.utils import timezone -from apps.slack.client import SlackClientWithErrorHandling +from apps.slack.client import SlackClient from apps.slack.tasks import populate_slack_channels_for_team @@ -35,9 +35,7 @@ def test_populate_slack_channels_for_team(make_organization_with_slack_team_iden False, ) - with patch.object( - SlackClientWithErrorHandling, "paginated_api_call_with_ratelimit", return_value=(response, cursor, rate_limited) - ): + with patch.object(SlackClient, "paginated_api_call_with_ratelimit", return_value=(response, cursor, rate_limited)): populate_slack_channels_for_team(slack_team_identity.pk) channels = slack_team_identity.cached_channels.all() @@ -110,7 +108,7 @@ def test_populate_slack_channels_for_team_ratelimit( expected_channel_ids = {"C111111111", "C222222222", "C333333333"} with patch.object( - SlackClientWithErrorHandling, + SlackClient, "paginated_api_call_with_ratelimit", return_value=(response_1, cursor_1, rate_limited_1), ): @@ -131,7 +129,7 @@ def test_populate_slack_channels_for_team_ratelimit( assert mocked_start_populate_slack_channels_for_team.call_count == 1 with patch.object( - SlackClientWithErrorHandling, + SlackClient, "paginated_api_call_with_ratelimit", return_value=(response_2, cursor_2, rate_limited_2), ): @@ -152,7 +150,7 @@ def test_populate_slack_channels_for_team_ratelimit( assert mocked_start_populate_slack_channels_for_team.call_count == 2 with patch.object( - SlackClientWithErrorHandling, + SlackClient, "paginated_api_call_with_ratelimit", return_value=(response_3, cursor_3, rate_limited_3), ): 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 18727c3a..bb8a39c5 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 @@ -3,7 +3,7 @@ from unittest.mock import patch import pytest from apps.alerts.models import AlertGroup -from apps.slack.client import SlackAPIException +from apps.slack.errors import SlackAPIRestrictedActionError from apps.slack.models import SlackMessage from apps.slack.scenarios.scenario_step import ScenarioStep from apps.slack.tests.conftest import build_slack_response @@ -25,8 +25,8 @@ def test_restricted_action_error( step = SlackAlertShootingStep(slack_team_identity) with patch.object(step._slack_client, "api_call") as mock_slack_api_call: - mock_slack_api_call.side_effect = SlackAPIException( - "error!", response=build_slack_response({"error": "restricted_action"}) + mock_slack_api_call.side_effect = SlackAPIRestrictedActionError( + response=build_slack_response({"error": "restricted_action"}) ) step._post_alert_group_to_slack(slack_team_identity, alert_group, alert, None, "channel-id", []) diff --git a/engine/apps/slack/tests/test_scenario_steps/test_resolution_note.py b/engine/apps/slack/tests/test_scenario_steps/test_resolution_note.py index 9a5d43f3..7e2b500c 100644 --- a/engine/apps/slack/tests/test_scenario_steps/test_resolution_note.py +++ b/engine/apps/slack/tests/test_scenario_steps/test_resolution_note.py @@ -3,7 +3,8 @@ from unittest.mock import patch import pytest -from apps.slack.client import SlackAPIException, SlackClientWithErrorHandling +from apps.slack.client import SlackClient +from apps.slack.errors import SlackAPIViewNotFoundError from apps.slack.scenarios.scenario_step import ScenarioStep from apps.slack.tests.conftest import build_slack_response from common.api_helpers.utils import create_engine_url @@ -184,9 +185,9 @@ def test_get_resolution_notes_blocks_latest_limit( @pytest.mark.django_db @patch.object( - SlackClientWithErrorHandling, + SlackClient, "api_call", - side_effect=SlackAPIException("error!", response=build_slack_response({"ok": False, "error": "not_found"})), + side_effect=SlackAPIViewNotFoundError(response=build_slack_response({"ok": False, "error": "not_found"})), ) def test_resolution_notes_modal_closed_before_update( mock_slack_api_call, diff --git a/engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py b/engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py index 0c163ac2..2758ca5c 100644 --- a/engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py +++ b/engine/apps/slack/tests/test_scenario_steps/test_slack_channel_integration.py @@ -3,7 +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.errors import SlackAPIMessageNotFoundError from apps.slack.scenarios.slack_channel_integration import SlackChannelMessageEventStep from apps.slack.tests.conftest import build_slack_response @@ -291,7 +291,7 @@ class TestSlackChannelMessageEventStep: step = SlackChannelMessageEventStep(slack_team_identity, organization, user) step._slack_client = Mock() step._slack_client.chat_getPermalink.side_effect = [ - SlackAPIException("error!", response=build_slack_response({"ok": False, "error": "message_not_found"})) + SlackAPIMessageNotFoundError(response=build_slack_response({"ok": False, "error": "message_not_found"})) ] payload = { diff --git a/engine/apps/slack/tests/test_slack_client.py b/engine/apps/slack/tests/test_slack_client.py index e0fdd5eb..75bd73ed 100644 --- a/engine/apps/slack/tests/test_slack_client.py +++ b/engine/apps/slack/tests/test_slack_client.py @@ -1,23 +1,243 @@ import json +from contextlib import suppress from unittest.mock import patch import pytest +from django.utils import timezone +from slack_sdk.web import SlackResponse -from apps.slack.client import SlackAPIRateLimitException, SlackClientWithErrorHandling +from apps.slack.client import SlackClient, server_error_retry_handler +from apps.slack.errors import ( + SlackAPICannotDMBotError, + SlackAPIChannelArchivedError, + SlackAPIChannelInactiveError, + SlackAPIChannelNotFoundError, + SlackAPIError, + SlackAPIFetchMembersFailedError, + SlackAPIInvalidAuthError, + SlackAPIMessageNotFoundError, + SlackAPIMethodNotSupportedForChannelTypeError, + SlackAPIPermissionDeniedError, + SlackAPIPlanUpgradeRequiredError, + SlackAPIRatelimitError, + SlackAPIRestrictedActionError, + SlackAPIServerError, + SlackAPITokenError, + SlackAPIUsergroupNotFoundError, + SlackAPIUserNotFoundError, + SlackAPIViewNotFoundError, +) -@pytest.mark.parametrize("error", ["ratelimited", "rate_limited", "message_limit_exceeded"]) -def test_slack_client_ratelimit(monkeypatch, error): - # undo engine.conftest.mock_slack_api_call - monkeypatch.undo() +@pytest.mark.django_db +@patch( + "slack_sdk.web.base_client.BaseClient._perform_urllib_http_request_internal", + return_value={"status": 200, "body": '{"ok": true}', "headers": {}}, +) +def test_slack_client_ok(mock_request, monkeypatch, make_organization_with_slack_team_identity): + monkeypatch.undo() # undo engine.conftest.mock_slack_api_call - return_value = {"status": 429, "body": json.dumps({"ok": False, "error": error}), "headers": {"Retry-After": "42"}} + _, slack_team_identity = make_organization_with_slack_team_identity() + client = SlackClient(slack_team_identity) + client.api_call("auth.test") + mock_request.assert_called_once() + + +@pytest.mark.parametrize("status", [500, 503, 504]) +@patch.object( + server_error_retry_handler.interval_calculator, + "calculate_sleep_duration", + return_value=0, # speed up the retries +) +@pytest.mark.django_db +def test_slack_client_unexpected_response(_, monkeypatch, status, make_organization_with_slack_team_identity): + monkeypatch.undo() # undo engine.conftest.mock_slack_api_call + + _, slack_team_identity = make_organization_with_slack_team_identity() + client = SlackClient(slack_team_identity) + + return_value = {"status": status, "body": "non-json", "headers": {}} with patch( "slack_sdk.web.base_client.BaseClient._perform_urllib_http_request_internal", return_value=return_value ) as mock_request: - with pytest.raises(SlackAPIRateLimitException) as exc_info: - SlackClientWithErrorHandling().api_call("auth.test") + with pytest.raises(SlackAPIServerError) as exc_info: + client.api_call("auth.test") + assert type(exc_info.value.response) is dict + + assert mock_request.call_count == server_error_retry_handler.max_retry_count + 1 + + +@pytest.mark.parametrize("error", ["internal_error", "fatal_error"]) +@patch.object( + server_error_retry_handler.interval_calculator, + "calculate_sleep_duration", + return_value=0, # speed up the retries +) +@pytest.mark.django_db +def test_slack_client_slack_server_error(_, monkeypatch, error, make_organization_with_slack_team_identity): + monkeypatch.undo() # undo engine.conftest.mock_slack_api_call + + _, slack_team_identity = make_organization_with_slack_team_identity() + client = SlackClient(slack_team_identity) + + return_value = {"status": 200, "body": json.dumps({"ok": False, "error": error}), "headers": {}} + with patch( + "slack_sdk.web.base_client.BaseClient._perform_urllib_http_request_internal", return_value=return_value + ) as mock_request: + with pytest.raises(SlackAPIServerError) as exc_info: + client.api_call("auth.test") + assert type(exc_info.value.response) is dict + + assert mock_request.call_count == server_error_retry_handler.max_retry_count + 1 + + +@pytest.mark.django_db +@patch( + "slack_sdk.web.base_client.BaseClient._perform_urllib_http_request_internal", + return_value={"status": 200, "body": '{"ok": false, "error": "random_error_123"}', "headers": {}}, +) +def test_slack_client_generic_error(mock_request, monkeypatch, make_organization_with_slack_team_identity): + monkeypatch.undo() # undo engine.conftest.mock_slack_api_call + + _, slack_team_identity = make_organization_with_slack_team_identity() + client = SlackClient(slack_team_identity) + + with pytest.raises(SlackAPIError) as exc_info: + client.api_call("auth.test") + assert type(exc_info.value) is SlackAPIError + assert type(exc_info.value.response) is SlackResponse + + mock_request.assert_called_once() + + +@pytest.mark.parametrize( + "error,error_class", + [ + ("account_inactive", SlackAPITokenError), + ("cannot_dm_bot", SlackAPICannotDMBotError), + ("channel_not_found", SlackAPIChannelNotFoundError), + ("fatal_error", SlackAPIServerError), + ("fetch_members_failed", SlackAPIFetchMembersFailedError), + ("internal_error", SlackAPIServerError), + ("invalid_auth", SlackAPIInvalidAuthError), + ("is_archived", SlackAPIChannelArchivedError), + ("is_inactive", SlackAPIChannelInactiveError), + ("message_limit_exceeded", SlackAPIRatelimitError), + ("message_not_found", SlackAPIMessageNotFoundError), + ("method_not_supported_for_channel_type", SlackAPIMethodNotSupportedForChannelTypeError), + ("no_such_subteam", SlackAPIUsergroupNotFoundError), + ("not_found", SlackAPIViewNotFoundError), + ("permission_denied", SlackAPIPermissionDeniedError), + ("plan_upgrade_required", SlackAPIPlanUpgradeRequiredError), + ("rate_limited", SlackAPIRatelimitError), + ("ratelimited", SlackAPIRatelimitError), + ("restricted_action", SlackAPIRestrictedActionError), + ("token_revoked", SlackAPITokenError), + ("user_not_found", SlackAPIUserNotFoundError), + ], +) +@patch.object( + server_error_retry_handler.interval_calculator, + "calculate_sleep_duration", + return_value=0, # speed up the retries if any +) +@pytest.mark.django_db +def test_slack_client_specific_error(_, error, error_class, monkeypatch, make_organization_with_slack_team_identity): + monkeypatch.undo() # undo engine.conftest.mock_slack_api_call + + _, slack_team_identity = make_organization_with_slack_team_identity() + client = SlackClient(slack_team_identity) + + with patch( + "slack_sdk.web.base_client.BaseClient._perform_urllib_http_request_internal", + return_value={"status": 200, "body": json.dumps({"ok": False, "error": error}), "headers": {}}, + ): + with pytest.raises(SlackAPIError) as exc_info: + client.api_call("auth.test") + assert type(exc_info.value) is error_class + assert type(exc_info.value.response) is SlackResponse + + +@pytest.mark.parametrize("error", ["ratelimited", "rate_limited", "message_limit_exceeded"]) +@pytest.mark.django_db +def test_slack_client_ratelimit(monkeypatch, error, make_organization_with_slack_team_identity): + monkeypatch.undo() # undo engine.conftest.mock_slack_api_call + + _, slack_team_identity = make_organization_with_slack_team_identity() + client = SlackClient(slack_team_identity) + + return_value = {"status": 429, "body": json.dumps({"ok": False, "error": error}), "headers": {"Retry-After": "42"}} + with patch( + "slack_sdk.web.base_client.BaseClient._perform_urllib_http_request_internal", return_value=return_value + ) as mock_request: + with pytest.raises(SlackAPIRatelimitError) as exc_info: + client.api_call("auth.test") mock_request.assert_called_once() assert exc_info.value.retry_after == 42 + + +@pytest.mark.parametrize("error", ["account_inactive", "token_revoked"]) +@pytest.mark.django_db +def test_slack_client_mark_token_revoked(error, monkeypatch, make_organization_with_slack_team_identity): + monkeypatch.undo() # undo engine.conftest.mock_slack_api_call + + _, slack_team_identity = make_organization_with_slack_team_identity() + client = SlackClient(slack_team_identity) + assert slack_team_identity.detected_token_revoked is None + + with patch( + "slack_sdk.web.base_client.BaseClient._perform_urllib_http_request_internal", + return_value={"status": 200, "body": json.dumps({"ok": False, "error": error}), "headers": {}}, + ) as mock_request: + with pytest.raises(SlackAPITokenError): + client.api_call("auth.test") + + mock_request.assert_called_once() + slack_team_identity.refresh_from_db() + assert slack_team_identity.detected_token_revoked is not None + + +@pytest.mark.parametrize("error", ["account_inactive", "token_revoked"]) +@pytest.mark.django_db +def test_slack_client_cant_unmark_token_revoked(error, monkeypatch, make_organization_with_slack_team_identity): + monkeypatch.undo() # undo engine.conftest.mock_slack_api_call + + now = timezone.now() + _, slack_team_identity = make_organization_with_slack_team_identity(detected_token_revoked=now) + client = SlackClient(slack_team_identity) + assert slack_team_identity.detected_token_revoked == now + + with patch( + "slack_sdk.web.base_client.BaseClient._perform_urllib_http_request_internal", + return_value={"status": 200, "body": json.dumps({"ok": False, "error": error}), "headers": {}}, + ) as mock_request: + with pytest.raises(SlackAPITokenError): + client.api_call("auth.test") + + mock_request.assert_called_once() + slack_team_identity.refresh_from_db() + assert slack_team_identity.detected_token_revoked == now + + +@pytest.mark.parametrize("body", [{"ok": False, "error": "ratelimited"}, {"ok": True}]) +@pytest.mark.django_db +def test_slack_client_unmark_token_revoked(body, monkeypatch, make_organization_with_slack_team_identity): + monkeypatch.undo() # undo engine.conftest.mock_slack_api_call + + now = timezone.now() + _, slack_team_identity = make_organization_with_slack_team_identity(detected_token_revoked=now) + client = SlackClient(slack_team_identity) + assert slack_team_identity.detected_token_revoked == now + + with patch( + "slack_sdk.web.base_client.BaseClient._perform_urllib_http_request_internal", + return_value={"status": 200, "body": json.dumps(body), "headers": {}}, + ) as mock_request: + with suppress(SlackAPIError): + client.api_call("auth.test") + + mock_request.assert_called_once() + slack_team_identity.refresh_from_db() + assert slack_team_identity.detected_token_revoked is None diff --git a/engine/apps/slack/tests/test_user_group.py b/engine/apps/slack/tests/test_user_group.py index 6626f862..2f4d354d 100644 --- a/engine/apps/slack/tests/test_user_group.py +++ b/engine/apps/slack/tests/test_user_group.py @@ -3,9 +3,14 @@ from unittest.mock import PropertyMock, patch import pytest from apps.schedules.models.on_call_schedule import OnCallScheduleQuerySet, OnCallScheduleWeb -from apps.slack.client import SlackClientWithErrorHandling +from apps.slack.client import SlackClient from apps.slack.models import SlackUserGroup -from apps.slack.tasks import start_update_slack_user_group_for_schedules, update_slack_user_group_for_schedules +from apps.slack.tasks import ( + populate_slack_usergroups_for_team, + start_update_slack_user_group_for_schedules, + update_slack_user_group_for_schedules, +) +from apps.slack.tests.conftest import build_slack_response from apps.user_management.models import Organization @@ -16,7 +21,7 @@ def test_update_members(make_organization_with_slack_team_identity, make_slack_u slack_ids = ["slack_id_1", "slack_id_2"] - with patch.object(SlackClientWithErrorHandling, "api_call") as mock: + with patch.object(SlackClient, "api_call") as mock: user_group.update_members(slack_ids) mock.assert_called() @@ -91,3 +96,61 @@ def test_start_update_slack_user_group_for_schedules_organization_deleted( with patch.object(update_slack_user_group_for_schedules, "delay") as mock: start_update_slack_user_group_for_schedules() mock.assert_not_called() + + +@patch.object( + SlackClient, + "usergroups_users_list", + return_value=build_slack_response({"ok": True, "users": ["test_user_1", "test_user_2"]}), +) +@patch.object( + SlackClient, + "usergroups_list", + return_value=build_slack_response( + { + "ok": True, + "usergroups": [{"id": "test_slack_id", "name": "test_name", "handle": "test_handle", "date_delete": 0}], + } + ), +) +@pytest.mark.django_db +def test_update_or_create_slack_usergroup_from_slack( + mock_usergroups_list, mock_usergroups_users_list, make_organization_with_slack_team_identity +): + organization, slack_team_identity = make_organization_with_slack_team_identity() + SlackUserGroup.update_or_create_slack_usergroup_from_slack("test_slack_id", slack_team_identity) + + usergroup = SlackUserGroup.objects.get() + assert usergroup.name == "test_name" + assert usergroup.handle == "test_handle" + assert usergroup.members == ["test_user_1", "test_user_2"] + assert usergroup.is_active + + +@patch.object( + SlackClient, + "usergroups_users_list", + return_value=build_slack_response({"ok": True, "users": ["test_user_1", "test_user_2"]}), +) +@patch.object( + SlackClient, + "usergroups_list", + return_value=build_slack_response( + { + "ok": True, + "usergroups": [{"id": "test_slack_id", "name": "test_name", "handle": "test_handle", "date_delete": 0}], + } + ), +) +@pytest.mark.django_db +def test_populate_slack_usergroups_for_team( + mock_usergroups_list, mock_usergroups_users_list, make_organization_with_slack_team_identity +): + organization, slack_team_identity = make_organization_with_slack_team_identity() + populate_slack_usergroups_for_team(slack_team_identity.pk) + + usergroup = SlackUserGroup.objects.get() + assert usergroup.name == "test_name" + assert usergroup.handle == "test_handle" + assert usergroup.members == ["test_user_1", "test_user_2"] + assert usergroup.is_active diff --git a/engine/apps/slack/utils.py b/engine/apps/slack/utils.py index ab52746e..0412f09a 100644 --- a/engine/apps/slack/utils.py +++ b/engine/apps/slack/utils.py @@ -2,7 +2,8 @@ import enum import typing from datetime import datetime -from apps.slack.client import SlackAPIException, SlackClientWithErrorHandling +from apps.slack.client import SlackClient +from apps.slack.errors import SlackAPIChannelNotFoundError if typing.TYPE_CHECKING: from apps.user_management.models import Organization @@ -61,15 +62,14 @@ class SlackDateFormat(enum.StrEnum): def post_message_to_channel(organization: "Organization", channel_id: str, text: str) -> None: - if organization.slack_team_identity: - slack_client = SlackClientWithErrorHandling(organization.slack_team_identity.bot_access_token) - try: - slack_client.chat_postMessage(channel=channel_id, text=text) - except SlackAPIException as e: - if e.response["error"] == "channel_not_found": - pass - else: - raise e + if not organization.slack_team_identity: + return + + slack_client = SlackClient(organization.slack_team_identity) + try: + slack_client.chat_postMessage(channel=channel_id, text=text) + except SlackAPIChannelNotFoundError: + pass def _format_datetime_to_slack(timestamp: float, format: str) -> str: diff --git a/engine/apps/slack/views.py b/engine/apps/slack/views.py index fc11fedc..60a2ebee 100644 --- a/engine/apps/slack/views.py +++ b/engine/apps/slack/views.py @@ -15,7 +15,8 @@ from rest_framework.views import APIView from apps.api.permissions import RBACPermission from apps.auth_token.auth import PluginAuthentication from apps.base.utils import live_settings -from apps.slack.client import SlackAPIException, SlackAPITokenException, SlackClientWithErrorHandling +from apps.slack.client import SlackClient +from apps.slack.errors import SlackAPIError from apps.slack.scenarios.alertgroup_appearance import STEPS_ROUTING as ALERTGROUP_APPEARANCE_ROUTING # Importing routes from scenarios @@ -41,6 +42,7 @@ from apps.user_management.models import Organization from common.insight_log import ChatOpsEvent, ChatOpsTypePlug, write_chatops_insight_log from common.oncall_gateway import delete_slack_connector +from .errors import SlackAPITokenError from .models import SlackMessage, SlackTeamIdentity, SlackUserIdentity SCENARIOS_ROUTES: ScenarioRoute.RoutingSteps = [] @@ -189,14 +191,12 @@ class SlackEventApiEndpointView(APIView): logger.info(f"Team {slack_team_identity.slack_id} has no keys, dropping request.") return Response() - sc = SlackClientWithErrorHandling(slack_team_identity.bot_access_token) + sc = SlackClient(slack_team_identity) - if slack_team_identity.detected_token_revoked is not None: - # check if token is still invalid + if slack_team_identity.detected_token_revoked: try: - sc.auth_test(team=slack_team_identity) - except SlackAPITokenException: - logger.info(f"Team {slack_team_identity.slack_id} has revoked token, dropping request.") + sc.auth_test() # check if token is still invalid + except SlackAPITokenError: return Response(status=200) Step = None @@ -504,14 +504,12 @@ class SlackEventApiEndpointView(APIView): step = ScenarioStep(slack_team_identity) try: step.open_warning_window(payload, warning_text) - except SlackAPIException as e: + except SlackAPIError as e: logger.info( f"Failed to open pop-up for unpopulated SlackTeamIdentity {slack_team_identity.pk}\n" f"Error: {e}" ) - def _open_warning_for_unconnected_user( - self, slack_client: SlackClientWithErrorHandling, payload: EventPayload - ) -> None: + def _open_warning_for_unconnected_user(self, slack_client: SlackClient, payload: EventPayload) -> None: if payload.get("trigger_id") is None: return diff --git a/engine/conftest.py b/engine/conftest.py index 3ffaed41..aca32ffe 100644 --- a/engine/conftest.py +++ b/engine/conftest.py @@ -69,7 +69,7 @@ from apps.schedules.tests.factories import ( OnCallScheduleICalFactory, ShiftSwapRequestFactory, ) -from apps.slack.client import SlackClientWithErrorHandling +from apps.slack.client import SlackClient from apps.slack.tests.factories import ( SlackChannelFactory, SlackMessageFactory, @@ -144,7 +144,7 @@ def mock_slack_api_call(monkeypatch): "team": {"name": "TEST_TEAM"}, } - monkeypatch.setattr(SlackClientWithErrorHandling, "api_call", mock_api_call) + monkeypatch.setattr(SlackClient, "api_call", mock_api_call) @pytest.fixture(autouse=True)