Skip to content

fix(telegram): route PTB NetworkError/TimedOut from polling heartbeat into reconnect path (#62047) - #63007

Closed
SilentKnight87 wants to merge 1 commit into
NousResearch:mainfrom
SilentKnight87:fix/telegram-heartbeat-ptb-network-errors
Closed

fix(telegram): route PTB NetworkError/TimedOut from polling heartbeat into reconnect path (#62047)#63007
SilentKnight87 wants to merge 1 commit into
NousResearch:mainfrom
SilentKnight87:fix/telegram-heartbeat-ptb-network-errors

Conversation

@SilentKnight87

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes #62047 — the polling heartbeat's probe handler only routed asyncio.TimeoutError/OSError into the reconnect path. In PTB 22.x, telegram.error.NetworkError and TimedOut inherit from TelegramError (not OSError), so real connectivity failures fell through to a generic except Exception and were silently ignored — the gateway kept reporting Telegram as connected while the heartbeat never initiated recovery.

Related Issue

Closes #62047

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • plugins/platforms/telegram/adapter.py
    • Heartbeat probe handler now catches Exception and classifies via the existing _looks_like_network_error() helper, so PTB NetworkError/TimedOut enter the same recovery ladder as TimeoutError/OSError. Non-connectivity errors keep the old behavior (deferred to PTB's own handlers).
    • New _is_request_validation_error() guard: PTB's BadRequest subclasses NetworkError, so classification by base class alone would send HTTP 400 request-validation failures into the reconnect ladder — where a successful start_polling resets the retry counter and the churn never escalates to fatal. BadRequest stays non-retryable.
    • Reconnect-trigger log line now redacts the error via _redact_telegram_error_text() (PTB transport errors can embed the bot-token request URL), matching every sibling logging site.
    • _probe_pending_updates' webhook-info handler keeps its original deliberate deferral (a failed probe is the get_me() path's job to confirm; first-failure teardown of a healthy poller would regress Telegram gateway connects and polls successfully but incoming DMs produce zero log output and no response (WSL2, v0.16.0, PTB 22.6) #42909's debounce design) — PTB connectivity errors now get that same deferral instead of leaking upward as unclassified exceptions.
  • Tests (tests/gateway/test_telegram_network_reconnect.py, test_telegram_pending_update_probe.py)
    • Reconnect triggers on PTB NetworkError/TimedOut (parametrized); no reconnect on InvalidToken; no reconnect on BadRequest despite it being a NetworkError subclass; webhook-info probe defers connectivity errors and propagates non-connectivity errors.
    • Test exceptions are built with real-PTB-shaped hierarchies (TelegramError-derived, not OSError-derived): the pre-existing gateway-conftest mock derives NetworkError from OSError, which the unfixed code already caught — tests written against it pass with or without this fix. Verified these tests fail on unfixed main.
    • Fixed the file-local telegram mock in both files to register telegram.error (the new top-level import made the standalone fallback a guaranteed ModuleNotFoundError).

How to Test

scripts/run_tests.sh tests/gateway/test_telegram_network_reconnect.py tests/gateway/test_telegram_pending_update_probe.py -- -q

43 passed. Also verified the regression tests fail against unfixed main (reverted adapter, reran: 3 failures, all new tests).

Notes for reviewers

  • The conftest-level telegram mock (tests/gateway/conftest.py) still models NetworkError as an OSError subclass, diverging from real PTB. This PR works around it locally; aligning the shared mock with PTB's real hierarchy would benefit the whole gateway suite but touches many tests, so it's left out of this fix.
  • _verify_polling_after_reconnect has the mirror-image gap (routes every get_me() failure, including InvalidToken, into recovery, and bypasses the _polling_error_task dedup guard). Deliberately out of scope; can file separately if useful.

Checklist

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've added tests for my changes (6 new tests; verified non-vacuous against unfixed main)
  • I've tested on my platform: macOS
  • I've considered cross-platform impact (exception classification only; no platform-specific code)

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Telegram regression analysis. This is already implemented on current main by a broader fix.

  • Automated hermes-sweeper review verified commit 97fb9e1f629c633e1c60d4b64ba490e7c3a61e60 (fix(telegram): classify PTB heartbeat transport errors) is an ancestor of origin/main.
  • plugins/platforms/telegram/adapter.py:1185 centrally classifies PTB transport errors while excluding BadRequest, InvalidToken, Forbidden, and RetryAfter.
  • plugins/platforms/telegram/adapter.py:2227 routes classified heartbeat failures into _schedule_polling_recovery().
  • Regression coverage in tests/gateway/test_telegram_network_reconnect.py:710 covers NetworkError/TimedOut on both heartbeat probe paths and semantic-error non-recovery.

Closing as redundant with the implementation already on main.

@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages duplicate This issue or pull request already exists labels Jul 12, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #62098 (merged 2026-07-10, canonical salvage that closed #62047). That PR already shipped the same PTB NetworkError/TimedOut classification routing through _looks_like_network_error() / _schedule_polling_recovery() in plugins/platforms/telegram/adapter.py on main. This PR was created after that merge and re-implements the same fix. Closing candidate; the underlying heartbeat classification gap is already resolved on main. (Sibling re-implementations #62104/#62106 were likewise duplicates of #62098.)

@teknium1 teknium1 closed this Jul 12, 2026
@teknium1 teknium1 added sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
@SilentKnight87

SilentKnight87 commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! this was in flight before I saw #62098 had landed; good to see it converged on the same classification design (BadRequest/InvalidToken exclusions, routing through _schedule_polling_recovery).

One bug still open is #62098: _verify_polling_after_reconnect() still feeds unclassified failures (including InvalidToken) into the reconnect ladder and bypasses the _polling_error_task guard. Filed as #63243, fix in #63247.

@SilentKnight87
SilentKnight87 deleted the fix/telegram-heartbeat-ptb-network-errors branch July 13, 2026 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery duplicate This issue or pull request already exists P1 High — major feature broken, no workaround platform/telegram Telegram bot adapter sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Telegram polling heartbeat swallows python-telegram-bot NetworkError, so reconnect is never triggered

3 participants