Skip to content

fix: restore support for falsy wait values and callable + strategy - #694

Merged
mergify[bot] merged 1 commit into
mainfrom
devs/jd/fix/wait-regressions/restore-support-falsy-wait-values-callable--a40c32a2
Aug 6, 2026
Merged

fix: restore support for falsy wait values and callable + strategy#694
mergify[bot] merged 1 commit into
mainfrom
devs/jd/fix/wait-regressions/restore-support-falsy-wait-values-callable--a40c32a2

Conversation

@jd

@jd jd commented Aug 5, 2026

Copy link
Copy Markdown
Owner

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().
    refactor: drop always-true truthiness checks and enable truthy-bool #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 fix: annotate wait_base.__radd__ as taking the int seed from sum() #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.

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
@jd

jd commented Aug 5, 2026

Copy link
Copy Markdown
Owner Author

This pull request is part of a Mergify stack:

# Pull Request Link
1 fix: restore support for falsy wait values and callable + strategy #694 👈
2 fix: read the tornado global live instead of an import-time snapshot #695

@jd
jd marked this pull request as ready for review August 6, 2026 06:43
@mergify

mergify Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-08-06 06:44 UTC · Rule: default · triggered by rule autoqueue
  • Checks skipped · PR is already up-to-date
  • Merged2026-08-06 06:44 UTC · at 6e425f390dc7dfa1f3e83e7ab9fc44b4618d2bb7 · squash

This pull request spent 7 seconds in the queue, including 1 second running CI.

Required conditions to merge

@mergify
mergify Bot merged commit 389aab8 into main Aug 6, 2026
9 checks passed
@mergify
mergify Bot deleted the devs/jd/fix/wait-regressions/restore-support-falsy-wait-values-callable--a40c32a2 branch August 6, 2026 06:44
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.

1 participant