Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
fixes:
- |
``async_retry_base`` operators (``|``, ``&``, and their reflected
forms) now validate their operand at the operator site, matching
the behaviour already in place for the sync ``retry_base``. A
non-callable, non-``retry_base`` value used to silently construct
an ``async_retry_any``/``async_retry_all`` whose ``__call__``
later raised ``TypeError: object <type> can't be used in 'await'
expression`` from inside the retry loop, far from the original
misuse. The operators now raise a clear ``TypeError`` naming the
bad operand at the construction site. ``retry_any.__ror__`` and
``retry_all.__rand__`` also validate the leaf operand on their
flattening fallthrough path, so ``True | retry_any(...)`` and
``None & retry_all(...)`` fail fast with the same actionable
message instead of slipping a non-callable through the async
operator chain.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
fixes:
- |
``wait_chain()`` now raises ``ValueError`` at construction time when
called with no strategies, instead of raising a confusing
``IndexError`` from ``__call__`` the first time it is used. Empty
wait chains have no useful behaviour, so failing early gives a
clearer error message.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
fixes:
- |
``retry_base`` operators (``|``, ``&``, and their reflected forms)
now validate their operand at the operator site. A non-callable,
non-``retry_base`` value (e.g. ``True | retry_always``, ``retry & 42``,
``"nope" | retry``, ``None & retry``) used to silently construct a
``retry_any``/``retry_all`` whose ``__call__`` later raised
``TypeError: '<type>' object is not callable`` from inside the retry
loop, far from the original misuse. The operators now raise a clear
``TypeError`` naming the bad operand at the construction site, so
common typos like ``retry or True`` fail fast with an actionable
message.
18 changes: 18 additions & 0 deletions tenacity/asyncio/retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,25 @@ async def __call__(self, retry_state: "RetryCallState") -> bool: # type: ignore
def __and__( # type: ignore[override]
self, other: "retry_base | async_retry_base"
) -> "retry_all":
self._validate_predicate(other)
return retry_all(self, other)

def __rand__( # type: ignore[misc,override]
self, other: "retry_base | async_retry_base"
) -> "retry_all":
self._validate_predicate(other)
return retry_all(other, self)

def __or__( # type: ignore[override]
self, other: "retry_base | async_retry_base"
) -> "retry_any":
self._validate_predicate(other)
return retry_any(self, other)

def __ror__( # type: ignore[misc,override]
self, other: "retry_base | async_retry_base"
) -> "retry_any":
self._validate_predicate(other)
return retry_any(other, self)


Expand Down Expand Up @@ -109,6 +113,14 @@ async def __call__(self, retry_state: "RetryCallState") -> bool: # type: ignore
def __ror__( # type: ignore[misc,override]
self, other: "retry_base | async_retry_base"
) -> "retry_any":
# async_retry_base.__ror__ already validated the operand for
# the common `retry | non_retry` path, but retry_any itself
# defines __ror__ to flatten nested retry_any groupings. The
# flattening branch (``isinstance(other, retry_any)``) only
# composes with another retry_any so the operand is already
# known to be a valid retry_base; the leaf branch must still
# validate the raw value before folding it in.
self._validate_predicate(other)
if isinstance(other, retry_any):
return retry_any(*other.retries, *self.retries)
return retry_any(other, *self.retries)
Expand All @@ -131,6 +143,12 @@ async def __call__(self, retry_state: "RetryCallState") -> bool: # type: ignore
def __rand__( # type: ignore[misc,override]
self, other: "retry_base | async_retry_base"
) -> "retry_all":
# async_retry_base.__rand__ already validated the operand for
# the common `non_retry & retry` path, but retry_all itself
# defines __rand__ to flatten nested retry_all groupings. Same
# reasoning as retry_any.__ror__ above: the flatten branch is
# safe by construction, the leaf branch must validate.
self._validate_predicate(other)
if isinstance(other, retry_all):
return retry_all(*other.retries, *self.retries)
return retry_all(other, *self.retries)
20 changes: 20 additions & 0 deletions tenacity/retry.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,24 @@ class retry_base(abc.ABC):
def __call__(self, retry_state: "RetryCallState") -> bool:
pass

