Skip to content

fix(telegram): eliminate 409 polling race between PTB retry loop and conflict handler - #30158

Closed
luyao618 wants to merge 1 commit into
NousResearch:mainfrom
luyao618:fix/telegram-409-polling-race-30130
Closed

fix(telegram): eliminate 409 polling race between PTB retry loop and conflict handler#30158
luyao618 wants to merge 1 commit into
NousResearch:mainfrom
luyao618:fix/telegram-409-polling-race-30130

Conversation

@luyao618

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a persistent race condition in the Telegram gateway where polling enters a ~31-second cycle of "409 Conflict, retry, resume, Conflict", causing missed updates during the recovery window.

Two independent retry paths were racing inside gateway/platforms/telegram.py:

  1. PTB internal network_retry_loop (configured with max_retries=-1) — after invoking our error callback, PTB still independently retried getUpdates with exponential backoff (1s, 1.5s, ..., 30s).
  2. Our _handle_polling_conflict handlerawait updater.stop(), 20s sleep, drain pools, then start_polling() again.

Because both ran as concurrent asyncio tasks and start_polling() defaulted to a 10s long-poll timeout, PTB retried getUpdates (with a fresh httpx connection) overlapped with the stale one from the previous session, triggering another 409 almost immediately. The cycle repeated forever.

Related Issue

Fixes #30130

Type of Change

  • Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/platforms/telegram.py

    • Add _polling_conflict_recovery_in_progress flag on TelegramAdapter (initialised in __init__).
    • Split _handle_polling_conflict into a guarded outer wrapper plus _handle_polling_conflict_inner. The wrapper sets the flag, runs the inner method under try/finally, and clears the flag, making concurrent invocations no-op cleanly (idempotent).
    • Make _polling_error_callback early-return while a conflict recovery is in progress so PTB retry loop cannot queue overlapping handlers or race the restart.
    • Pass timeout=timedelta(seconds=60) to all three start_polling() call sites (initial startup, network-error retry path, conflict-recovery retry). This raises the long-poll cycle from PTB 10s default to 60s, dramatically shrinking the overlap window between sessions.
    • Add timedelta to the existing datetime import.
  • tests/gateway/test_telegram_conflict.py

    • test_start_polling_passes_long_timeout asserts start_polling() is invoked with a timeout kwarg greater than 10s.
    • test_handle_polling_conflict_is_idempotent asserts that while the recovery flag is set, additional invocations do not trigger another restart attempt or advance the conflict counter; once the flag clears, a new invocation proceeds normally.

How to Test

  1. Repro setup (per the issue): Telegram gateway with a token still held open by a prior session, observe the recurring 409 cycle without this patch.
  2. Apply this PR. The first 409 still triggers a single recovery, but PTB retry loop no longer races the restart, and the new 60s long-poll dramatically reduces re-conflict probability.
  3. Unit tests: pytest tests/gateway/test_telegram_conflict.py -q to 8 passed.

Checklist

Code

  • I have read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(telegram): ...)
  • I searched for existing PRs to make sure this is not a duplicate
  • My PR contains only changes related to this fix
  • I have added tests for my changes
  • I have tested on my platform: macOS 15

Documentation and Housekeeping

  • N/A, no docs/config/architecture changes
  • N/A, no tool/schema changes

Notes

  • python-telegram-bot==22.6 is pinned in this repo. Updater.start_polling timeout signature is int | datetime.timedelta (verified against the installed package), so timedelta(seconds=60) is the correct API surface.
  • read_timeout (configured via HERMES_TELEGRAM_HTTP_READ_TIMEOUT) is intentionally left alone, it is user-tunable and the 60s long-poll is the more impactful knob.
  • Behaviour for non-conflict errors (network errors, fatal errors after MAX_CONFLICT_RETRIES) is preserved unchanged.

…conflict handler

Resolves NousResearch#30130.

Two independent retry paths were racing in the Telegram adapter:
1. PTB's internal `network_retry_loop` (max_retries=-1, exponential backoff)
2. Our `_handle_polling_conflict` handler that stops + restarts polling

After our error callback scheduled a recovery task, PTB would independently
retry `getUpdates` with a fresh httpx connection, briefly overlapping the
stale one from the restart and triggering another 409. The result was a
persistent ~31s cycle of Conflict → retry → resume → Conflict.

This change:

- Adds `_polling_conflict_recovery_in_progress` flag (try/finally guarded)
  so the conflict handler is idempotent — concurrent invocations from PTB's
  retry loop no-op cleanly.
- Suppresses `_polling_error_callback` while a recovery is in progress so
  PTB's retry loop cannot queue overlapping handlers or race the restart.
- Passes `timeout=timedelta(seconds=60)` to all three `start_polling()`
  call sites (initial startup, network-error retry, conflict-recovery retry).
  This raises the long-poll cycle from PTB's 10s default to 60s, dramatically
  shrinking the overlap window that caused the 409 race.

Added unit tests in tests/gateway/test_telegram_conflict.py covering:
- start_polling() is called with a long timeout (>10s)
- conflict handler is idempotent under concurrent invocation
@daimon-nous daimon-nous Bot added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter P1 High — major feature broken, no workaround labels May 22, 2026
@daimon-nous

daimon-nous Bot commented May 22, 2026

Copy link
Copy Markdown
Contributor

Competes with #27317 (open) which takes a different approach — delegating all recovery to PTB rather than adding a conflict recovery guard. Both fix #30130.

@luyao618

Copy link
Copy Markdown
Contributor Author

Closing — open too long, no longer relevant.

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 P1 High — major feature broken, no workaround platform/telegram Telegram bot adapter type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Persistent Telegram 409 polling conflicts caused by PTB network_retry_loop racing with _handle_polling_conflict

1 participant