Skip to content

on_connect_check_health raises indistinguishable TimeoutError from pipeline response read timeout #4240

Description

@jangeja

Bug

pipeline.execute() can raise redis.exceptions.TimeoutError: Timeout reading from <host>
from two completely different code paths that have opposite semantics:

  1. Response read timeout (_execute_pipelineread_response): The write commands already landed in Redis. The timeout occurred while reading the response back. The write succeeded.

  2. Connection health check timeout (get_connectionensure_connectionconnectconnect_check_healthon_connect_check_healthread_response): The connection could not be established. The write never reached Redis.

Both raise identical exceptions:

redis.exceptions.TimeoutError: Timeout reading from <host>:6379

There is no attribute, subclass, or message difference to distinguish them.

Impact

Any caller that needs to treat these differently (e.g. to decide whether to retry a write or treat it as already applied) must resort to walking exc.__traceback__ and inspecting frame names — a fragile approach that breaks if redis-py internal function names change.

Repro

The traceback below comes from get_connection during pipeline execution — the write never reached Redis, but the exception is indistinguishable from a response read timeout:

File "redis/asyncio/client.py", line 1662, in execute
    conn = await self.connection_pool.get_connection()
  File "redis/asyncio/connection.py", line 1259, in get_connection
    await self.ensure_connection(connection)
  ...
  File "redis/asyncio/connection.py", line 355, in connect_check_health
    await self.on_connect_check_health(check_health=check_health)
  File "redis/asyncio/connection.py", line 512, in on_connect_check_health
    await self.read_response()
  File "redis/asyncio/connection.py", line 660, in read_response
    raise TimeoutError(f"Timeout reading from {host_error}")
redis.exceptions.TimeoutError: Timeout reading from <host>:6379

Proposed Fix

on_connect_check_health (and connect_check_health) should catch TimeoutError from read_response and re-raise as ConnectionError, which already semantically represents "failed to establish a usable connection":

async def on_connect_check_health(self, check_health: bool = True) -> None:
    ...
    try:
        await self.read_response()
    except TimeoutError as e:
        raise ConnectionError(f"Timeout during connection health check to {self.host}:{self.port}") from e

This makes TimeoutError from pipeline.execute() unambiguously mean "response read timed out after write was sent."

Version

redis-py 7.2.0

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions