Merge remote-tracking branch 'origin/master' into ck_py312

Author: gerzse

.github/workflows/integration.yaml

@@ -25,8 +25,8 @@ permissions:
 
 env:
   CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
-  REDIS_IMAGE: redis/redis-stack-server:7.4.0-rc1
-  REDIS_STACK_IMAGE: redis/redis-stack-server:7.4.0-rc1
+  REDIS_IMAGE: redis:7.4-rc2
+  REDIS_STACK_IMAGE: redis/redis-stack-server:7.4.0-rc2
 
 jobs:
   dependency-audit:

redis/asyncio/connection.py

@@ -27,6 +27,8 @@
 )
 from urllib.parse import ParseResult, parse_qs, unquote, urlparse
 
+from ..utils import format_error_message
+
 # the functionality is available in 3.11.x but has a major issue before
 # 3.11.3. See https://github.com/redis/redis-py/issues/2633
 if sys.version_info >= (3, 11, 3):
@@ -345,9 +347,8 @@ async def _connect(self):
     def _host_error(self) -> str:
         pass
 
-    @abstractmethod
     def _error_message(self, exception: BaseException) -> str:
-        pass
+        return format_error_message(self._host_error(), exception)
 
     async def on_connect(self) -> None:
         """Initialize the connection, authenticate and select a database"""
@@ -799,27 +800,6 @@ async def _connect(self):
     def _host_error(self) -> str:
         return f"{self.host}:{self.port}"
 
-    def _error_message(self, exception: BaseException) -> str:
-        # args for socket.error can either be (errno, "message")
-        # or just "message"
-
-        host_error = self._host_error()
-
-        if not exception.args:
-            # asyncio has a bug where on Connection reset by peer, the
-            # exception is not instanciated, so args is empty. This is the
-            # workaround.
-            # See: https://github.com/redis/redis-py/issues/2237
-            # See: https://github.com/python/cpython/issues/94061
-            return f"Error connecting to {host_error}. Connection reset by peer"
-        elif len(exception.args) == 1:
-            return f"Error connecting to {host_error}. {exception.args[0]}."
-        else:
-            return (
-                f"Error {exception.args[0]} connecting to {host_error}. "
-                f"{exception}."
-            )
-
 
 class SSLConnection(Connection):
     """Manages SSL connections to and from the Redis server(s).
@@ -971,20 +951,6 @@ async def _connect(self):
     def _host_error(self) -> str:
         return self.path
 
-    def _error_message(self, exception: BaseException) -> str:
-        # args for socket.error can either be (errno, "message")
-        # or just "message"
-        host_error = self._host_error()
-        if len(exception.args) == 1:
-            return (
-                f"Error connecting to unix socket: {host_error}. {exception.args[0]}."
-            )
-        else:
-            return (
-                f"Error {exception.args[0]} connecting to unix socket: "
-                f"{host_error}. {exception.args[1]}."
-            )
-
 
 FALSE_STRINGS = ("0", "F", "FALSE", "N", "NO")
 

redis/commands/core.py

@@ -5126,9 +5126,8 @@ def hexpire(
             lt: Set expiry only when the new expiry is less than the current one.
 
         Returns:
-            If the key does not exist, returns an empty list. If the key exists, returns
-            a list which contains for each field in the request:
-                - `-2` if the field does not exist.
+            Returns a list which contains for each field in the request:
+                - `-2` if the field does not exist, or if the key does not exist.
                 - `0` if the specified NX | XX | GT | LT condition was not met.
                 - `1` if the expiration time was set or updated.
                 - `2` if the field was deleted because the specified expiration time is
@@ -5187,9 +5186,8 @@ def hpexpire(
             lt: Set expiry only when the new expiry is less than the current one.
 
         Returns:
-            If the key does not exist, returns an empty list. If the key exists, returns
-            a list which contains for each field in the request:
-                - `-2` if the field does not exist.
+            Returns a list which contains for each field in the request:
+                - `-2` if the field does not exist, or if the key does not exist.
                 - `0` if the specified NX | XX | GT | LT condition was not met.
                 - `1` if the expiration time was set or updated.
                 - `2` if the field was deleted because the specified expiration time is
@@ -5248,9 +5246,8 @@ def hexpireat(
             lt: Set expiry only when the new expiry is less than the current one.
 
         Returns:
-            If the key does not exist, returns an empty list. If the key exists, returns
-            a list which contains for each field in the request:
-                - `-2` if the field does not exist.
+            Returns a list which contains for each field in the request:
+                - `-2` if the field does not exist, or if the key does not exist.
                 - `0` if the specified NX | XX | GT | LT condition was not met.
                 - `1` if the expiration time was set or updated.
                 - `2` if the field was deleted because the specified expiration time is
@@ -5315,9 +5312,8 @@ def hpexpireat(
             lt: Set expiry only when the new expiry is less than the current one.
 
         Returns:
-            If the key does not exist, returns an empty list. If the key exists, returns
-            a list which contains for each field in the request:
-                - `-2` if the field does not exist.
+            Returns a list which contains for each field in the request:
+                - `-2` if the field does not exist, or if the key does not exist.
                 - `0` if the specified NX | XX | GT | LT condition was not met.
                 - `1` if the expiration time was set or updated.
                 - `2` if the field was deleted because the specified expiration time is
@@ -5362,9 +5358,8 @@ def hpersist(self, name: KeyT, *fields: str) -> ResponseT:
                     expiration time.
 
         Returns:
-            If the key does not exist, returns an empty list. If the key exists, returns
-            a list which contains for each field in the request:
-                - `-2` if the field does not exist.
+            Returns a list which contains for each field in the request:
+                - `-2` if the field does not exist, or if the key does not exist.
                 - `-1` if the field exists but has no associated expiration time.
                 - `1` if the expiration time was successfully removed from the field.
         """
