Repository navigation
Fail a task that raises its own TimeoutError under a Timeout - #581
Merged
Merged
Conversation
Timeout.run catches asyncio.TimeoutError to detect its deadline. On Python 3.11+ that is the builtin TimeoutError, so a TimeoutError from the task body ended the run as a success, with no ERROR log and no Retry. Now, when the task has finished, run() returns its result or re-raises its exception. Found when a Perpetual task's storage read timed out and docket recorded the run as a success. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The wait_for test now asserts the logged exception came from the task body, so a substitute TimeoutError raised by Timeout.run fails it on Python 3.10 too. The run loop becomes while True, since a finished task now returns. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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.
The problem. A task with a
Timeoutdependency that raisesTimeoutErrorfrom its own body is recorded as a success.Timeout.runwaits withasyncio.wait_for(asyncio.shield(task), ...)and catchesasyncio.TimeoutErrorto re-check its deadline. On Python 3.11+asyncio.TimeoutErroris the builtinTimeoutError, so the task's own error lands in thatexcept, the loop sees the task done, andrun()returnsNone. The worker then takes the success path: no ERROR log, noRetry, and success counters count it. Any body that can time out on its own, such as a storage read or an innerasyncio.wait_for, fails silently. On 3.10 the same happens for an innerasyncio.wait_fortimeout.The fix. In that
except, if the task is done, returntask.result(), which re-raises the task's own exception. The overrun path is unchanged: when the deadline passes the task is still running, so docket still cancels it and raises its own timeout. The same branch also reports the task's real outcome when it finishes on the tick the deadline fires, where the old loop returnedNone. The loop is nowwhile True, since a finished task always returns.New tests in
tests/test_timeout_task_errors.pycover a body raisingTimeoutError(withValueErroras the control), an innerwait_fortimeout,Retrybeing honoured, and the overrun path. The first three fail on main on 3.14, and the innerwait_fortest fails on 3.10. The full suite passes at 100% coverage.Session context
Found when a
Perpetualtask's object-storage read hit its read timeout and docket logged the run as finished, scheduling the next run a day later. A mutation check showed the first version of the innerwait_fortest could not tell the task's error from docket's own overrun on 3.10, so it now checks that the logged exception came from the task body.🤖 Generated with Claude Code