fix apps.telegram.tasks.send_log_and_actions_message retrying tasks (#4851)

# What this PR does

It _appears_ like Telegram may have changed one of the error messages
they return for `telegram.error.BadRequest`. This _may_ be causing us to
infinitely retry some of these tasks.

Previously we were checking for two variants of the same type of error
message:
- "Message to reply not found"
- "Replied message not found"

_However_, if I search for the following [in the
logs](https://ops.grafana-ops.net/goto/hMgBb8CSR?orgId=1):
```logql
{namespace="amixr-prod"} |~ `(Message to be replied not found|Message to reply not found|Replied message not found)`
````
I _only_ see references to "Message to be replied not found". I have
updated references to the former to this new error log message we are
seeing.

Also:
- deduplicate some of the words we check for in
`telegram.error.BadRequest` and `telegram.error.Unauthorized` into
`apps.telegram.client.TelegramClient.BadRequestMessage` and
`apps.telegram.client.TelegramClient.UnauthorizedMessage` respectively
- deduplicate some of the wording we use in the `reason` arg passed to
`TelegramToUserConnector.create_telegram_notification_error` into
`apps.telegram.models.connectors.personal.TelegramToUserConnector.NotificationErrorReason`
- standardize how we check the `message` attribute of
`telegram.error.TelegramError`s into a new `error_message_is` static
method on `apps.telegram.client.TelegramClient`
- previously we would check these error messages in two different ways:
  ```python3
  # style 1
  if "error message to check" in e.message:
    # do something

  # style 2
  if error.message == "error message to check":
    # do something
  ```

## Which issue(s) this PR closes

Closes https://github.com/grafana/oncall-private/issues/2868

## Checklist

- [x] Unit, integration, and e2e (if applicable) tests updated
- [x] Documentation added (or `pr:no public docs` PR label added if not
required)
- [x] Added the relevant release notes label (see labels prefixed w/
`release:`). These labels dictate how your PR will
    show up in the autogenerated release notes.

.nvmrc

@@ -0,0 +1 @@
+20.16.0

engine/apps/telegram/client.py

@@ -3,7 +3,7 @@ from typing import Optional, Tuple, Union
 
 from django.conf import settings
 from telegram import Bot, InlineKeyboardMarkup, Message, ParseMode
-from telegram.error import BadRequest, InvalidToken, Unauthorized
+from telegram.error import BadRequest, InvalidToken, TelegramError, Unauthorized
 from telegram.utils.request import Request
 
 from apps.alerts.models import AlertGroup
@@ -27,6 +27,18 @@ class TelegramClient:
         if self.token is None:
             raise InvalidToken()
 
+    class BadRequestMessage:
+        CHAT_NOT_FOUND = "Chat not found"
+        MESSAGE_IS_NOT_MODIFIED = "Message is not modified"
+        MESSAGE_TO_EDIT_NOT_FOUND = "Message to edit not found"
+        NEED_ADMIN_RIGHTS_IN_THE_CHANNEL = "Need administrator rights in the channel chat"
+        MESSAGE_TO_BE_REPLIED_NOT_FOUND = "Message to be replied not found"
+
+    class UnauthorizedMessage:
+        BOT_WAS_BLOCKED_BY_USER = "Forbidden: bot was blocked by the user"
+        INVALID_TOKEN = "Invalid token"
+        USER_IS_DEACTIVATED = "Forbidden: user is deactivated"
+
     @property
     def api_client(self) -> Bot:
         return Bot(self.token, request=Request(read_timeout=15))
@@ -96,7 +108,7 @@ class TelegramClient:
                 disable_web_page_preview=False,
             )
         except BadRequest as e:
-            logger.warning("Telegram BadRequest: {}".format(e.message))
+            logger.warning(f"Telegram BadRequest: {e.message}")
             raise
 
         return message
@@ -171,3 +183,7 @@ class TelegramClient:
             raise Exception(f"_get_message_and_keyboard with type {message_type} is not implemented")
 
         return text, keyboard
+
+    @staticmethod
+    def error_message_is(error: TelegramError, messages: list[str]) -> bool:
+        return error.message in messages

engine/apps/telegram/decorators.py

@@ -42,7 +42,7 @@ def ignore_message_unchanged(f):
         try:
             return f(*args, **kwargs)
         except error.BadRequest as e:
-            if "Message is not modified" in e.message:
+            if TelegramClient.error_message_is(e, [TelegramClient.BadRequestMessage.MESSAGE_IS_NOT_MODIFIED]):
                 logger.warning(
                     f"Tried to change Telegram message, but update is identical to original message. "
                     f"args: {args}, kwargs: {kwargs}"
@@ -59,7 +59,7 @@ def ignore_message_to_edit_deleted(f):
         try:
             return f(*args, **kwargs)
         except error.BadRequest as e:
-            if "Message to edit not found" in e.message:
+            if TelegramClient.error_message_is(e, [TelegramClient.BadRequestMessage.MESSAGE_TO_EDIT_NOT_FOUND]):
                 logger.warning(
                     f"Tried to edit Telegram message, but message was deleted. args: {args}, kwargs: {kwargs}"
                 )
@@ -75,7 +75,7 @@ def ignore_reply_to_message_deleted(f):
         try:
             return f(*args, **kwargs)
         except error.BadRequest as e:
-            if "Replied message not found" in e.message:
+            if TelegramClient.error_message_is(e, [TelegramClient.BadRequestMessage.MESSAGE_TO_BE_REPLIED_NOT_FOUND]):
                 logger.warning(
                     f"Tried to reply to Telegram message, but message was deleted. args: {args}, kwargs: {kwargs}"
                 )

engine/apps/telegram/models/connectors/channel.py

@@ -123,15 +123,16 @@ class TelegramToOrganizationConnector(models.Model):
                 chat_id=self.channel_chat_id, message_type=TelegramMessage.ALERT_GROUP_MESSAGE, alert_group=alert_group
             )
         except error.BadRequest as e:
-            if e.message == "Need administrator rights in the channel chat":
+            if TelegramClient.error_message_is(
+                e,
+                [
+                    TelegramClient.BadRequestMessage.NEED_ADMIN_RIGHTS_IN_THE_CHANNEL,
+                    TelegramClient.BadRequestMessage.CHAT_NOT_FOUND,
+                ],
+            ):
                 logger.warning(
                     f"Could not send alert group to Telegram channel with id {self.channel_chat_id} "
-                    f"due to lack of admin rights. alert_group {alert_group.pk}"
+                    f"due to {e.message}. alert_group {alert_group.pk}"
-                )
-            elif e.message == "Chat not found":
-                logger.warning(
-                    f"Could not send alert group to Telegram channel with id {self.channel_chat_id} "
-                    f"due to 'Chat not found'. alert_group {alert_group.pk}"
                 )
             else:
                 telegram_client.send_message(

engine/apps/telegram/models/connectors/personal.py

@@ -25,6 +25,14 @@ class TelegramToUserConnector(models.Model):
     class Meta:
         unique_together = (("user", "telegram_chat_id"),)
 
+    class NotificationErrorReason:
+        USER_WAS_DISABLED = "Telegram user was disabled"
+        INVALID_TOKEN = "Invalid token"
+        BOT_BLOCKED_BY_USER = "Bot was blocked by the user"
+        FORMATTING_ERROR_IN_RENDERED_TEMPLATE = (
+            "Notification sent but there was a formatting error in the rendered template"
+        )
+
     @classmethod
     def notify_user(cls, user: User, alert_group: AlertGroup, notification_policy: UserNotificationPolicy) -> None:
         try:
@@ -98,7 +106,7 @@ class TelegramToUserConnector(models.Model):
                 self.user,
                 notification_policy,
                 UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_TOKEN_ERROR,
-                reason="Invalid token",
+                reason=self.NotificationErrorReason.INVALID_TOKEN,
             )
             return
 
@@ -123,7 +131,7 @@ class TelegramToUserConnector(models.Model):
                     self.user,
                     notification_policy,
                     UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_FORMATTING_ERROR,
-                    reason="Notification sent but there was a formatting error in the rendered template",
+                    reason=self.NotificationErrorReason.FORMATTING_ERROR_IN_RENDERED_TEMPLATE,
                 )
                 telegram_client.send_message(
                     chat_id=self.telegram_chat_id,
@@ -131,29 +139,29 @@ class TelegramToUserConnector(models.Model):
                     alert_group=alert_group,
                 )
             except error.Unauthorized as e:
-                if e.message == "Forbidden: bot was blocked by the user":
+                if TelegramClient.error_message_is(e, [TelegramClient.UnauthorizedMessage.BOT_WAS_BLOCKED_BY_USER]):
                     TelegramToUserConnector.create_telegram_notification_error(
                         alert_group,
                         self.user,
                         notification_policy,
                         UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_BOT_IS_DELETED,
-                        reason="Bot was blocked by the user",
+                        reason=self.NotificationErrorReason.BOT_BLOCKED_BY_USER,
                     )
-                elif e.message == "Invalid token":
+                elif TelegramClient.error_message_is(e, [TelegramClient.UnauthorizedMessage.INVALID_TOKEN]):
                     TelegramToUserConnector.create_telegram_notification_error(
                         alert_group,
                         self.user,
                         notification_policy,
                         UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_TOKEN_ERROR,
-                        reason="Invalid token",
+                        reason=self.NotificationErrorReason.INVALID_TOKEN,
                     )
-                elif e.message == "Forbidden: user is deactivated":
+                elif TelegramClient.error_message_is(e, [TelegramClient.UnauthorizedMessage.USER_IS_DEACTIVATED]):
                     TelegramToUserConnector.create_telegram_notification_error(
                         alert_group,
                         self.user,
                         notification_policy,
                         UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_USER_IS_DEACTIVATED,
-                        reason="Telegram user was disabled",
+                        reason=self.NotificationErrorReason.USER_WAS_DISABLED,
                     )
                 else:
                     raise e
@@ -175,7 +183,7 @@ class TelegramToUserConnector(models.Model):
                 self.user,
                 notification_policy,
                 UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_TOKEN_ERROR,
-                reason="Invalid token",
+                reason=self.NotificationErrorReason.INVALID_TOKEN,
             )
             return
 
@@ -193,29 +201,29 @@ class TelegramToUserConnector(models.Model):
                 alert_group=alert_group,
             )
         except error.Unauthorized as e:
-            if e.message == "Forbidden: bot was blocked by the user":
+            if TelegramClient.error_message_is(e, [TelegramClient.UnauthorizedMessage.BOT_WAS_BLOCKED_BY_USER]):
                 TelegramToUserConnector.create_telegram_notification_error(
                     alert_group,
                     self.user,
                     notification_policy,
                     UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_BOT_IS_DELETED,
-                    reason="Bot was blocked by the user",
+                    reason=self.NotificationErrorReason.BOT_BLOCKED_BY_USER,
                 )
-            elif e.message == "Invalid token":
+            elif TelegramClient.error_message_is(e, [TelegramClient.UnauthorizedMessage.INVALID_TOKEN]):
                 TelegramToUserConnector.create_telegram_notification_error(
                     alert_group,
                     self.user,
                     notification_policy,
                     UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_TOKEN_ERROR,
-                    reason="Invalid token",
+                    reason=self.NotificationErrorReason.INVALID_TOKEN,
                 )
-            elif e.message == "Forbidden: user is deactivated":
+            elif TelegramClient.error_message_is(e, [TelegramClient.UnauthorizedMessage.USER_IS_DEACTIVATED]):
                 TelegramToUserConnector.create_telegram_notification_error(
                     alert_group,
                     self.user,
                     notification_policy,
                     UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_USER_IS_DEACTIVATED,
-                    reason="Telegram user was disabled",
+                    reason=self.NotificationErrorReason.USER_WAS_DISABLED,
                 )
             else:
                 raise e

engine/apps/telegram/tasks.py

@@ -66,7 +66,7 @@ def edit_message(self, message_pk):
     try:
         telegram_client.edit_message(message=message)
     except error.BadRequest as e:
-        if "Message is not modified" in e.message:
+        if TelegramClient.error_message_is(e, [TelegramClient.BadRequestMessage.MESSAGE_IS_NOT_MODIFIED]):
             pass
     except (error.RetryAfter, error.TimedOut) as e:
         countdown = getattr(e, "retry_after", 3)
@@ -165,19 +165,18 @@ def send_log_and_actions_message(self, channel_chat_id, group_chat_id, channel_m
                         reply_to_message_id=reply_to_message_id,
                     )
             except error.BadRequest as e:
-                if e.message == "Chat not found":
+                if TelegramClient.error_message_is(
+                    e,
+                    [
+                        TelegramClient.BadRequestMessage.CHAT_NOT_FOUND,
+                        TelegramClient.BadRequestMessage.MESSAGE_TO_BE_REPLIED_NOT_FOUND,
+                    ],
+                ):
                     logger.warning(
                         f"Could not send log and actions messages to Telegram group with id {group_chat_id} "
-                        f"due to 'Chat not found'. alert_group {alert_group.pk}"
+                        f"due to '{e.message}'. alert_group {alert_group.pk}"
                     )
                     return
-                elif e.message == "Message to reply not found":
-                    logger.warning(
-                        f"Could not send log and actions messages to Telegram group with id {group_chat_id} "
-                        f"due to 'Message to reply not found'. alert_group {alert_group.pk}"
-                    )
-                    return
-                else:
                 raise
 
 

engine/apps/telegram/tests/test_personal_connector.py

@@ -8,7 +8,11 @@ from apps.telegram.client import TelegramClient
 from apps.telegram.models import TelegramMessage, TelegramToUserConnector
 
 
-@patch.object(TelegramClient, "send_raw_message", side_effect=error.BadRequest("Replied message not found"))
+@patch.object(
+    TelegramClient,
+    "send_raw_message",
+    side_effect=error.BadRequest(TelegramClient.BadRequestMessage.MESSAGE_TO_BE_REPLIED_NOT_FOUND),
+)
 @pytest.mark.django_db
 def test_personal_connector_replied_message_not_found(
     mock_send_message,
@@ -53,19 +57,19 @@ def test_personal_connector_replied_message_not_found(
     "side_effect,notification_error_code,reason",
     [
         (
-            error.Unauthorized("Forbidden: bot was blocked by the user"),
+            error.Unauthorized(TelegramClient.UnauthorizedMessage.BOT_WAS_BLOCKED_BY_USER),
             UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_BOT_IS_DELETED,
-            "Bot was blocked by the user",
+            TelegramToUserConnector.NotificationErrorReason.BOT_BLOCKED_BY_USER,
         ),
         (
-            error.Unauthorized("Invalid token"),
+            error.Unauthorized(TelegramClient.UnauthorizedMessage.INVALID_TOKEN),
             UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_TOKEN_ERROR,
-            "Invalid token",
+            TelegramToUserConnector.NotificationErrorReason.INVALID_TOKEN,
         ),
         (
-            error.Unauthorized("Forbidden: user is deactivated"),
+            error.Unauthorized(TelegramClient.UnauthorizedMessage.USER_IS_DEACTIVATED),
             UserNotificationPolicyLogRecord.ERROR_NOTIFICATION_TELEGRAM_USER_IS_DEACTIVATED,
-            "Telegram user was disabled",
+            TelegramToUserConnector.NotificationErrorReason.USER_WAS_DISABLED,
         ),
     ],
 )

engine/apps/telegram/tests/test_tasks.py

@@ -7,8 +7,10 @@ from apps.telegram.client import TelegramClient
 from apps.telegram.models import TelegramMessage
 from apps.telegram.tasks import send_log_and_actions_message
 
+bad_request_error_msg = TelegramClient.BadRequestMessage.MESSAGE_TO_BE_REPLIED_NOT_FOUND
 
-@patch.object(TelegramClient, "send_raw_message", side_effect=error.BadRequest("Message to reply not found"))
+
+@patch.object(TelegramClient, "send_raw_message", side_effect=error.BadRequest(bad_request_error_msg))
 @pytest.mark.django_db
 def test_send_log_and_actions_replied_message_not_found(
     mock_send_message,
@@ -42,6 +44,6 @@ def test_send_log_and_actions_replied_message_not_found(
 
     expected_msg = (
         f"Could not send log and actions messages to Telegram group with id group_chat_id "
-        f"due to 'Message to reply not found'. alert_group {alert_group.pk}"
+        f"due to '{bad_request_error_msg}'. alert_group {alert_group.pk}"
     )
     assert expected_msg in caplog.text

grafana-plugin/.nvmrc

@@ -1 +0,0 @@
-20.x