fix(telegram): probe polling liveness after reconnect to detect wedged Updater - #18088
Closed
jslizar wants to merge 1 commit into
Closed
fix(telegram): probe polling liveness after reconnect to detect wedged Updater#18088jslizar wants to merge 1 commit into
jslizar wants to merge 1 commit into
Conversation
…d Updater After a transient Telegram 502, _handle_polling_network_error's stop()+start_polling() cycle can leave PTB's Updater with `running=True` but a wedged consumer task that never makes progress. No error_callback fires in that state, so the reconnect ladder never advances past attempt 1, the MAX_NETWORK_RETRIES fatal-error path is never reached, and the gateway sits silent indefinitely. Schedule a heartbeat probe (60s after a successful reconnect) that verifies Updater.running is still True and bot.get_me() responds within a tight asyncio.wait_for timeout. Either failure feeds back into the reconnect ladder so the existing escalation path fires. No PTB-internal coupling, no Application rebuild — minimal additive defense inside the existing reconnect abstraction. Tests cover healthy / Updater non-running / probe timeout / probe network error / already-fatal cases, plus an integration check that the probe is actually scheduled after a successful start_polling(). Closes the silent-wedge case observed in the wild after a transient Telegram 502; existing reconnect tests updated to mock bot.get_me() now that the success path schedules a heartbeat probe.
Contributor
|
Merged via #18751 — your commit cherry-picked onto current main with authorship preserved via rebase-merge. One test-file merge conflict resolved (HEAD had drain-connection tests from a prior salvage; kept both those and your heartbeat-probe tests). Thanks for the detailed issue write-up and the thoughtful defense-in-depth fix! |
This was referenced Aug 3, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #18086
Detect wedged Telegram polling after reconnect via heartbeat probe
Summary
_handle_polling_network_errorcurrently treatsUpdater.start_polling()returning successfully as proof that polling has resumed. In practice, the underlying long-poll task can be left wedged on a stale httpx connection —Updater.runningisTruebut nogetUpdatescalls actually progress, no error callback fires, and the reconnect ladder sits at attempt 1 forever. The fatal-error path is never reached, so the gateway runs indefinitely with no working Telegram polling.This PR adds a deferred heartbeat probe scheduled after each successful
start_polling()in the reconnect path. The probe verifies that the bot endpoint is reachable through the same client a healthy long-poll would use; on failure it re-enters the existing reconnect ladder so the proven escalation path (eventually_set_fatal_error(retryable=True)afterMAX_NETWORK_RETRIES) can fire.Why
See the linked issue for the full repro and log evidence: a single transient
Bad GatewayfromgetUpdatestriggered the customer-side wedge —Updater.stop()raised aTimedOutfrom_get_updates_cleanup(logged as "Suppressing error to ensure graceful shutdown"), the surroundingtry / except Exception: passswallowed it, thenstart_polling()returned without raising but polling never actually consumed any further updates. No "polling resumed" log, no "reconnecting in 10s, attempt 2/10", no fatal-error path — just silence for 11 hours until the container was manually restarted.The first reconnect attempt looks like recovery in the logs; only the absence of subsequent activity reveals the wedge. The existing safeguards (
MAX_NETWORK_RETRIES, fatal-error retryable=True with supervisor restart) are sound but unreachable when polling silently wedges.Approach
Two-part fix, both inside the existing reconnect abstraction (no PTB-internal coupling, no Application rebuild):
After a successful
start_polling()in_handle_polling_network_error, schedule_verify_polling_after_reconnect()as a background task.The probe waits
HEARTBEAT_PROBE_DELAY(60s, comfortably above one healthy long-poll cycle), then verifiesUpdater.runningis still True andBot.get_me()returns withinPROBE_TIMEOUT(10s) viaasyncio.wait_for. Either failure feeds back into_handle_polling_network_errorso the reconnect ladder advances.This is a minimal additive layer — no behavior change on the happy path, and on the wedged path the existing
MAX_NETWORK_RETRIESladder eventually escalates to fatal-error so external supervisors (systemdRestart=on-failure) can do their job.Considered alternatives
Updater.stop()exceptions. Worth doing on its own, but doesn't help in the case wherestop()cleans up successfully andstart_polling()is the silent failure point.Bot.get_me()was chosen as the probe because it shares the bot's httpx client (so a wedged pool fails the probe) and doesn't conflict withUpdater.getUpdates()(no 409).Test plan
tests/platforms/test_polling_heartbeat.py(added in this PR) covers:Tests use
unittest.mock.AsyncMockfor the PTB Application/Updater/Bot and patchasyncio.sleep/asyncio.wait_forso the suite runs in milliseconds.End-to-end repro (operator-side, optional): boot the gateway against a transparent proxy that returns
502 Bad GatewayforgetUpdatesfor 30s then 200; observe the heartbeat probe firing and the reconnect ladder progressing past attempt 1 instead of silencing.Files changed
gateway/platforms/telegram.py— schedules_verify_polling_after_reconnect()after a successful reconnect; defines the new method.tests/platforms/test_polling_heartbeat.py— unit tests for the probe.Open questions for maintainers
HEARTBEAT_PROBE_DELAYandPROBE_TIMEOUTare currently hardcoded; happy to move them next toMAX_NETWORK_RETRIES/BASE_DELAYas module-level constants or env knobs if there's a preference.except Exception: passaroundUpdater.stop()in the reconnect path. That's a behavioral change worth its own discussion, kept out of this PR for scope.