Repository: python-trio/trio

Issue:
`fail_after` deadline is set on initialization not context entry
This is a duplicate of https://github.com/agronholm/anyio/issues/514 — they are following the convention established here in trio.

When using a `fail_after` context, the deadline is set at "initialization" time rather than `__enter__`. This behavior can result in unexpected bugs if the context is declared before it is entered. This is not particularly intuitive for a Python context manager — I'd expect the timer to start when the context is entered. I do not think changing it would be complex, but it could be considered breaking.

```python
import trio


async def main():
    ctx = trio.fail_after(5)
    await trio.sleep(5)
    with ctx:
        for i in range(1, 6):
            print(i)
            await trio.sleep(1)


trio.run(main)
```

```
❯ python example.py  
1
Traceback (most recent call last):
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-39/lib/python3.9/site-packages/trio/_timeouts.py", line 106, in fail_at
    yield scope
  File "/Users/mz/dev/prefect/example.py", line 168, in main
    await trio.sleep(1)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-39/lib/python3.9/site-packages/trio/_timeouts.py", line 76, in sleep
    await sleep_until(trio.current_time() + seconds)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-39/lib/python3.9/site-packages/trio/_timeouts.py", line 57, in sleep_until
    await sleep_forever()
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-39/lib/python3.9/site-packages/trio/_timeouts.py", line 40, in sleep_forever
    await trio.lowlevel.wait_task_rescheduled(lambda _: trio.lowlevel.Abort.SUCCEEDED)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-39/lib/python3.9/site-packages/trio/_core/_traps.py", line 166, in wait_task_rescheduled
    return (await _async_yield(WaitTaskRescheduled(abort_func))).unwrap()
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-39/lib/python3.9/site-packages/outcome/_impl.py", line 138, in unwrap
    raise captured_error
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-39/lib/python3.9/site-packages/trio/_core/_run.py", line 1222, in raise_cancel
    raise Cancelled._create()
trio.Cancelled: Cancelled

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/mz/dev/prefect/example.py", line 171, in <module>
    trio.run(main)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-39/lib/python3.9/site-packages/trio/_core/_run.py", line 2010, in run
    raise runner.main_task_outcome.error
  File "/Users/mz/dev/prefect/example.py", line 168, in main
    await trio.sleep(1)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-39/lib/python3.9/contextlib.py", line 137, in __exit__
    self.gen.throw(typ, value, traceback)
  File "/opt/homebrew/Caskroom/miniconda/base/envs/orion-dev-39/lib/python3.9/site-packages/trio/_timeouts.py", line 108, in fail_at
    raise TooSlowError
trio.TooSlowError
```