@@ -5382,9 +5377,8 @@ def hexpiretime(self, key: KeyT, *fields: str) -> ResponseT:
                     time.
 
         Returns:
-            If the key does not exist, returns an empty list. If the key exists, returns
-            a list which contains for each field in the request:
-                - `-2` if the field does not exist.
+            Returns a list which contains for each field in the request:
+                - `-2` if the field does not exist, or if the key does not exist.
                 - `-1` if the field exists but has no associated expire time.
                 - A positive integer representing the expiration Unix timestamp in
                   seconds, if the field has an associated expiration time.
@@ -5405,9 +5399,8 @@ def hpexpiretime(self, key: KeyT, *fields: str) -> ResponseT:
                     time.
 
         Returns:
-            If the key does not exist, returns an empty list. If the key exists, returns
-            a list which contains for each field in the request:
-                - `-2` if the field does not exist.
+            Returns a list which contains for each field in the request:
+                - `-2` if the field does not exist, or if the key does not exist.
                 - `-1` if the field exists but has no associated expire time.
                 - A positive integer representing the expiration Unix timestamp in
                   milliseconds, if the field has an associated expiration time.
@@ -5428,9 +5421,8 @@ def httl(self, key: KeyT, *fields: str) -> ResponseT:
             fields: A list of fields within the hash for which to get the TTL.
 
         Returns:
-            If the key does not exist, returns an empty list. If the key exists, returns
-            a list which contains for each field in the request:
-                - `-2` if the field does not exist.
+            Returns a list which contains for each field in the request:
+                - `-2` if the field does not exist, or if the key does not exist.
                 - `-1` if the field exists but has no associated expire time.
                 - A positive integer representing the TTL in seconds if the field has
                   an associated expiration time.
@@ -5451,9 +5443,8 @@ def hpttl(self, key: KeyT, *fields: str) -> ResponseT:
             fields: A list of fields within the hash for which to get the TTL.
 
         Returns:
-            If the key does not exist, returns an empty list. If the key exists, returns
-            a list which contains for each field in the request:
-                - `-2` if the field does not exist.
+            Returns a list which contains for each field in the request:
+                - `-2` if the field does not exist, or if the key does not exist.
                 - `-1` if the field exists but has no associated expire time.
                 - A positive integer representing the TTL in milliseconds if the field
                   has an associated expiration time.

redis/connection.py

