fix(connection): fail fast on unrecoverable connect errors (#4026)

redis/asyncio/connection.py

@@ -1,5 +1,6 @@
 import asyncio
 import copy
+import errno
 import inspect
 import math
 import socket
@@ -344,6 +345,25 @@ def set_parser(self, parser_class: Type[BaseParser]) -> None:
         """
         self._parser = parser_class(socket_read_size=self._socket_read_size)
 
+    # OS-level errno values for which reconnecting can never succeed, so
+    # there is no point spending the retry/backoff budget on them.
+    _UNRETRYABLE_CONNECT_ERRNOS = frozenset({errno.ENOENT})
+
+    @classmethod
+    def _is_retryable_connect_error(cls, error: BaseException) -> bool:
+        """Whether a failed connection attempt is worth retrying.
+
+        Returns ``False`` for definitively unrecoverable socket errors (e.g. a
+        Unix socket path that does not exist, ``ENOENT``) so that ``connect()``
+        fails fast instead of backing off for every retry. Transient errors
+        such as ``ECONNREFUSED`` during server startup stay retryable.
+        """
+        cause = error.__cause__
+        return not (
+            isinstance(cause, OSError)
+            and cause.errno in cls._UNRETRYABLE_CONNECT_ERRNOS
+        )
+
     async def connect(self):
         """Connects to the Redis server if not already connected"""
         # try once the socket connect with the handshake, retry the whole
@@ -355,6 +375,7 @@ async def connect(self):
             lambda error, failure_count: self.disconnect(
                 error=error, failure_count=failure_count
             ),
+            is_retryable=self._is_retryable_connect_error,
             with_failure_count=True,
         )
 
@@ -395,17 +416,19 @@ def failure_callback(error, failure_count):
             )
             raise e
         except OSError as e:
-            e = ConnectionError(self._error_message(e))
+            conn_error = ConnectionError(self._error_message(e))
             await record_error_count(
                 server_address=getattr(self, "host", None),
                 server_port=getattr(self, "port", None),
                 network_peer_address=getattr(self, "host", None),
                 network_peer_port=getattr(self, "port", None),
-                error_type=e,
+                error_type=conn_error,
                 retry_attempts=actual_retry_attempts,
                 is_internal=False,
             )
-            raise e
+            # Preserve the original OSError (with its errno) as the cause so
+            # the retry policy can tell unrecoverable failures apart.
+            raise conn_error from e
         except Exception as exc:
             raise ConnectionError(exc) from exc
 

redis/connection.py

@@ -1,4 +1,5 @@
 import copy
+import errno
 import os
 import socket
 import sys
@@ -1000,6 +1001,25 @@ def set_parser(self, parser_class):
     def _get_parser(self) -> Union[_HiredisParser, _RESP3Parser, _RESP2Parser]:
         return self._parser
 
+    # OS-level errno values for which reconnecting can never succeed, so
+    # there is no point spending the retry/backoff budget on them.
+    _UNRETRYABLE_CONNECT_ERRNOS = frozenset({errno.ENOENT})
+
+    @classmethod
+    def _is_retryable_connect_error(cls, error: BaseException) -> bool:
+        """Whether a failed connection attempt is worth retrying.
+
+        Returns ``False`` for definitively unrecoverable socket errors (e.g. a
+        Unix socket path that does not exist, ``ENOENT``) so that ``connect()``
+        fails fast instead of backing off for every retry. Transient errors
+        such as ``ECONNREFUSED`` during server startup stay retryable.
+        """
+        cause = error.__cause__
+        return not (
+            isinstance(cause, OSError)
+            and cause.errno in cls._UNRETRYABLE_CONNECT_ERRNOS
+        )
+
     def connect(self):
         "Connects to the Redis server if not already connected"
         # try once the socket connect with the handshake, retry the whole
@@ -1009,6 +1029,7 @@ def connect(self):
                 check_health=True, retry_socket_connect=False
             ),
             lambda error: self.disconnect(error),
+            is_retryable=self._is_retryable_connect_error,
         )
 
     def connect_check_health(
@@ -1044,16 +1065,18 @@ def failure_callback(error, failure_count):
             )
             raise e
         except OSError as e:
-            e = ConnectionError(self._error_message(e))
+            conn_error = ConnectionError(self._error_message(e))
             record_error_count(
                 server_address=getattr(self, "host", None),
                 server_port=getattr(self, "port", None),
                 network_peer_address=getattr(self, "host", None),
                 network_peer_port=getattr(self, "port", None),
-                error_type=e,
+                error_type=conn_error,
                 retry_attempts=actual_retry_attempts[0],
             )
-            raise e
+            # Preserve the original OSError (with its errno) as the cause so
+            # the retry policy can tell unrecoverable failures apart.
+            raise conn_error from e
 
         self._sock = sock
         try:

tests/test_asyncio/test_connection.py

@@ -3,7 +3,7 @@
 import ssl
 import types
 from unittest import mock
-from errno import ECONNREFUSED
+from errno import ECONNREFUSED, ENOENT
 from unittest.mock import patch
 
 import pytest
@@ -411,6 +411,37 @@ async def test_connect_timeout_error_without_retry():
     assert conn._connect.call_count == 1
 
 
+@pytest.mark.fixed_client
+async def test_connect_without_retry_on_unrecoverable_oserror():
+    """A connect error that can never succeed on retry (e.g. a missing Unix
+    socket path, ENOENT) fails fast instead of consuming the retry budget."""
+    conn = Connection(retry=Retry(NoBackoff(), 3))
+    conn._connect = mock.AsyncMock()
+    conn._connect.side_effect = OSError(ENOENT, "No such file or directory")
+
+    with pytest.raises(ConnectionError) as excinfo:
+        await conn.connect()
+    # no retries: _connect is called exactly once
+    assert conn._connect.call_count == 1
+    # the original OSError is preserved as the cause for classification
+    assert isinstance(excinfo.value.__cause__, OSError)
+    assert excinfo.value.__cause__.errno == ENOENT
+
+
+@pytest.mark.fixed_client
+async def test_connect_retries_on_transient_oserror():
+    """A transient socket error such as ECONNREFUSED (server not up yet)
+    stays retryable, so the whole connect flow is retried as before."""
+    conn = Connection(retry=Retry(NoBackoff(), 2))
+    conn._connect = mock.AsyncMock()
+    conn._connect.side_effect = OSError(ECONNREFUSED, "Connection refused")
+
+    with pytest.raises(ConnectionError):
+        await conn.connect()
+    # 2 retries --> 3 attempts
+    assert conn._connect.call_count == 3
+
+
 @pytest.mark.onlynoncluster
 async def test_connection_parse_response_resume(r: redis.Redis):
     """

tests/test_connection.py

@@ -7,7 +7,7 @@
 import ssl
 import threading
 import types
-from errno import ECONNREFUSED, EWOULDBLOCK
+from errno import ECONNREFUSED, ENOENT, EWOULDBLOCK
 from typing import Any
 from unittest import mock
 from unittest.mock import call, patch, MagicMock, Mock
@@ -502,6 +502,35 @@ def test_connect_timeout_error_without_retry(self):
         assert conn._connect.call_count == 1
         self.clear(conn)
 
+    def test_connect_without_retry_on_unrecoverable_oserror(self):
+        """A connect error that can never succeed on retry (e.g. a missing Unix
+        socket path, ENOENT) fails fast instead of consuming the retry budget."""
+        conn = Connection(retry=Retry(NoBackoff(), 3))
+        conn._connect = mock.Mock()
+        conn._connect.side_effect = OSError(ENOENT, "No such file or directory")
+
+        with pytest.raises(ConnectionError) as excinfo:
+            conn.connect()
+        # no retries: _connect is called exactly once
+        assert conn._connect.call_count == 1
+        # the original OSError is preserved as the cause for classification
+        assert isinstance(excinfo.value.__cause__, OSError)
+        assert excinfo.value.__cause__.errno == ENOENT
+        self.clear(conn)
+
+    def test_connect_retries_on_transient_oserror(self):
+        """A transient socket error such as ECONNREFUSED (server not up yet)
+        stays retryable, so the whole connect flow is retried as before."""
+        conn = Connection(retry=Retry(NoBackoff(), 2))
+        conn._connect = mock.Mock()
+        conn._connect.side_effect = OSError(ECONNREFUSED, "Connection refused")
+
+        with pytest.raises(ConnectionError):
+            conn.connect()
+        # 2 retries --> 3 attempts
+        assert conn._connect.call_count == 3
+        self.clear(conn)
+
 
 @pytest.mark.onlynoncluster
 @pytest.mark.parametrize(