Skip to content

fix(telegram): stop reusing the getUpdates connection Telegram closes at ~39s - #84495

Open
Gulyiyen wants to merge 1 commit into
NousResearch:mainfrom
Gulyiyen:fix/telegram-getupdates-keepalive
Open

Gulyiyen wants to merge 1 commit into
NousResearch:mainfrom
Gulyiyen:fix/telegram-getupdates-keepalive

Conversation

@Gulyiyen

Copy link
Copy Markdown

What does this PR do?

A healthy Telegram bot on a healthy network reconnects every ~44 seconds, forever. In one 20-minute sample: 133 disconnects, 134 recoveries. It never looks broken — the attempt counter resets to 1 each time, so the reconnect ladder never escalates and the bot keeps working — but ~11% of wall-clock time is spent not polling, and each gap adds up to 5s of latency.

The cause is not a flaky network. It is a metronome:

resumed at failed at delta
23:03:45 23:04:25 39.3s
23:04:30 23:05:09 39.2s
23:05:14 23:05:53 39.3s
23:05:59 23:06:38 39.3s
23:06:43 23:07:22 39.4s

Root cause: api.telegram.org closes a pooled connection roughly 39 seconds after it is opened. PTB's long poll runs back-to-back (Updater.start_polling() defaults to poll_interval=0), so the socket is never idle. keepalive_expiry measures idle time, so it never fires and the same TCP connection is reused indefinitely. At ~39s the server closes it, httpx hands the next getUpdates that dead socket and raises a bare httpx.ReadError (empty message: the connection went away mid-read). _handle_polling_network_error classifies that as a network fault and answers with a 5s backoff + start_polling(). Hence the ~44s cycle: 39s of polling, 5s blind.

The fix: give the getUpdates pool limits that never keep a connection alive, so every long poll starts on a fresh socket that cannot already be dead.

Only that pool opts out. Ordinary Bot API calls keep the shared tuned keepalive from platform_httpx_limits() — they are short and sporadic, reuse is a win there, and the ~39s lifetime is invisible to them.

Cost is one TLS handshake per poll: ~35ms to the IPv6 endpoint, ~150ms to IPv4, once per timeout seconds. Against a 5s blind window every 44s, that trade is worth making.

The part that is easy to get wrong

The limits must reach whichever object owns the connection pool.

When a custom transport is passed to httpx.AsyncClient, httpx builds no transport of its own and the client-level limits are silently ignored — they exist only to construct the default transport. On the fallback-IP branch TelegramFallbackTransport owns the pool, so the limits go into it; it forwards **transport_kwargs to the httpx.AsyncHTTPTransport it creates per IP. This matches the rule the surrounding code already documents for the general pool.

Wiring them only into the client passes review, changes nothing on that branch, and leaves the bug fully intact. That was the first attempt at this fix and it looked correct in the diff.

keepalive_expiry

Carried from the shared tuned limits even though it is moot while max_keepalive_connections=0 — nothing is kept alive to expire. #31599's invariant is that no pool is left on httpx's 5.0 default, and the value must already be right if the keepalive count is ever raised.

Related Issue

No open issue tracks this specific symptom. The closest is #31599 (CLOSE_WAIT fd leak in the general pool), which is closed and was fixed by #51541 — this PR does not re-fix it. The code and tests here reference #31599 because they extend the invariant it established to the getUpdates pool.

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • plugins/platforms/telegram/adapter.py
    • _updates_limits() — builds httpx.Limits(max_keepalive_connections=0, ...) for the getUpdates pool, preserving PTB's connection_pool_size as max_connections and carrying keepalive_expiry from the shared tuned limits.
    • _with_updates_limits() — injects those limits at the client level on the proxy and direct branches, where httpx honours the client-level limits kwarg.
    • fallback-IP branch — copies the shared _transport_kwargs and overrides limits with _updates_limits(), so they reach TelegramFallbackTransport itself. The client-level kwarg is deliberately not set here; httpx would discard it next to a custom transport.
    • The general request pool is untouched on every branch.
  • tests/gateway/test_telegram_closewait_limits_31599.py
    • _assert_updates_pool_never_reuses() — new helper asserting the getUpdates pool contract.
    • The proxy-branch test now asserts both contracts (general pool keeps reuse, updates pool has none).
    • The fallback-branch test asserts both contracts on TelegramFallbackTransport._transport_kwargs["limits"] rather than through the client-level helpers, because that branch deliberately does not set client-level limits.

