From 09b106bb1591ec8f641b1ff98c8a4d261106a408 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9s=20S=C3=A1nchez?= Date: Thu, 6 Aug 2026 07:50:39 -0600 Subject: [PATCH] fix: retry the second cross-loop RuntimeError wording too (3.8.8) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 3.8.7 made the async client per event loop, which is what actually stops cross-loop reuse, and added 'is bound to a different event loop' to the retry-on-fresh-connection predicate as a defensive belt. That belt only covered one of the two wordings CPython emits: asyncio/mixins.py phrases it ' is bound to a different event loop' when a pool primitive is touched from the wrong loop, but asyncio/tasks.py raises 'Task ... got Future ... attached to a different loop' when a Task awaits another loop's Future. The second string matched no predicate, so it propagated out of async_execute and async_query/async_mutate folded it into a GraphQL error list, where it reads as an unexplained failure rather than a loop problem. Observed against 3.8.6 in a Temporal worker that runs each activity through its own asyncio.run — a fresh loop per run against a process-shared client. Both wordings now self-heal via drop-and-rebuild. Predicate cases cover both strings, an end-to-end test drives async_execute through the tasks.py wording, and a per-loop test pins that the dead loop's client is released without aclose() ever being awaited on it. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 4 ++ pygqlc/GraphQLClient.py | 3 ++ pygqlc/__version__.py | 2 +- .../gql_client/test_async_client_per_loop.py | 32 ++++++++++++-- .../test_transient_transport_retry.py | 42 +++++++++++++++++++ 5 files changed, 79 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f138f1..6d8263e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # CHANGELOG +## [3.8.8] - 2026-08-06 + +- [Fixed] The retry-on-fresh-connection predicate now also admits CPython's second cross-loop wording, `RuntimeError: Task ... got Future ... attached to a different loop` (`asyncio/tasks.py`), alongside 3.8.7's `is bound to a different event loop` (`asyncio/mixins.py`). 3.8.7's per-loop client cache is what actually prevents cross-loop reuse and remains the fix; this only completes the defensive belt, so any stale cross-loop state that still reaches a request self-heals via drop-and-rebuild instead of escaping to the caller — where `async_query`/`async_mutate` fold it into a GraphQL error list and it reads as an unexplained blank failure. Observed against 3.8.6 in a Temporal worker running each activity through its own `asyncio.run` (a fresh loop per run against a process-shared client). + ## [3.8.7] - 2026-07-30 - [Fixed] The cached async client is now PER EVENT LOOP. 3.8.6's shared client bound its httpx connection-pool primitives to whichever loop first used it; any consumer running coroutines on more than one loop — e.g. a Temporal worker's main loop plus a subscription callback thread using `asyncio.run` per event (valuechainos-queues' `trigger_by_subscription`) — then failed with `RuntimeError: is bound to a different event loop` (observed live as `Error processing workflow QUEUE_REPLENISHMENT_FOR_CSV_REPORT`). Each loop now gets (and reuses) its own client; per-loop reuse keeps 3.8.6's no-churn goal, `_close()` schedules `aclose()` on each client's own loop, and "is bound to a different event loop" joined the retryable-on-fresh-connection predicate as a defensive self-heal. diff --git a/pygqlc/GraphQLClient.py b/pygqlc/GraphQLClient.py index d705533..6d412db 100644 --- a/pygqlc/GraphQLClient.py +++ b/pygqlc/GraphQLClient.py @@ -1173,7 +1173,10 @@ def _should_retry_on_fresh_connection(error: Exception) -> bool: or "client has been closed" in msg # Defensive: cannot arise with per-loop clients, but if stale cross-loop # state ever surfaces, drop-and-rebuild self-heals instead of failing. + # Both wordings CPython emits: asyncio/mixins.py for a pool primitive, + # asyncio/tasks.py when a Task awaits another loop's Future. or "is bound to a different event loop" in msg + or "attached to a different loop" in msg ): return True return isinstance(error, TRANSIENT_TRANSPORT_ERRORS) diff --git a/pygqlc/__version__.py b/pygqlc/__version__.py index 08f7211..a48af93 100644 --- a/pygqlc/__version__.py +++ b/pygqlc/__version__.py @@ -1 +1 @@ -__version__ = "3.8.7" +__version__ = "3.8.8" diff --git a/tests/pygqlc/gql_client/test_async_client_per_loop.py b/tests/pygqlc/gql_client/test_async_client_per_loop.py index 6b3097c..dd1f870 100644 --- a/tests/pygqlc/gql_client/test_async_client_per_loop.py +++ b/tests/pygqlc/gql_client/test_async_client_per_loop.py @@ -40,7 +40,10 @@ def test_each_loop_gets_its_own_client(client): async def grab(): seen.append(await client._get_async_client()) - with patch("pygqlc.GraphQLClient.httpx.AsyncClient", side_effect=lambda **_: AsyncMock(is_closed=False)): + with patch( + "pygqlc.GraphQLClient.httpx.AsyncClient", + side_effect=lambda **_: AsyncMock(is_closed=False), + ): asyncio.run(grab()) asyncio.run(grab()) @@ -57,7 +60,8 @@ async def grab_twice(): seen.append(await client._get_async_client()) with patch( - "pygqlc.GraphQLClient.httpx.AsyncClient", side_effect=lambda **_: AsyncMock(is_closed=False) + "pygqlc.GraphQLClient.httpx.AsyncClient", + side_effect=lambda **_: AsyncMock(is_closed=False), ) as ctor: asyncio.run(grab_twice()) @@ -77,10 +81,32 @@ def make_client(**_): created.append(mock) return mock - with patch("pygqlc.GraphQLClient.httpx.AsyncClient", side_effect=make_client) as ctor: + with patch( + "pygqlc.GraphQLClient.httpx.AsyncClient", side_effect=make_client + ) as ctor: asyncio.run(client.async_execute("query { ok }")) asyncio.run(client.async_execute("query { ok }")) assert ctor.call_count == 2, "second loop must build its own client" created[0].post.assert_awaited_once() created[1].post.assert_awaited_once() + + +def test_stale_loops_client_is_released_without_awaiting_on_its_dead_loop(client): + """The first loop's client is dropped, not awaited: its loop is closed by the + time the second run starts, so any aclose() there would itself raise.""" + response = MagicMock(status_code=200, content=b'{"data": {"ok": true}}') + created = [] + + def make_client(**_): + mock = AsyncMock(is_closed=False) + mock.post.return_value = response + created.append(mock) + return mock + + with patch("pygqlc.GraphQLClient.httpx.AsyncClient", side_effect=make_client): + asyncio.run(client.async_execute("query { ok }")) + asyncio.run(client.async_execute("query { ok }")) + + created[0].aclose.assert_not_awaited() + assert created[0].post.await_count == 1, "dead loop's client must not be reused" diff --git a/tests/pygqlc/gql_client/test_transient_transport_retry.py b/tests/pygqlc/gql_client/test_transient_transport_retry.py index 7c43250..72d876c 100644 --- a/tests/pygqlc/gql_client/test_transient_transport_retry.py +++ b/tests/pygqlc/gql_client/test_transient_transport_retry.py @@ -43,6 +43,21 @@ def gql_env(): (httpx.ConnectError(""), True), (RuntimeError("Event loop is closed"), True), (RuntimeError("Cannot send a request, as the client has been closed."), True), + # Both cross-loop wordings CPython emits: asyncio/mixins.py phrases it + # "is bound to a different event loop", asyncio/tasks.py "attached to a + # different loop". Per-loop clients should prevent both; retrying is the belt. + ( + RuntimeError( + " is bound to a different event loop" + ), + True, + ), + ( + RuntimeError( + "Task got Future attached to a different loop" + ), + True, + ), (httpx.ReadTimeout(""), False), # genuine slow request — don't auto-retry (ValueError("nope"), False), ], @@ -95,6 +110,33 @@ async def test_async_execute_rebuilds_client_on_closed_event_loop(gql_env, monke dropped.assert_awaited_once() # whole client rebuilt for a dead event loop +@pytest.mark.asyncio +async def test_async_execute_rebuilds_client_on_cross_loop_future(gql_env, monkeypatch): + payload = {"data": {"createBulkThings": {"successful": True}}} + # Per-loop clients keep this from happening; if stale cross-loop state ever + # reaches a post anyway, the client is rebuilt rather than surfacing to the caller. + stale = AsyncMock() + stale.post = AsyncMock( + side_effect=RuntimeError( + "Task got Future attached to a different loop" + ) + ) + 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 + fresh.post.assert_awaited_once() + dropped.assert_awaited_once() + + @pytest.mark.asyncio async def test_async_execute_does_not_retry_read_timeout(gql_env, monkeypatch): client = AsyncMock()