fix(task): tolerate transient result backend errors in wait_result - #657
Open
timyjsong wants to merge 1 commit into
Open
fix(task): tolerate transient result backend errors in wait_result#657timyjsong wants to merge 1 commit into
timyjsong wants to merge 1 commit into
Conversation
timyjsong
marked this pull request as draft
August 6, 2026 19:33
timyjsong
marked this pull request as ready for review
August 6, 2026 20:01
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Refs #655.
AsyncTaskiqTask.wait_resultpollsis_ready()in a loop with no handler, andis_ready()wraps every result backend exception inResultIsReadyError. Onefailed poll therefore ends the wait, even though the
timeoutbudget isuntouched 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
timeoutbudget runsout, is deliberate.
timeoutdefaults to-1.0, which means wait forever, sospending 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_erroralready pins thebehaviour 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=0restores today's behaviour exactly,which is the same escape hatch pointed the other way.
The default of 3 matches
default_retry_countinsimple_retry_middleware.pyand
smart_retry_middleware.py.One consequence worth naming:
is_ready()converts every backend exceptioninto
ResultIsReadyError, so a genuine bug in a custom backend is now retriedas well, and surfaces after
max_poll_failures + 1checks rather than on thefirst. It does still surface, with the original exception as
__cause__.Narrowing
is_ready()would be the real answer to that and would change thecontract 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
ResultGetErrorout ofwait_result. I left it out because it is a differentdecision rather than the same one twice: once the backend has reported ready, a
failed read can mean the result expired,
result_ex_timebeing set, just aseasily as it can mean the connection blipped, and retrying then waits out a
result that is never coming.
gather()intaskiq/funcs.pyhas the readiness defect too. Fixing it meanschanging
test_gather_result_backend_error, which currently asserts the fatalbehaviour, so it is a behaviour change with its own argument.
Backing
check_intervaloff while the backend is erroring, which the issuesuggests 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 backendstyle 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 setwhile retrying (
TaskiqResultTimeoutErrorstill wins).uv run pre-commit run -a black,... ruffand... mypyall pass.