Skip to content

Reject NaN deadlines on asyncio to match Trio - #1317

Open
dylanpulver wants to merge 2 commits into
agronholm:masterfrom
dylanpulver:fix/nan-deadline-backend-parity
Open

Reject NaN deadlines on asyncio to match Trio#1317
dylanpulver wants to merge 2 commits into
agronholm:masterfrom
dylanpulver:fix/nan-deadline-backend-parity

Conversation

@dylanpulver

Copy link
Copy Markdown

NOTE Erasing or replacing the contents of this template will result in your pull
request being summarily closed without consideration!

Changes

Fixes #.

CancelScope on Trio raises ValueError for a NaN deadline; on asyncio the NaN was stored as-is. Every deadline comparison against NaN is false, so the scope never fired — fail_after(math.nan) returned instead of raising TimeoutError, and move_on_after(math.nan) never cancelled. A NaN delay reaching a timeout silently removes the timeout.

This is the same backend inconsistency #1307 fixed for sleep(), on the surfaces that were left. Measured before the change:

call asyncio Trio
sleep(nan) ValueError ValueError
CancelScope(deadline=nan) returns ValueError: deadline must not be NaN
scope.deadline = nan returns ValueError: deadline must not be NaN
move_on_at(nan) / fail_at(nan) returns ValueError: deadline must not be NaN
move_on_after(nan) / fail_after(nan) returns ValueError: deadline must not be NaN

Negative and -inf delays already agree across backends; only NaN diverged. The asyncio CancelScope now rejects it in the constructor and the deadline setter, with Trio's wording, which covers all six entry points since they all funnel through the scope.

Checklist

If this is a user-facing code change, like a bugfix or a new feature, please ensure that
you've fulfilled the following conditions (where applicable):

  • You've added tests (in tests/) which would fail without your patch
  • You've updated the documentation (in docs/), in case of behavior changes or new
    features
  • You've added a new changelog entry (in docs/versionhistory.rst).

Reverting _backends/_asyncio.py fails 18 of the 24 new test cases — every asyncio variant (asyncio, asyncio+uvloop, asyncio+eager) — while the 6 Trio ones still pass, which is the inconsistency itself. Full suite: 3972 passed, 299 skipped, 5 xfailed. pre-commit run clean on both changed files.

No documentation change: fail_after/move_on_after do not currently document delay validation, and sleep()'s :raises ValueError: note from #1307 is the only such mention. Happy to add matching notes if you'd like them.

🤖 Generated with Claude Code

https://claude.ai/code/session_011M5uTyCU4WcNTsPvGrErDo

dylanpulver and others added 2 commits September 9, 2026 02:06
CancelScope on Trio raises ValueError for a NaN deadline; on asyncio the
NaN was stored as-is. Every deadline comparison against NaN is false, so
the scope simply never fired: fail_after(math.nan) returned instead of
raising TimeoutError, and move_on_after(math.nan) never cancelled.

This is the same backend inconsistency agronholm#1307 fixed for sleep(), on the
five remaining public surfaces that take a deadline or delay:
CancelScope(deadline=...), the deadline setter, move_on_at, fail_at,
move_on_after and fail_after.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011M5uTyCU4WcNTsPvGrErDo
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011M5uTyCU4WcNTsPvGrErDo
Comment thread tests/test_taskgroups.py
Comment on lines +2545 to +2548
pytest.param(lambda: move_on_at(math.nan), id="move_on_at"),
pytest.param(lambda: fail_at(math.nan), id="fail_at"),
pytest.param(lambda: move_on_after(math.nan), id="move_on_after"),
pytest.param(lambda: fail_after(math.nan), id="fail_after"),

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we explicitly testing these too? They all come down to CancelScope anyway.

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.

2 participants