From 957cf955cefc3049b828ed6a33342824dd8d21b5 Mon Sep 17 00:00:00 2001 From: acrogenesis Date: Wed, 24 Jun 2026 20:01:53 -0600 Subject: [PATCH 1/2] fix: retry transient transport errors on a fresh async connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit httpx reuses pooled keep-alive connections; when the server has already closed one, the next async request fails immediately with a transient transport error — commonly `ReadError('')` (empty message), also `RemoteProtocolError`, `ConnectError`, pool/connect timeouts. `async_execute` previously retried only on a closed event loop and surfaced everything else, so a single stale socket became a hard failure (after 3.8.3, visible as `[{'message': "ReadError('')"}]`). It now drops the stale client and retries once on a fresh connection for those transient transport errors. `ReadTimeout` is intentionally excluded — a genuinely slow request would just time out again. New `_should_retry_on_fresh_connection` helper + `TRANSIENT_TRANSPORT_ERRORS`; sync `execute` already had this behavior. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Uhj6eA8d9jcAzzHuzqtPod --- CHANGELOG.md | 4 + pygqlc/GraphQLClient.py | 54 ++++++++---- pygqlc/__version__.py | 2 +- .../test_transient_transport_retry.py | 83 +++++++++++++++++++ 4 files changed, 127 insertions(+), 16 deletions(-) create mode 100644 tests/pygqlc/gql_client/test_transient_transport_retry.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0065b77..8dcfe64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # CHANGELOG +## [3.8.4] - 2026-06-25 + +- [Fixed] `async_execute` now retries once on a fresh connection when a request fails with a transient transport error (`httpx.NetworkError` incl. `ReadError`/`ConnectError`, `RemoteProtocolError`, `PoolTimeout`, `ConnectTimeout`), not only on a closed event loop. httpx reuses pooled keep-alive connections; when the server has already closed one, the next request fails immediately — commonly `ReadError('')` with an empty message. Previously that surfaced to the caller (after 3.8.3, as `errors=[{'message': "ReadError('')"}]`); now the stale client is dropped and the request is retried once on a new connection. `ReadTimeout` is deliberately NOT retried (a genuinely slow request would just time out again). Added `_should_retry_on_fresh_connection` helper + `TRANSIENT_TRANSPORT_ERRORS`. The sync `execute` path already retried once on a fresh client, so only the async path changed. + ## [3.8.3] - 2026-06-24 - [Fixed] `query`, `mutate`, `async_query`, and `async_mutate` no longer collapse a raised transport exception into `[{"message": ""}]`. httpx timeout and connection-reset exceptions carry an empty message (`str(httpx.ReadTimeout(""))` is `""`), so the previous `errors = [{"message": str(e)}]` surfaced a blank, undiagnosable error — most visibly on `async_mutate`, where async httpx timeouts always stringify to "". A new `exception_errors` helper falls back to `repr(error)` when the message is blank, preserving the exception class name (e.g. `ReadTimeout('')`) so the caller can tell what actually failed. Added an integration test that drives a real `GraphQLClient` against a local HTTP server stalled past `post_timeout`. diff --git a/pygqlc/GraphQLClient.py b/pygqlc/GraphQLClient.py index 801af56..8250f0a 100644 --- a/pygqlc/GraphQLClient.py +++ b/pygqlc/GraphQLClient.py @@ -39,6 +39,18 @@ websocket.WebSocketConnectionClosedException, ) +# HTTP transport failures where the connection in hand is dead/unusable — most +# commonly a stale keep-alive socket the server already closed, which surfaces as +# ReadError/RemoteProtocolError (often with an empty message). A single retry on a +# fresh connection typically succeeds. ReadTimeout is intentionally excluded: a +# genuinely slow request would just time out again, doubling the wait. +TRANSIENT_TRANSPORT_ERRORS = ( + httpx.NetworkError, # ReadError, WriteError, ConnectError, CloseError + httpx.RemoteProtocolError, # server closed the connection without a full response + httpx.PoolTimeout, + httpx.ConnectTimeout, +) + # * Custom Exception class for GraphQL responses @@ -1159,6 +1171,18 @@ async def _drop_async_client(self): # Loop gone / client partially torn down — nothing more we can do log(LogLevel.WARNING, f"Warning: Error closing async client: {str(e)}") + @staticmethod + def _should_retry_on_fresh_connection(error: Exception) -> bool: + """Whether `error` warrants one retry on a brand-new connection. + + True for a closed event loop and for transient transport failures + (`TRANSIENT_TRANSPORT_ERRORS`) — both mean the current connection is + unusable but a fresh one is likely to succeed. + """ + if "Event loop is closed" in str(error): + return True + return isinstance(error, TRANSIENT_TRANSPORT_ERRORS) + async def async_execute(self, query: str, variables: dict | None = None) -> dict: """Async version of execute method that executes instructions of a query or mutation. @@ -1196,22 +1220,22 @@ async def async_execute(self, query: str, variables: dict | None = None) -> dict timeout=float(env.get("post_timeout", 60)), ) except (httpx.RequestError, RuntimeError) as e: - # Check if this is an event loop issue or a network issue - if "Event loop is closed" in str(e): - # Event loop was closed - need to get a new client with a valid loop - # Best-effort close the stale client instead of leaking its transports - await self._drop_async_client() - # Try again with a new client - client = await self._get_async_client() - response = await client.post( - env["url"], - json=data, - headers=headers, - timeout=float(env.get("post_timeout", 60)), - ) - else: - # Some other request error, re-raise + # Retry once on a fresh connection when the failure is a closed event + # loop or a transient transport error (e.g. a stale keep-alive socket + # the server already closed, surfacing as ReadError/RemoteProtocolError). + # Anything else (real timeouts, genuine network outages) re-raises. + if not self._should_retry_on_fresh_connection(e): raise + # Best-effort close the stale client instead of leaking its transports, + # then retry once with a brand-new client/connection. + await self._drop_async_client() + client = await self._get_async_client() + response = await client.post( + env["url"], + json=data, + headers=headers, + timeout=float(env.get("post_timeout", 60)), + ) if response.status_code == 200: return orjson.loads(response.content) diff --git a/pygqlc/__version__.py b/pygqlc/__version__.py index 429dcfe..9fd9cda 100644 --- a/pygqlc/__version__.py +++ b/pygqlc/__version__.py @@ -1 +1 @@ -__version__ = "3.8.3" +__version__ = "3.8.4" diff --git a/tests/pygqlc/gql_client/test_transient_transport_retry.py b/tests/pygqlc/gql_client/test_transient_transport_retry.py new file mode 100644 index 0000000..945883a --- /dev/null +++ b/tests/pygqlc/gql_client/test_transient_transport_retry.py @@ -0,0 +1,83 @@ +"""A stale/closed connection must be retried once on a fresh connection. + +httpx reuses pooled keep-alive connections; when the server has already closed +one, the next request on it fails immediately with a transient transport error +(commonly `ReadError`/`RemoteProtocolError`, often with an empty message). A +single retry on a brand-new connection succeeds. `async_execute` must do that +retry instead of surfacing the error to the caller. +""" + +from unittest.mock import AsyncMock + +import httpx +import orjson +import pytest + +from pygqlc import GraphQLClient + + +def _fake_response(payload): + resp = httpx.Response(status_code=200, content=orjson.dumps(payload)) + return resp + + +@pytest.fixture +def gql_env(): + gql = GraphQLClient() + gql.addEnvironment( + "retry-test", + url="http://127.0.0.1:1/api", + wss="ws://127.0.0.1:1/socket/websocket", + headers={"Authorization": "Bearer test"}, + post_timeout=5, + default=True, + ) + return gql + + +@pytest.mark.parametrize( + "error,expected", + [ + (httpx.ReadError(""), True), + (httpx.RemoteProtocolError(""), True), + (httpx.ConnectTimeout(""), True), + (httpx.PoolTimeout(""), True), + (httpx.ConnectError(""), True), + (RuntimeError("Event loop is closed"), True), + (httpx.ReadTimeout(""), False), # genuine slow request — don't auto-retry + (ValueError("nope"), False), + ], +) +def test_should_retry_on_fresh_connection(error, expected): + assert GraphQLClient._should_retry_on_fresh_connection(error) is expected + + +@pytest.mark.asyncio +async def test_async_execute_retries_once_on_stale_connection(gql_env, monkeypatch): + payload = {"data": {"createBulkThings": {"successful": True}}} + client = AsyncMock() + # First post hits a stale socket (ReadError); the retry on a fresh client succeeds. + client.post = AsyncMock(side_effect=[httpx.ReadError(""), _fake_response(payload)]) + + monkeypatch.setattr(gql_env, "_get_async_client", AsyncMock(return_value=client)) + dropped = AsyncMock() + monkeypatch.setattr(gql_env, "_drop_async_client", dropped) + + result = await gql_env.async_execute("query { things { id } }") + + assert result == payload + assert client.post.call_count == 2 # original + one retry + dropped.assert_awaited_once() # stale client was dropped before retrying + + +@pytest.mark.asyncio +async def test_async_execute_does_not_retry_read_timeout(gql_env, monkeypatch): + client = AsyncMock() + client.post = AsyncMock(side_effect=httpx.ReadTimeout("")) + monkeypatch.setattr(gql_env, "_get_async_client", AsyncMock(return_value=client)) + monkeypatch.setattr(gql_env, "_drop_async_client", AsyncMock()) + + with pytest.raises(httpx.ReadTimeout): + await gql_env.async_execute("query { things { id } }") + + assert client.post.call_count == 1 # not retried From 60793f2e4af89380760e5d5011a53fbe6022bd55 Mon Sep 17 00:00:00 2001 From: acrogenesis Date: Wed, 24 Jun 2026 20:18:10 -0600 Subject: [PATCH 2/2] review: tighten retry test (distinct stale/fresh clients), trim comments/docs Address Copilot review on #77: - async_execute retry test now uses two distinct clients (stale -> fresh) so it actually asserts a brand-new connection is used on retry. - Fix misleading inline comment (the block does retry ConnectTimeout/ConnectError); shortened the comments, helper docstring, and CHANGELOG entry. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01Uhj6eA8d9jcAzzHuzqtPod --- CHANGELOG.md | 2 +- pygqlc/GraphQLClient.py | 27 ++++++----------- .../test_transient_transport_retry.py | 29 ++++++++++--------- 3 files changed, 26 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8dcfe64..27d3a24 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## [3.8.4] - 2026-06-25 -- [Fixed] `async_execute` now retries once on a fresh connection when a request fails with a transient transport error (`httpx.NetworkError` incl. `ReadError`/`ConnectError`, `RemoteProtocolError`, `PoolTimeout`, `ConnectTimeout`), not only on a closed event loop. httpx reuses pooled keep-alive connections; when the server has already closed one, the next request fails immediately — commonly `ReadError('')` with an empty message. Previously that surfaced to the caller (after 3.8.3, as `errors=[{'message': "ReadError('')"}]`); now the stale client is dropped and the request is retried once on a new connection. `ReadTimeout` is deliberately NOT retried (a genuinely slow request would just time out again). Added `_should_retry_on_fresh_connection` helper + `TRANSIENT_TRANSPORT_ERRORS`. The sync `execute` path already retried once on a fresh client, so only the async path changed. +- [Fixed] `async_execute` retries once on a fresh connection for transient transport errors (`httpx.NetworkError`/`ReadError`, `RemoteProtocolError`, `PoolTimeout`, `ConnectTimeout`) — typically a stale keep-alive socket the server closed, surfacing as `ReadError('')`. `ReadTimeout` is excluded (a slow request would just time out again). Sync `execute` already behaved this way. ## [3.8.3] - 2026-06-24 diff --git a/pygqlc/GraphQLClient.py b/pygqlc/GraphQLClient.py index 8250f0a..29d8b51 100644 --- a/pygqlc/GraphQLClient.py +++ b/pygqlc/GraphQLClient.py @@ -39,14 +39,13 @@ websocket.WebSocketConnectionClosedException, ) -# HTTP transport failures where the connection in hand is dead/unusable — most -# commonly a stale keep-alive socket the server already closed, which surfaces as -# ReadError/RemoteProtocolError (often with an empty message). A single retry on a -# fresh connection typically succeeds. ReadTimeout is intentionally excluded: a -# genuinely slow request would just time out again, doubling the wait. +# Transport failures where the connection is dead but a fresh one will likely +# work — commonly a stale keep-alive socket (surfaces as ReadError('')). Retried +# once on a new connection. ReadTimeout is excluded: a slow request would just +# time out again. TRANSIENT_TRANSPORT_ERRORS = ( httpx.NetworkError, # ReadError, WriteError, ConnectError, CloseError - httpx.RemoteProtocolError, # server closed the connection without a full response + httpx.RemoteProtocolError, httpx.PoolTimeout, httpx.ConnectTimeout, ) @@ -1173,12 +1172,8 @@ async def _drop_async_client(self): @staticmethod def _should_retry_on_fresh_connection(error: Exception) -> bool: - """Whether `error` warrants one retry on a brand-new connection. - - True for a closed event loop and for transient transport failures - (`TRANSIENT_TRANSPORT_ERRORS`) — both mean the current connection is - unusable but a fresh one is likely to succeed. - """ + """True when the connection is unusable but a fresh one should work: + a closed event loop or a transient transport error.""" if "Event loop is closed" in str(error): return True return isinstance(error, TRANSIENT_TRANSPORT_ERRORS) @@ -1220,14 +1215,10 @@ async def async_execute(self, query: str, variables: dict | None = None) -> dict timeout=float(env.get("post_timeout", 60)), ) except (httpx.RequestError, RuntimeError) as e: - # Retry once on a fresh connection when the failure is a closed event - # loop or a transient transport error (e.g. a stale keep-alive socket - # the server already closed, surfacing as ReadError/RemoteProtocolError). - # Anything else (real timeouts, genuine network outages) re-raises. + # Stale keep-alive socket or closed loop: drop the client and retry + # once on a fresh connection. ReadTimeout and others re-raise. if not self._should_retry_on_fresh_connection(e): raise - # Best-effort close the stale client instead of leaking its transports, - # then retry once with a brand-new client/connection. await self._drop_async_client() client = await self._get_async_client() response = await client.post( diff --git a/tests/pygqlc/gql_client/test_transient_transport_retry.py b/tests/pygqlc/gql_client/test_transient_transport_retry.py index 945883a..17f0abb 100644 --- a/tests/pygqlc/gql_client/test_transient_transport_retry.py +++ b/tests/pygqlc/gql_client/test_transient_transport_retry.py @@ -1,10 +1,8 @@ -"""A stale/closed connection must be retried once on a fresh connection. +"""A stale/closed pooled connection must be retried once on a fresh one. -httpx reuses pooled keep-alive connections; when the server has already closed -one, the next request on it fails immediately with a transient transport error -(commonly `ReadError`/`RemoteProtocolError`, often with an empty message). A -single retry on a brand-new connection succeeds. `async_execute` must do that -retry instead of surfacing the error to the caller. +When the server has closed a pooled keep-alive connection, the next request +fails with a transient transport error (e.g. `ReadError('')`). `async_execute` +should retry once on a new connection instead of surfacing it. """ from unittest.mock import AsyncMock @@ -55,19 +53,24 @@ def test_should_retry_on_fresh_connection(error, expected): @pytest.mark.asyncio async def test_async_execute_retries_once_on_stale_connection(gql_env, monkeypatch): payload = {"data": {"createBulkThings": {"successful": True}}} - client = AsyncMock() - # First post hits a stale socket (ReadError); the retry on a fresh client succeeds. - client.post = AsyncMock(side_effect=[httpx.ReadError(""), _fake_response(payload)]) - - monkeypatch.setattr(gql_env, "_get_async_client", AsyncMock(return_value=client)) + # Two distinct clients: the stale one fails, a brand-new one is used on retry. + stale = AsyncMock() + stale.post = AsyncMock(side_effect=httpx.ReadError("")) + fresh = AsyncMock() + fresh.post = AsyncMock(return_value=_fake_response(payload)) + + monkeypatch.setattr( + gql_env, "_get_async_client", AsyncMock(side_effect=[stale, fresh]) + ) dropped = AsyncMock() monkeypatch.setattr(gql_env, "_drop_async_client", dropped) result = await gql_env.async_execute("query { things { id } }") assert result == payload - assert client.post.call_count == 2 # original + one retry - dropped.assert_awaited_once() # stale client was dropped before retrying + stale.post.assert_awaited_once() # failed on the stale connection + fresh.post.assert_awaited_once() # retried on a brand-new connection + dropped.assert_awaited_once() # stale client dropped before retrying @pytest.mark.asyncio