mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
fix(agent): replace custom socket_options transport with httpx pool-level keepalive expiry
The custom ``httpx.HTTPTransport(socket_options=[SO_KEEPALIVE, ...])`` in ``_build_keepalive_http_client()`` was introduced to fix CLOSE-WAIT socket accumulation on long-lived connections (#10324). That approach broke streaming for providers behind reverse proxies (OpenResty, Cloudflare, etc.) because the custom socket options conflict with the proxy's chunked-transfer handling (#54049, #12952). It also stripped TCP_NODELAY, stalling TLS handshakes and SSE encoding. Narrow per-provider bypasses were added for Copilot (#50298), Codex (#36623, #12953), but the root cause remained. The fix moves connection lifecycle management from the socket layer to the HTTP pool layer: - ``httpx.Limits(keepalive_expiry=20.0)`` tells httpx to close idle pooled connections at 20 s, before a reverse proxy's typical 30-60 s timeout drops them and causes CLOSE-WAIT accumulation. - The default httpx transport preserves OS TCP defaults (including TCP_NODELAY), so TLS handshakes and SSE chunked encoding work correctly. - ``trust_env=False`` prevents httpx from double-dipping on env vars (we handle proxy detection ourselves via ``_get_proxy_for_base_url`` which respects NO_PROXY). - The Copilot host bypass (line 3632) is no longer needed since all providers now use the same standard httpx.Client. Closes #54049. Supersedes #12010, #36623, #12953, #50298.
This commit is contained in:
David Metcalfe
Teknium
parent
e02cef0d0d
commit
8324dd19ca
2 changed files with 70 additions and 37 deletions
76
run_agent.py
76
run_agent.py
|
|
@ -3896,30 +3896,70 @@ class AIAgent:
|
|||
|
||||
@staticmethod
|
||||
def _build_keepalive_http_client(base_url: str = "", *, verify: Any = True) -> Any:
|
||||
"""Build an httpx.Client with proactive idle-connection reaping.
|
||||
|
||||
Previously this method injected a custom ``httpx.HTTPTransport``
|
||||
with ``socket_options`` (``SO_KEEPALIVE``, ``TCP_KEEPIDLE``, …) to
|
||||
prevent CLOSE-WAIT accumulation on long-lived connections (#10324).
|
||||
|
||||
That approach broke streaming for providers behind reverse proxies
|
||||
(OpenResty, Cloudflare, etc.) because the custom socket options
|
||||
conflict with the proxy's chunked-transfer handling (#54049,
|
||||
#12952). It also stripped ``TCP_NODELAY``, stalling TLS handshakes
|
||||
and SSE encoding.
|
||||
|
||||
The fix moves connection lifecycle management from the socket layer
|
||||
to the HTTP pool layer: ``keepalive_expiry=20.0`` tells httpx to
|
||||
close idle pooled connections *before* a reverse proxy's typical
|
||||
30–60 s timeout drops them, preventing CLOSE-WAIT accumulation
|
||||
without modifying socket options. The default httpx transport
|
||||
preserves OS TCP defaults (including ``TCP_NODELAY``).
|
||||
|
||||
``verify`` carries per-provider ``ssl_ca_cert`` / ``ssl_verify`` and
|
||||
``HERMES_CA_BUNDLE`` settings. It is passed on the client AND on
|
||||
the plain no-proxy mounts (a mounted transport owns the SSL context
|
||||
for its scheme).
|
||||
"""
|
||||
try:
|
||||
import httpx as _httpx
|
||||
import socket as _socket
|
||||
|
||||
if "api.githubcopilot.com" in str(base_url or "").lower():
|
||||
return _httpx.Client(verify=verify)
|
||||
|
||||
_sock_opts = [(_socket.SOL_SOCKET, _socket.SO_KEEPALIVE, 1)]
|
||||
if hasattr(_socket, "TCP_KEEPIDLE"):
|
||||
_sock_opts.append((_socket.IPPROTO_TCP, _socket.TCP_KEEPIDLE, 30))
|
||||
_sock_opts.append((_socket.IPPROTO_TCP, _socket.TCP_KEEPINTVL, 10))
|
||||
_sock_opts.append((_socket.IPPROTO_TCP, _socket.TCP_KEEPCNT, 3))
|
||||
elif hasattr(_socket, "TCP_KEEPALIVE"):
|
||||
_sock_opts.append((_socket.IPPROTO_TCP, _socket.TCP_KEEPALIVE, 30))
|
||||
# When a custom transport is provided, httpx won't auto-read proxy
|
||||
# from env vars (allow_env_proxies = trust_env and transport is None).
|
||||
# Explicitly read proxy settings while still honoring NO_PROXY for
|
||||
# loopback / local endpoints such as a locally hosted sub2api.
|
||||
# Explicitly read proxy settings so requests route through
|
||||
# HTTP_PROXY / HTTPS_PROXY / NO_PROXY correctly.
|
||||
_proxy = _get_proxy_for_base_url(base_url)
|
||||
# verify lives on the transport: httpx ignores the client-level
|
||||
# ``verify`` when a custom ``transport=`` is supplied.
|
||||
|
||||
# Proactive pool reaping: close idle connections at 20 s,
|
||||
# before reverse proxies (30–60 s typical) send FIN and
|
||||
# cause CLOSE-WAIT accumulation.
|
||||
_limits = _httpx.Limits(
|
||||
max_keepalive_connections=20,
|
||||
max_connections=100,
|
||||
keepalive_expiry=20.0,
|
||||
)
|
||||
|
||||
# Timeouts: generous read=None for SSE streaming endpoints.
|
||||
_timeout = _httpx.Timeout(
|
||||
connect=15.0,
|
||||
read=None,
|
||||
write=15.0,
|
||||
pool=10.0,
|
||||
)
|
||||
|
||||
# When _proxy is None (NO_PROXY bypass or no proxy configured),
|
||||
# mount plain transports to prevent httpx from reading env proxy
|
||||
# vars and creating an HTTPProxy mount that would bypass our
|
||||
# NO_PROXY resolution.
|
||||
_mounts = {}
|
||||
if _proxy is None:
|
||||
_mounts = {
|
||||
"http://": _httpx.HTTPTransport(verify=verify),
|
||||
"https://": _httpx.HTTPTransport(verify=verify),
|
||||
}
|
||||
return _httpx.Client(
|
||||
transport=_httpx.HTTPTransport(socket_options=_sock_opts, verify=verify),
|
||||
limits=_limits,
|
||||
timeout=_timeout,
|
||||
proxy=_proxy,
|
||||
mounts=_mounts or None,
|
||||
verify=verify,
|
||||
)
|
||||
except Exception:
|
||||
return None
|
||||
|
|
|
|||
|
|
@ -1,22 +1,14 @@
|
|||
"""Regression guard: _create_openai_client must honor HTTP(S)_PROXY env vars.
|
||||
|
||||
When #11277 re-landed TCP keepalives, ``_create_openai_client`` began passing
|
||||
a custom ``transport=httpx.HTTPTransport(...)`` to ``httpx.Client``. httpx only
|
||||
auto-reads ``HTTP_PROXY`` / ``HTTPS_PROXY`` / ``ALL_PROXY`` when
|
||||
``transport is None`` (see ``Client.__init__``:
|
||||
``allow_env_proxies = trust_env and transport is None``). As a result, proxy
|
||||
env vars were silently ignored for the primary chat client, causing requests
|
||||
to bypass local proxies (Clash, corporate egress, etc.) and hit upstream
|
||||
directly from the raw interface.
|
||||
The keepalive client now uses ``httpx.Limits(keepalive_expiry=20.0)``
|
||||
instead of a custom ``httpx.HTTPTransport(socket_options=...)`` to
|
||||
prevent CLOSE-WAIT accumulation. This avoids breaking streaming for
|
||||
providers behind reverse proxies (#54049, #12952) while still reaping
|
||||
idle connections before a proxy's timeout drops them.
|
||||
|
||||
For users on WSL2 + Clash TUN this surfaced as Cloudflare ``cf-mitigated:
|
||||
challenge`` 403s against ``chatgpt.com/backend-api/codex`` once they upgraded
|
||||
past #11277. The fix forwards the proxy URL explicitly to ``httpx.Client``
|
||||
while keeping the keepalive-enabled transport in place.
|
||||
|
||||
This test pins that the constructed ``httpx.Client`` mounts an ``HTTPProxy``
|
||||
pool when a proxy env var is set, AND that the socket-level keepalive
|
||||
transport is still installed on the no-proxy default path.
|
||||
This test pins that the constructed ``httpx.Client`` mounts an
|
||||
``HTTPProxy`` pool when a proxy env var is set, and that no
|
||||
custom socket-options transport is used (default httpx transport).
|
||||
"""
|
||||
from unittest.mock import patch
|
||||
|
||||
|
|
@ -117,8 +109,8 @@ def test_create_openai_client_routes_via_proxy_when_env_set(mock_openai, monkeyp
|
|||
|
||||
@patch("run_agent.OpenAI")
|
||||
def test_create_openai_client_no_proxy_when_env_unset(mock_openai, monkeypatch):
|
||||
"""Without proxy env vars, the keepalive transport must still be installed
|
||||
and no HTTPProxy mount should exist."""
|
||||
"""Without proxy env vars, no HTTPProxy mount should exist and
|
||||
no custom socket-options transport should be installed."""
|
||||
for key in ("HTTPS_PROXY", "HTTP_PROXY", "ALL_PROXY",
|
||||
"https_proxy", "http_proxy", "all_proxy"):
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
|
|
@ -147,7 +139,8 @@ def test_create_openai_client_no_proxy_when_env_unset(mock_openai, monkeypatch):
|
|||
|
||||
@patch("run_agent.OpenAI")
|
||||
def test_create_openai_client_uses_plain_httpx_client_for_copilot(mock_openai, monkeypatch):
|
||||
"""Copilot Claude chat-completions rejects the custom socket-options transport."""
|
||||
"""All providers now use a standard httpx.Client (no custom socket-options
|
||||
transport) so Copilot Claude chat-completions works without a host bypass."""
|
||||
for key in ("HTTPS_PROXY", "HTTP_PROXY", "ALL_PROXY",
|
||||
"https_proxy", "http_proxy", "all_proxy"):
|
||||
monkeypatch.delenv(key, raising=False)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue