From edd304a79885b5bf1df3c6822995e25f33733f61 Mon Sep 17 00:00:00 2001 From: Will Miller Date: Fri, 2 Feb 2024 18:10:47 +0000 Subject: [PATCH 1/2] Fix retry logic for pubsub and pipeline Extend the fix from bea72995fd39b01e2f0a1682b16b6c7690933f36 to apply to pipeline and pubsub as well. Fixes #2973 --- redis/asyncio/client.py | 36 ++++++++++++++++++----------- redis/client.py | 50 ++++++++++++++++++++++++++++------------- 2 files changed, 57 insertions(+), 29 deletions(-) diff --git a/redis/asyncio/client.py b/redis/asyncio/client.py index 88de893f5b..69600d6b38 100644 --- a/redis/asyncio/client.py +++ b/redis/asyncio/client.py @@ -924,11 +924,15 @@ async def connect(self): async def _disconnect_raise_connect(self, conn, error): """ Close the connection and raise an exception - if retry_on_timeout is not set or the error - is not a TimeoutError. Otherwise, try to reconnect + if retry_on_error is not set or the error is not one + of the specified error types. Otherwise, try to + reconnect """ await conn.disconnect() - if not (conn.retry_on_timeout and isinstance(error, TimeoutError)): + if ( + conn.retry_on_error is None + or isinstance(error, tuple(conn.retry_on_error)) is False + ): raise error await conn.connect() @@ -1341,8 +1345,8 @@ async def _disconnect_reset_raise(self, conn, error): """ Close the connection, reset watching state and raise an exception if we were watching, - retry_on_timeout is not set, - or the error is not a TimeoutError + if retry_on_error is not set or the error is not one + of the specified error types. """ await conn.disconnect() # if we were already watching a variable, the watch is no longer @@ -1353,9 +1357,12 @@ async def _disconnect_reset_raise(self, conn, error): raise WatchError( "A ConnectionError occurred on while watching one or more keys" ) - # if retry_on_timeout is not set, or the error is not - # a TimeoutError, raise it - if not (conn.retry_on_timeout and isinstance(error, TimeoutError)): + # if retry_on_error is not set or the error is not one + # of the specified error types, raise it + if ( + conn.retry_on_error is None + or isinstance(error, tuple(conn.retry_on_error)) is False + ): await self.aclose() raise @@ -1530,8 +1537,8 @@ async def load_scripts(self): async def _disconnect_raise_reset(self, conn: Connection, error: Exception): """ Close the connection, raise an exception if we were watching, - and raise an exception if retry_on_timeout is not set, - or the error is not a TimeoutError + and raise an exception if retry_on_error is not set or the + error is not one of the specified error types. """ await conn.disconnect() # if we were watching a variable, the watch is no longer valid @@ -1541,9 +1548,12 @@ async def _disconnect_raise_reset(self, conn: Connection, error: Exception): raise WatchError( "A ConnectionError occurred on while watching one or more keys" ) - # if retry_on_timeout is not set, or the error is not - # a TimeoutError, raise it - if not (conn.retry_on_timeout and isinstance(error, TimeoutError)): + # if retry_on_error is not set or the error is not one + # of the specified error types, raise it + if ( + conn.retry_on_error is None + or isinstance(error, tuple(conn.retry_on_error)) is False + ): await self.reset() raise diff --git a/redis/client.py b/redis/client.py index 2d4c512699..3c882f87e3 100755 --- a/redis/client.py +++ b/redis/client.py @@ -25,7 +25,12 @@ SentinelCommands, list_or_args, ) -from redis.connection import ConnectionPool, SSLConnection, UnixDomainSocketConnection +from redis.connection import ( + ConnectionPool, + SSLConnection, + UnixDomainSocketConnection, + AbstractConnection, +) from redis.credentials import CredentialProvider from redis.exceptions import ( ConnectionError, @@ -837,11 +842,15 @@ def clean_health_check_responses(self) -> None: def _disconnect_raise_connect(self, conn, error) -> None: """ Close the connection and raise an exception - if retry_on_timeout is not set or the error - is not a TimeoutError. Otherwise, try to reconnect + if retry_on_error is not set or the error is not one + of the specified error types. Otherwise, try to + reconnect """ conn.disconnect() - if not (conn.retry_on_timeout and isinstance(error, TimeoutError)): + if ( + conn.retry_on_error is None + or isinstance(error, tuple(conn.retry_on_error)) is False + ): raise error conn.connect() @@ -1318,8 +1327,8 @@ def _disconnect_reset_raise(self, conn, error) -> None: """ Close the connection, reset watching state and raise an exception if we were watching, - retry_on_timeout is not set, - or the error is not a TimeoutError + if retry_on_error is not set or the error is not one + of the specified error types. """ conn.disconnect() # if we were already watching a variable, the watch is no longer @@ -1330,9 +1339,12 @@ def _disconnect_reset_raise(self, conn, error) -> None: raise WatchError( "A ConnectionError occurred on while watching one or more keys" ) - # if retry_on_timeout is not set, or the error is not - # a TimeoutError, raise it - if not (conn.retry_on_timeout and isinstance(error, TimeoutError)): + # if retry_on_error is not set or the error is not one + # of the specified error types, raise it + if ( + conn.retry_on_error is None + or isinstance(error, tuple(conn.retry_on_error)) is False + ): self.reset() raise @@ -1490,11 +1502,15 @@ def load_scripts(self): if not exist: s.sha = immediate("SCRIPT LOAD", s.script) - def _disconnect_raise_reset(self, conn: Redis, error: Exception) -> None: + def _disconnect_raise_reset( + self, + conn: AbstractConnection, + error: Exception, + ) -> None: """ Close the connection, raise an exception if we were watching, - and raise an exception if TimeoutError is not part of retry_on_error, - or the error is not a TimeoutError + and raise an exception if retry_on_error is not set or the + error is not one of the specified error types. """ conn.disconnect() # if we were watching a variable, the watch is no longer valid @@ -1504,11 +1520,13 @@ def _disconnect_raise_reset(self, conn: Redis, error: Exception) -> None: raise WatchError( "A ConnectionError occurred on while watching one or more keys" ) - # if TimeoutError is not part of retry_on_error, or the error - # is not a TimeoutError, raise it - if not ( - TimeoutError in conn.retry_on_error and isinstance(error, TimeoutError) + # if retry_on_error is not set or the error is not one + # of the specified error types, raise it + if ( + conn.retry_on_error is None + or isinstance(error, tuple(conn.retry_on_error)) is False ): + self.reset() raise error From 819d65b37f6515c82526940f969a6d0e6e88b31d Mon Sep 17 00:00:00 2001 From: Will Miller Date: Mon, 12 Feb 2024 08:23:05 +0000 Subject: [PATCH 2/2] fix isort --- redis/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/redis/client.py b/redis/client.py index 3c882f87e3..ca0700393b 100755 --- a/redis/client.py +++ b/redis/client.py @@ -26,10 +26,10 @@ list_or_args, ) from redis.connection import ( + AbstractConnection, ConnectionPool, SSLConnection, UnixDomainSocketConnection, - AbstractConnection, ) from redis.credentials import CredentialProvider from redis.exceptions import (