From 6e425f390dc7dfa1f3e83e7ab9fc44b4618d2bb7 Mon Sep 17 00:00:00 2001 From: Julien Danjou Date: Wed, 5 Aug 2026 23:14:08 +0200 Subject: [PATCH] fix: restore support for falsy wait values and callable + strategy Three runtime regressions from the mypy-strictness stack (#679, #677), all A/B verified against a2af454. None were caught by the test suite, and all three only fire once a retry actually happens, so happy-path callers see nothing until production. 1. `Retrying(wait=None)`, `wait=0` and `wait=sum([])` now raise `TypeError: 'NoneType' object is not callable` from inside `iter()`. #679 removed the `if self.wait:` guard on the grounds that `truthy-bool` proved it always true -- but that only holds for the *declared* type. Untyped callers pass `None`/`0` to mean "no wait", and `sum([])` over an empty strategy list returns the int 0. The `TypeError` is raised outside the attempt's try/except, so it escapes uncaught and destroys the caller's real exception. Restore the guard with a local `truthy-bool` suppression explaining why the "impossible" branch is reachable. `before`, `after`, `before_sleep` and `retry_error_callback` all kept their `is not None` guards; `wait` was the only one dropped. 2. `plain_callable + wait_strategy` now raises `TypeError`. `WaitBaseT` admits plain callables, and a function has no `__add__`, so this went through `wait_base.__radd__` -- which #677 narrowed to `int`. Widen it to `WaitBaseT | int` and build the `wait_combine` again. `5 + strategy` is still rejected, now via `NotImplemented` rather than a combination that fails later. The parameter stays `int` rather than `Literal[0]` because typeshed's `sum()` protocol requires `__radd__(x: int)`; narrowing it would make every `sum()` over strategies need a `type: ignore`. 3. `wait_combine.__call__` passed the state as `retry_state=`, which crashes on any `WaitBaseT` callable whose parameter has another name. Pass it positionally, as `BaseRetrying._run_wait` already does. Adds regression tests for all three plus the async path -- the original change shipped with no test covering any of them. Change-Id: Ia40c32a22cdac09cbfba4280fb0a7f0c2cb7b7e0 --- ...alsy-wait-regression-3b7e91c4a5d20f68.yaml | 17 +++++++ tenacity/__init__.py | 9 +++- tenacity/asyncio/__init__.py | 11 +++-- tenacity/wait.py | 29 ++++++++--- tests/test_asyncio.py | 13 +++++ tests/test_tenacity.py | 49 +++++++++++++++++++ 6 files changed, 116 insertions(+), 12 deletions(-) create mode 100644 releasenotes/notes/fix-falsy-wait-regression-3b7e91c4a5d20f68.yaml diff --git a/releasenotes/notes/fix-falsy-wait-regression-3b7e91c4a5d20f68.yaml b/releasenotes/notes/fix-falsy-wait-regression-3b7e91c4a5d20f68.yaml new file mode 100644 index 00000000..fa480eef --- /dev/null +++ b/releasenotes/notes/fix-falsy-wait-regression-3b7e91c4a5d20f68.yaml @@ -0,0 +1,17 @@ +--- +fixes: + - | + Restored support for falsy ``wait`` values. ``Retrying(wait=None)``, + ``Retrying(wait=0)`` and ``wait=sum([])`` raised ``TypeError`` from inside + the retry loop instead of not waiting. The same applied to + ``AsyncRetrying``. The error surfaced only once a retry actually happened, + so it escaped happy-path tests. + - | + Restored ``callable + wait_strategy``. A plain callable is a valid + ``WaitBaseT``, but adding one on the left raised ``TypeError`` instead of + building a ``wait_combine``. + - | + ``wait_combine`` now passes the retry state positionally, matching how + ``BaseRetrying`` invokes ``wait``. It previously passed it as the keyword + ``retry_state=``, which crashed on any callable whose parameter had a + different name. diff --git a/tenacity/__init__.py b/tenacity/__init__.py index 9ceaf16d..51925efa 100644 --- a/tenacity/__init__.py +++ b/tenacity/__init__.py @@ -415,7 +415,14 @@ def _run_retry(self, retry_state: "RetryCallState") -> None: self.iter_state.retry_run_result = self.retry(retry_state) def _run_wait(self, retry_state: "RetryCallState") -> None: - retry_state.upcoming_sleep = self.wait(retry_state) + # `wait` is annotated as always set, so a type checker sees this guard + # as always true -- but untyped callers legitimately pass `None` or `0` + # to mean "no wait", and `sum([])` over an empty list of strategies + # yields the int 0. Keep honouring those. + if not self.wait: # type: ignore[truthy-bool] + retry_state.upcoming_sleep = 0.0 + else: + retry_state.upcoming_sleep = self.wait(retry_state) def _run_stop(self, retry_state: "RetryCallState") -> None: self.statistics["delay_since_first_attempt"] = retry_state.seconds_since_start diff --git a/tenacity/asyncio/__init__.py b/tenacity/asyncio/__init__.py index a6c2253b..aa4a7560 100644 --- a/tenacity/asyncio/__init__.py +++ b/tenacity/asyncio/__init__.py @@ -147,9 +147,14 @@ async def _run_retry(self, retry_state: "RetryCallState") -> None: # type: igno @override async def _run_wait(self, retry_state: "RetryCallState") -> None: # type: ignore[override] - retry_state.upcoming_sleep = await _utils.wrap_to_async_func(self.wait)( - retry_state - ) + # See BaseRetrying._run_wait: falsy `wait` values mean "no wait" and + # reach us from untyped callers. + if not self.wait: # type: ignore[truthy-bool] + retry_state.upcoming_sleep = 0.0 + else: + retry_state.upcoming_sleep = await _utils.wrap_to_async_func(self.wait)( + retry_state + ) @override async def _run_stop(self, retry_state: "RetryCallState") -> None: # type: ignore[override] diff --git a/tenacity/wait.py b/tenacity/wait.py index e137b357..6607e4fd 100644 --- a/tenacity/wait.py +++ b/tenacity/wait.py @@ -37,13 +37,22 @@ def __call__(self, retry_state: "RetryCallState") -> float: def __add__(self, other: "wait_base") -> "wait_combine": return wait_combine(self, other) - def __radd__(self, other: int) -> "wait_base": - # `sum()` seeds its accumulator with the int 0, so tolerate that to - # make summing waits work. Any other left operand is a `wait_base`, - # whose `__add__` never defers to us. - if other != 0: + # `other` is `int` rather than `Literal[0]` because typeshed's `sum()` + # protocol demands `__radd__(x: int)`; narrowing it would make every + # `sum()` over wait strategies need a `type: ignore`. A non-zero number is + # rejected at runtime instead, below. + def __radd__(self, other: "WaitBaseT | int") -> "wait_combine | wait_base": + if isinstance(other, int): + # `sum()` seeds its accumulator with the int 0; treat that as + # identity so summing a list of strategies works. Any other number + # is not a wait strategy, and saying so here raises TypeError at + # the `+` rather than building a combination that fails when called. + if other == 0: + return self return NotImplemented - return self + # A plain callable -- `WaitBaseT` admits those, and a function has no + # `__add__` of its own to handle `callable + strategy`. + return wait_combine(self, other) WaitBaseT = wait_base | typing.Callable[["RetryCallState"], float | int] @@ -86,12 +95,16 @@ def __call__(self, retry_state: "RetryCallState") -> float: class wait_combine(wait_base): """Combine several waiting strategies.""" - def __init__(self, *strategies: wait_base) -> None: + def __init__(self, *strategies: "WaitBaseT") -> None: self.wait_funcs = strategies @override def __call__(self, retry_state: "RetryCallState") -> float: - return sum(x(retry_state=retry_state) for x in self.wait_funcs) + # Positional, like `BaseRetrying._run_wait`: a `WaitBaseT` callable is + # only guaranteed to take the state positionally, so passing it by + # keyword crashed on any callable whose parameter is not named + # `retry_state`. + return float(sum(x(retry_state) for x in self.wait_funcs)) class wait_chain(wait_base): diff --git a/tests/test_asyncio.py b/tests/test_asyncio.py index c9d58027..f4fded13 100644 --- a/tests/test_asyncio.py +++ b/tests/test_asyncio.py @@ -83,6 +83,19 @@ async def test_retry(self) -> None: await _retryable_coroutine(thing) assert thing.counter == thing.count + @asynctest + async def test_wait_falsy_values_mean_no_wait(self) -> None: + # Mirrors the sync test: falsy `wait` values reach AsyncRetrying from + # untyped callers and must not raise from inside iter(). + for wait in (None, 0): + thing = NoIOErrorAfterCount(2) + retrying = AsyncRetrying( + wait=wait, # type: ignore[arg-type] + stop=stop_after_attempt(5), + ) + await retrying(_async_function, thing) + assert thing.counter == thing.count + @asynctest async def test_iscoroutinefunction(self) -> None: assert inspect.iscoroutinefunction(_retryable_coroutine) diff --git a/tests/test_tenacity.py b/tests/test_tenacity.py index 1acd3f8d..1ad92757 100644 --- a/tests/test_tenacity.py +++ b/tests/test_tenacity.py @@ -506,6 +506,55 @@ def test_wait_arbitrary_sum(self) -> None: self.assertLess(w, 9) self.assertGreaterEqual(w, 6) + def test_wait_falsy_values_mean_no_wait(self) -> None: + # Untyped callers pass None or 0 to mean "no wait", and `sum([])` + # over an empty list of strategies yields the int 0. All must reach + # the retry path without blowing up inside iter(). + def make_flaky() -> typing.Callable[[], str]: + attempts = [] + + def flaky() -> str: + attempts.append(1) + if len(attempts) < 2: + raise ValueError("boom") + return "ok" + + return flaky + + for wait in (None, 0, sum([])): + with self.subTest(wait=wait): + flaky = make_flaky() + r = Retrying( + wait=wait, # type: ignore[arg-type] + stop=tenacity.stop_after_attempt(3), + ) + self.assertEqual(r(flaky), "ok") + + def test_wait_radd_plain_callable(self) -> None: + # A plain callable is a valid WaitBaseT, and functions have no + # __add__, so `callable + strategy` goes through wait_base.__radd__. + def cb(retry_state: RetryCallState) -> float: + return 2.0 + + combined = cb + tenacity.wait_fixed(1) + self.assertIsInstance(combined, tenacity.wait_combine) + self.assertEqual(combined(make_retry_state(1, 5)), 3.0) + + def test_wait_combine_passes_state_positionally(self) -> None: + # A WaitBaseT callable only promises to take the state positionally; + # its parameter name is its own business. + combined = tenacity.wait_combine( + tenacity.wait_fixed(1), + lambda rs: 2.0, + ) + self.assertEqual(combined(make_retry_state(1, 5)), 3.0) + + def test_wait_radd_rejects_non_zero_number(self) -> None: + with self.assertRaises(TypeError): + # Statically accepted -- see the comment on wait_base.__radd__ -- + # so the runtime rejection is what has to be tested. + 5 + tenacity.wait_fixed(1) + def _assert_range(self, wait: float, min_: float, max_: float) -> None: self.assertLess(wait, max_) self.assertGreaterEqual(wait, min_)