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_)