asyncio: passing the sync redis.retry.Retry silently disables retries and failure callbacks

[Brumbelow]

Description

Filed from the review of #4260 (see #4252): passing the sync redis.retry.Retry to any redis.asyncio client, pool, or connection is accepted silently and degrades every retry seam to a single attempt with no failure callback. It's an easy mistake — same class name, and the sync import is muscle memory — and the #4252 reproduction script did exactly this:

from redis.retry import Retry            # sync — wrong for asyncio, accepted silently
from redis.backoff import ExponentialBackoff
import redis.asyncio as aredis

pool = aredis.ConnectionPool.from_url(
    "redis://localhost:6379/0",
    retry=Retry(ExponentialBackoff(), 3),
    retry_on_error=[aredis.ConnectionError, aredis.TimeoutError],
)

Mechanism

Async call sites do await retry.call_with_retry(do, fail) where do is a coroutine function. The sync Retry.call_with_retry is a plain def: it calls do(), which for a coroutine function just creates the coroutine without raising, so the sync wrapper returns that coroutine immediately — its try/except around do() never observes the eventual exception. The caller then awaits the returned coroutine directly; when it fails:

• the exception propagates on the first attempt (no retry loop, no backoff), and
• the fail callback never runs — so e.g. Connection.check_health()'s _ping_failed (which disconnects the connection so the retried PING reconnects) and execute_command's disconnect-on-error handler are skipped.

conn.retry.get_retries() still reports the configured count and update_supported_errors() works, so the misconfiguration is invisible to the checks the #4252 reporter ran.

This affects every async retry seam: Connection.connect(), connect_check_health(), check_health(), Redis.execute_command(), and the cluster paths. Observed concretely while diagnosing #4252: the health-check PING failure on a server-closed connection surfaced unretried instead of disconnect-reconnect-retry.

The reverse direction (async redis.asyncio.retry.Retry passed to the sync client) fails loudly — commands return unawaited coroutine objects and Python emits "coroutine was never awaited" warnings — so it's the sync-into-async direction that needs a guard.

Suggested guard

Either, at the async boundary (redis.asyncio AbstractConnection.__init__ and the client/pool constructors that store a retry):

1. Convert: if the object isn't the async Retry but is an AbstractRetry, rebuild it as redis.asyncio.retry.Retry(retry._backoff, retry._retries, retry._supported_errors) (optionally with a warning). Preserves the user's intent and silently repairs configurations that are broken today; or
2. Reject: raise TypeError with a pointed message ("use redis.asyncio.retry.Retry with redis.asyncio clients").

Option 1 seems preferable since code affected today appears to work — its retries just never happen — and a hard error would turn silent misbehavior into a crash on upgrade.

Version

Present on master (3be919d3) and at least back through 8.0.x; not a regression, a long-standing silent-misconfiguration trap.

[petyaslavova]

Hey @Brumbelow, thanks for the follow-up!