@staticmethod
def _validate_predicate(other: "RetryBaseT") -> None:
# The RetryBaseT union documents that | / & accept either a
# retry_base subclass or a plain callable. Without this guard a
# non-callable (e.g. ``True | retry_always``) builds a
# retry_any/retry_all whose ``__call__`` later raises
# ``TypeError: 'bool' object is not callable`` far from the
# original misuse. Rejecting the value at the operator site
# gives a clear error pointing at the bad operand.
if isinstance(other, retry_base) or callable(other):
return
raise TypeError(
"Retry predicates must be retry_base instances or callables, "
f"got {type(other).__name__}: {other!r}"
)

def __and__(self, other: "RetryBaseT") -> "retry_all":
self._validate_predicate(other)
if isinstance(other, retry_base):
return other.__rand__(self)
# Plain callable: flatten if self is already a retry_all
Expand All @@ -38,12 +55,14 @@ def __and__(self, other: "RetryBaseT") -> "retry_all":
return retry_all(self, other)

def __rand__(self, other: "RetryBaseT") -> "retry_all":
self._validate_predicate(other)
# Flatten if other is already a retry_all
if isinstance(other, retry_all):
return retry_all(*other.retries, self)
return retry_all(other, self)

def __or__(self, other: "RetryBaseT") -> "retry_any":
self._validate_predicate(other)
if isinstance(other, retry_base):
return other.__ror__(self)
# Plain callable: flatten if self is already a retry_any
Expand All @@ -52,6 +71,7 @@ def __or__(self, other: "RetryBaseT") -> "retry_any":
return retry_any(self, other)

