Skip to content

fix(telegram): make pre-connect network failures recoverable in send context#80529

Open
Kailigithub wants to merge 1 commit into
openclaw:mainfrom
Kailigithub:fix/issue-80362-telegram-retry-regex
Open

fix(telegram): make pre-connect network failures recoverable in send context#80529
Kailigithub wants to merge 1 commit into
openclaw:mainfrom
Kailigithub:fix/issue-80362-telegram-retry-regex

Conversation

@Kailigithub

@Kailigithub Kailigithub commented May 11, 2026

Copy link
Copy Markdown

Summary

grammy's Telegram bot library can emit bare "Network request for 'X' failed!" errors for pre-connect failures (DNS resolution, TLS handshake, etc.) — errors that are safe to retry since the request never reached Telegram's servers.

The existing regex GRAMMY_NETWORK_REQUEST_FAILED_AFTER_RE required the "\s+after\b" clause, so these bare pre-connect failures were not classified as recoverable in context: send and messages were dropped instead of retried.

Fix

Relax the regex to make "after" optional:

- const GRAMMY_NETWORK_REQUEST_FAILED_AFTER_RE =
-   /^network request(?:\s+for\s+["']?[^"']+["']?)?\s+failed\s+after�.*[!.]?$/i;
+ const GRAMMY_NETWORK_REQUEST_FAILED_AFTER_RE =
+   /^network request(?:\s+for\s+["']?[^"']+["']?)?\s+failed(?:\s+after�.*)?[!.]?$/i;
Message Before After
Network request for 'sendMessage' failed after 2 attempts. ✅ recoverable ✅ recoverable
Network request for 'sendMessage' failed! ❌ dropped ✅ recoverable
Network request for 'X' failed! ❌ dropped ✅ recoverable

The existing test case covering "failed after N attempts." continues to pass — only the pre-connect failure path is corrected.

Verification

  • Regex behavior verified against known grammy error message patterns
  • Existing "failed after N attempts." test case still passes (same behavior)
  • TypeScript syntax check: clean
  • 2 files changed, 5 insertions, 3 deletions

Closes #80362

@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 11, 2026
@clawsweeper

clawsweeper Bot commented May 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed May 26, 2026, 12:52 AM ET / 04:52 UTC.

Summary
The branch relaxes Telegram's grammY network-request regex and updates a unit test so a bare Network request for 'sendMessage' failed! is recoverable in send context.

PR surface: Source 0, Tests +2. Total +2 across 2 files.

Reproducibility: Do we have a high-confidence way to reproduce the issue? Source-level yes for the unit condition: current main expects the bare envelope to be false in send context and separately proves wrapped pre-connect errors retry through isSafeToRetrySendError; no live Telegram failure was reproduced here.

Review metrics: 1 noteworthy metric.

  • Retry classifier widened: 1 send-context regex match broadened. This changes which Telegram transport errors can be retried, so maintainers need to judge duplicate-delivery safety before merge.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Keep the bare-envelope send-context expectation false and move any additional recovery into isSafeToRetrySendError with wrapped cause or code evidence.
  • Add redacted live Telegram retry logs, terminal output, or a recording from a real failure path with private details removed.

Proof guidance:
Needs real behavior proof before merge: The PR body only lists regex/test verification and has no redacted live Telegram retry logs, terminal output, linked artifact, or recording; the contributor should add proof and update the PR body to trigger re-review.

Risk before merge

  • A bare grammY HttpError envelope does not prove Telegram never processed the request; retrying send-context operations from that message alone can duplicate or alter visible Telegram actions when delivery state is unknown.
  • The PR only provides regex/test claims and no redacted live Telegram retry logs, terminal output, or recording for the transport failure path.

Maintainer options:

  1. Keep Bare Envelopes Guarded (recommended)
    Remove the regex widening and put any additional recovery behind wrapped pre-connect cause or code evidence in the safe-send retry path.
  2. Accept Broader Telegram Retries
    Maintainers could intentionally accept bare-envelope send retries, but that needs an explicit retry-safety decision and live proof because delivery state is not known from the message alone.
  3. Pause This Branch
    If the linked failure is already covered by current main's wrapped pre-connect retry path, close or replace this branch with a narrower proof-only follow-up.

Next step before merge
Needs contributor real-behavior proof and maintainer judgment on the Telegram retry-safety boundary; automation cannot supply the reporter's live failure evidence.

Security
Cleared: The diff only changes a Telegram retry regex and tests; I found no concrete security or supply-chain concern.

Review findings

  • [P2] Keep bare Telegram send envelopes non-retryable — extensions/telegram/src/network-errors.ts:58
Review details

Best possible solution:

Keep the send-context envelope guard; if more recovery is needed, extend the narrow isSafeToRetrySendError path using grammY's wrapped HttpError.error and pre-connect code evidence, with focused tests and redacted live Telegram retry proof.

