Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/robusta/core/sinks/telegram/telegram_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,18 @@ def send_message(self, message: str, disable_links_preview: bool = True):
response = requests.post(url, json=message_json)

if response.status_code != 200:
logging.error(
f"Failed to send telegram message: chat_id - {self.chat_id} reason - {response.reason} {response.text}"
logging.warning(
f"Failed to send telegram message with Markdown parse_mode: chat_id - {self.chat_id} "
f"reason - {response.reason} {response.text}. Retrying without parse_mode."
)
# Retry without parse_mode to handle messages with unescaped Markdown characters
message_json.pop("parse_mode", None)
response = requests.post(url, json=message_json)
Comment on lines +34 to +36
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Retry only Markdown parse failures

The new fallback retries on every non-200 response, not just Markdown parsing errors. That means rate-limit (429) and transient/server failures will immediately trigger a second sendMessage call with identical payload except parse_mode, which can worsen throttling and increase dropped/duplicated alert risk under load; previously those cases made only one request. Gate this retry to parse-related failures (e.g., 400 with parse-entity error text) instead of unconditional non-200 responses.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Missing timeout on the retry requests.post call.

Ruff S113 flags this. A hanging network call on the retry path can block the calling thread indefinitely. The original call on line 27 is also missing a timeout (pre-existing), but since line 36 is new code this should be addressed here. Consider adding a consistent timeout to both calls.

⏱️ Proposed fix
-            response = requests.post(url, json=message_json)
+            response = requests.post(url, json=message_json, timeout=10)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
response = requests.post(url, json=message_json)
response = requests.post(url, json=message_json, timeout=10)
🧰 Tools
🪛 Ruff (0.15.12)

[error] 36-36: Probable use of requests call without timeout

(S113)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/robusta/core/sinks/telegram/telegram_client.py` at line 36, The retry
HTTP POST call currently uses requests.post(url, json=message_json) without a
timeout which can hang; update the retry in telegram_client.py (the line
assigning response = requests.post(...)) to pass a sensible timeout parameter
(e.g., timeout=TIMEOUT_SECONDS or a constant) and apply the same timeout
consistently to the initial send call as well so both the first attempt and the
retry use the same timeout value; ensure the timeout constant is defined near
the module or function scope and referenced by both calls (identify the calls by
the assignment response = requests.post(...) within the send/send_with_retry
logic).

Comment on lines 29 to +36

if response.status_code != 200:
Comment on lines +30 to +38
logging.error(
f"Failed to send telegram message: chat_id - {self.chat_id} reason - {response.reason} {response.text}"
)
Comment on lines 29 to +41
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Retry fires on all non-200 responses, not just Markdown parse failures.

The current condition retries without parse_mode for any non-200 response (429 Too Many Requests, 500 Internal Server Error, etc.). For those cases, stripping parse_mode won't help and the warning message ("Failed to send telegram message with Markdown parse_mode") will be misleading. The fix should be scoped to the specific Telegram error that indicates a Markdown parsing failure (HTTP 400 with "can't parse entities" in the response body).

🛡️ Proposed fix to scope the retry to Markdown parse failures
-        if response.status_code != 200:
-            logging.warning(
-                f"Failed to send telegram message with Markdown parse_mode: chat_id - {self.chat_id} "
-                f"reason - {response.reason} {response.text}. Retrying without parse_mode."
-            )
-            # Retry without parse_mode to handle messages with unescaped Markdown characters
-            message_json.pop("parse_mode", None)
-            response = requests.post(url, json=message_json)
-
-            if response.status_code != 200:
-                logging.error(
-                    f"Failed to send telegram message: chat_id - {self.chat_id} reason - {response.reason} {response.text}"
-                )
+        if response.status_code != 200:
+            if response.status_code == 400 and "can't parse entities" in response.text.lower():
+                logging.warning(
+                    f"Failed to send telegram message with Markdown parse_mode: chat_id - {self.chat_id} "
+                    f"reason - {response.reason} {response.text}. Retrying without parse_mode."
+                )
+                # Retry without parse_mode to handle messages with unescaped Markdown characters
+                message_json.pop("parse_mode", None)
+                response = requests.post(url, json=message_json)
+                if response.status_code != 200:
+                    logging.error(
+                        f"Failed to send telegram message: chat_id - {self.chat_id} reason - {response.reason} {response.text}"
+                    )
+            else:
+                logging.error(
+                    f"Failed to send telegram message: chat_id - {self.chat_id} reason - {response.reason} {response.text}"
+                )
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if response.status_code != 200:
logging.error(
f"Failed to send telegram message: chat_id - {self.chat_id} reason - {response.reason} {response.text}"
logging.warning(
f"Failed to send telegram message with Markdown parse_mode: chat_id - {self.chat_id} "
f"reason - {response.reason} {response.text}. Retrying without parse_mode."
)
# Retry without parse_mode to handle messages with unescaped Markdown characters
message_json.pop("parse_mode", None)
response = requests.post(url, json=message_json)
if response.status_code != 200:
logging.error(
f"Failed to send telegram message: chat_id - {self.chat_id} reason - {response.reason} {response.text}"
)
if response.status_code != 200:
if response.status_code == 400 and "can't parse entities" in response.text.lower():
logging.warning(
f"Failed to send telegram message with Markdown parse_mode: chat_id - {self.chat_id} "
f"reason - {response.reason} {response.text}. Retrying without parse_mode."
)
# Retry without parse_mode to handle messages with unescaped Markdown characters
message_json.pop("parse_mode", None)
response = requests.post(url, json=message_json)
if response.status_code != 200:
logging.error(
f"Failed to send telegram message: chat_id - {self.chat_id} reason - {response.reason} {response.text}"
)
else:
logging.error(
f"Failed to send telegram message: chat_id - {self.chat_id} reason - {response.reason} {response.text}"
)
🧰 Tools
🪛 Ruff (0.15.12)

[error] 36-36: Probable use of requests call without timeout

(S113)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/robusta/core/sinks/telegram/telegram_client.py` around lines 29 - 41, The
retry logic currently triggers on any non-200 response; change it to only retry
when Telegram returns a 400 Bad Request with a body indicating a markdown parse
error (e.g., the response.text contains "can't parse entities" or similar).
Inside the method where requests.post is called (use the existing response,
message_json, parse_mode, self.chat_id, requests.post symbols), check if
response.status_code == 400 and "can't parse entities" in response.text
(case-insensitive) before popping message_json["parse_mode"] and re-posting;
otherwise log the actual failure and do not attempt the parse_mode retry. Ensure
the warning message reflects that the retry is specific to markdown parse
failures and keep the final logging/error path unchanged for other status codes.


def send_file(self, file_name: str, contents: bytes):
file_type = "Photo" if is_image(file_name) else "Document"
Expand Down
Loading