def __ror__(self, other: "RetryBaseT") -> "retry_any":
self._validate_predicate(other)
# Flatten if other is already a retry_any
if isinstance(other, retry_any):
return retry_any(*other.retries, self)
Expand Down
2 changes: 2 additions & 0 deletions tenacity/wait.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,8 @@ def wait_chained():
"""

def __init__(self, *strategies: wait_base) -> None:
if not strategies:
raise ValueError("wait_chain() requires at least one strategy")
self.strategies = strategies

def __call__(self, retry_state: "RetryCallState") -> float:
Expand Down
97 changes: 97 additions & 0 deletions tests/test_asyncio.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,103 @@ async def test_enabled_false_aiter_succeeds_on_first_attempt(self) -> None:
assert call_count == 1


class TestAsyncRetryComposition(unittest.TestCase):
"""The async retry operator overloads (| and &) should reject non-callable,
non-retry_base operands at the construction site, matching the sync
behaviour added in test_retry_composition_rejects_non_callable. Without
the guard, an operand like ``True | async_retry_if_exception(...)``
used to silently build a retry_any whose ``__call__`` later raised
``TypeError: object bool can't be used in 'await' expression`` from
inside the retry loop, far from the original misuse."""

def test_async_retry_composition_rejects_non_callable(self) -> None:
async_retry = tasyncio.retry.retry_if_exception(lambda exc: True)

# `True | async_retry` exercises async_retry_base.__ror__
with self.assertRaises(TypeError) as ctx:
True | async_retry # noqa: B018
self.assertIn("callable", str(ctx.exception))
self.assertIn("bool", str(ctx.exception))

# `async_retry | False` exercises async_retry_base.__or__
with self.assertRaises(TypeError) as ctx:
async_retry | False # noqa: B018
self.assertIn("callable", str(ctx.exception))
self.assertIn("bool", str(ctx.exception))

# An int is a non-callable, non-retry_base
with self.assertRaises(TypeError) as ctx:
async_retry & 42 # noqa: B018
self.assertIn("callable", str(ctx.exception))
self.assertIn("int", str(ctx.exception))

# A string is a non-callable, non-retry_base
with self.assertRaises(TypeError) as ctx:
"nope" | async_retry # noqa: B018
self.assertIn("callable", str(ctx.exception))
self.assertIn("str", str(ctx.exception))

# The same guard must apply on the right side of & via __rand__
with self.assertRaises(TypeError) as ctx:
None & async_retry # noqa: B018
self.assertIn("callable", str(ctx.exception))
self.assertIn("NoneType", str(ctx.exception))

def test_async_retry_composition_accepts_valid_operands(self) -> None:
"""The guard must let through retry_base instances, sync retry_base
instances, and plain callables, so legitimate compositions keep
working unchanged."""
async_retry = tasyncio.retry.retry_if_exception(lambda exc: True)
other_async = tasyncio.retry.retry_if_result(lambda r: r is None)
sync_retry = tenacity.retry_always
plain_callable = lambda _state: True # noqa: E731

# async | async
combined = async_retry | other_async
self.assertIsInstance(combined, tasyncio.retry.retry_any)
# async | sync
combined = async_retry | sync_retry
self.assertIsInstance(combined, tasyncio.retry.retry_any)
# async | callable
combined = async_retry | plain_callable
self.assertIsInstance(combined, tasyncio.retry.retry_any)
# async & async
combined = async_retry & other_async
self.assertIsInstance(combined, tasyncio.retry.retry_all)
# async & sync
combined = async_retry & sync_retry
self.assertIsInstance(combined, tasyncio.retry.retry_all)
# async & callable
combined = async_retry & plain_callable
self.assertIsInstance(combined, tasyncio.retry.retry_all)

def test_async_retry_any_ror_validates_leaf_operand(self) -> None:
"""``retry_any.__ror__`` flattens nested retry_any groupings, but
the leaf branch (when ``other`` is not a retry_any) must still
validate that the raw value is a retry_base or callable. Without
that guard, ``True | some_async_retry`` would skip the
async_retry_base.__ror__ validator and build a retry_any
containing a non-callable."""
any_obj = tasyncio.retry.retry_any(
tasyncio.retry.retry_if_exception(lambda exc: True)
)
with self.assertRaises(TypeError) as ctx:
True | any_obj # noqa: B018
self.assertIn("callable", str(ctx.exception))
self.assertIn("bool", str(ctx.exception))

def test_async_retry_all_rand_validates_leaf_operand(self) -> None:
"""Mirror of test_async_retry_any_ror_validates_leaf_operand for
the ``&`` operator and retry_all."""
all_obj = tasyncio.retry.retry_all(
tasyncio.retry.retry_if_exception(lambda exc: True)
)
with self.assertRaises(TypeError) as ctx:
None & all_obj # noqa: B018
self.assertIn("callable", str(ctx.exception))
self.assertIn("NoneType", str(ctx.exception))


@unittest.skipIf(not have_trio, "trio not installed")
class TestTrio(unittest.TestCase):
def test_trio_basic(self) -> None:
Expand Down
40 changes: 36 additions & 4 deletions tests/test_tenacity.py
Original file line number Diff line number Diff line change
Expand Up @@ -541,10 +541,13 @@ def always_return_1() -> int:
self.assertEqual(sleep_intervals, [1.0, 2.0, 3.0, 3.0])
sleep_intervals[:] = []

# Clear and restart retrying.
self.assertRaises(tenacity.RetryError, always_return_1)
self.assertEqual(sleep_intervals, [1.0, 2.0, 3.0, 3.0])
sleep_intervals[:] = []
def test_wait_chain_requires_at_least_one_strategy(self) -> None:
with self.assertRaises(ValueError):
tenacity.wait_chain()
# Confirm the wrapped Retrying path surfaces the same ValueError
# instead of an opaque IndexError from self.strategies[-1].
with self.assertRaises(ValueError):
Retrying(wait=tenacity.wait_chain())

def test_wait_random_exponential(self) -> None:
fn = tenacity.wait_random_exponential(0.5, 60.0)
Expand Down Expand Up @@ -837,6 +840,35 @@ def test_retry_and_coalesces(self) -> None:
self.assertIsInstance(combined, retry_all)
self.assertEqual(len(combined.retries), 3)

def test_retry_composition_rejects_non_callable(self) -> None:
"""Non-callable, non-retry_base operands (bool/int/None/str/...)
used to silently construct a retry_any/retry_all that raised
``TypeError: '<type>' object is not callable`` from inside the
retry loop. The operator should now raise a clear TypeError at
the construction site that names the bad operand."""
retry = tenacity.retry_always

# `True | retry` exercises __ror__ on retry_base
with self.assertRaises(TypeError) as ctx:
True | retry # noqa: B018
self.assertIn("callable", str(ctx.exception))

# `retry | False` exercises __or__ on retry_base
with self.assertRaises(TypeError):
retry | False # noqa: B018

# An int is a non-callable, non-retry_base
with self.assertRaises(TypeError):
retry & 42 # noqa: B018

# A string is a non-callable, non-retry_base
with self.assertRaises(TypeError):
"nope" | retry # noqa: B018

# The same guard must apply on the right side of & via __rand__
with self.assertRaises(TypeError):
None & retry # noqa: B018

def _raise_try_again(self) -> None:
self._attempts += 1
if self._attempts < 3:
Expand Down
Loading