Transient retry ignores the server's retry_after_seconds hint #1380

Summary

TransientRetryHandler decides whether to retry from the server's response, but decides when from local backoff only. Infrahub now publishes a retry hint the handler never reads, so an opted-in client retries far sooner than the server advised.

Detail

Infrahub is adding a bounded worker RPC (opsmill/infrahub#10668). When no worker answers within INFRAHUB_BROKER_RPC_TIMEOUT (30s by default), the API returns the catalogued WORKER_TIMEOUT error with HTTP status 504 and a typed payload:

{"operation": "git.repository.connectivity", "timeout_seconds": 30, "retry_after_seconds": 30}

On the SDK side:

• retry.py — is_transient_status matches 504 against DEFAULT_RETRY_STATUS_CODES ({500, 502, 503, 504}), and graphql_error_status reads extensions.http_status / extensions.code to classify a transient error inside a 200 GraphQL envelope. So the error is correctly identified as retryable.
• next_delay then computes the wait purely from compute_backoff(state.attempts) — base_delay * 2**(attempt - 1) with equal jitter, capped at max_delay. Nothing reads extensions.data.retry_after_seconds.

With retry_on_failure=True and the defaults (retry_delay=5, retry_max_delay=60), a 30-second worker timeout is retried after roughly 2.5-5s — about 6-12x sooner than advised, and against a worker fleet that is by definition still saturated. Spacing only converges to something sensible by the third attempt.

Suggested fix

Have next_delay prefer a server-supplied hint over computed backoff, clamped to max_delay, mirroring what rate_limit.py already does with the Retry-After header on 429 (parse_retry_after → min(retry_after, self.backoff_max)).

The hint has to come out of the GraphQL error payload rather than a header: Infrahub returns GraphQL errors inside a 200 envelope with extensions.http_status, so there is no response header to read on that path. A Retry-After header on the REST 504 is being tracked separately on the Infrahub side and would cover plain HTTP clients.

Severity

Low, but it should land before it can matter:

• retry_on_failure defaults to False, so only opted-in clients are affected.
• All three of Infrahub's RPC call sites are idempotent reads on the worker side (GitRepositoryConnectivity from RepositoryFinalizer.post_create and from the ValidateRepositoryConnectivity mutation, GitFileGet from api/file.py::get_file), so premature retries waste capacity rather than duplicating work.
• The behaviour being replaced upstream was an unbounded hang, so a premature retry is still strictly better than what shipped before.

This is a live gap rather than a latent one: Infrahub already answers 504 for QueryTimeoutError (database query timeouts) and HTTPServerTimeoutError through the generic handler that maps Error.HTTP_CODE, so an opted-in client retries those on local backoff today. WORKER_TIMEOUT is simply the first 504 to carry an explicit hint, which is what makes the missing read visible. It wants fixing before any non-idempotent RPC caller is added on the Infrahub side.

References

• Infrahub PR: feat(backend): bound every worker RPC by a configurable timeout [IFC-3148] infrahub#10668
• Jira: IFC-3148
• Originally surfaced by a cubic review comment on that PR, then verified against origin/infrahub-develop.

This issue was written by Claude (AI) on behalf of @ogenstad.