Discussion:
@Fuyukai: So don't do that? I fail to see any good design where creating a context manager and then not using it would make sense.@zanieb: Often, we'll create a context manager ahead of time (during a configuration step, for example) then downstream enter the context. In my application, we are using `fail_after` to enforce configurable timeouts. We set up the context but do not want to enter it until right before we call the (library user's) code that needs a timeout. At other times, when there is not a timeout, we'll use a null context to simplify downstream logic.

If we weren't ever using the context manager, this wouldn't be an issue. The issue here is that the default behavior is antithetical to typical context manager behavior. It's like having the following return the amount of time since initialization rather than within the context:

```python
timer = timer()
with timer:
   ...
```

It's also not an atypical design to use things like an `AsyncExitStack` — in this case developers may create the contexts beforehand then enter them in a loop.
@agronholm: Trio would have to change their semantics from what has been documented, and so far there haven't been any practical use cases that would warrant this.@zanieb: @agronholm can you link to the documentation that covers this behavior? I did not see any explicit mention of this.@agronholm: Here: https://trio.readthedocs.io/en/stable/reference-core.html#trio.fail_after

> Creates a cancel scope with the given timeout, and raises an error if it is actually cancelled.

Implies that the cancel scope gets **created** with a set deadline. Says nothing about the deadline getting set when _entering_ the scope.@zanieb: Ah I think that can be interpreted either way based on your assumptions.. the example shows `with fail_after() as cancel_scope` so it's pretty ambiguous when that scope is created — it could just as well be created on `__enter__` and yielded. In fact, you cannot get the `CancelScope` object itself until after entering the context (in anyio).@agronholm: As there are 0 examples of separating the instantiation of this context manager from its use in a `with` statement in the documentation of either trio or anyio, I think it would be safe to assume that it was not a supported use case.@zanieb: If it was not a supported use-case then what is the objection to changing the behavior to be clearer?@zanieb: To be clear: I can work around this and will need to do so for compatibility with older versions of anyio/trio no matter what is decided here. I will likely add my own utility with the behavior I need — I am only advocating for an API that is less likely to be misleading here.@agronholm: > If it was not a supported use-case then what is the objection to changing the behavior to be clearer?

If anybody anywhere is already using the context manager in such a disjointed manner correctly, their code will be broken. I wouldn't expect anybody to be doing that, but then again they wouldn't report that to us unless something wasn't working. I also didn't expect anybody to be using the cm like this at all, so who knows?@agronholm: I think a change in semantics should be considered but only if someone brings forth a plausible use case where doing this is necessary or at least beneficial.@zanieb: Yeah unfortunately timeouts are kind of fuzzy in the first place — we had it like this for a long time before we got a bug report from a user. From a cursory search on GitHub, I don't see any uses apart from ours where the initialization and entry are separated. I'll open a pull request to document this caveat. We can see if more people chime in on this issue, I see no reason to rush an API change.@njsmith: I don't think there's any problem necessarily with calculating the deadline at the last moment, though I can imagine someone being confused the other way as well. ("I created the `fail_after` object early because I wanted that to be when the deadline was relative to.")

BTW, re:

> In my application, we are using fail_after to enforce configurable timeouts. We set up the context but do not want to enter it until right before we call the (library user's) code that needs a timeout. At other times, when there is not a timeout, we'll use a null context to simplify downstream logic.

Trio accepts `math.inf` as a timeout everywhere, meaning "never timeout". So you could pass the bare timeout value around everywhere (either the configured timeout, or `math.inf` if there's no timeout), and then unconditionally call `fail_after` around the operation that has the timeout. Might be easier to understand, independently of whatever happens with this discussion?@smurfix: I'd vote +0.25 for changing the behavior (but +1 for documenting things more stringently).

> ("I created the fail_after object early because I wanted that to be when the deadline was relative to."

You can easily calculate an absolute timeout at that time instead.

The way things currently are, you can't achieve the behavior @madkinsz wants as easily, thus this'd be a net win for that kind of flexibility in case anybody ever wants it.@zanieb: Thanks for the additional responses!

> Trio accepts math.inf as a timeout everywhere, meaning "never timeout". So you could pass the bare timeout value around everywhere (either the configured timeout, or math.inf if there's no timeout), and then unconditionally call fail_after around the operation that has the timeout. Might be easier to understand, independently of whatever happens with this discussion?

Definitely! I plan on doing this in the short term.

For more context: I'm also working on a refactor of how we're orchestrating user code to reduce the duplication of some complex portions of code and something I'm exploring is passing a list of contexts to a function that uses an exit stack to enter them all. This separation of configuration of the contexts that should be active during execution of the user code and management of actual execution of the code greatly simplifies our runtime logic. Of course, I can make this accept `Callable[[], ContextManager]` too to get past this particular issue, but then for more complex context managers I'll be passing around a bunch of `partial` objects 🤷‍♀️ tradeoffs.

@Zac-HD: I think it would make sense for `trio.fail_after(x)` to be relative to entering the context manager; `trio.fail_at(trio.current_time() + x)` seems a more natural way to express the relative-to-construction-time semantics.  (and the same for `trio.move_on_{at,after}`)

Either way, we should document the chosen semantics.@jakkdl: Concretely thinking about possible ways of going about implementing a change in behaviour (sorry if somewhat rambly/stream-of-thought, I think I personally prefer the new class that transforms into a cancelscope)

### new attribute
One option would be introducing an attribute, `relative`, to `CancelScope` - such that when `CancelScope.__enter__` is called it does
```python
if self.relative: self._deadline = trio.current_time() + self._deadline
```
* Small change
* easy to change the relative deadline before entering, and to change whether it's relative or not
* downside: existing code might break if they're inspecting `CancelScope`s (before entering) and doing logic based on the value of `deadline`, if they are passed `CancelScope`s that have the `relative` flag set
* it's unclear what should happen if modifying `deadline` after entering the `CancelScope` [depending on the value of relative]. Or if the value of `relative` is modified.

Could also add an attribute `relative_deadline`... but that brings up problems with what should happen if you access the `deadline` attribute.

### new class that transforms into `CancelScope`
We can introduce a new class (`PendingTimeout`, `PendingRelativeCancelScope`, `RelativeCancelScope`...?) which when `__enter__`ed returns a `CancelScope`. It does need to be exposed for the sake of typing, and you probably want to be able to modify the timeout after creation, before entering. It probably makes sense to also expose `shield`.

There's still some danger of deadline confusion if writing:
```python
my_timeout = fail_after(5)
with my_timeout as timeout:
    # oh no, we want 7 seconds, not 5
    my_timeout.deadline = 7
    # ...except that's not how cancelscopes work
```

although I guess that has always been the case.
If trying to access `deadline` on this object you will get an error (since it's not defined as an attribute at all), so some of the code relying on current behavior will get caught this way, and still others will get caught by typing errors.

example usage
```python
my_timeout: PendingTimeout = fail_after(5)
my_timeout.shield = True
my_timeout.seconds = 7
my_timeout.fail = False # this is now a move_on_after instead
with pytest.raises(
    AttributeError,
    match="'PendingTimeout' object has no attribute 'deadline'"
    ):
  my_timeout.deadline += 2
cs: CancelScope
with my_timeout as cs:
  with pytest.raises(
      AttributeError,
      match="'CancelScope' object has no attribute 'seconds'"
  ):
    cs.seconds += 2

# this
with trio.PendingTimeout(5, shield=True, fail = False):
  ...
# would be equivalent to
with trio.move_on_after(5) as cs:
  cs.shield = True
  ...
```

### new subclass
A third option would be introducing a subclass of `CancelScope`, `RelativeCancelScope`, which has an attribute `seconds` (or `timeout`? The parameter to `fail_` & `move_on_after` is currently called `seconds`). Trying to get/set `deadline` on the object before entering will raise an error (which means code relying on current functionality will get a helpful error mesage), but `seconds` can be modified freely. Once entered though, if modifying `seconds` the change will flow down to the underlying `CancelScope`, and if modifying `deadline` the `seconds` parameter will also be updated.
Code using current functionality will not get any typing errors, since `RelativeCancelScope` is a subclass of `CancelScope`.

example usage
```python
my_timeout: RelativeCancelScope = fail_after(9)
with pytest.raises(
    AttributeError, # or some other error
    match='cannot set deadline on RelativeCancelScope before entering.'
  ):
  my_timeout.deadline += 3
my_timeout.seconds = 5

with my_timeout: # __enter__ returns Self, so don't even need to do `as`
  # assume that trio.current_time() was 100 upon entering
  assert my_timeout.deadline == 105
  my_timeout.seconds = 20
  assert my_timeout.deadline == 120
  my_timeout.deadline = 150
  assert my_timeout.seconds = 50
  my_timeout.deadline = 97 # nothing illegal about setting deadlines in the past on cancelscopes
  assert my_timeout.seconds = -3
```

I'm not sure how much this actually differs from adding a new attribute tbh.


### a new, entirely independent, class
could also go all the way to introducing a fully new `RelativeCancelScope` that does not inherit from `CancelScope`. But that seems overkill.@CoolCat467: I think a new subclass would work well. On the `seconds` side of things, in the past I've wished it were measured in smaller time increments, but I think it's probably fine to leave as seconds. The division operation to make things into seconds isn't really going to be that bad.@Zac-HD: I favor adding a `._relative_deadline` attribute to the existing `CancelScope` class.  
- `fail_after()` then creates a `CancelScope(deadline=inf)` and sets the `_relative_deadline`
- `CancelScope.__enter__` then sets `deadline = min(deadline, _relative_deadline + current_time())`

Modifying the deadline or relative deadline before entering seems fine, though I'm still using a leading underscore to discourage the latter.  We could make the relative deadline a property so that attempting to modify it after entering checks `if self._has_been_entered` and raises an error.@TeamSpen210: After being entered, modifying either the absolute or relative deadline seems reasonable to me - it'd just implicitly fetch the current time then set the other to match.@jakkdl: Finishing up the PR, when I realized that the behaviour of `relative_deadline` after entering is not obvious.

```python
with CancelScope(relative_deadline = 5) as cs:
  await trio.sleep(1)
  print(cs.relative_deadline)  # should this print 5, or 4?
  cs.relative_deadline = 3 # does this mean "deadline in 3 seconds", or "3 seconds after entering"?
```

My first intuition (and implementation) was that it would print 5, and then "deadline in 3 seconds" - but that means that the value of `cs.relative_deadline` becomes ~worthless:

```python
with CancelScope(relative_deadline = 5) as cs:
  await trio.sleep(random.randint(3))
  cs.relative_deadline = 1 + random.randint(3)
  await trio.sleep(random.randint(3))
  print(cs.relative_deadline)  # this value now has zero relation to when the scope will time out.
```

Possible alternatives:
1. accessing the `relative_deadline` property after entering will return `deadline - current_time()`. The answers to the first questions are "4 seconds" and "deadline in 3 seconds".
2. Add another attribute `_time_of_entering` to the cs, set to `current_time()` upon entering. All relative times are now relative to entering the scope. The answer to the first questions are "5 seconds" and "3 seconds after entering".
3. Raise an error if accessing or setting `relative_deadline` after entering to remove a potential footgun. The 1st option doesn't actually add any functionality after all, `cs.relative_deadline = x` is equivalent to `cs.deadline = trio.current_time() + x` and `cs.relative_deadline` is equivalent to `cs.deadline - trio.current_time()`. 

I think the first one might be more intuitive and useful? But can see a case for any of the options. I'll push to #3010 soonish with an implementation.

Solution patch:
diff --git a/docs/source/conf.py b/docs/source/conf.py
index ecdbdd470..2e84d9153 100755
--- a/docs/source/conf.py
+++ b/docs/source/conf.py
@@ -229,6 +229,7 @@ def setup(app: Sphinx) -> None:
     "pyopenssl": ("https://www.pyopenssl.org/en/stable/", None),
     "sniffio": ("https://sniffio.readthedocs.io/en/latest/", None),
     "trio-util": ("https://trio-util.readthedocs.io/en/latest/", None),
+    "flake8-async": ("https://flake8-async.readthedocs.io/en/latest/", None),
 }
 
 # See https://sphinx-hoverxref.readthedocs.io/en/latest/configuration.html
diff --git a/docs/source/reference-core.rst b/docs/source/reference-core.rst
index 392eac7d2..4b023292e 100644
--- a/docs/source/reference-core.rst
+++ b/docs/source/reference-core.rst
@@ -527,8 +527,12 @@ objects.
 
    .. autoattribute:: deadline
 
+   .. autoattribute:: relative_deadline
+
    .. autoattribute:: shield
 
+   .. automethod:: is_relative()
+
    .. automethod:: cancel()
 
    .. attribute:: cancelled_caught
@@ -561,7 +565,8 @@ situation of just wanting to impose a timeout on some code:
 .. autofunction:: fail_at
    :with: cancel_scope
 
-Cheat sheet:
+Cheat sheet
++++++++++++
 
 * If you want to impose a timeout on a function, but you don't care
   whether it timed out or not:
@@ -597,7 +602,6 @@ which is sometimes useful:
 
 .. autofunction:: current_effective_deadline
 
-
 .. _tasks:
 
 Tasks let you do multiple things at once
diff --git a/newsfragments/2512.breaking.rst b/newsfragments/2512.breaking.rst
new file mode 100644
index 000000000..f9a6b4263
--- /dev/null
+++ b/newsfragments/2512.breaking.rst
@@ -0,0 +1,2 @@
+:func:`trio.move_on_after` and :func:`trio.fail_after` previously set the deadline relative to initialization time, instead of more intuitively upon entering the context manager. This might change timeouts if a program relied on this behavior. If you want to restore previous behavior you should instead use ``trio.move_on_at(trio.current_time() + ...)``.
+flake8-async has a new rule to catch this, in case you're supporting older trio versions. See :ref:`ASYNC122`.
diff --git a/newsfragments/2512.feature.rst b/newsfragments/2512.feature.rst
new file mode 100644
index 000000000..fe9197cde
--- /dev/null
+++ b/newsfragments/2512.feature.rst
@@ -0,0 +1 @@
+:meth:`CancelScope.relative_deadline` and :meth:`CancelScope.is_relative` added, as well as a ``relative_deadline`` parameter to ``__init__``. This allows initializing scopes ahead of time, but where the specified relative deadline doesn't count down until the scope is entered.
diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py
index 8921ed7f1..5de75e8e6 100644
--- a/src/trio/_core/_run.py
+++ b/src/trio/_core/_run.py
@@ -13,7 +13,7 @@
 from contextlib import AbstractAsyncContextManager, contextmanager, suppress
 from contextvars import copy_context
 from heapq import heapify, heappop, heappush
-from math import inf
+from math import inf, isnan
 from time import perf_counter
 from typing import (
     TYPE_CHECKING,
@@ -543,8 +543,21 @@ class CancelScope:
     cancelled_caught: bool = attrs.field(default=False, init=False)
 
     # Constructor arguments:
-    _deadline: float = attrs.field(default=inf, kw_only=True, alias="deadline")
-    _shield: bool = attrs.field(default=False, kw_only=True, alias="shield")
+    _relative_deadline: float = attrs.field(default=inf, kw_only=True)
+    _deadline: float = attrs.field(default=inf, kw_only=True)
+    _shield: bool = attrs.field(default=False, kw_only=True)
+
+    def __attrs_post_init__(self) -> None:
+        if isnan(self._deadline):
+            raise ValueError("deadline must not be NaN")
+        if isnan(self._relative_deadline):
+            raise ValueError("relative deadline must not be NaN")
+        if self._relative_deadline < 0:
+            raise ValueError("timeout must be non-negative")
+        if self._relative_deadline != inf and self._deadline != inf:
+            raise ValueError(
+                "Cannot specify both a deadline and a relative deadline",
+            )
 
     @enable_ki_protection
     def __enter__(self) -> Self:
@@ -554,6 +567,12 @@ def __enter__(self) -> Self:
                 "Each CancelScope may only be used for a single 'with' block",
             )
         self._has_been_entered = True
+
+        if self._relative_deadline != inf:
+            assert self._deadline == inf
+            self._deadline = current_time() + self._relative_deadline
+            self._relative_deadline = inf
+
         if current_time() >= self._deadline:
             self.cancel()
         with self._might_change_registered_deadline():
@@ -734,13 +753,70 @@ def deadline(self) -> float:
         this can be overridden by the ``deadline=`` argument to
         the :class:`~trio.CancelScope` constructor.
         """
+        if self._relative_deadline != inf:
+            assert self._deadline == inf
+            warnings.warn(
+                DeprecationWarning(
+                    "unentered relative cancel scope does not have an absolute deadline. Use `.relative_deadline`",
+                ),
+                stacklevel=2,
+            )
+            return current_time() + self._relative_deadline
         return self._deadline
 
     @deadline.setter
     def deadline(self, new_deadline: float) -> None:
+        if isnan(new_deadline):
+            raise ValueError("deadline must not be NaN")
+        if self._relative_deadline != inf:
+            assert self._deadline == inf
+            warnings.warn(
+                DeprecationWarning(
+                    "unentered relative cancel scope does not have an absolute deadline. Transforming into an absolute cancel scope. First set `.relative_deadline = math.inf` if you do want an absolute cancel scope.",
+                ),
+                stacklevel=2,
+            )
+            self._relative_deadline = inf
         with self._might_change_registered_deadline():
             self._deadline = float(new_deadline)
 
+    @property
+    def relative_deadline(self) -> float:
+        if self._has_been_entered:
+            return self._deadline - current_time()
+        elif self._deadline != inf:
+            assert self._relative_deadline == inf
+            raise RuntimeError(
+                "unentered non-relative cancel scope does not have a relative deadline",
+            )
+        return self._relative_deadline
+
+    @relative_deadline.setter
+    def relative_deadline(self, new_relative_deadline: float) -> None:
+        if isnan(new_relative_deadline):
+            raise ValueError("relative deadline must not be NaN")
+        if new_relative_deadline < 0:
+            raise ValueError("relative deadline must be non-negative")
+        if self._has_been_entered:
+            with self._might_change_registered_deadline():
+                self._deadline = current_time() + float(new_relative_deadline)
+        elif self._deadline != inf:
+            assert self._relative_deadline == inf
+            raise RuntimeError(
+                "unentered non-relative cancel scope does not have a relative deadline",
+            )
+        else:
+            self._relative_deadline = new_relative_deadline
+
+    @property
+    def is_relative(self) -> bool | None:
+        """Returns None after entering. Returns False if both deadline and
+        relative_deadline are inf."""
+        assert not (self._deadline != inf and self._relative_deadline != inf)
+        if self._has_been_entered:
+            return None
+        return self._relative_deadline != inf
+
     @property
     def shield(self) -> bool:
         """Read-write, :class:`bool`, default :data:`False`. So long as
diff --git a/src/trio/_timeouts.py b/src/trio/_timeouts.py
index b20a05fb6..1a5e2e1db 100644
--- a/src/trio/_timeouts.py
+++ b/src/trio/_timeouts.py
@@ -1,11 +1,15 @@
 from __future__ import annotations
 
 import math
-from contextlib import AbstractContextManager, contextmanager
+import sys
+from contextlib import contextmanager
 from typing import TYPE_CHECKING
 
 import trio
 
+if TYPE_CHECKING:
+    from collections.abc import Generator
+
 
 def move_on_at(deadline: float, *, shield: bool = False) -> trio.CancelScope:
     """Use as a context manager to create a cancel scope with the given
@@ -20,27 +24,38 @@ def move_on_at(deadline: float, *, shield: bool = False) -> trio.CancelScope:
       ValueError: if deadline is NaN.
 
     """
-    if math.isnan(deadline):
-        raise ValueError("deadline must not be NaN")
+    # CancelScope validates that deadline isn't math.nan
     return trio.CancelScope(deadline=deadline, shield=shield)
 
 
-def move_on_after(seconds: float, *, shield: bool = False) -> trio.CancelScope:
+def move_on_after(
+    seconds: float,
+    *,
+    shield: bool = False,
+) -> trio.CancelScope:
     """Use as a context manager to create a cancel scope whose deadline is
     set to now + *seconds*.
 
+    The deadline of the cancel scope is calculated upon entering.
+
     Args:
       seconds (float): The timeout.
       shield (bool): Initial value for the `~trio.CancelScope.shield` attribute
           of the newly created cancel scope.
 
     Raises:
-      ValueError: if timeout is less than zero or NaN.
+      ValueError: if ``seconds`` is less than zero or NaN.
 
     """
+    # duplicate validation logic to have the correct parameter name
     if seconds < 0:
-        raise ValueError("timeout must be non-negative")
-    return move_on_at(trio.current_time() + seconds, shield=shield)
+        raise ValueError("`seconds` must be non-negative")
+    if math.isnan(seconds):
+        raise ValueError("`seconds` must not be NaN")
+    return trio.CancelScope(
+        shield=shield,
+        relative_deadline=seconds,
+    )
 
 
 async def sleep_forever() -> None:
@@ -84,7 +99,7 @@ async def sleep(seconds: float) -> None:
 
     """
     if seconds < 0:
-        raise ValueError("duration must be non-negative")
+        raise ValueError("`seconds` must be non-negative")
     if seconds == 0:
         await trio.lowlevel.checkpoint()
     else:
@@ -98,9 +113,12 @@ class TooSlowError(Exception):
     """
 
 
-# workaround for PyCharm not being able to infer return type from @contextmanager
-# see https://youtrack.jetbrains.com/issue/PY-36444/PyCharm-doesnt-infer-types-when-using-contextlib.contextmanager-decorator
-def fail_at(deadline: float, *, shield: bool = False) -> AbstractContextManager[trio.CancelScope]:  # type: ignore[misc]
+@contextmanager
+def fail_at(
+    deadline: float,
+    *,
+    shield: bool = False,
+) -> Generator[trio.CancelScope, None, None]:
     """Creates a cancel scope with the given deadline, and raises an error if it
     is actually cancelled.
 
@@ -129,15 +147,12 @@ def fail_at(deadline: float, *, shield: bool = False) -> AbstractContextManager[
         raise TooSlowError
 
 
-if not TYPE_CHECKING:
-    fail_at = contextmanager(fail_at)
-
-
+@contextmanager
 def fail_after(
     seconds: float,
     *,
     shield: bool = False,
-) -> AbstractContextManager[trio.CancelScope]:
+) -> Generator[trio.CancelScope, None, None]:
     """Creates a cancel scope with the given timeout, and raises an error if
     it is actually cancelled.
 
@@ -148,6 +163,8 @@ def fail_after(
     it's caught and discarded. When it reaches :func:`fail_after`, then it's
     caught and :exc:`TooSlowError` is raised in its place.
 
+    The deadline of the cancel scope is calculated upon entering.
+
     Args:
       seconds (float): The timeout.
       shield (bool): Initial value for the `~trio.CancelScope.shield` attribute
@@ -159,6 +176,17 @@ def fail_after(
       ValueError: if *seconds* is less than zero or NaN.
 
     """
-    if seconds < 0:
-        raise ValueError("timeout must be non-negative")
-    return fail_at(trio.current_time() + seconds, shield=shield)
+    with move_on_after(seconds, shield=shield) as scope:
+        yield scope
+    if scope.cancelled_caught:
+        raise TooSlowError
+
+
+# Users don't need to know that fail_at & fail_after wraps move_on_at and move_on_after
+# and there is no functional difference. So we replace the return value when generating
+# documentation.
+if "sphinx" in sys.modules:  # pragma: no cover
+    import inspect
+
+    for c in (fail_at, fail_after):
+        c.__signature__ = inspect.Signature.from_callable(c).replace(return_annotation=trio.CancelScope)  # type: ignore[union-attr]
diff --git a/test-requirements.txt b/test-requirements.txt
index 47aea97d8..1173aff40 100644
--- a/test-requirements.txt
+++ b/test-requirements.txt
@@ -105,7 +105,7 @@ pylint==3.2.7
     # via -r test-requirements.in
 pyopenssl==24.2.1
     # via -r test-requirements.in
-pyright==1.1.378
+pyright==1.1.381
     # via -r test-requirements.in
 pytest==8.3.2
     # via -r test-requirements.in


Test patch:
diff --git a/src/trio/_core/_tests/test_run.py b/src/trio/_core/_tests/test_run.py
index e6ee5e869..9d2eab787 100644
--- a/src/trio/_core/_tests/test_run.py
+++ b/src/trio/_core/_tests/test_run.py
@@ -9,7 +9,7 @@
 import types
 import weakref
 from contextlib import ExitStack, contextmanager, suppress
-from math import inf
+from math import inf, nan
 from typing import TYPE_CHECKING, Any, NoReturn, TypeVar, cast
 
 import outcome
@@ -365,6 +365,27 @@ async def test_cancel_scope_repr(mock_clock: _core.MockClock) -> None:
     assert "exited" in repr(scope)
 
 
+async def test_cancel_scope_validation() -> None:
+    with pytest.raises(
+        ValueError,
+        match="^Cannot specify both a deadline and a relative deadline$",
+    ):
+        _core.CancelScope(deadline=7, relative_deadline=3)
+    scope = _core.CancelScope()
+
+    with pytest.raises(ValueError, match="^deadline must not be NaN$"):
+        scope.deadline = nan
+    with pytest.raises(ValueError, match="^relative deadline must not be NaN$"):
+        scope.relative_deadline = nan
+
+    with pytest.raises(ValueError, match="^relative deadline must be non-negative$"):
+        scope.relative_deadline = -3
+    scope.relative_deadline = 5
+    assert scope.relative_deadline == 5
+
+    # several related tests of CancelScope are implicitly handled by test_timeouts.py
+
+
 def test_cancel_points() -> None:
     async def main1() -> None:
         with _core.CancelScope() as scope:
diff --git a/src/trio/_tests/test_timeouts.py b/src/trio/_tests/test_timeouts.py
index b2486c7d2..515f536c1 100644
--- a/src/trio/_tests/test_timeouts.py
+++ b/src/trio/_tests/test_timeouts.py
@@ -4,9 +4,20 @@
 import outcome
 import pytest
 
+import trio
+
 from .. import _core
 from .._core._tests.tutil import slow
-from .._timeouts import *
+from .._timeouts import (
+    TooSlowError,
+    fail_after,
+    fail_at,
+    move_on_after,
+    move_on_at,
+    sleep,
+    sleep_forever,
+    sleep_until,
+)
 from ..testing import assert_checkpoints
 
 T = TypeVar("T")
@@ -155,7 +166,7 @@ async def test_timeouts_raise_value_error() -> None:
     ):
         with pytest.raises(
             ValueError,
-            match="^(duration|deadline|timeout) must (not )*be (non-negative|NaN)$",
+            match="^(deadline|`seconds`) must (not )*be (non-negative|NaN)$",
         ):
             await fun(val)
 
@@ -169,7 +180,79 @@ async def test_timeouts_raise_value_error() -> None:
     ):
         with pytest.raises(
             ValueError,
-            match="^(duration|deadline|timeout) must (not )*be (non-negative|NaN)$",
+            match="^(deadline|`seconds`) must (not )*be (non-negative|NaN)$",
         ):
             with cm(val):
                 pass  # pragma: no cover
+
+
+async def test_timeout_deadline_on_entry(mock_clock: _core.MockClock) -> None:
+    rcs = move_on_after(5)
+    assert rcs.relative_deadline == 5
+
+    mock_clock.jump(3)
+    start = _core.current_time()
+    with rcs as cs:
+        assert cs.is_relative is None
+
+        # This would previously be start+2
+        assert cs.deadline == start + 5
+        assert cs.relative_deadline == 5
+
+        cs.deadline = start + 3
+        assert cs.deadline == start + 3
+        assert cs.relative_deadline == 3
+
+        cs.relative_deadline = 4
+        assert cs.deadline == start + 4
+        assert cs.relative_deadline == 4
+
+    rcs = move_on_after(5)
+    assert rcs.shield is False
+    rcs.shield = True
+    assert rcs.shield is True
+
+    mock_clock.jump(3)
+    start = _core.current_time()
+    with rcs as cs:
+        assert cs.deadline == start + 5
+
+        assert rcs is cs
+
+
+async def test_invalid_access_unentered(mock_clock: _core.MockClock) -> None:
+    cs = move_on_after(5)
+    mock_clock.jump(3)
+    start = _core.current_time()
+
+    match_str = "^unentered relative cancel scope does not have an absolute deadline"
+    with pytest.warns(DeprecationWarning, match=match_str):
+        assert cs.deadline == start + 5
+    mock_clock.jump(1)
+    # this is hella sketchy, but they *have* been warned
+    with pytest.warns(DeprecationWarning, match=match_str):
+        assert cs.deadline == start + 6
+
+    with pytest.warns(DeprecationWarning, match=match_str):
+        cs.deadline = 7
+    # now transformed into absolute
+    assert cs.deadline == 7
+    assert not cs.is_relative
+
+    cs = move_on_at(5)
+
+    match_str = (
+        "^unentered non-relative cancel scope does not have a relative deadline$"
+    )
+    with pytest.raises(RuntimeError, match=match_str):
+        assert cs.relative_deadline
+    with pytest.raises(RuntimeError, match=match_str):
+        cs.relative_deadline = 7
+
+
+@pytest.mark.xfail(reason="not implemented")
+async def test_fail_access_before_entering() -> None:  # pragma: no cover
+    my_fail_at = fail_at(5)
+    assert my_fail_at.deadline  # type: ignore[attr-defined]
+    my_fail_after = fail_after(5)
+    assert my_fail_after.relative_deadline  # type: ignore[attr-defined]