@@ -39,6 +39,7 @@
     HIREDIS_AVAILABLE,
     HIREDIS_PACK_AVAILABLE,
     SSL_AVAILABLE,
+    format_error_message,
     get_lib_version,
     str_if_bytes,
 )
@@ -338,9 +339,8 @@ def _connect(self):
     def _host_error(self):
         pass
 
-    @abstractmethod
     def _error_message(self, exception):
-        pass
+        return format_error_message(self._host_error(), exception)
 
     def on_connect(self):
         "Initialize the connection, authenticate and select a database"
@@ -733,27 +733,6 @@ def _connect(self):
     def _host_error(self):
         return f"{self.host}:{self.port}"
 
-    def _error_message(self, exception):
-        # args for socket.error can either be (errno, "message")
-        # or just "message"
-
-        host_error = self._host_error()
-
-        if len(exception.args) == 1:
-            try:
-                return f"Error connecting to {host_error}. \
-                        {exception.args[0]}."
-            except AttributeError:
-                return f"Connection Error: {exception.args[0]}"
-        else:
-            try:
-                return (
-                    f"Error {exception.args[0]} connecting to "
-                    f"{host_error}. {exception.args[1]}."
-                )
-            except AttributeError:
-                return f"Connection Error: {exception.args[0]}"
-
 
 class SSLConnection(Connection):
     """Manages SSL connections to and from the Redis server(s).
@@ -948,20 +927,6 @@ def _connect(self):
     def _host_error(self):
         return self.path
 
-    def _error_message(self, exception):
-        # args for socket.error can either be (errno, "message")
-        # or just "message"
-        host_error = self._host_error()
-        if len(exception.args) == 1:
-            return (
-                f"Error connecting to unix socket: {host_error}. {exception.args[0]}."
-            )
-        else:
-            return (
-                f"Error {exception.args[0]} connecting to unix socket: "
-                f"{host_error}. {exception.args[1]}."
-            )
-
 
 FALSE_STRINGS = ("0", "F", "FALSE", "N", "NO")
 

redis/utils.py

@@ -141,3 +141,15 @@ def get_lib_version():
     except metadata.PackageNotFoundError:
         libver = "99.99.99"
     return libver
+
+
+def format_error_message(host_error: str, exception: BaseException) -> str:
+    if not exception.args:
+        return f"Error connecting to {host_error}."
+    elif len(exception.args) == 1:
+        return f"Error {exception.args[0]} connecting to {host_error}."
+    else:
+        return (
+            f"Error {exception.args[0]} connecting to {host_error}. "
+            f"{exception.args[1]}."
+        )

tests/test_asyncio/test_connection.py

@@ -12,7 +12,12 @@
     _AsyncRESPBase,
 )
 from redis.asyncio import ConnectionPool, Redis
-from redis.asyncio.connection import Connection, UnixDomainSocketConnection, parse_url
+from redis.asyncio.connection import (
+    Connection,
+    SSLConnection,
+    UnixDomainSocketConnection,
+    parse_url,
+)
 from redis.asyncio.retry import Retry
 from redis.backoff import NoBackoff
 from redis.exceptions import ConnectionError, InvalidResponse, TimeoutError
@@ -494,18 +499,50 @@ async def test_connection_garbage_collection(request):
 
 
 @pytest.mark.parametrize(
-    "error, expected_message",
+    "conn, error, expected_message",
     [
-        (OSError(), "Error connecting to localhost:6379. Connection reset by peer"),
-        (OSError(12), "Error connecting to localhost:6379. 12."),
+        (SSLConnection(), OSError(), "Error connecting to localhost:6379."),
+        (SSLConnection(), OSError(12), "Error 12 connecting to localhost:6379."),
         (
+            SSLConnection(),
             OSError(12, "Some Error"),
-            "Error 12 connecting to localhost:6379. [Errno 12] Some Error.",
+            "Error 12 connecting to localhost:6379. Some Error.",
+        ),
+        (
+            UnixDomainSocketConnection(path="unix:///tmp/redis.sock"),
+            OSError(),
+            "Error connecting to unix:///tmp/redis.sock.",
+        ),
+        (
+            UnixDomainSocketConnection(path="unix:///tmp/redis.sock"),
+            OSError(12),
+            "Error 12 connecting to unix:///tmp/redis.sock.",
+        ),
+        (
+            UnixDomainSocketConnection(path="unix:///tmp/redis.sock"),
+            OSError(12, "Some Error"),
+            "Error 12 connecting to unix:///tmp/redis.sock. Some Error.",
         ),
     ],
 )
-async def test_connect_error_message(error, expected_message):
+async def test_format_error_message(conn, error, expected_message):
     """Test that the _error_message function formats errors correctly"""
-    conn = Connection()
     error_message = conn._error_message(error)
     assert error_message == expected_message
+
+
+async def test_network_connection_failure():
+    with pytest.raises(ConnectionError) as e:
+        redis = Redis(host="127.0.0.1", port=9999)
+        await redis.set("a", "b")
+    assert str(e.value).startswith("Error 111 connecting to 127.0.0.1:9999. Connect")
+
+
+async def test_unix_socket_connection_failure():
+    with pytest.raises(ConnectionError) as e:
+        redis = Redis(unix_socket_path="unix:///tmp/a.sock")
+        await redis.set("a", "b")
+    assert (
+        str(e.value)
+        == "Error 2 connecting to unix:///tmp/a.sock. No such file or directory."
+    )

tests/test_asyncio/test_hash.py

@@ -45,7 +45,7 @@ async def test_hexpire_conditions(r):
 @skip_if_server_version_lt("7.3.240")
 async def test_hexpire_nonexistent_key_or_field(r):
     await r.delete("test:hash")
-    assert await r.hexpire("test:hash", 1, "field1") == []
+    assert await r.hexpire("test:hash", 1, "field1") == [-2]
     await r.hset("test:hash", "field1", "value1")
     assert await r.hexpire("test:hash", 1, "nonexistent_field") == [-2]
 
@@ -105,7 +105,7 @@ async def test_hpexpire_conditions(r):
 @skip_if_server_version_lt("7.3.240")
 async def test_hpexpire_nonexistent_key_or_field(r):
     await r.delete("test:hash")
-    assert await r.hpexpire("test:hash", 500, "field1") == []
+    assert await r.hpexpire("test:hash", 500, "field1") == [-2]
     await r.hset("test:hash", "field1", "value1")
     assert await r.hpexpire("test:hash", 500, "nonexistent_field") == [-2]
 
@@ -163,7 +163,7 @@ async def test_hexpireat_conditions(r):
 async def test_hexpireat_nonexistent_key_or_field(r):
     await r.delete("test:hash")
     future_exp_time = int((datetime.now() + timedelta(seconds=1)).timestamp())
-    assert await r.hexpireat("test:hash", future_exp_time, "field1") == []
+    assert await r.hexpireat("test:hash", future_exp_time, "field1") == [-2]
     await r.hset("test:hash", "field1", "value1")
     assert await r.hexpireat("test:hash", future_exp_time, "nonexistent_field") == [-2]
 
@@ -228,7 +228,7 @@ async def test_hpexpireat_nonexistent_key_or_field(r):
     future_exp_time = int(
         (datetime.now() + timedelta(milliseconds=500)).timestamp() * 1000
     )
-    assert await r.hpexpireat("test:hash", future_exp_time, "field1") == []
+    assert await r.hpexpireat("test:hash", future_exp_time, "field1") == [-2]
     await r.hset("test:hash", "field1", "value1")
     assert await r.hpexpireat("test:hash", future_exp_time, "nonexistent_field") == [-2]
 

tests/test_asyncio/test_json.py

@@ -131,7 +131,7 @@ async def test_mset(decoded_r: redis.Redis):
 async def test_clear(decoded_r: redis.Redis):
     await decoded_r.json().set("arr", Path.root_path(), [0, 1, 2, 3, 4])
     assert 1 == await decoded_r.json().clear("arr", Path.root_path())
-    assert_resp_response(decoded_r, await decoded_r.json().get("arr"), [], [[[]]])
+    assert_resp_response(decoded_r, await decoded_r.json().get("arr"), [], [])
 
 
 @pytest.mark.redismod