feat(client): retry transient failures, optionally indefinitely by fatih-acar · Pull Request #1323 · opsmill/infrahub-sdk-python

fatih-acar · 2026-09-04T07:41:56Z

Summary

Generators and other long-running SDK jobs abort as soon as a single GraphQL call hits a transient infrastructure error, even though the task worker already runs the client with retry_on_failure=True. That switch only covered connection errors inside execute_graphql, gave up after 5 minutes, spun without any delay on HTTP 5xx, and never looked at REST calls or at GraphQL error envelopes. This PR turns retry_on_failure into a real transient-error policy and lets operators opt into retrying indefinitely, so a multi-hour generator survives a database failover or a network outage instead of failing.

Key changes

With retry_on_failure enabled, connection errors, read timeouts, HTTP 500/502/503/504 responses and GraphQL errors the server flags with one of those statuses are retried. Everything else still fails fast, so a bad query or a schema error surfaces immediately even in unlimited mode.
A connection dropped before any response arrives is retried too. httpx reports that as RemoteProtocolError rather than a network error, so it used to escape the retry handler entirely; it is now mapped to ServerNotReachableError like any other lost connection, on all six transport paths of both clients. See the section below for how this was found.
Retries now cover every request path: GraphQL queries and mutations, the generator's query_gql_query data collection, schema loading and the other REST endpoints. The GraphQL-envelope retry shares one time budget and attempt counter with the transport layer.
Exponential backoff with jitter replaces the fixed delay. retry_delay is the base and the new retry_max_delay caps it. This also fixes the 5xx busy loop.
max_retry_duration=0 now means retry indefinitely. On the task worker, INFRAHUB_MAX_RETRY_DURATION=0 is enough to opt in because the worker already sets retry_on_failure=True.
The new retry_status_codes setting controls which statuses count as transient. 500 is included because Infrahub reports some transient database errors (Neo4j transient or session errors exhausting the server-side retries) without further classification.
Every retry is logged with the attempt number and elapsed time, escalating from WARNING to ERROR after five minutes so an indefinitely retrying job stays visible.
Once the budget is exhausted the original error is raised instead of a generic "resp hasn't been initialized" error.
client.retry_on_failure and client.retry_delay become properties so they can still be toggled at runtime, for example by a generator.

Verified against a real failover

The policy was exercised against a real Infrahub deployment from infrahub-testcontainers (two API replicas behind HAProxy, plus Neo4j, RabbitMQ, Redis and Prefect) rather than against mocks alone. The server's /api/response-delay endpoint makes every GraphQL request sit for 10 seconds, which is a wide enough window to kill a container while a mutation is genuinely in flight.

That found the feature aborting on the exact scenario it exists for. With retry_on_failure=True and max_retry_duration=0, restarting HAProxy during a mutation killed the operation after a single attempt:

MUTATION FAILED after 10.2s: httpx.RemoteProtocolError: Server disconnected without sending a response.

A load balancer that goes away mid-request closes the socket before sending a single byte of the response, and httpx reports that as RemoteProtocolError, a ProtocolError rather than a NetworkError. Only NetworkError and ReadTimeout were mapped to the SDK exceptions the retry handler treats as transient, so a failover bypassed the handler entirely: zero retries, unlimited budget notwithstanding. With the mapping added here, the same failover only delays the mutation:

WARNING infrahub_sdk: Transient failure on .../graphql/main: Unable to connect to 'http://localhost:18000'.
                     Retry 1 in 0.9s (10s elapsed, no time limit)
INFO  httpx: HTTP Request: POST .../graphql/main "HTTP/1.1 200 OK"

A failover reaches the client in one of two shapes, depending on which side of the load balancer goes away. Both are now covered:

Restarted	What the client sees	Retried by
HAProxy	The connection dropped before any response (`RemoteProtocolError`)	The mapping added in this PR
The API servers behind it	HTTP 502 for the request in flight, 503 while they boot	`retry_status_codes`, which already worked