How to Test

  1. Reproduce without a bot token (does not disturb a running gateway — plain GETs, no getUpdates, no 409 conflict). Save the script below as repro_telegram_conn_lifetime.py:

    python repro_telegram_conn_lifetime.py            # reproduce the bug
    python repro_telegram_conn_lifetime.py --fixed    # apply the fix
    

    Observed:

    [   0.1s] first connection ...:55099  (HTTP 302)
    [  19.7s] !! httpx.ReadError:   (connection age 38.7s)
    [  39.8s] first connection ...:60172  (HTTP 302)
    [  59.6s] !! httpx.ReadError:   (connection age 38.9s)
    
    result: 2 errors in 95s
    

    38.7s and 38.9s — matching the adapter's 39.3s. With --fixed the same probe runs 100s with zero errors.

    repro_telegram_conn_lifetime.py (no bot token needed)
    """Reproduce the ~39s getUpdates ReadError without a bot token.
    
    api.telegram.org closes a pooled connection roughly 39 seconds after it is
    opened. The Telegram adapter's long poll runs back-to-back (PTB's
    poll_interval defaults to 0), so the socket is never idle, keepalive_expiry
    never fires, and the same connection is reused until the server kills it —
    at which point httpx writes the next getUpdates to a dead socket and raises a
    bare httpx.ReadError.
    
    This script shows the connection lifetime with plain GETs, so it needs no bot
    token and does not disturb a running gateway (no getUpdates, no 409 conflict).
    The 1s gap is shorter than keepalive_expiry=2.0, so the connection stays in
    the pool and is reused — the same condition the long poll creates.
    
        python repro_telegram_conn_lifetime.py            # reproduce the bug
        python repro_telegram_conn_lifetime.py --fixed    # apply the fix
    
    Expected:
        default  -> ReadError at ~38-39s of connection age, repeatedly
        --fixed  -> zero errors (max_keepalive_connections=0, no reuse)
    """
    
    import asyncio
    import sys
    import time
    
    import httpx
    
    URL = "https://api.telegram.org/"
    DURATION_S = 100
    GAP_S = 1.0
    
    
    def socket_id(resp: httpx.Response) -> str:
        """Local address of the socket that served this response, if exposed."""
        try:
            stream = resp.extensions.get("network_stream")
            if stream is None:
                return "?"
            addr = stream.get_extra_info("client_addr")
            return f"{addr[0]}:{addr[1]}" if addr else "?"
        except Exception:
            return "?"
    
    
    async def main(fixed: bool) -> int:
        if fixed:
            # The fix: never keep a connection alive, so the poll can never be
            # handed a socket the server has already closed.
            limits = httpx.Limits(max_keepalive_connections=0, keepalive_expiry=2.0)
        else:
            # What the adapter uses today, via platform_httpx_limits().
            limits = httpx.Limits(max_keepalive_connections=10, keepalive_expiry=2.0)
    
        print(f"mode: {'FIXED (no keepalive)' if fixed else 'CURRENT (keepalive)'}")
        print(f"limits: {limits}\n")
    
        t0 = time.monotonic()
        current = None
        conn_started = t0
        errors = 0
    
        async with httpx.AsyncClient(limits=limits, timeout=20.0) as client:
            while time.monotonic() - t0 < DURATION_S:
                elapsed = time.monotonic() - t0
                try:
                    resp = await client.get(URL)
                    sid = socket_id(resp)
                    if sid != current:
                        if current is not None:
                            age = time.monotonic() - conn_started
                            print(f"[{elapsed:6.1f}s] connection replaced "
                                  f"{current} -> {sid}  (previous lived {age:.1f}s)")
                        else:
                            print(f"[{elapsed:6.1f}s] first connection {sid}  "
                                  f"(HTTP {resp.status_code})")
                        current = sid
                        conn_started = time.monotonic()
                except Exception as exc:
                    errors += 1
                    age = time.monotonic() - conn_started
                    print(f"[{elapsed:6.1f}s] !! {type(exc).__module__}."
                          f"{type(exc).__name__}: {exc!s:.60}  "
                          f"(connection age {age:.1f}s)")
                    current = None
                    conn_started = time.monotonic()
                await asyncio.sleep(GAP_S)
    
        print(f"\nresult: {errors} error(s) in {DURATION_S}s")
        return errors
    
    
    if __name__ == "__main__":
        is_fixed = "--fixed" in sys.argv
        error_count = asyncio.run(main(is_fixed))
        # With --fixed, any error is a failure. Without it, errors are the point.
        sys.exit(1 if (is_fixed and error_count) else 0)
  2. Unit tests:

    pytest tests/gateway/test_telegram_closewait_limits_31599.py -q   # 2 passed
    
  3. On a live gateway — same bot, same network, same machine. Before: 133 disconnects in 20 minutes, one every ~44s without exception. After: 25 minutes 34 seconds of uptime, 0 disconnects, 0 ReadError. The gateway is live throughout (12.1s CPU accumulated, dispatcher and housekeeping both ticking) — the silence is an absence of errors, not an absence of work. At the old rate that window would have contained roughly 34 reconnects.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 11 Pro (build 26200), Python 3.11

