From 762a74acd23494aeea8f039be1d26b251bf833a2 Mon Sep 17 00:00:00 2001
From: Alexey Shalaev <75322386+AlexeyShalaev@users.noreply.github.com>
Date: Mon, 7 Sep 2026 10:00:22 +0300
Subject: [PATCH] fix: hand redis.asyncio.Redis the async Retry, not the sync
 one

build_redis_retry built redis.retry.Retry for both factories, and the
async factory handed that to redis.asyncio.Redis. The sync class's
call_with_retry does not await, so the client awaited the coroutine
outside the retry loop and retry.enabled=True, max_attempts=N never
retried on an async client: 0.50 s to fail against a paused Redis with
socket_timeout=0.5, where the sync client took 3.42 s.

Both helpers take asyncio=False now; the async factory passes True and
gets redis.asyncio.retry.Retry, for the exponential object and for the
zero-retry one alike. The sync path is unchanged. The API rows and rule
5 on the agents page and the retry section of the configuration guide
say so.
---
 docs/agents.md                            | 18 +++++--
 docs/guide/configuration.md               |  6 +++
 redis_client_kit/aio/factory.py           |  4 +-
 redis_client_kit/utils.py                 | 35 +++++++++---
 tests/integration/test_redis_lifecycle.py | 65 +++++++++++++++++++++++
 tests/unit/aio/test_client.py             |  2 +-
 tests/unit/aio/test_resilience.py         | 24 ++++++---
 tests/unit/test_utils.py                  | 39 +++++++++++++-
 8 files changed, 172 insertions(+), 21 deletions(-)

diff --git a/docs/agents.md b/docs/agents.md
index 8480381..74d5220 100644
--- a/docs/agents.md
+++ b/docs/agents.md
@@ -205,8 +205,8 @@ Everything in this table is importable from `redis_client_kit` itself.
 | `check_redis_health` | `(client)` | `bool` — never raises |
 | `close_async_redis_client` | `await (client)` | `None` — shielded, 10 s timeout, never raises |
 | `close_redis_client` | `(client)` | `None` — never raises |
-| `build_base_redis_kwargs` | `(settings)` | `dict[str, object]` of `redis-py` keyword arguments |
-| `build_redis_retry` | `(settings)` | `redis.retry.Retry` — `Retry(NoBackoff(), 0)` when retries are off |
+| `build_base_redis_kwargs` | `(settings, asyncio=False)` | `dict[str, object]` of `redis-py` keyword arguments; `asyncio=True` for a `redis.asyncio` client |
+| `build_redis_retry` | `(settings, asyncio=False)` | `redis.retry.Retry`, or `redis.asyncio.retry.Retry` with `asyncio=True` — `Retry(NoBackoff(), 0)` when retries are off |
 | `parse_redis_url_node` | `(node)` | `tuple[str, int]` — `ValueError` on a node with no port |
 | `AsyncRedisClient` | type alias | `redis.asyncio.Redis | redis.asyncio.cluster.RedisCluster` |
 | `SyncRedisClient` | type alias | `redis.Redis | redis.cluster.RedisCluster` |
@@ -309,7 +309,11 @@ See rules 15 to 17.
    `Retry(NoBackoff(), 0)` explicitly — never nothing, because `redis-py` given no
    `Retry` retries on its own since 6.0 (three times; ten since 8.0, with jittered
    backoff), which turns a `socket_timeout=0.5` failure into ten seconds or more. With
-   retries off, the first `ConnectionError` or `TimeoutError` is the one you get.
+   retries off, the first `ConnectionError` or `TimeoutError` is the one you get. The
+   flavour follows the client: the async factory hands `redis.asyncio.retry.Retry`, the
+   sync factory `redis.retry.Retry`, and both helpers build the sync class unless called
+   with `asyncio=True`. A sync `Retry` on a `redis.asyncio` client never retries — its
+   `call_with_retry` does not await, so the failure never reaches the loop.
 6. **Retries cover connection failures, not command failures.** `redis-py`'s `Retry`
    defaults to `ConnectionError`, `TimeoutError` and `socket.timeout`; a `ResponseError`
    from a bad command is raised on the first try. The delay is
@@ -404,6 +408,14 @@ BaseRedisSettings(
 )
 ```
 
+```python
+# WRONG — a sync Retry on an async client never retries: its call_with_retry does not await
+client = redis.asyncio.Redis(**build_base_redis_kwargs(settings), host="localhost", port=6379)
+
+# RIGHT — asyncio=True builds redis.asyncio.retry.Retry, which is what the async factory does
+client = redis.asyncio.Redis(**build_base_redis_kwargs(settings, asyncio=True), host="localhost", port=6379)
+```
+
 ```python
 # WRONG — metrics_enabled does nothing, and the client is never instrumented
 settings = BaseRedisSettings(key_prefix="myapp", metrics_enabled=True)