The at-least-once caveat below turned out not to be theoretical. The first run used a plain create and the retry came back with Violates uniqueness constraint 'name': the server keeps processing a request whose client has gone away, so attempt one had already written the node before the socket closed. The integration tests use save(allow_upsert=True) for that reason.

Caveats

A mutation whose connection was lost may have been applied by the server before the retry, as observed above. Upserts are idempotent. A bare create that already succeeded fails on retry with a non-transient error, which is raised.
Because 500 is in the default set, a genuine bug that returns 500 is retried until the budget expires (5 minutes by default). Remove 500 from retry_status_codes to fail fast on it.
A per-generator opt-in in .infrahub.yml needs an Infrahub-side change to forward the flag to the client and is left as a follow-up.

Documentation

docs/docs/python-sdk/guides/client.mdx: new "Retry transient failures" section under advanced use cases, listing a dropped connection among the transient failures.
Regenerated docs/docs/python-sdk/reference/config.mdx and docs/docs/python-sdk/sdk_ref/infrahub_sdk/client.mdx.
Changelog fragments changelog/+transient-retry.added.md, changelog/+retry-5xx-busy-loop.fixed.md and changelog/+retry-connection-dropped-mid-request.fixed.md.

Test plan

uv run pytest tests/unit/sdk/test_retry.py: 78 tests covering classification, backoff, budget and unlimited mode, the shared budget across layers, the opt-in default, runtime toggling, environment plumbing and async/sync parity. 12 of them are new and drive a lost connection through httpx itself, so the RemoteProtocolError mapping is covered without needing Docker.
uv run pytest tests/integration/test_retry_on_failover.py: 4 tests against the deployment described above, 3m30s. Three restart HAProxy mid-mutation (retries disabled, async retry-forever, sync retry-forever) and all three fail without the mapping, so they are a real regression test rather than a demonstration. The fourth restarts the API servers instead and asserts the retry happened on a 502, not merely that the mutation eventually succeeded.
Two details that reviewers may wonder about are documented in the test file: the load balancer is published on a pinned host port, because Docker assigns a new one on every container start and the client would otherwise retry against a dead address; and the response delay lives in the memory of each API worker process, so the test that restarts them re-applies it in teardown.
uv run invoke lint-code passes. uv run invoke lint-docs passes rumdl; vale was not available locally.
Full unit suite passes apart from the known pre-existing tests/unit/ctl failures.

🤖 Generated with Claude Code

Summary by cubic

Turns retry_on_failure in the infrahub_sdk client into a real transient-error policy. Previously it only retried connection errors inside execute_graphql, capped at 5 minutes, and never covered REST calls, GraphQL error envelopes, the no-delay 5xx busy loop, or connections dropped before a response arrived; now connection errors, dropped connections, read timeouts, HTTP 500/502/503/504 responses, and GraphQL envelopes flagged with those statuses are retried on every request path—including multipart uploads and streamed downloads—with exponential backoff and jitter, optionally forever.

Key changes

New retry_max_delay caps the backoff; retry_status_codes tunes which statuses count as transient (500 is in the default set because Infrahub reports some transient DB errors without classification).
GraphQL-envelope retries share one time budget and attempt counter with transport-level retries, and each retry is logged, escalating from WARNING to ERROR after five minutes.
Budget exhaustion raises the original error instead of a generic "resp hasn't been initialized" error.
retry_on_failure and retry_delay can now be toggled at runtime.
A connection dropped before any response arrives (the shape a failover takes) is now mapped to ServerNotReachableError and retried; opt-in integration tests (INFRAHUB_TESTING_FAILOVER=1) restart HAProxy mid-mutation to cover it, with the sync case time-bounded.
A multipart mutation answered with a transient GraphQL error envelope retries with the file rewound before each attempt; non-seekable streams (pipes, sockets) are copied to a temp file first (off the event loop in the async client) so a retried send carries the full body, and a cancelled async upload keeps draining that copy loop—absorbing repeated cancellations—until the thread ends before closing the buffer.
Streamed transient responses are only pre-read when a retry will actually happen, so a spent budget returns the response open instead of fetching its body.