On the two boxes above, precisely:

  • Full suite not claimed. I did not run pytest tests/ -q green, so I have not ticked it. pytest-asyncio is not in the dev deps, so every @pytest.mark.asyncio test errors out in my environment regardless of this patch. What I did instead is a before/after comparison on the same interpreter and selection: pytest tests/gateway -k telegram --ignore=tests/gateway/relay gives 221 failed / 367 passed on untouched main and 221 failed / 367 passed on this branch, with identical failure sets — no test fails here that does not already fail without the patch. Happy to re-run anything specific.
  • Tests are new assertions, not a new test function. The two existing tests in test_telegram_closewait_limits_31599.py already drive both branches of connect(), so the new getUpdates contract is asserted there via a new helper rather than by adding a third test that would duplicate the setup.

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A — N/A; the reasoning lives in docstrings and comments on the new helpers, where the existing pool-limit rationale already sits.
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A — N/A; no new config keys or env vars. This is a behaviour fix, not a knob.
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A — N/A
  • I've considered cross-platform impact (Windows, macOS) — or N/A — Pure httpx pool configuration; no paths, processes, signals or shell commands. Developed and verified on Windows 11.
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A — N/A

Screenshots / Logs

Before — the loop, roughly every 44 seconds, forever:

WARNING [Telegram] Telegram network error, scheduling reconnect: httpx.ReadError:
WARNING [Telegram] Telegram network error (attempt 1/10), reconnecting in 5s. Error: httpx.ReadError:
INFO    [Telegram] Telegram polling resumed after network error (attempt 1)

After — the last httpx.ReadError in the log is timestamped 23:37:09, twenty-one seconds before the patched gateway finished starting at 23:37:30. Nothing after it.


Related work: #29326 (open) proposed isolating the getUpdates transport in May and deserves the credit for raising it first; it targets gateway/platforms/telegram.py, which no longer exists after the adapter moved to plugins/platforms/telegram/, and it also adds polling-timing env vars. This PR is rebased on current main, limited to the pool-limits fix, and additionally covers the fallback-IP transport branch. Maintainers may well prefer to revive that one instead — happy either way.

