From a40ccf708367d443139e9b83136ee2ca3eb6b31c Mon Sep 17 00:00:00 2001 From: Infrahub Date: Tue, 8 Sep 2026 15:56:27 +0000 Subject: [PATCH] feat(client): add connect_timeout to bound the connect phase The SDK passed a single integer timeout to httpx, which applied it to the connect phase as well, so an unreachable address consumed the whole request budget (60s by default) before failing. A new connect_timeout setting (default 10s, INFRAHUB_CONNECT_TIMEOUT) now bounds the TCP/TLS handshake on every request path, capped by the per-request timeout. Since the transient retry handler already treats a connect timeout as a lost connection, a client behind a DNS round robin or a failing-over load balancer now moves on to its next attempt after connect_timeout seconds instead of waiting out the full request timeout. Co-Authored-By: Claude Fable 5.1 --- changelog/+connect-timeout.added.md | 1 + docs/docs/python-sdk/reference/config.mdx | 11 +- infrahub_sdk/client.py | 28 +++- infrahub_sdk/config.py | 13 +- tests/unit/sdk/test_connect_timeout.py | 184 ++++++++++++++++++++++ 5 files changed, 229 insertions(+), 8 deletions(-) create mode 100644 changelog/+connect-timeout.added.md create mode 100644 tests/unit/sdk/test_connect_timeout.py diff --git a/changelog/+connect-timeout.added.md b/changelog/+connect-timeout.added.md new file mode 100644 index 00000000..30294599 --- /dev/null +++ b/changelog/+connect-timeout.added.md @@ -0,0 +1 @@ +Add the `connect_timeout` setting (`INFRAHUB_CONNECT_TIMEOUT`, default 10 seconds) so the TCP/TLS connection phase of every request is bounded separately from the request `timeout`. An unreachable address now fails fast instead of consuming the whole request budget, which lets endpoints published through DNS round robin fall over to the next address quickly. diff --git a/docs/docs/python-sdk/reference/config.mdx b/docs/docs/python-sdk/reference/config.mdx index 5ffe9c05..4e25eacb 100644 --- a/docs/docs/python-sdk/reference/config.mdx +++ b/docs/docs/python-sdk/reference/config.mdx @@ -234,12 +234,21 @@ The following settings can be defined in the `Config` class ## timeout -**Description**: Default connection timeout in seconds
+**Description**: Default request timeout in seconds, applied to the read, write and pool phases of a request.
**Type**: `integer`
**Default value**: 60
**Environment variable**: `INFRAHUB_TIMEOUT`
+## connect_timeout + + +**Description**: Timeout in seconds for establishing the TCP/TLS connection to Infrahub. Kept short so an unreachable address fails fast; it never exceeds the request timeout.
+**Type**: `integer`
+**Default value**: 10
+**Environment variable**: `INFRAHUB_CONNECT_TIMEOUT`
+ + ## transport diff --git a/infrahub_sdk/client.py b/infrahub_sdk/client.py index 51d3eca6..33a70091 100644 --- a/infrahub_sdk/client.py +++ b/infrahub_sdk/client.py @@ -380,6 +380,16 @@ def _merge_request_headers(self, headers: dict | None) -> dict: merged.update(headers) return merged + def _build_timeout(self, timeout: int) -> httpx.Timeout: + """Build the httpx timeout for a single request. + + ``timeout`` bounds the read, write and pool phases. The connect phase (TCP and TLS + handshake) is bounded by ``config.connect_timeout`` instead, so an unreachable address + fails fast rather than consuming the whole request budget, and the caller's ``timeout`` + remains an upper bound for it. + """ + return httpx.Timeout(timeout, connect=min(self.config.connect_timeout, timeout)) + @property def request_context(self) -> RequestContext | None: return self._request_context @@ -1615,7 +1625,9 @@ async def send() -> httpx.Response: _rewind_multipart_files(files) async with httpx.AsyncClient(**self._build_proxy_config(), verify=self.config.tls_context) as client: try: - return await client.post(url=url, headers=headers, timeout=timeout, files=files) + return await client.post( + url=url, headers=headers, timeout=self._build_timeout(timeout), files=files + ) except CONNECTION_LOST_EXCEPTIONS as exc: raise ServerNotReachableError(address=self.address) from exc except httpx.TimeoutException as exc: @@ -1722,7 +1734,9 @@ async def send() -> httpx.Response: stack = AsyncExitStack() try: response = await stack.enter_async_context( - client.stream(method="GET", url=url, headers=headers, timeout=request_timeout) + client.stream( + method="GET", url=url, headers=headers, timeout=self._build_timeout(request_timeout) + ) ) except CONNECTION_LOST_EXCEPTIONS as exc: raise ServerNotReachableError(address=self.address) from exc @@ -1793,7 +1807,7 @@ async def _default_request_method( method=method.value, url=url, headers=headers, - timeout=timeout, + timeout=self._build_timeout(timeout), **params, ) except CONNECTION_LOST_EXCEPTIONS as exc: @@ -2652,7 +2666,7 @@ def send() -> httpx.Response: _rewind_multipart_files(files) with httpx.Client(**self._build_proxy_config(), verify=self.config.tls_context) as client: try: - return client.post(url=url, headers=headers, timeout=timeout, files=files) + return client.post(url=url, headers=headers, timeout=self._build_timeout(timeout), files=files) except CONNECTION_LOST_EXCEPTIONS as exc: raise ServerNotReachableError(address=self.address) from exc except httpx.TimeoutException as exc: @@ -3910,7 +3924,9 @@ def send() -> httpx.Response: stack = ExitStack() try: response = stack.enter_context( - client.stream(method="GET", url=url, headers=headers, timeout=request_timeout) + client.stream( + method="GET", url=url, headers=headers, timeout=self._build_timeout(request_timeout) + ) ) except CONNECTION_LOST_EXCEPTIONS as exc: raise ServerNotReachableError(address=self.address) from exc @@ -4013,7 +4029,7 @@ def _default_request_method( method=method.value, url=url, headers=headers, - timeout=timeout, + timeout=self._build_timeout(timeout), **params, ) except CONNECTION_LOST_EXCEPTIONS as exc: diff --git a/infrahub_sdk/config.py b/infrahub_sdk/config.py index b5c5e1ae..9ec43b4e 100644 --- a/infrahub_sdk/config.py +++ b/infrahub_sdk/config.py @@ -122,7 +122,18 @@ class ConfigBase(BaseSettings): schema_converge_timeout: int = Field( default=60, description="Number of seconds to wait for schema to have converged" ) - timeout: int = Field(default=60, description="Default connection timeout in seconds") + timeout: int = Field( + default=60, + description="Default request timeout in seconds, applied to the read, write and pool phases of a request.", + ) + connect_timeout: int = Field( + default=10, + gt=0, + description=( + "Timeout in seconds for establishing the TCP/TLS connection to Infrahub. Kept short so an " + "unreachable address fails fast; it never exceeds the request timeout." + ), + ) transport: RequesterTransport = Field( default=RequesterTransport.HTTPX, description="Set an alternate transport using a predefined option" ) diff --git a/tests/unit/sdk/test_connect_timeout.py b/tests/unit/sdk/test_connect_timeout.py new file mode 100644 index 00000000..dfd5555f --- /dev/null +++ b/tests/unit/sdk/test_connect_timeout.py @@ -0,0 +1,184 @@ +"""Connect-phase timeout handling on the async and sync clients. + +Covers the split connect/request timeout handed to httpx, the cap of the connect timeout by the +per-request timeout, and that a connect timeout still surfaces as ``ServerNotReachableError`` on every +request path and is retried by ``retry_on_failure``, so the shorter connect phase composes with retries. +""" + +from __future__ import annotations + +import asyncio +import io +import time +from typing import TYPE_CHECKING + +import httpx +import pytest +from pydantic import ValidationError + +from infrahub_sdk import Config, InfrahubClient, InfrahubClientSync +from infrahub_sdk.exceptions import ServerNotReachableError +from infrahub_sdk.types import HTTPMethod + +if TYPE_CHECKING: + from pytest_httpx import HTTPXMock + +CLIENT_TYPES = ["standard", "sync"] + +# ``_request_multipart`` and ``_get_streaming`` build their own httpx client, so each path is driven +# separately to prove they all share the same timeout and error handling. +REQUEST_PATHS = ["regular", "multipart", "streaming"] + +URL = "http://mock/graphql/main" + + +def _make_client( + client_type: str, *, connect_timeout: int = 10, retry_on_failure: bool = False +) -> InfrahubClient | InfrahubClientSync: + """Build a client with no ``requester`` override so the real (mocked) httpx transport is used.""" + cfg = Config(address="http://mock", connect_timeout=connect_timeout, retry_on_failure=retry_on_failure) + if client_type == "standard": + return InfrahubClient(config=cfg) + return InfrahubClientSync(config=cfg) + + +def _patch_sleep(monkeypatch: pytest.MonkeyPatch) -> list[float]: + """Replace the retry handler's async and sync sleeps with recorders that never wait.""" + recorded: list[float] = [] + + async def fake_async_sleep(delay: float) -> None: + recorded.append(delay) + + def fake_sync_sleep(delay: float) -> None: + recorded.append(delay) + + monkeypatch.setattr(asyncio, "sleep", fake_async_sleep) + monkeypatch.setattr(time, "sleep", fake_sync_sleep) + return recorded + + +async def _drive_path(client: InfrahubClient | InfrahubClientSync, path: str, timeout: int) -> httpx.Response: + """Send one request on the selected path and return the (fully read) response.""" + if path == "regular": + if isinstance(client, InfrahubClient): + return await client._request(url=URL, method=HTTPMethod.POST, headers={}, timeout=timeout, payload={}) + return client._request(url=URL, method=HTTPMethod.POST, headers={}, timeout=timeout, payload={}) + + if path == "multipart": + files = {"file": ("upload.bin", io.BytesIO(b"file body"), "application/octet-stream")} + if isinstance(client, InfrahubClient): + return await client._request_multipart(url=URL, headers={}, timeout=timeout, files=files) + return client._request_multipart(url=URL, headers={}, timeout=timeout, files=files) + + if isinstance(client, InfrahubClient): + async with client._get_streaming(url=URL, timeout=timeout) as response: + await response.aread() + return response + with client._get_streaming(url=URL, timeout=timeout) as response: + response.read() + return response + + +# --- Configuration ------------------------------------------------------------------------------ + + +def test_default_connect_timeout_is_shorter_than_request_timeout() -> None: + config = Config(address="http://mock") + + assert config.connect_timeout == 10 + assert config.connect_timeout < config.timeout + + +def test_connect_timeout_from_env_var(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("INFRAHUB_CONNECT_TIMEOUT", "3") + + assert Config(address="http://mock").connect_timeout == 3 + + +def test_connect_timeout_must_be_positive() -> None: + with pytest.raises(ValidationError): + Config(address="http://mock", connect_timeout=0) + + +# --- Timeout handed to httpx ------------------------------------------------------------------- + + +@pytest.mark.parametrize("path", REQUEST_PATHS) +@pytest.mark.parametrize("client_type", CLIENT_TYPES) +async def test_request_uses_connect_timeout_for_connect_phase_only( + client_type: str, path: str, httpx_mock: HTTPXMock +) -> None: + """Every request path bounds the connect phase with ``connect_timeout`` and the rest with ``timeout``.""" + httpx_mock.add_response(status_code=200, json={"data": {}}) + client = _make_client(client_type, connect_timeout=4) + + await _drive_path(client=client, path=path, timeout=30) + + request = httpx_mock.get_request() + assert request is not None + assert request.extensions["timeout"] == {"connect": 4.0, "read": 30.0, "write": 30.0, "pool": 30.0} + + +@pytest.mark.parametrize("client_type", CLIENT_TYPES) +async def test_connect_timeout_is_capped_by_request_timeout(client_type: str, httpx_mock: HTTPXMock) -> None: + """A per-request ``timeout`` shorter than ``connect_timeout`` also bounds the connect phase.""" + httpx_mock.add_response(status_code=200, json={"data": {}}) + client = _make_client(client_type, connect_timeout=10) + + await _drive_path(client=client, path="regular", timeout=3) + + request = httpx_mock.get_request() + assert request is not None + assert request.extensions["timeout"]["connect"] == pytest.approx(3.0) + + +# --- ConnectTimeout is a connection failure ----------------------------------------------------- + + +@pytest.mark.parametrize("path", REQUEST_PATHS) +@pytest.mark.parametrize("client_type", CLIENT_TYPES) +async def test_connect_timeout_raises_server_not_reachable(client_type: str, path: str, httpx_mock: HTTPXMock) -> None: + """``httpx.ConnectTimeout`` surfaces as ``ServerNotReachableError`` on every request path.""" + httpx_mock.add_exception(httpx.ConnectTimeout("timed out")) + client = _make_client(client_type) + + with pytest.raises(ServerNotReachableError) as excinfo: + await _drive_path(client=client, path=path, timeout=10) + + assert isinstance(excinfo.value.__cause__, httpx.ConnectTimeout) + assert excinfo.value.address == "http://mock" + + +@pytest.mark.parametrize("client_type", CLIENT_TYPES) +async def test_execute_graphql_retries_after_connect_timeout( + client_type: str, httpx_mock: HTTPXMock, monkeypatch: pytest.MonkeyPatch +) -> None: + """With ``retry_on_failure`` enabled, a connect timeout is retried and the next attempt succeeds.""" + recorded_sleeps = _patch_sleep(monkeypatch) + httpx_mock.add_exception(httpx.ConnectTimeout("timed out")) + httpx_mock.add_response(status_code=200, json={"data": {"ok": True}}) + client = _make_client(client_type, retry_on_failure=True) + + if isinstance(client, InfrahubClient): + data = await client.execute_graphql(query="query { ok }") + else: + data = client.execute_graphql(query="query { ok }") + + assert data == {"ok": True} + assert len(httpx_mock.get_requests()) == 2 + assert len(recorded_sleeps) == 1 + + +@pytest.mark.parametrize("client_type", CLIENT_TYPES) +async def test_execute_graphql_surfaces_connect_timeout_without_retry(client_type: str, httpx_mock: HTTPXMock) -> None: + """Without ``retry_on_failure``, a connect timeout is raised once as ``ServerNotReachableError``.""" + httpx_mock.add_exception(httpx.ConnectTimeout("timed out")) + client = _make_client(client_type) + + with pytest.raises(ServerNotReachableError): + if isinstance(client, InfrahubClient): + await client.execute_graphql(query="query { ok }") + else: + client.execute_graphql(query="query { ok }") + + assert len(httpx_mock.get_requests()) == 1