Caveats

A timed-out mutation may have been applied before the retry; upserts are idempotent, but a bare create that succeeded fails on retry with a non-transient error.
Remove 500 from retry_status_codes if you want genuine bugs to fail fast instead of retrying until the budget expires.
Per-generator opt-in via .infrahub.yml is left as a follow-up.

^{Written for commit 34f6717. Summary will update on new commits.}

…lly forever Generators that run for hours aborted whenever a single GraphQL call hit a transient infrastructure error, because retry_on_failure only covered connection errors inside execute_graphql, gave up after max_retry_duration, spun without delay on HTTP 5xx, and never looked at REST calls or at GraphQL error envelopes. Introduce a TransientRetryHandler wired into _request for both clients so connection errors, read timeouts and transient HTTP statuses are retried on every path, including query_gql_query and schema loading. execute_graphql also retries GraphQL envelopes whose errors all carry a transient extensions.http_status, sharing one time budget with the transport layer. Retries use exponential backoff with jitter capped by retry_max_delay, and max_retry_duration=0 now means retry indefinitely. retry_status_codes controls which statuses count as transient; 500 is included because Infrahub reports some transient database errors without further classification. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

codecov · 2026-09-04T07:44:55Z

Codecov Report

❌ Patch coverage is 97.53521% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
infrahub_sdk/client.py	95.23%	6 Missing and 1 partial ⚠️

@@            Coverage Diff             @@
##           stable    #1323      +/-   ##
==========================================
+ Coverage   84.24%   84.77%   +0.53%     
==========================================
  Files         147      148       +1     
  Lines       13068    13257     +189     
  Branches     1940     1951      +11     
==========================================
+ Hits        11009    11239     +230     
+ Misses       1494     1453      -41     
  Partials      565      565

