From c58dbf8b9c7c93f5c15b99557a2ec0c6294201e7 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Tue, 17 Jan 2023 13:30:51 -0800 Subject: [PATCH] fix: pass in slack error messages properly (#22727) --- superset/reports/notifications/slack.py | 14 +++++++++----- .../integration_tests/reports/commands_tests.py | 17 +++++++++++++++-- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/superset/reports/notifications/slack.py b/superset/reports/notifications/slack.py index 41ccad392a1f4..91470c5b02736 100644 --- a/superset/reports/notifications/slack.py +++ b/superset/reports/notifications/slack.py @@ -192,10 +192,14 @@ def send(self) -> None: SlackRequestError, SlackClientConfigurationError, ) as ex: - raise NotificationParamException from ex + raise NotificationParamException(str(ex)) from ex except SlackObjectFormationError as ex: - raise NotificationMalformedException from ex + raise NotificationMalformedException(str(ex)) from ex except SlackTokenRotationError as ex: - raise NotificationAuthorizationException from ex - except (SlackClientNotConnectedError, SlackClientError, SlackApiError) as ex: - raise NotificationUnprocessableException from ex + raise NotificationAuthorizationException(str(ex)) from ex + except (SlackClientNotConnectedError, SlackApiError) as ex: + raise NotificationUnprocessableException(str(ex)) from ex + except SlackClientError as ex: + # this is the base class for all slack client errors + # keep it last so that it doesn't interfere with @backoff + raise NotificationUnprocessableException(str(ex)) from ex diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py index 49f5b73b90e49..eb606ffec3ee8 100644 --- a/tests/integration_tests/reports/commands_tests.py +++ b/tests/integration_tests/reports/commands_tests.py @@ -1208,8 +1208,21 @@ def test_slack_chart_report_schedule_with_errors( ).run() db.session.commit() - # Assert errors are being logged) - assert get_error_logs_query(create_report_slack_chart).count() == (idx + 1) * 2 + + # Assert errors are being logged + + # Only one notification log is sent because it's in grace period + # for the rest of the reports + notification_logs_count = get_notification_error_sent_count( + create_report_slack_chart + ) + error_logs = get_error_logs_query(create_report_slack_chart) + + # check that we have two logs for each error + assert error_logs.count() == (len(slack_errors) + notification_logs_count) * 2 + + # check that each error has a message + assert len([log.error_message for log in error_logs]) == error_logs.count() @pytest.mark.usefixtures(