Skip to content

fix(task): tolerate transient result backend errors in wait_result - #657

Open
timyjsong wants to merge 1 commit into
taskiq-python:masterfrom
timyjsong:fix/wait-result-transient-backend-error
Open

fix(task): tolerate transient result backend errors in wait_result#657
timyjsong wants to merge 1 commit into
taskiq-python:masterfrom
timyjsong:fix/wait-result-transient-backend-error

Conversation

@timyjsong

@timyjsong timyjsong commented Aug 6, 2026

Copy link
Copy Markdown

Why

Refs #655.

AsyncTaskiqTask.wait_result polls is_ready() in a loop with no handler, and
is_ready() wraps every result backend exception in ResultIsReadyError. One
failed poll therefore ends the wait, even though the timeout budget is
untouched and the task itself is fine. With a network-backed result backend, a
single Redis read timeout, failover or connection reset is enough to trigger it.

The loop now counts consecutive failures. Any successful check resets the
count, and the error is re-raised once max_poll_failures (new parameter,
default 3) checks have failed in a row. Each tolerated failure is logged at
warning level with the traceback.

Counting consecutively, rather than retrying until the timeout budget runs
out, is deliberate. timeout defaults to -1.0, which means wait forever, so
spending the budget would turn a loud error into a silent hang for the most
likely real cause: the backend is down.
tests/test_funcs.py::test_gather_result_backend_error already pins the
behaviour for that case, using a mock that fails on every call. Bounding by
consecutive failures fixes the transient case from the issue and leaves that
intact, and it behaves the same whether or not a timeout is set.

The issue also offers an opt-in flag as the more conservative option. I went
with a bounded default instead, because the issue describes a bug, and a flag
that is off by default leaves that bug in place for everyone who does not go
looking for it. max_poll_failures=0 restores today's behaviour exactly,
which is the same escape hatch pointed the other way.

The default of 3 matches default_retry_count in simple_retry_middleware.py
and smart_retry_middleware.py.

One consequence worth naming: is_ready() converts every backend exception
into ResultIsReadyError, so a genuine bug in a custom backend is now retried
as well, and surfaces after max_poll_failures + 1 checks rather than on the
first. It does still surface, with the original exception as __cause__.
Narrowing is_ready() would be the real answer to that and would change the
contract every third-party result backend is written against, so it is not in
this PR.

What this deliberately leaves

This PR is narrower than the issue it refers to, so the issue should stay open
after it merges. The report also notes that get_result() has the same shape,
and a blip in the window between "ready" and the read still raises
ResultGetError out of wait_result. I left it out because it is a different
decision rather than the same one twice: once the backend has reported ready, a
failed read can mean the result expired, result_ex_time being set, just as
easily as it can mean the connection blipped, and retrying then waits out a
result that is never coming.

gather() in taskiq/funcs.py has the readiness defect too. Fixing it means
changing test_gather_result_backend_error, which currently asserts the fatal
behaviour, so it is a behaviour change with its own argument.

Backing check_interval off while the backend is erroring, which the issue
suggests as well, would need a backoff convention the library does not have
yet.

Happy to send any of the three as a follow-up, in whatever order is useful.

Testing

uv run pytest -q -n auto . gives 302 passed, against 297 on master.

Five tests were added to tests/test_task.py, using the hand-rolled backend
style already in that file: one transient failure then ready, a backend that
never recovers (still raises, and does not hang), failures separated by
successful polls (the counter resets), max_poll_failures=0, and a timeout set
while retrying (TaskiqResultTimeoutError still wins).

uv run pre-commit run -a black, ... ruff and ... mypy all pass.

@timyjsong
timyjsong marked this pull request as draft August 6, 2026 19:33
@timyjsong
timyjsong marked this pull request as ready for review August 6, 2026 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant