Skip to content

retry operators: validate | and & operands on both sync and async retry_base#649

Open
HrachShah wants to merge 3 commits into
jd:mainfrom
HrachShah:fix/retry-predicate-rejects-non-callable
Open

retry operators: validate | and & operands on both sync and async retry_base#649
HrachShah wants to merge 3 commits into
jd:mainfrom
HrachShah:fix/retry-predicate-rejects-non-callable

Conversation

@HrachShah

@HrachShah HrachShah commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

What

The | and & operators on retry_base and async_retry_base
accept either a retry strategy or a plain callable. A non-callable
non-retry_base value (e.g. True | retry_always, retry & 42,
None & async_retry) used to silently build a retry_any or
retry_all whose __call__ later raised TypeError: '<type>' object is not callable (sync) or TypeError: object bool can't be used in 'await' expression (async) from inside the running 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 or True | async_retry fail fast
with an actionable message.

Where

  • tenacity/retry.py: retry_base._validate_predicate (static
    method), called from __and__, __rand__, __or__, __ror__.
  • tenacity/asyncio/retry.py: same guard added to
    async_retry_base.__and__, __rand__, __or__, __ror__,
    and to the leaf branches of retry_any.__ror__ and
    retry_all.__rand__ (which are reached when Python's reflected
    operator dispatch goes through the subclass's override rather than
    the base class's).

Tests

  • 5 new sync tests in tests/test_tenacity.py::TestRetryConditions::test_retry_composition_rejects_non_callable
    covering True | retry, retry | False, retry & 42,
    "nope" | retry, and None & retry.
  • 4 new async tests in tests/test_asyncio.py::TestAsyncRetryComposition:
    test_async_retry_composition_rejects_non_callable (mirrors the
    sync test with async operators), test_async_retry_composition_accepts_valid_operands
    (regression guard for valid compositions still building retry_any /
    retry_all), test_async_retry_any_ror_validates_leaf_operand and
    test_async_retry_all_rand_validates_leaf_operand (cover the
    subclass-operator leaf branches that don't go through
    async_retry_base\.__ror__ / __rand__).
  • python3 -m pytest tests/ passes 171 tests (was 167).
  • ruff check tenacity/asyncio/retry.py tenacity/retry.py clean.
  • Verified manually: True | tenacity.retry_always raises
    TypeError: Retry predicates must be retry_base instances or callables, got bool: True; True | tenacity.asyncio.retry.retry_if_exception(lambda exc: True)
    raises the same; legitimate compositions
    async_retry | sync_retry, async_retry | async_retry,
    async_retry | lambda still build a retry_any with the
    expected operands.

Release notes

releasenotes/notes/reject-non-callable-retry-operands-7c4e9f2b1a3d8c5e.yaml
(sync, added previously) and
releasenotes/notes/fix-async-retry-composition-validates-operand-8f3a2b1e6c4d9a72.yaml
(async, this commit) per the project's reno convention.

jd
jd previously requested changes Jul 3, 2026
Comment thread .github/workflows/ci.yaml
steps:
- name: Checkout 🛎️
uses: actions/checkout@v7.0.0
uses: actions/checkout@v6.0.3

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.

unrelated

The sync retry_base operators | and & got a guard in jd#648 that rejects
non-callable, non-retry_base operands with a clear TypeError at the
construction site. The async_retry_base (and async retry_any.__ror__ /
retry_all.__rand__ that override the inherited methods) define the same
operator surface but did not call _validate_predicate, so the guard did
not extend to the async path.

Before this fix:
  True | retry_if_exception(...)  # built silently
  ...later, inside the retry loop, awaited a bool and crashed with
  'TypeError: object bool can t be used in await expression', far from
  the line that actually wrote the bad expression.

After this fix the same misuse raises synchronously at the | / & site
with a clear message naming the bad operand's type and repr, matching
the sync behaviour.

Adds four new tests in tests/test_asyncio.py:
- test_async_retry_composition_rejects_non_callable: covers the four
  operator overloads (|, __ror__, &, __rand__) with bool/int/str/None.
- test_async_retry_composition_accepts_valid_operands: regression guard
  for legitimate async | async, async | sync, async | callable
  compositions.
- test_async_retry_any_ror_validates_leaf_operand: covers the override
  in retry_any that flattens nested retry_any groups; the leaf branch
  must still validate.
- test_async_retry_all_rand_validates_leaf_operand: mirror of the above
  for retry_all.__rand__.

171 tests pass (was 167).
@HrachShah HrachShah changed the title retry_base: validate | and & operands are retry_base or callable retry operators: validate | and & operands on both sync and async retry_base Jul 4, 2026
@mergify mergify Bot dismissed jd’s stale review July 4, 2026 21:33

Pull request has been modified.

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