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
This issue was written by Claude (AI) on behalf of @ogenstad.
Summary
TransientRetryHandlerdecides 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 cataloguedWORKER_TIMEOUTerror 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_statusmatches 504 againstDEFAULT_RETRY_STATUS_CODES({500, 502, 503, 504}), andgraphql_error_statusreadsextensions.http_status/extensions.codeto classify a transient error inside a 200 GraphQL envelope. So the error is correctly identified as retryable.next_delaythen computes the wait purely fromcompute_backoff(state.attempts)—base_delay * 2**(attempt - 1)with equal jitter, capped atmax_delay. Nothing readsextensions.data.retry_after_seconds.With
retry_on_failure=Trueand 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_delayprefer a server-supplied hint over computed backoff, clamped tomax_delay, mirroring whatrate_limit.pyalready does with theRetry-Afterheader 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. ARetry-Afterheader 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_failuredefaults toFalse, so only opted-in clients are affected.GitRepositoryConnectivityfromRepositoryFinalizer.post_createand from theValidateRepositoryConnectivitymutation,GitFileGetfromapi/file.py::get_file), so premature retries waste capacity rather than duplicating work.This is a live gap rather than a latent one: Infrahub already answers 504 for
QueryTimeoutError(database query timeouts) andHTTPServerTimeoutErrorthrough the generic handler that mapsError.HTTP_CODE, so an opted-in client retries those on local backoff today.WORKER_TIMEOUTis 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
origin/infrahub-develop.This issue was written by Claude (AI) on behalf of @ogenstad.