… at ~39s

api.telegram.org closes a pooled connection roughly 39 seconds after it is
opened. PTB's long poll runs back-to-back (poll_interval defaults to 0), so
the socket is never idle and keepalive_expiry -- which measures *idle* time --
never fires. httpx therefore hands the next getUpdates a socket the server has
already closed and raises a bare `httpx.ReadError`. The polling error callback
reads that as a network fault and answers with a 5s reconnect, so a healthy
bot on a healthy network reconnects every ~44s forever, losing a 5s window of
updates each time. Left running it is also the "sustained reconnect storm"
the send path already warns about, where PTB's pool can report
SendResult(success=True) for sends that never transmit.

Measured against the live endpoint with plain GETs on a reused pool (no bot
token involved): the connection died at 38.7s and 38.9s, matching the
adapter's observed 39.3s reconnect period. With max_keepalive_connections=0
the same probe ran 100s with zero errors.

Give the getUpdates pool limits that never keep a connection alive. Only that
pool opts out; ordinary Bot API calls keep the shared tuned keepalive, where
reuse is a win and the ~39s lifetime is invisible because those requests are
short and sporadic. Cost is one TLS handshake per poll (~35ms to the IPv6
endpoint) against a 5s blind window every 44s.

The limits must reach whichever object owns the pool. When a custom transport
is supplied, httpx builds no transport of its own and the client-level
`limits` are silently ignored, so the fallback-IP branch passes them into
TelegramFallbackTransport, which forwards **transport_kwargs to the
httpx.AsyncHTTPTransport it creates per IP. Wiring them only into the client
looks correct and changes nothing on that branch.

keepalive_expiry is carried from the shared tuned limits even though it is
moot at max_keepalive_connections=0, so NousResearch#31599's "no pool on httpx's 5.0
default" invariant still holds if the keepalive count is ever raised.

test_telegram_closewait_limits_31599 now asserts the two pools separately:
the general pool keeps tight keepalive (1..50 connections), the getUpdates
pool must have none.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working comp/plugins Plugin system and bundled plugins platform/telegram Telegram bot adapter P3 Low — cosmetic, nice to have sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Aug 12, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(telegram): stop reusing the getUpdates connection Telegram closes at ~39s

  • tests/gateway/test_telegram_closewait_limits_31599.py _assert_updates_pool_never_reuses(): the assertion assumes instances[1] is the getUpdates pool purely by construction order. If connect() ever builds the requests in a different order (or a third pool is added), the test silently checks the wrong pool. Consider identifying the pool by a distinguishing kwarg (e.g. presence of the no-keepalive limits) instead of by index.
  • _updates_limits() is evaluated once for the transport branch and once per _with_updates_limits() call; the transport-kwargs copy correctly avoids mutating the shared _transport_kwargs. The one-TLS-handshake-per-poll cost is documented and acceptable.
  • Behavior is otherwise well-reasoned: only the getUpdates pool opts out of reuse, the general pool keeps reuse, and keepalive_expiry is still carried to preserve the Telegram adapter leaks httpx general-pool connections through HTTP proxy (CLOSED sockets accumulate, fd limit hit after ~2 days) #31599 invariant.

@teknium1

Copy link
Copy Markdown
Collaborator

Heads-up: PR #99691 (just opened for #87057, salvaging #87111 + #87265) sets max_keepalive_connections=0 on the dedicated getUpdates request pool, so the long-poll never picks up a pooled connection Telegram already closed — which overlaps with this PR's core mechanism. Once #99691 lands, it's worth re-checking whether the ~39s reconnect metronome you measured still reproduces; if your PR contains distinct changes beyond the no-reuse pool, a rebase against that would isolate them. Leaving this open for maintainer review rather than closing, since the symptom you documented (steady-state churn, not the Windows deadlock) is distinct.

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

Labels

comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have platform/telegram Telegram bot adapter 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.

4 participants