fix: retry Telegram message without parse_mode on Markdown parsing failure#2073
fix: retry Telegram message without parse_mode on Markdown parsing failure#2073dashitongzhi wants to merge 2 commits into
Conversation
…ilure When alert text contains unescaped Markdown special characters (e.g. underscores in Kubernetes pod names like 'crowdsec-agent_k8vkt'), the Telegram API rejects the message with a 'can't parse entities' error. This change adds a fallback mechanism: if sending with parse_mode='Markdown' fails, the message is retried without any parse_mode so it is delivered as plain text rather than being silently dropped. Fixes robusta-dev#1982
WalkthroughThe Telegram sink message sending logic now handles parsing failures gracefully by falling back to plain text mode. When the initial POST request with ChangesTelegram message send retry fallback
🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 652ef4849e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # Retry without parse_mode to handle messages with unescaped Markdown characters | ||
| message_json.pop("parse_mode", None) | ||
| response = requests.post(url, json=message_json) |
There was a problem hiding this comment.
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 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@src/robusta/core/sinks/telegram/telegram_client.py`:
- 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).
- Around line 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.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 951275ea-8b80-4a25-aec4-a3de7451268f
📒 Files selected for processing (1)
src/robusta/core/sinks/telegram/telegram_client.py
| 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}" | ||
| ) |
There was a problem hiding this comment.
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.
| 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.
| ) | ||
| # Retry without parse_mode to handle messages with unescaped Markdown characters | ||
| message_json.pop("parse_mode", None) | ||
| response = requests.post(url, json=message_json) |
There was a problem hiding this comment.
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.
| 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).
There was a problem hiding this comment.
Pull request overview
Adds a fallback delivery path for the Telegram sink so that messages that fail Telegram’s Markdown parsing (e.g., due to unescaped underscores) are retried as plain text, improving notification reliability and addressing #1982.
Changes:
- Log a warning when a Markdown-formatted Telegram message fails to send.
- Retry the same message without
parse_modeto bypass Markdown parsing failures. - Only log an error if the plain-text retry also fails.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 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: |
| 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) |
|
@cla-bot recheck |
|
Hi! 👋 This PR has passed all CI checks and looks ready for review. Could a maintainer please take a look when they have a chance? Thank you! |
Problem
The Telegram sink uses
parse_mode: "Markdown"but does not escape special Markdown characters in message content. When alert text contains underscores (common in Kubernetes pod names likecrowdsec-agent_k8vkt), Telegram interprets them as italic markers and fails to parse the message, silently dropping the notification.Fixes #1982
Fix
This change adds a fallback mechanism in
TelegramClient.send_message(): if sending withparse_mode="Markdown"fails (e.g. due to unescaped special characters), the message is automatically retried without any parse_mode, ensuring it gets delivered as plain text rather than being silently dropped.The fix:
parse_modeFiles modified
src/robusta/core/sinks/telegram/telegram_client.py- Added retry logic