feat(backend): bound every worker RPC by a configurable timeout [IFC-3148]
Why
When the API asks a worker something over the message bus, it waits for the reply with no time limit. If the worker fleet is saturated, restarting, or simply gone, the request never finishes, the caller is held open until something else gives up. There is no error to show the user and nothing in the logs that says a worker was the reason.
Goal: put a bound on that wait, and give the caller a real error when it expires.
Non-goals: this does not make any individual operation faster, change how work is distributed, or add retry/backoff. It also does not fix infrahub.api.file::get_file swallowing worker-side errors — that is a separate pre-existing defect, descoped and filed on its own ticket.
Part of the IFC-3101 commit-visibility feature, which needs a bounded read before it can ask a worker for a commit log. It lands on its own because it changes behaviour for every existing rpc caller.
What changed
Behavioral changes
- Every message-bus request is now bounded by a new
INFRAHUB_BROKER_RPC_TIMEOUTsetting, 30 seconds by default. Both the RabbitMQ and NATS adapters honour it. - When the bound expires the caller gets
WORKER_TIMEOUT, a catalogued error carrying the operation that timed out and how long it was allowed. GraphQL reports it with logical status 504; the REST envelope surfaces it as a 504 with the detail in the message text. - Adding a repository no longer leaves debris behind. The connectivity check already deleted the repository and raised when a worker reported failure, but a timeout skipped that branch — so a slow or unreachable remote left a repository created, never cloned, and impossible to re-add because the name was taken. The timeout now goes down the same delete-and-raise path.
- A worker that answers after the caller gave up is now an ordinary event rather than an error raised inside the broker consumer callback.
Implementation notes
- The bound is resolved from the adapter's own injected
BrokerSettings, not a module global, so a bus constructed with explicit settings actually honours them. - The waiting future is removed in a
finallythat also covers a failed publish, so a broker outage cannot accumulate abandoned futures for the life of the process. - Two orderings matter and are both covered by tests. A late reply can find the future already cancelled by the timeout but still in the pending map, because the timeout cancels synchronously while the cleanup runs a loop turn later — resolving it there raised
InvalidStateErrorinside the consumer. And theexcept TimeoutErroris scoped tightly to the wait, so a publish-level timeout is not mislabelled as "no worker answered". - The exception keeps the true wait as a float so its message reports what was actually waited; the whole-second rounding happens where the published integer payload is built.
What stayed the same
- No database or GraphQL schema changes. The
rpcsignature change is backward compatible —timeoutdefaults toNone, and no existing caller passes one. BusSimulator(the in-process bus) accepts the parameter and ignores it, so component tests that drive handlers directly are unaffected.
Suggested review order
backend/infrahub/services/adapters/message_bus/rabbitmq.pyandnats.py— the bound itself and the two callback orderings. The rest follows from these.backend/infrahub/repositories/create_repository.py— the regression this PR would otherwise have introduced.backend/infrahub/errors/+graphql/error_formatter.py— the published error surface.- Tests, then the regenerated artefacts (
schema/error-catalogue.json, the frontend bindings, the reference docs) which are mechanical.
How to review
Focus on the two adapters' rpc and on_callback. The interesting question is future lifecycle: can an entry leak, and can a reply land on a future that is gone or already cancelled?
Mechanical, skim only: schema/error-catalogue.json, frontend/app/src/shared/api/errors/catalogue.generated.ts, docs/docs/reference/{error-catalogue,configuration}.mdx are all generated. The one hand-written frontend line adds the new code to an exhaustive switch guard that tsc enforces.
Where I'd want extra scrutiny: whether 30 seconds is the right default. git.repository.connectivity shells out to git ls-remote, which can legitimately exceed 30s against a slow or firewalled remote — in which case a valid repository would now be deleted rather than added. The timeout= parameter exists to raise the bound at that specific call site if we decide it should be higher; I have not used it, so today every caller takes the default.
How to test
# The bounded wait, both adapters, and the callback orderings. No broker or database needed.
uv run pytest backend/tests/unit/services/adapters/message_bus/test_rpc_timeout.py -v
# The published error surface
uv run pytest backend/tests/unit/graphql/test_error_formatter.py backend/tests/unit/errors -v
# The repository is really gone after a timeout (needs a database)
uv run pytest backend/tests/component/repositories/test_create_repository.py -v
Manually: stop every task worker, then add a repository through the UI. Expect a WORKER_TIMEOUT error within 30 seconds and no leftover repository — previously this hung, then left an uncloned repository behind.
Impact & rollout
- Backward compatibility: no breaking API changes. The new error code is additive and marked
evolving; clients are already required to treat unrecognised codes asUNDEFINED_ERROR. - Behaviour change worth flagging: calls that previously blocked indefinitely now fail after 30 seconds. Any operation that legitimately took longer than that will start returning an error — see the scrutiny note above.
- Config/env changes: new
INFRAHUB_BROKER_RPC_TIMEOUT(default 30, minimum 1), wired into both compose files. - Deployment notes: safe to deploy on its own; no coordinated release or migration needed.
Checklist
- Tests added/updated
- Changelog entry added
- External docs updated (if user-facing or ops-facing change) — the generated configuration and error-catalogue references are updated; no prose page needed
- Internal .md docs updated (the IFC-3101 spec set records the two decisions taken during review)
- I have reviewed AI generated content
Assisted-by: opsmill-dev-pr 0.2.0