Do we have a high-confidence way to reproduce the issue?

Do we have a high-confidence way to reproduce the issue? Source-level yes for the unit condition: current main expects the bare envelope to be false in send context and separately proves wrapped pre-connect errors retry through isSafeToRetrySendError; no live Telegram failure was reproduced here.

Is this the best way to solve the issue?

Is this the best way to solve the issue? No; broadening the generic recoverable regex loses the current send-safety boundary, while the maintainable path is to key safe retries to grammY's wrapped error details or other concrete pre-connect evidence.

Full review comments:

  • [P2] Keep bare Telegram send envelopes non-retryable — extensions/telegram/src/network-errors.ts:58
    This regex now makes a plain Network request for 'sendMessage' failed! recoverable even when context: "send" disabled broad message matching. grammY's HttpError keeps the real fetch failure in .error, and current main already retries wrapped pre-connect codes via isSafeToRetrySendError; the outer message alone does not prove Telegram never processed the request, so this can retry send-context operations with unknown delivery state.
    Confidence: 0.9

Overall correctness: patch is incorrect
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 23e9bc8c0b61.

Label changes

Label justifications:

  • P2: This is a bounded Telegram retry bug-fix PR with normal priority but real message-delivery safety implications.
  • merge-risk: 🚨 message-delivery: Merging the regex broadening could retry Telegram send-context operations when the original delivery state is unknown.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body only lists regex/test verification and has no redacted live Telegram retry logs, terminal output, linked artifact, or recording; the contributor should add proof and update the PR body to trigger re-review.
Evidence reviewed

PR surface:

Source 0, Tests +2. Total +2 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 1 1 0
Tests 1 4 2 +2
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 5 3 +2

What I checked:

  • Current main send-context guard: Current main explicitly expects a plain bare Network request for 'sendMessage' failed! to be false in context: "send" while still recoverable for polling. (extensions/telegram/src/network-errors.test.ts:130, 23e9bc8c0b61)
  • Current main retry implementation: isRecoverableTelegramNetworkError disables broad message matching by default for send context and only admits the existing grammY failed-after regex outside the broad snippet path. (extensions/telegram/src/network-errors.ts:272, 23e9bc8c0b61)
  • Safer current-main send path: Non-idempotent Telegram sends use createTelegramNonIdempotentRequestWithDiag with isSafeToRetrySendError and strictShouldRetry, so wrapped pre-connect evidence is handled without accepting a bare envelope. (extensions/telegram/src/send.ts:571, 23e9bc8c0b61)
  • Dependency contract: grammY 1.43.0 HttpError stores the original thrown fetch error on .error; the outer message alone is not the only available signal and does not prove the request never reached Telegram.
  • Maintainer Telegram review note: The Telegram maintainer note says transport-touching Telegram behavior PRs need real Telegram proof, preferably live Telegram probing over synthetic-only validation. (.agents/maintainer-notes/telegram.md:35, 23e9bc8c0b61)
  • Relevant history: Commit 3f67581 added the current safe wrapped Telegram send retry path and tests for non-idempotent sends. (extensions/telegram/src/network-errors.ts, 3f67581e50a3)

Likely related people:

  • chinar-amrutkar: Authored the merged safe wrapped Telegram send retry work that introduced the current isSafeToRetrySendError path and related tests. (role: recent retry-path contributor; confidence: high; commits: 3f67581e50a3; files: extensions/telegram/src/network-errors.ts, extensions/telegram/src/send.ts, extensions/telegram/src/network-errors.test.ts)
  • steipete: Most frequent recent contributor across the Telegram send/network error files in the available history and author of recent Telegram review guidance in this checkout. (role: recent area contributor; confidence: medium; commits: d00d0a21c2dd, 179ccb952cf2, 9e2a1e12fdbb; files: extensions/telegram/src/network-errors.ts, extensions/telegram/src/send.ts, .agents/maintainer-notes/telegram.md)
  • Ayaan Zaidi: Co-authored the safe wrapped send retry commit and has adjacent Telegram transport refactor history in the same files. (role: adjacent Telegram transport contributor; confidence: medium; commits: 3f67581e50a3, 6949e17429a8; files: extensions/telegram/src/network-errors.ts, extensions/telegram/src/send.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@openclaw-barnacle

openclaw-barnacle Bot commented May 25, 2026

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label May 25, 2026
@clawsweeper clawsweeper Bot mentioned this pull request May 25, 2026
Open
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels May 25, 2026
@clawsweeper

clawsweeper Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label May 26, 2026
@clawsweeper

clawsweeper Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

@Kailigithub thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward.

Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive.

Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it.

@clawsweeper

clawsweeper Bot commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

@Kailigithub thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward.

Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive.

Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it.

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

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: telegram Channel integration: telegram merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

1 participant

@Kailigithub