fix: retry transient transport errors on a fresh async connection

[acrogenesis]

Problem

After 3.8.3 made transport failures legible, a recurring ReadError('') surfaced on UPSERT_KPI_DATA (femsa KPI_ENGINE_ON_DEMAND) as errors=[{'message': "ReadError('')"}]. An empty-message ReadError is the classic signature of a stale keep-alive connection: httpx reuses a pooled connection the server has already closed, so the next request fails instantly. async_execute only retried on a closed event loop and surfaced everything else, so one stale socket = one hard failure.

Fix

async_execute now drops the stale client and retries once on a fresh connection for transient transport errors:

TRANSIENT_TRANSPORT_ERRORS = (
    httpx.NetworkError,        # ReadError, WriteError, ConnectError, CloseError
    httpx.RemoteProtocolError, # server closed connection without a full response
    httpx.PoolTimeout,
    httpx.ConnectTimeout,
)

ReadTimeout is intentionally excluded — a genuinely slow request would just time out again and double the latency. The sync execute path already retried once on a fresh client, so only the async path changed.

Tests

tests/pygqlc/gql_client/test_transient_transport_retry.py: parametrized coverage of the retry predicate (ReadError/RemoteProtocolError/ConnectTimeout/PoolTimeout/ConnectError/event-loop → retry; ReadTimeout/generic → no), plus an integration test that async_execute retries on a fresh connection after a first-call ReadError and a negative test that ReadTimeout is not retried. 10 pass; existing timeout test still green; no new lint.

Context

This is the transport-layer root cause behind the KPI ReadError failures. The companion fix repairs the dead @retry (the retrying library is a no-op on async def) in the kpis package.

🤖 Generated with Claude Code

[palantir-valiot]

Overall Assessment

This PR adds retry logic to async_execute for transient transport errors (stale keep-alive sockets manifesting as ReadError(''), RemoteProtocolError, etc.), mirroring the existing sync execute behavior. The implementation is sound: a new _should_retry_on_fresh_connection predicate classifies errors, ReadTimeout is correctly excluded to avoid doubling latency on slow requests, and the retry path drops the stale client before creating a fresh one. No blocking bugs found.

Findings

No actionable findings.

Notes

• Test coverage is strong: the new test_transient_transport_retry.py provides parametrized predicate coverage plus an integration test that verifies the retry path (first ReadError triggers _drop_async_client + fresh client, second call succeeds) and a negative test confirming ReadTimeout is not retried.
• The docstring on _should_retry_on_fresh_connection and the module-level TRANSIENT_TRANSPORT_ERRORS comment clearly document the design rationale (why ReadTimeout is excluded, why NetworkError/RemoteProtocolError are included).
• The PR follows the repo's TDD and release conventions (test added, version bumped, CHANGELOG entry written).

[Copilot]

Pull request overview

This PR hardens GraphQLClient.async_execute against stale pooled HTTP connections by retrying exactly once on a freshly-created httpx.AsyncClient when specific transient transport errors occur, aligning the async path with the existing sync behavior.

Changes:

• Added TRANSIENT_TRANSPORT_ERRORS and _should_retry_on_fresh_connection() to centralize the async retry predicate.
• Updated async_execute to drop the stale async client and retry once on a new client for transient transport failures (excluding ReadTimeout).
• Added a new test module covering the retry predicate and async retry behavior; bumped version to 3.8.4 and documented the change in CHANGELOG.md.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
pygqlc/GraphQLClient.py	Adds transient transport error classification + a helper predicate, and retries async_execute once after dropping the async client.
tests/pygqlc/gql_client/test_transient_transport_retry.py	New tests for retry predicate and async retry/no-retry behavior.
pygqlc/__version__.py	Bumps package version to 3.8.4.
CHANGELOG.md	Documents the async fresh-connection retry behavior and rationale.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[acrogenesis]

Addressed @Copilot's review:

• Retry test: now uses two distinct mocks (stale → fresh) via _get_async_client side_effect, asserting stale.post/fresh.post each awaited once — so it actually proves a brand-new connection is used on retry.
• Misleading comment: reworded (the block intentionally retries ConnectTimeout/ConnectError); also trimmed the comments, helper docstring, and CHANGELOG entry for brevity.

[palantir-valiot]

Overall Assessment

This PR adds retry-on-fresh-connection logic to async_execute for transient transport errors (httpx.NetworkError, RemoteProtocolError, PoolTimeout, ConnectTimeout) plus the existing "Event loop is closed" case. The sync execute path already retried on any exception; the async path now does so selectively. Implementation and tests are sound; no blocking bugs found.

Findings

No actionable findings.

Notes

• The predicate _should_retry_on_fresh_connection is extracted as a static method with a clear docstring, and the TRANSIENT_TRANSPORT_ERRORS tuple is documented at the module level.
• ReadTimeout is correctly excluded from the retry set (it is caught by except httpx.RequestError but _should_retry... returns False, causing a re-raise); ConnectTimeout/PoolTimeout are included because they indicate connection establishment failure rather than request slowness.
• The new test file provides parametrized coverage of the predicate plus an integration test that verifies the stale→fresh client swap and a negative test for ReadTimeout. All tests are fast and hermetic (no real sockets).
• The sync execute retries on a bare Exception; the async change narrows this to a well-defined set while preserving the one-shot retry pattern.