Skip to content

fix(telegram): check res.ok for Markdown fallback in sendMessage#1152

Closed
BrianYang-123 wants to merge 2 commits intoNVIDIA:mainfrom
BrianYang-123:fix/telegram-sendmessage-fallback
Closed

fix(telegram): check res.ok for Markdown fallback in sendMessage#1152
BrianYang-123 wants to merge 2 commits intoNVIDIA:mainfrom
BrianYang-123:fix/telegram-sendmessage-fallback

Conversation

@BrianYang-123
Copy link
Copy Markdown

@BrianYang-123 BrianYang-123 commented Mar 31, 2026

Summary

Fix silent message loss when LLM output contains invalid Markdown.

Problem

tgApi() resolves with {ok: false} on Telegram API errors instead of rejecting the promise. The existing .catch() fallback for Markdown parse failures therefore never triggers, causing messages to be silently dropped with no error logged.

This affects all Telegram bridge users whenever the LLM outputs unbalanced Markdown entities (*, _, etc.), which is common with local models. Telegram returns:

{"ok": false, "error_code": 400, "description": "Bad Request: can't parse entities: Can't find end of the entity starting at byte offset 1883"}

But since tgApi() resolves (not rejects), .catch() is skipped and the plain-text retry never executes.

Changes

  • scripts/telegram-bridge.js: Check res.ok explicitly instead of relying on .catch() for the Markdown-to-plain-text fallback.

Test plan

  • Send message triggering LLM response with broken Markdown (unmatched *)
  • Before fix: message silently lost, no error in logs
  • After fix: Markdown attempt fails → automatic plain-text retry → message delivered
  • Normal messages with valid Markdown still render correctly

Summary by CodeRabbit

  • Bug Fixes
    • Improved message resend behavior for formatting errors: when a message fails due to Markdown parsing, it is now retried automatically as plain text. Other delivery failures no longer trigger automatic retries, reducing duplicate attempts and improving overall messaging reliability.

Signed-off-by: brianyang brianyang@signalpro.com.tw

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 31, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4fdc9a0f-64e9-4d18-acb5-51e930995636

📥 Commits

Reviewing files that changed from the base of the PR and between 1a0eebb and a866579.

📒 Files selected for processing (1)
  • scripts/telegram-bridge.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • scripts/telegram-bridge.js

📝 Walkthrough

Walkthrough

sendMessage now awaits the Telegram API response and inspects it. It first sends with parse_mode: "Markdown"; if the API returns error_code === 400 with a description matching /can't parse entities/i it retries once without parse_mode. Other { ok: false } responses are returned unchanged.

Changes

Cohort / File(s) Summary
Telegram message logic
scripts/telegram-bridge.js
Reworked sendMessage to await the Telegram API response and conditionally retry without parse_mode only when error_code === 400 and description matches "can't parse entities" (case-insensitive). Removed unconditional .catch()-based fallback retry.

Sequence Diagram

sequenceDiagram
    actor Caller
    participant sendMessage
    participant TelegramAPI

    Caller->>sendMessage: sendMessage(chatId, text, replyTo)
    sendMessage->>TelegramAPI: POST /sendMessage (parse_mode: "Markdown")
    TelegramAPI-->>sendMessage: { ok: true, result: ... } or { ok: false, error_code, description }
    alt ok === true
        sendMessage-->>Caller: return result
    else ok === false
        alt error_code == 400 and description =~ /can't parse entities/i
            sendMessage->>TelegramAPI: POST /sendMessage (no parse_mode) [retry]
            TelegramAPI-->>sendMessage: { ok: true/false, result/description }
            sendMessage-->>Caller: return final response
        else
            sendMessage-->>Caller: return original { ok: false, description }
        end
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

A rabbit taps the Telegram stream,
Markdown trips, I flip the scheme,
One cautious retry, plain and bright,
Words now hop into the light. 🐇✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately summarizes the main change: adding a check for res.ok to implement a Markdown fallback mechanism in sendMessage, which directly addresses the PR's core objective of fixing silent message loss.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@scripts/telegram-bridge.js`:
- Around line 83-95: The send/retry logic around tgApi("sendMessage") should
only retry when Telegram explicitly rejects Markdown parsing: instead of
unconditionally treating any !res.ok as retryable, call tgApi for sendMessage
and inspect the response/error — if you receive an HTTP 400 with a description
containing "can't parse entities" (or similar markdown parse message) then retry
the call without parse_mode; for any other non-OK response or a
transport/network exception, rethrow or propagate the original error (do not
convert every catch into { ok: false }). Update the code paths around tgApi,
res, chunk, replyTo and chatId to perform targeted checking and only fall back
for the markdown-parse case while surfacing other errors.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ddb87539-6fa5-4e57-9b06-350a3f1d0f22

📥 Commits

Reviewing files that changed from the base of the PR and between 36f4456 and 6409583.

📒 Files selected for processing (1)
  • scripts/telegram-bridge.js

@wscurran wscurran added bug Something isn't working Integration: Telegram Use this label to identify Telegram bot integration issues with NemoClaw. labels Apr 1, 2026
tgApi() resolves with {ok: false} on Telegram API errors instead of
rejecting. The existing .catch() fallback for Markdown parse failures
therefore never triggers, causing messages to be silently lost when
LLM output contains unbalanced Markdown entities (*, _, etc.).

Check res.ok explicitly so the plain-text retry actually executes.

Reproduction:
1. Send a message that triggers a long LLM response with Markdown
2. Telegram rejects with "can't parse entities" (HTTP 400, ok: false)
3. Before fix: message silently dropped, no error logged
4. After fix: automatic retry without parse_mode delivers the message

Signed-off-by: brianyang <brianyang@signalpro.com.tw>
Address review feedback: only retry without parse_mode when Telegram
returns 400 with "can't parse entities", instead of retrying on all
failures. This avoids masking real errors (403, 429, invalid chat).

Signed-off-by: brianyang <brianyang@signalpro.com.tw>
@BrianYang-123 BrianYang-123 force-pushed the fix/telegram-sendmessage-fallback branch from feb1595 to a866579 Compare April 2, 2026 01:22
@BrianYang-123
Copy link
Copy Markdown
Author

Closing — the target file scripts/telegram-bridge.js was deleted in #1081 (merged 2026-04-05). The sendMessage function and its flawed .catch() Markdown fallback no longer exist in NemoClaw.

Telegram message sending is now handled natively by OpenClaw inside the sandbox. Whether OpenClaw correctly retries on Markdown parse failures (error_code 400 + "can't parse entities") is unknown from NemoClaw's side. If the bug persists, it should be filed against OpenClaw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working Integration: Telegram Use this label to identify Telegram bot integration issues with NemoClaw.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants