Skip to content

fix(telegram): release fallback transport pools on connect failure - #71593

Closed
aneym wants to merge 1 commit into
NousResearch:mainfrom
aneym:fix/telegram-fallback-transport-fd-leak
Closed

aneym wants to merge 1 commit into
NousResearch:mainfrom
aneym:fix/telegram-fallback-transport-fd-leak

Conversation

@aneym

@aneym aneym commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Summary

TelegramFallbackTransport leaks a file descriptor on every failed fallback connect. On a host where api.telegram.org became unreachable, this exhausted the gateway's descriptor limit and wedged the whole process.

The per-IP httpx.AsyncHTTPTransport pools are built once in __init__ and never torn down. A connect that reaches ESTABLISHED and is then closed by the peer leaves its socket in CLOSE_WAIT inside the pool, and the failure path only logs and continues — so the poisoned pool is retained and leaks one descriptor per retry.

Two changes:

  • Build fallback transports lazily and discard them on a retryable connect failure (_reset_fallback pops the pool and aclose()s it, releasing its sockets).
  • Bound every pool at max_connections=8. httpx defaults to 100 per pool, so the two seed IPs plus primary could alone exceed a default file limit.

Impact observed

The bot gateway accumulated 177 sockets in CLOSE_WAIT to 149.154.166.110 (_SEED_FALLBACK_IPS[0]) against launchd's 256 soft limit. Once exhausted, the failure cascaded well beyond Telegram:

ERROR asyncio: socket.accept() out of system resource
socket: <asyncio.TransportSocket fd=36, laddr=('127.0.0.1', 8643)>
OSError: [Errno 24] Too many open files
  • accept() on the gateway port failed, so the gateway stopped answering entirely
  • config reads failed (failed to read .drain_request.json: [Errno 24])
  • DNS resolution failed (nodename nor servname provided), which made the primary path fail too and fed more traffic into the leaking fallback path
  • MCP servers could not connect (tempo parked after 5 attempts)

The DNS symptom is what makes this self-reinforcing: descriptor exhaustion breaks the resolver, which forces every subsequent request down the fallback path, which leaks faster.

Test plan

Regression test drives 50 consecutive retryable connect failures through the transport:

  • before: 1 pool retained indefinitely, 0 closes
  • after: 0 pools retained, 50 aclose() calls, max_connections=8 applied
ok  pools bounded: max_connections=8
ok  no pools retained after 50 failed attempts (was: 1 held forever)
ok  every failed pool was closed (50 aclose calls)
ok  aclose() clean

Also verified in production: after applying this the gateway holds ~113 descriptors with 0 in CLOSE_WAIT, /health returns 200, and Telegram reconnects cleanly (set_my_commands OK, 60 cmds).

Note for operators: steady-state usage is ~113 descriptors, so a 256 soft limit leaves little headroom even without a leak. Raising NumberOfFiles on the service is worthwhile independently of this fix.

Generated with Claude Code

The per-IP httpx transports were built once in __init__ and never torn
down. A connect that reached ESTABLISHED and was then closed by the peer
left its socket in CLOSE_WAIT inside the pool, and the failure path only
logged and continued — so the poisoned pool was retained and leaked one
descriptor per retry.

With DNS for api.telegram.org failing, every poll fell through to the
seed IP and leaked another fd every ~2.5s. The bot gateway reached 177
CLOSE_WAIT sockets against launchd's 256 soft limit and wedged: accept()
on the gateway port, config reads and DNS resolution all failed with
EMFILE, which in turn made the primary path fail and fed the loop.

Build fallback transports lazily and discard them on a retryable connect
failure, and bound every pool at 8 connections (httpx defaults to 100,
so two seed IPs plus primary could alone exceed the fd ceiling).

Generated with [Claude Code](https://claude.ai/code)
via [Happy](https://happy.engineering)

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Happy <yesreply@happy.engineering>
@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 labels Jul 25, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related: #58982 merged the inner-transport keepalive limits; this PR additionally discards a retry-failed fallback pool. #39336 is an open broader limits/launchd proposal in the same reliability family.

teknium1 added a commit that referenced this pull request Jul 27, 2026
The salvaged fix (#71593) rebuilds Telegram fallback pools lazily and
discards+aclose()s a pool on retryable connect failure (_reset_fallback),
bounding each at Limits(max_connections=8) as a setdefault default. The PR
shipped no test.

Add tests/gateway/test_telegram_fallback_pool_release_71593.py:
  * failed fallback pool is aclose()d and dropped from _fallbacks (the
    discard-on-failure path — reverting the _reset_fallback call fails it)
  * a recovered pool is retained, only the failed one discarded
  * _reset_fallback is a no-op when the pool was never built
  * caller-supplied limits win over the _POOL_LIMITS setdefault default
  * the max_connections=8 default applies when the caller omits limits

Update the eager-build assumptions in test_telegram_network.py to the new
lazy contract (fallbacks materialize via _get_fallback, not in __init__).
@teknium1

Copy link
Copy Markdown
Collaborator

Merged via #72359 with your commit cherry-picked onto current main — authorship preserved. Added the regression test on top (the PR body referenced one but the diff shipped none). Thanks for the FD-leak fix!

@teknium1 teknium1 closed this Jul 27, 2026
kilhyeonjun pushed a commit to kilhyeonjun/hermes-agent that referenced this pull request Jul 30, 2026
…rd-on-failure

The salvaged fix (NousResearch#71593) rebuilds Telegram fallback pools lazily and
discards+aclose()s a pool on retryable connect failure (_reset_fallback),
bounding each at Limits(max_connections=8) as a setdefault default. The PR
shipped no test.

Add tests/gateway/test_telegram_fallback_pool_release_71593.py:
  * failed fallback pool is aclose()d and dropped from _fallbacks (the
    discard-on-failure path — reverting the _reset_fallback call fails it)
  * a recovered pool is retained, only the failed one discarded
  * _reset_fallback is a no-op when the pool was never built
  * caller-supplied limits win over the _POOL_LIMITS setdefault default
  * the max_connections=8 default applies when the caller omits limits

Update the eager-build assumptions in test_telegram_network.py to the new
lazy contract (fallbacks materialize via _get_fallback, not in __init__).

(cherry picked from commit 71c9910)
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…rd-on-failure

The salvaged fix (NousResearch#71593) rebuilds Telegram fallback pools lazily and
discards+aclose()s a pool on retryable connect failure (_reset_fallback),
bounding each at Limits(max_connections=8) as a setdefault default. The PR
shipped no test.

Add tests/gateway/test_telegram_fallback_pool_release_71593.py:
  * failed fallback pool is aclose()d and dropped from _fallbacks (the
    discard-on-failure path — reverting the _reset_fallback call fails it)
  * a recovered pool is retained, only the failed one discarded
  * _reset_fallback is a no-op when the pool was never built
  * caller-supplied limits win over the _POOL_LIMITS setdefault default
  * the max_connections=8 default applies when the caller omits limits

Update the eager-build assumptions in test_telegram_network.py to the new
lazy contract (fallbacks materialize via _get_fallback, not in __init__).
prmartinow pushed a commit to prmartinow/hermes-agent that referenced this pull request Aug 26, 2026
…rd-on-failure

The salvaged fix (NousResearch#71593) rebuilds Telegram fallback pools lazily and
discards+aclose()s a pool on retryable connect failure (_reset_fallback),
bounding each at Limits(max_connections=8) as a setdefault default. The PR
shipped no test.

Add tests/gateway/test_telegram_fallback_pool_release_71593.py:
  * failed fallback pool is aclose()d and dropped from _fallbacks (the
    discard-on-failure path — reverting the _reset_fallback call fails it)
  * a recovered pool is retained, only the failed one discarded
  * _reset_fallback is a no-op when the pool was never built
  * caller-supplied limits win over the _POOL_LIMITS setdefault default
  * the max_connections=8 default applies when the caller omits limits

Update the eager-build assumptions in test_telegram_network.py to the new
lazy contract (fallbacks materialize via _get_fallback, not in __init__).
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
…rd-on-failure

The salvaged fix (NousResearch#71593) rebuilds Telegram fallback pools lazily and
discards+aclose()s a pool on retryable connect failure (_reset_fallback),
bounding each at Limits(max_connections=8) as a setdefault default. The PR
shipped no test.

Add tests/gateway/test_telegram_fallback_pool_release_71593.py:
  * failed fallback pool is aclose()d and dropped from _fallbacks (the
    discard-on-failure path — reverting the _reset_fallback call fails it)
  * a recovered pool is retained, only the failed one discarded
  * _reset_fallback is a no-op when the pool was never built
  * caller-supplied limits win over the _POOL_LIMITS setdefault default
  * the max_connections=8 default applies when the caller omits limits

Update the eager-build assumptions in test_telegram_network.py to the new
lazy contract (fallbacks materialize via _get_fallback, not in __init__).
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 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.

3 participants