diff --git a/docs/guide/configuration.md b/docs/guide/configuration.md
index 70b03ba..d3b82aa 100644
--- a/docs/guide/configuration.md
+++ b/docs/guide/configuration.md
@@ -161,6 +161,12 @@ or more. Disabled means the first `ConnectionError` or `TimeoutError` is raised
 with `socket_timeout=0.5`, a command against an unreachable Redis fails in about half a
 second.
 
+The async factory hands redis-py `redis.asyncio.retry.Retry` and the sync factory
+`redis.retry.Retry`; they are not interchangeable, because the sync class's
+`call_with_retry` does not await, so on a `redis.asyncio` client it never retries. If you
+assemble the keyword arguments yourself, `build_base_redis_kwargs(settings, asyncio=True)`
+and `build_redis_retry(settings, asyncio=True)` build the async flavour.
+
 Retry logic uses exponential backoff:
 ```
 delay = min(backoff_cap, backoff_base * (2 ** failures))
diff --git a/redis_client_kit/aio/factory.py b/redis_client_kit/aio/factory.py
index 998c1c1..c386806 100644
--- a/redis_client_kit/aio/factory.py
+++ b/redis_client_kit/aio/factory.py
@@ -50,7 +50,7 @@ def _create_async_single_client(
         extra={"host": settings.connection.host, "port": settings.connection.port},
     )
 
-    client_kwargs: dict[str, Any] = build_base_redis_kwargs(settings)
+    client_kwargs: dict[str, Any] = build_base_redis_kwargs(settings, asyncio=True)
     client_kwargs.update(
         {
             "host": settings.connection.host,
@@ -87,7 +87,7 @@ def _create_async_cluster_client(
         },
     )
 
-    cluster_kwargs: dict[str, Any] = build_base_redis_kwargs(settings)
+    cluster_kwargs: dict[str, Any] = build_base_redis_kwargs(settings, asyncio=True)
     cluster_kwargs.update(
         {
             "startup_nodes": startup_nodes,
diff --git a/redis_client_kit/utils.py b/redis_client_kit/utils.py
index 486f88f..bbcb4c8 100644
--- a/redis_client_kit/utils.py
+++ b/redis_client_kit/utils.py
@@ -3,8 +3,10 @@
 import base64
 import binascii
 from pathlib import Path
+from typing import Literal, overload
 from urllib.parse import urlparse
 
+from redis.asyncio.retry import Retry as AsyncRetry
 from redis.backoff import ExponentialBackoff, NoBackoff
 from redis.retry import Retry
 
@@ -34,8 +36,12 @@ def parse_redis_url_node(node: str) -> tuple[str, int]:
     return host, port
 
 
-def build_base_redis_kwargs(settings: RedisSettingsProtocol) -> dict[str, object]:
-    """Build base Redis client keyword arguments."""
+def build_base_redis_kwargs(settings: RedisSettingsProtocol, *, asyncio: bool = False) -> dict[str, object]:
+    """Build base Redis client keyword arguments.
+
+    ``asyncio=True`` builds them for a ``redis.asyncio`` client, whose ``retry`` has to be
+    ``redis.asyncio.retry.Retry``.
+    """
     if settings.ssl.enabled:
         if settings.ssl.ca_certs:
             _validate_pem_format(settings.ssl.ca_certs, "CERTIFICATE")
@@ -52,7 +58,7 @@ def build_base_redis_kwargs(settings: RedisSettingsProtocol) -> dict[str, object
         "socket_keepalive": settings.pool.socket_keepalive,
         "socket_keepalive_options": settings.pool.socket_keepalive_options,
         "health_check_interval": settings.health_check_interval,
-        "retry": build_redis_retry(settings),
+        "retry": build_redis_retry(settings, asyncio=asyncio),
         "decode_responses": settings.response.decode_responses,
         "encoding": settings.response.encoding,
         "client_name": settings.connection.client_name,
@@ -72,22 +78,39 @@ def build_base_redis_kwargs(settings: RedisSettingsProtocol) -> dict[str, object
     return kwargs
 
 
-def build_redis_retry(settings: RedisSettingsProtocol) -> Retry:
+@overload
+def build_redis_retry(settings: RedisSettingsProtocol, *, asyncio: Literal[False] = ...) -> Retry: ...
+
+
+@overload
+def build_redis_retry(settings: RedisSettingsProtocol, *, asyncio: Literal[True]) -> AsyncRetry: ...
+
+
+@overload
+def build_redis_retry(settings: RedisSettingsProtocol, *, asyncio: bool) -> Retry | AsyncRetry: ...
+
+
+def build_redis_retry(settings: RedisSettingsProtocol, *, asyncio: bool = False) -> Retry | AsyncRetry:
     """Build Redis Retry object from settings.
 
     Retries are off unless both ``retry.enabled`` and ``retry.max_attempts`` are set. Off is
     still a ``Retry``, with zero retries: handed nothing, redis-py retries on its own (three
     times since 6.0, ten since 8.0, with jittered backoff).
+
+    ``asyncio=True`` builds ``redis.asyncio.retry.Retry``, the one a ``redis.asyncio`` client
+    needs: the sync class's ``call_with_retry`` does not await, so on an async client it
+    never retries.
     """
+    retry_class = AsyncRetry if asyncio else Retry
     if settings.retry.enabled and settings.retry.max_attempts:
-        return Retry(
+        return retry_class(
             backoff=ExponentialBackoff(
                 cap=settings.retry.backoff_cap,
                 base=settings.retry.backoff_base,
             ),
             retries=settings.retry.max_attempts,
         )
-    return Retry(backoff=NoBackoff(), retries=0)
+    return retry_class(backoff=NoBackoff(), retries=0)
 
 
 def _validate_pem_format(path: str | Path, file_type: str) -> None:
diff --git a/tests/integration/test_redis_lifecycle.py b/tests/integration/test_redis_lifecycle.py
index a0260f3..298709c 100644
--- a/tests/integration/test_redis_lifecycle.py
+++ b/tests/integration/test_redis_lifecycle.py
@@ -107,6 +107,39 @@ async def test__async_redis_client__retry_disabled_and_redis_paused__raises_with
     assert elapsed < 2.0
 
 
+@pytest.mark.asyncio
+async def test__async_redis_client__retry_enabled_and_redis_paused__retries_before_raising(
+    redis_container: RedisContainer,
+) -> None:
+    # Arrange
+    settings = FakeRedisSettings()
+    settings.connection.host = redis_container.get_container_host_ip()
+    settings.connection.port = int(redis_container.get_exposed_port(redis_container.port))
+    settings.pool.socket_timeout = 0.5
+    settings.pool.socket_connect_timeout = 0.5
+    settings.retry.enabled = True
+    settings.retry.max_attempts = 3
+    settings.retry.backoff_base = 0.1
+    settings.retry.backoff_cap = 1.0
+    client = create_async_redis_client(settings)
+    container = redis_container.get_wrapped_container()
+    await client.ping()
+
+    # Act
+    container.pause()
+    try:
+        started = time.perf_counter()
+        with pytest.raises(RedisTimeoutError):
+            await client.get("paused")
+        elapsed = time.perf_counter() - started
+    finally:
+        container.unpause()
+        await close_async_redis_client(client)
+
+    # Assert - four attempts of 0.5 s plus 0.2 + 0.4 + 0.8 s of backoff, not a single attempt
+    assert 3.0 <= elapsed < 6.0
+
+
 def test__sync_redis_client__retry_disabled_and_redis_paused__raises_within_socket_timeout(
     redis_container: RedisContainer,
 ) -> None:
@@ -134,3 +167,35 @@ def test__sync_redis_client__retry_disabled_and_redis_paused__raises_within_sock
 
     # Assert
     assert elapsed < 2.0
+
+
+def test__sync_redis_client__retry_enabled_and_redis_paused__retries_before_raising(
+    redis_container: RedisContainer,
+) -> None:
+    # Arrange
+    settings = FakeRedisSettings()
+    settings.connection.host = redis_container.get_container_host_ip()
+    settings.connection.port = int(redis_container.get_exposed_port(redis_container.port))
+    settings.pool.socket_timeout = 0.5
+    settings.pool.socket_connect_timeout = 0.5
+    settings.retry.enabled = True
+    settings.retry.max_attempts = 3
+    settings.retry.backoff_base = 0.1
+    settings.retry.backoff_cap = 1.0
+    client = create_redis_client(settings)
+    container = redis_container.get_wrapped_container()
+    client.ping()
+
+    # Act
+    container.pause()
+    try:
+        started = time.perf_counter()
+        with pytest.raises(RedisTimeoutError):
+            client.get("paused")
+        elapsed = time.perf_counter() - started
+    finally:
+        container.unpause()
+        close_redis_client(client)
+
+    # Assert - four attempts of 0.5 s plus 0.2 + 0.4 + 0.8 s of backoff, not a single attempt
+    assert 3.0 <= elapsed < 6.0
diff --git a/tests/unit/aio/test_client.py b/tests/unit/aio/test_client.py
index 847507c..f92779e 100644
--- a/tests/unit/aio/test_client.py
+++ b/tests/unit/aio/test_client.py
@@ -3,9 +3,9 @@
 
 import pytest
 from redis.asyncio.cluster import RedisCluster
+from redis.asyncio.retry import Retry
 from redis.backoff import NoBackoff
 from redis.exceptions import RedisError
-from redis.retry import Retry
 
 from redis_client_kit.aio import (
     check_async_redis_health,
diff --git a/tests/unit/aio/test_resilience.py b/tests/unit/aio/test_resilience.py
index ec32af0..363ae9e 100644
--- a/tests/unit/aio/test_resilience.py
+++ b/tests/unit/aio/test_resilience.py
@@ -2,27 +2,37 @@
 
 import pytest
 from redis.asyncio import RedisCluster
+from redis.asyncio.retry import Retry
 from redis.exceptions import BusyLoadingError, ClusterDownError, TimeoutError
-from redis.retry import Retry
 
 from redis_client_kit import check_async_redis_health, create_async_redis_client
 
 
-def test__create_async_redis_client__retry_enabled__passes_retry_to_client(mock_redis_settings: MagicMock) -> None:
+@pytest.mark.parametrize(
+    "cluster_mode, expected_class",
+    [
+        (False, "InstrumentedRedis"),
+        (True, "InstrumentedRedisCluster"),
+    ],
+    ids=["single-node", "cluster"],
+)
+def test__create_async_redis_client__retry_enabled__passes_async_retry_to_client(
+    mock_redis_settings: MagicMock, cluster_mode: bool, expected_class: str
+) -> None:
     # Arrange
+    mock_redis_settings.cluster.enabled = cluster_mode
     mock_redis_settings.retry.enabled = True
     mock_redis_settings.retry.max_attempts = 5
-    mock_redis_settings.retry_backoff_base = 0.5
     mock_metrics = MagicMock()
 
     # Act
-    with patch("redis_client_kit.aio.factory.InstrumentedRedis") as mock_redis:
+    with patch(f"redis_client_kit.aio.factory.{expected_class}") as mock_class:
         create_async_redis_client(mock_redis_settings, metrics=mock_metrics)
 
         # Assert
-        kwargs = mock_redis.call_args[1]
-        assert "retry" in kwargs
-        assert isinstance(kwargs["retry"], Retry)
+        retry = mock_class.call_args[1]["retry"]
+        assert isinstance(retry, Retry)
+        assert retry._retries == 5
 
 
 def test__create_async_redis_client__ssl_enabled__passes_ssl_config_to_client(mock_redis_settings: MagicMock) -> None:
diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py
index 130ec77..3d441d8 100644
--- a/tests/unit/test_utils.py
+++ b/tests/unit/test_utils.py
@@ -1,6 +1,7 @@
 from unittest.mock import MagicMock, patch
 
 import pytest
+from redis.asyncio.retry import Retry as AsyncRetry
 from redis.backoff import NoBackoff
 from redis.retry import Retry
 
@@ -68,8 +69,34 @@ def test__build_redis_retry__retry_enabled__returns_retry_object(mock_redis_sett
     retry = build_redis_retry(mock_redis_settings)
 
     # Assert
-    assert retry is not None
-    assert getattr(retry, "_retries", 0) == 5
+    assert isinstance(retry, Retry)
+    assert retry._retries == 5
+
+
+def test__build_redis_retry__asyncio_retry_enabled__returns_async_retry_object(mock_redis_settings: MagicMock) -> None:
+    # Arrange
+    mock_redis_settings.retry.enabled = True
+    mock_redis_settings.retry.max_attempts = 5
+
+    # Act
+    retry = build_redis_retry(mock_redis_settings, asyncio=True)
+
+    # Assert
+    assert isinstance(retry, AsyncRetry)
+    assert retry._retries == 5
+
+
+def test__build_redis_retry__asyncio_retry_disabled__returns_async_zero_retries(mock_redis_settings: MagicMock) -> None:
+    # Arrange
+    mock_redis_settings.retry.enabled = False
+
+    # Act
+    retry = build_redis_retry(mock_redis_settings, asyncio=True)
+
+    # Assert
+    assert isinstance(retry, AsyncRetry)
+    assert retry._retries == 0
+    assert isinstance(retry._backoff, NoBackoff)
 
 
 def test__build_redis_retry__no_max_attempts__returns_zero_retries(mock_redis_settings: MagicMock) -> None:
@@ -121,6 +148,14 @@ def test__build_base_redis_kwargs__retry_disabled__passes_zero_retries(
     assert isinstance(retry._backoff, NoBackoff)
 
 
+def test__build_base_redis_kwargs__asyncio__passes_async_retry(mock_redis_settings: MagicMock) -> None:
+    # Act
+    kwargs = build_base_redis_kwargs(mock_redis_settings, asyncio=True)
+
+    # Assert
+    assert isinstance(kwargs["retry"], AsyncRetry)
+
+
 def test__build_base_redis_kwargs__ssl_enabled__includes_ssl_parameters(mock_redis_settings: MagicMock) -> None:
     # Arrange
     mock_redis_settings.ssl.enabled = True