-
Notifications
You must be signed in to change notification settings - Fork 4.8k
fix: enable TCP keepalive on default httpx client (NAT gateway hangs) #3287
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| import inspect | ||
| import logging | ||
| import platform | ||
| import socket | ||
| import warnings | ||
| import email.utils | ||
| from types import TracebackType | ||
|
|
@@ -831,11 +832,25 @@ def _idempotency_key(self) -> str: | |
| return f"stainless-python-retry-{uuid.uuid4()}" | ||
|
|
||
|
|
||
| def _get_default_socket_options() -> list[tuple[int, int, int]]: | ||
| """Return socket options to enable TCP keepalive for NAT/gateway environments.""" | ||
| # TCP keepalive: enables keep-alive packets to detect dead connections | ||
| # Needed for NAT gateways that drop idle connections after their timeout | ||
| if sys.platform == "darwin": | ||
| # macOS: TCP_KEEPALIVE = 16 | ||
| return [(socket.IPPROTO_TCP, socket.TCP_KEEPALIVE, 1)] | ||
| # Linux/others: TCP_KEEPIDLE (start keepalive after 60s idle) | ||
| return [(socket.IPPROTO_TCP, getattr(socket, "TCP_KEEPIDLE", 4), 60)] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For any non-macOS platform where Useful? React with 👍 / 👎. |
||
|
|
||
|
|
||
| class _DefaultHttpxClient(httpx.Client): | ||
| def __init__(self, **kwargs: Any) -> None: | ||
| kwargs.setdefault("timeout", DEFAULT_TIMEOUT) | ||
| kwargs.setdefault("limits", DEFAULT_CONNECTION_LIMITS) | ||
| kwargs.setdefault("follow_redirects", True) | ||
| # Enable TCP keepalive to prevent non-streaming calls from hanging | ||
| # behind NAT gateways that drop idle connections | ||
| kwargs.setdefault("socket_options", _get_default_socket_options()) | ||
| super().__init__(**kwargs) | ||
|
|
||
|
|
||
|
|
@@ -1423,6 +1438,7 @@ def __init__(self, **kwargs: Any) -> None: | |
| kwargs.setdefault("timeout", DEFAULT_TIMEOUT) | ||
| kwargs.setdefault("limits", DEFAULT_CONNECTION_LIMITS) | ||
| kwargs.setdefault("follow_redirects", True) | ||
| kwargs.setdefault("socket_options", _get_default_socket_options()) | ||
| super().__init__(**kwargs) | ||
|
|
||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This helper only sets
TCP_KEEPALIVE/TCP_KEEPIDLEbut never enablesSO_KEEPALIVE, so the socket-level keepalive mechanism remains off on Linux/macOS and the new NAT-hang mitigation is effectively not activated. In practice this means the change can leave idle pooled connections behaving exactly as before, so the targeted hang scenario is still possible.Useful? React with 👍 / 👎.