Skip to content

fix(telegram): bound and refresh general httpx pool - #43333

Closed
chengnas wants to merge 1 commit into
NousResearch:mainfrom
chengnas:fix/telegram-general-pool-recovery
Closed

fix(telegram): bound and refresh general httpx pool#43333
chengnas wants to merge 1 commit into
NousResearch:mainfrom
chengnas:fix/telegram-general-pool-recovery

Conversation

@chengnas

Copy link
Copy Markdown

Summary

  • bound Telegram HTTPXRequest pools to a smaller default and shared keepalive limits
  • refresh the general Bot API request pool after polling reconnects and send-path network errors
  • add regression tests for bounded HTTPX limits and general-pool refresh before retrying sends

Motivation

Telegram's python-telegram-bot integration keeps separate httpx request pools for polling (Bot._request[0]) and general Bot API calls (Bot._request[1]). The adapter already refreshed the polling pool after reconnects, but send/edit/set-command calls can still get stuck behind stale half-closed connections in the general pool, especially when routed through HTTP proxies.

The old default pool size of 512 also makes stale connections accumulate for a long time before surfacing as pool exhaustion. This change keeps the pool bounded and gives keepalive sockets a short lifetime.

Tests

  • python -m py_compile gateway/platforms/telegram.py tests/gateway/test_telegram_conflict.py tests/gateway/test_telegram_thread_fallback.py
  • python -m pytest tests/gateway/test_telegram_conflict.py tests/gateway/test_telegram_thread_fallback.py

Locally the targeted tests pass: 56 passed.

@liuhao1024

Copy link
Copy Markdown
Contributor

Verification Review

Well-structured pool management fix. Observations:

  1. Default pool size 512 → 20: This is a significant reduction for the HERMES_TELEGRAM_HTTP_POOL_SIZE default. For most self-hosted users this is fine (20 concurrent connections is generous for a single bot). High-traffic bots with webhook-mode multi-chat workloads may hit pool saturation sooner, but the env-var override provides an escape hatch.

  2. platform_httpx_limits() integration is safe: The getattr(..., None) guards handle the case where platform_httpx_limits() returns None (no custom limits configured) — httpx_kwargs stays empty and httpx uses its defaults. The min(max_keepalive, connection_pool_size) ensures keepalive never exceeds total pool size.

  3. General pool drain on send-path network errors: The _drain_general_connections() calls before retry in send() and after polling reconnect address the root cause (stale half-closed sockets occupying pool slots). The _drain_request_connections(request_index, label) refactor keeps the logic DRY.

  4. keepalive_expiry=2.0: Short keepalive reduces idle socket accumulation. Good for proxy-heavy setups (sing-box, etc.) where connections go stale frequently.

  5. Test coverage is solid: test_connect_uses_bounded_httpx_limits verifies the limits object is passed through to both request clients. test_drain_general_connections_resets_general_request_only verifies isolation between polling and general pools. test_send_refreshes_general_pool_before_retrying_network_error verifies the send-path drain.

LGTM.

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter labels Jun 10, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the diagnosis and targeted coverage. This is an automated hermes-sweeper review; current main now provides the underlying recovery more narrowly and safely.

The Telegram adapter has since moved to plugins/platforms/telegram/adapter.py, so this branch's gateway/platforms/telegram.py patch is no longer salvageable as-is.

@teknium1 teknium1 closed this Jul 14, 2026
@teknium1 teknium1 added sweeper:implemented-on-main Sweeper: behavior already present on current main sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
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 P2 Medium — degraded but workaround exists 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-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades 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