Flag	Coverage Δ
integration-tests	`38.77% <27.46%> (-0.42%)`	⬇️
python-3.10	`57.56% <74.29%> (+0.56%)`	⬆️
python-3.11	`57.57% <74.29%> (+0.57%)`	⬆️
python-3.12	`57.57% <74.29%> (+0.57%)`	⬆️
python-3.13	`57.56% <74.29%> (+0.56%)`	⬆️
python-3.14	`57.57% <74.29%> (+0.57%)`	⬆️
python-filler-3.12	`23.81% <26.05%> (+0.11%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
infrahub_sdk/config.py	`91.56% <100.00%> (+0.10%)`	⬆️
infrahub_sdk/retry.py	`100.00% <100.00%> (ø)`
infrahub_sdk/client.py	`83.41% <95.23%> (+3.64%)`	⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

cloudflare-workers-and-pages · 2026-09-04T07:45:51Z

Deploying infrahub-sdk-python with Cloudflare Pages

Latest commit:	`34f6717`
Status:	✅ Deploy successful!
Preview URL:	https://30746e45.infrahub-sdk-python.pages.dev
Branch Preview URL:	https://fac-retry-on-error-7mwrc.infrahub-sdk-python.pages.dev

View logs

cubic-dev-ai

All reported issues were addressed across 11 files

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

Testing retry_on_failure against a real Infrahub deployment showed the feature aborting on the exact scenario it exists for. Restarting the HAProxy container in front of the API servers during a mutation killed the operation after one attempt, even with max_retry_duration=0. A load balancer or server that goes away mid-request closes the socket before sending any response byte, and httpx reports that as RemoteProtocolError rather than a NetworkError. Only NetworkError and ReadTimeout were mapped to the SDK exceptions the retry handler treats as transient, so the failover escaped it entirely. Map RemoteProtocolError to ServerNotReachableError alongside NetworkError, on all six transport paths of both clients, via a shared CONNECTION_LOST_EXCEPTIONS tuple next to the transient classification it feeds. Add tests/integration/test_retry_on_failover.py, which reproduces the failover against a real deployment: the API is told to delay every GraphQL request, an upsert is started, and the load balancer is killed while the request is in flight. The upsert is idempotent on purpose, since the server finishes a request whose client has gone away and retrying is at-least-once. The load balancer is published on a pinned host port so its address survives the restart. All three tests fail without the mapping above. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

cubic-dev-ai

All reported issues were addressed across 6 files (changes from recent commits).

_{Reply with feedback, questions, or to request a fix.

Re-trigger cubic}

A failover reaches the client in two shapes, and only one of them was covered. Restarting HAProxy drops the connection before any response arrives. Restarting the API servers behind it leaves the connection to the client intact, and HAProxy answers the in-flight request itself with 502 and whatever arrives while the servers boot with 503. Add a test for the second shape, asserting the mutation was retried on a 502 rather than merely that it eventually succeeded. Restarting the API servers clears the response delay, which lives in the memory of each worker process, so a fixture re-applies it afterwards and keeps the test independent of the order tests run in. Fold the duplicated restart-while-in-flight dance into restart_after, and take a sequence of container names since the API servers are replicated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

cubic-dev-ai

All reported issues were addressed across 1 file (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

…ed downloads The transient retry handler only wrapped _request, so a multipart upload or a streamed download failed on the first lost connection, timeout or retryable status even with retry_on_failure enabled, and a multipart mutation answered with a transient GraphQL error envelope was never retried at all. Wire the handler around both paths on the two clients, retrying stream initiation the way the 429 handler already does, and give _execute_graphql_with_file the same envelope retry loop as execute_graphql on one shared budget, rewinding the file before every attempt. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

… checker Spell out the setting names in the configuration descriptions and avoid the terms the vale spelling rule rejects in the client guide. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

cubic-dev-ai

All reported issues were addressed across 8 files (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

…pt-in The synchronous failover test ran an unlimited-retry save with no time guard, unlike its async twins bounded by asyncio.wait_for, so a load balancer that never came back would have hung the whole suite. Run the save in a daemon thread and fail after RETRY_TIMEOUT instead. The teardown that re-applies the response delay after the API servers restart now waits for them to answer first: the delay is set by a broadcast that only reaches workers that are up. The module is skipped unless INFRAHUB_TESTING_FAILOVER=1 is set, so CI never runs it: the tests restart containers, pin a host port and take several minutes. The tests guide documents how to opt in. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

cubic-dev-ai

All reported issues were addressed across 2 files (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

…end carries the full body A retried upload re-sends the file from the start, which a pipe, a socket or another non-seekable stream cannot do: the rewind before each attempt silently fails and the retry carries an empty body. Copy such a stream to a temporary file before the first attempt and upload from that copy. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

… be retried The streaming path read and closed every transient response as soon as retries were enabled, even once the retry budget was spent and the handler was about to hand that response back. Decide with the shared retry state instead, so a response that will not be retried reaches the caller open and its body is not fetched needlessly. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…pires Failing the test left the worker thread retrying the save with an unlimited budget against the live deployment, where it could complete later and keep logging behind the next test. On timeout the guard now turns the client's retries off, so the next failed attempt raises and the thread ends, and waits a grace period for that before failing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

cubic-dev-ai

All reported issues were addressed across 3 files (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

…nc client Draining a stream that cannot be rewound is blocking I/O; done inline it stalled every other task on the loop until the whole source was read. The async upload path now performs that copy in a worker thread; the synchronous client keeps the plain copy. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…e copy Keep the None check in the context managers, where it narrows the stream for the copy, and run the async copy through a small closure that the thread offload can type; the test counts the offloaded call instead of naming it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

cubic-dev-ai

All reported issues were addressed across 2 files (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

…re closing the buffer Cancelling the async upload while a non-seekable source was being copied propagated at once and closed the temporary file while the worker thread was still writing to it. Shield the copy and, on cancellation, wait for the thread to end before re-raising, so no worker is left on a closed resource. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

cubic-dev-ai

All reported issues were addressed across 2 files (changes from recent commits).

_{Tip: Review your code locally with the cubic CLI to iterate faster.

Re-trigger cubic}

…ncellations A second cancellation arriving while the drain waited on the copy task cancelled that wait too, and the temporary file was closed under the worker thread after all. Drain in a loop that absorbs cancellations until the thread has ended, then re-raise the cancellation the caller holds. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

gmazoyer · 2026-09-04T15:16:20Z

+    retry_delay: int = Field(
+        default=5,
+        ge=0,
+        description=(
+            "Base delay in seconds before retrying a request that failed with a transient error. "
+            "The delay doubles after every attempt, with jitter, up to the maximum retry delay."
+        ),
+    )
+    retry_max_delay: int = Field(
+        default=60,
+        ge=0,
+        description="Maximum delay in seconds between two retries of a request that failed with a transient error.",
+    )


both of these settings use ge=0, and compute_backoff returns min(max_delay, ...), so either zero makes every retry sleep 0 seconds. Managed to make 20000+ attempts in one second against a persistent 502.

gmazoyer · 2026-09-04T15:28:31Z

+    while not task.done():
+        try:
+            await asyncio.wait({task})
+        except asyncio.CancelledError:
+            continue


What happens if the source stream never returns from read()? It feels like things could get stuck here even at default config, with retries off.

gmazoyer · 2026-09-04T15:31:15Z

+TRANSIENT_EXCEPTIONS = (ServerNotReachableError, ServerNotResponsiveError)
+"""Client-side failures (connection error, read timeout) that are always considered transient."""
+
+CONNECTION_LOST_EXCEPTIONS = (httpx.NetworkError, httpx.RemoteProtocolError)


Should ConnectTimeout, WriteTimeout and PoolTimeout be in that sequence to be retried too? WriteTimeout is used by the upload path this PR tries to harden.

gmazoyer · 2026-09-04T15:36:45Z

+            except CONNECTION_LOST_EXCEPTIONS as exc:
                raise ServerNotReachableError(address=self.address) from exc


Is this the right exception for a connection lost mid-download?

The catch around the yield now includes RemoteProtocolError, so a transfer truncated while the caller reads the body surfaces as "Unable to connect to ..." with no retry, and file_handler leaves a partial file with no hint, even though the server was reachable in the first place.

gmazoyer · 2026-09-04T15:38:01Z

@@ -0,0 +1 @@
+With `retry_on_failure` enabled, the client now retries every transient failure instead of only connection errors: a connection dropped before any response arrives (the shape a load balancer failover or a server restart takes on an in-flight request), read timeouts, HTTP `500`/`502`/`503`/`504` responses and GraphQL errors the server flags with one of those statuses (for example a database that became unavailable mid-run). Retries apply to every request path, including multipart uploads, streamed downloads and REST endpoints such as `query_gql_query` used by generators, and use exponential backoff with jitter (`retry_delay` as the base, capped by the new `retry_max_delay`) instead of retrying HTTP 5xx responses in a tight loop without any delay. The new `retry_status_codes` setting tunes which statuses count as transient, and `max_retry_duration=0` retries indefinitely so long-running generators can survive an outage rather than abort. Retries are logged with the attempt number and elapsed time, escalating from `WARNING` to `ERROR` after five minutes, and once the budget is exhausted the original error is raised instead of a generic one.


This can probably be shorten without losing its meaning.

fatih-acar added the type/feature New feature or request label Sep 4, 2026

github-actions Bot added the type/documentation Improvements or additions to documentation label Sep 4, 2026