Make RedisException the canonical error contract for all public Redis operations

AGENTS.md

@@ -14,6 +14,10 @@ authoritative files named below over copying details into this document.
   `schema_utils.py`. Portal-facing APIs are primarily `ff_utils.py` (request/auth and
   metadata functions), `portal_utils.py` (the higher-level `Portal` wrapper),
   `portal_object_utils.py`, `structured_data.py`, and `submitr/`.
+- `redis_utils.py` owns the public Redis error contract: every public operation there and in
+  `redis_tools.py` is wrapped in `translate_redis_exceptions`, so driver failures surface as
+  `RedisException` and consumers (notably Snovault) never import `redis.exceptions`. Any new
+  `RedisBase` method must carry that decorator; `test/test_redis_error_contract.py` enforces it.
 - Integration modules are grouped by the system named in the file: AWS (`s3_utils.py`,
   `ecs_utils.py`, `ecr_utils.py`, `cloudformation_utils.py`, `secrets_utils.py`, etc.),
   search (`es_utils.py`, `opensearch_utils.py`), Redis, Docker, and deployment utilities.

CHANGELOG.rst

@@ -7,6 +7,31 @@ Change Log
 ----------
 
 
+8.20.0
+======
+* willronchetti / 2026-08-07 / branch: fm/dcicutils-redis-error-contract-4w
+  - Made ``RedisException`` the canonical error contract for every public Redis operation.
+    ``create_redis_client``, all ``RedisBase`` methods, and all ``RedisSessionToken``
+    operations (``from_redis``, ``store_session_token``, ``validate_session_token``,
+    ``update_session_token``, ``delete_session_token``) now translate redis driver
+    failures - ``redis.exceptions.RedisError`` and ``redis.exceptions.RedisClusterException``
+    and their subclasses - into ``RedisException``, chaining the original as ``__cause__``.
+    Consumers such as Snovault no longer need to import ``redis.exceptions``.
+  - Added the public ``dcicutils.redis_utils.translate_redis_exceptions`` decorator and
+    ``REDIS_DRIVER_EXCEPTIONS`` tuple used to implement this.
+  - Behavior change: ``RedisSessionToken.store_session_token`` previously caught *any*
+    ``Exception`` and re-raised a bare ``RedisException``. It now translates only redis
+    driver failures, so programmer errors (``TypeError``, ``AttributeError``, ...) propagate
+    unchanged instead of being masked. Absence and unreachability also stay distinct:
+    ``validate_session_token`` still returns ``False`` for a missing token but raises
+    ``RedisException`` when Redis cannot be reached.
+  - ``RedisException`` remains importable from both ``dcicutils.redis_utils`` and
+    ``dcicutils.redis_tools``; no signatures or return values changed.
+  - Added ``test/test_redis_error_contract.py``, which injects representative driver
+    connection/timeout/response failures into every affected public operation. These tests
+    use mocks and do not require a running redis-server.
+
+
 8.19.0
 ======
 * ajs/wrr/sn 2026-07-29 / branch: sn_refactor_custom_excel

dcicutils/redis_tools.py

@@ -2,8 +2,12 @@
 import datetime
 import structlog
 import jwt
-from dcicutils.redis_utils import RedisBase, RedisException
+from dcicutils.redis_utils import (
+    RedisBase, RedisException, REDIS_DRIVER_EXCEPTIONS, translate_redis_exceptions,
+)
 
+# RedisException is re-exported here on purpose: consumers of the session token API
+# (notably Snovault) import it from this module and must not need redis.exceptions.
 
 log = structlog.getLogger(__name__)
 
@@ -88,13 +92,16 @@ def get_email(self) -> str:
         return self.email
 
     @classmethod
+    @translate_redis_exceptions
     def from_redis(cls, *, redis_handler: RedisBase, namespace: str, token: str):
         """ Builds a RedisSessionToken from an existing record - allows extracting JWT
             given a session token internally.
         :param redis_handler: handle to Redis API
         :param namespace: namespace to search under
         :param token: value of the token
-        :return: A RedisSessionToken object built from an existing record in Redis
+        :return: A RedisSessionToken object built from an existing record in Redis,
+                 or None if no such record exists
+        :raises RedisException: if Redis cannot be reached or reports a failure
         """
         redis_key = f'{namespace}:session:{token}'
         redis_entry = redis_handler.get(redis_key)
@@ -116,34 +123,44 @@ def decode_jwt(self, audience: str, secret: str, leeway: int = 30, algorithms: l
         return jwt.decode(self.jwt, secret, audience=audience, leeway=leeway,
                           options={'verify_signature': True}, algorithms=algorithms)
 
+    @translate_redis_exceptions
     def store_session_token(self, *, redis_handler: RedisBase) -> bool:
         """ Stores the created session token object as an hset in Redis
         :param redis_handler: handle to Redis API
-        :return: True if successful, raise Exception otherwise
+        :return: True if successful
+        :raises RedisException: if Redis cannot be reached or reports a failure
         """
         try:
             redis_handler.set(self.redis_key, f'{self.jwt}:{self.email or ""}', exp=self.expiration)
-        except Exception as e:
+        except (RedisException, *REDIS_DRIVER_EXCEPTIONS) as e:
+            # A raw driver exception can only reach here if the caller passed something other than
+            # a RedisBase; either way the enclosing decorator normalizes it to RedisException.
             log.error(str(e))
-            raise RedisException()
+            raise
         return True
 
+    @translate_redis_exceptions
     def validate_session_token(self, *, redis_handler: RedisBase) -> bool:
         """ Validates the given session token against that stored in redis
         :param redis_handler: handle to Redis API
-        :return: True if token matches that in Redis and is not expired
+        :return: True if token matches that in Redis and is not expired, False if no such
+                 token is stored - note that an unreachable Redis raises rather than
+                 returning False, so callers can distinguish absence from failure
+        :raises RedisException: if Redis cannot be reached or reports a failure
         """
         redis_token = redis_handler.get(self.redis_key)
         if not redis_token:
             return False  # if it doesn't exist it's not valid
         return True  # if it does exist it must be valid since we always send with TTL
 
+    @translate_redis_exceptions
     def update_session_token(self, *, redis_handler: RedisBase, jwt: str, email: str) -> bool:
         """ Refreshes the session token, jwt (if different) and expiration stored in Redis
         :param redis_handler: handle to Redis API
         :param jwt: jwt of user
         :param email: email of user
-        :return: True if successful, raise Exception otherwise
+        :return: True if successful
+        :raises RedisException: if Redis cannot be reached or reports a failure
         """
         # remove old token
         self.delete_session_token(redis_handler=redis_handler)
@@ -155,9 +172,11 @@ def update_session_token(self, *, redis_handler: RedisBase, jwt: str, email: str
         self.email = email
         return self.store_session_token(redis_handler=redis_handler)
 
+    @translate_redis_exceptions
     def delete_session_token(self, *, redis_handler: RedisBase) -> bool:
         """ Deletes the session token from redis, effectively logging out
         :param redis_handler: handle to Redis API
-        :return: True if successful, False otherwise
+        :return: True if a token was removed, False if there was nothing to remove
+        :raises RedisException: if Redis cannot be reached or reports a failure
         """
         return 1 == redis_handler.delete(self.redis_key)

dcicutils/redis_utils.py

@@ -1,24 +1,66 @@
+import functools
 import redis
+import redis.exceptions
 import datetime
 from typing import Union
 # Low level utilities for working with Redis
 
 
+class RedisException(Exception):
+    """ Canonical exception raised by this library for any operational failure reported by the
+        underlying Redis driver.
+
+        This is the public error contract: callers of create_redis_client, RedisBase and
+        dcicutils.redis_tools should catch this exception and never need to import
+        redis.exceptions themselves. The originating driver exception is always attached as
+        __cause__ for callers that want to inspect it.
+    """
+    pass
+
+
+# Driver exceptions that represent an operational failure talking to Redis (connection refused,
+# socket timeout, protocol/response error, cluster failure, ...). redis.exceptions.RedisError is
+# the root of nearly all of them; RedisClusterException deliberately sits outside that tree, so
+# it is named explicitly. Python-level programmer errors (TypeError, AttributeError, ValueError,
+# ...) are intentionally NOT listed and propagate unchanged.
+REDIS_DRIVER_EXCEPTIONS = (redis.exceptions.RedisError, redis.exceptions.RedisClusterException)
+
+
+def translate_redis_exceptions(fn):
+    """ Decorator translating Redis driver failures into the canonical RedisException.
+
+        Applied to every public operation in this module and in dcicutils.redis_tools so that
+        consumers have a single exception type to handle. Exceptions that are not driver
+        failures - including RedisException itself, so nesting decorated calls is harmless -
+        are re-raised untouched.
+
+    :param fn: function issuing one or more Redis commands
+    :return: wrapped function raising RedisException in place of driver exceptions
+    """
+    @functools.wraps(fn)
+    def wrapper(*args, **kwargs):
+        try:
+            return fn(*args, **kwargs)
+        except REDIS_DRIVER_EXCEPTIONS as e:
+            raise RedisException(f'Redis operation {fn.__name__} failed:'
+                                 f' {e.__class__.__name__}: {e}') from e
+    return wrapper
+
+
+@translate_redis_exceptions
 def create_redis_client(*, url: str, ping=True) -> redis.Redis:
     """ Creates a Redis client connecting from a host:port or a URL
         Because this creation pings Redis should re-use this as much as possible, but
         you can skip the ping if you're confident it will come up
+
+        Raises RedisException if the driver cannot connect (or the ping fails).
     """
     r = redis.from_url(url)
     if ping:
         r.ping()
     return r
 
 
-class RedisException(Exception):
-    pass
-
-
 class RedisBase(object):
     """ This class contains low level methods meant to implement useful Redis APIs. The idea is these functions
         are used to implement the methods needed in Redis Tools.
@@ -54,12 +96,14 @@ def _decode_value(value: Union[bytes, int, float]) -> str:
         """
         return value.decode('utf-8') if (value and isinstance(value, bytes)) else value
 
+    @translate_redis_exceptions
     def info(self) -> dict:
         """ Returns info about the Redis server https://redis.io/commands/info/
         :return: a dictionary of information about the redis server
         """
         return self.redis.info()
 
+    @translate_redis_exceptions
     def set(self, key: str, value: Union[str, int, float], exp: Union[int, datetime.timedelta] = None) -> str:
         """ Sets the given key to the given value https://redis.io/commands/set/
         :param key: string to store value under
@@ -72,6 +116,7 @@ def set(self, key: str, value: Union[str, int, float], exp: Union[int, datetime.
             kwargs['ex'] = exp
         return self.redis.set(self._encode_value(key), self._encode_value(value), **kwargs)
 
+    @translate_redis_exceptions
     def get(self, key: str) -> str:
         """ Gets the given key from Redis https://redis.io/commands/get/
         :param key: key to check for a value store in Redis
@@ -82,6 +127,7 @@ def get(self, key: str) -> str:
             val = self._decode_value(val)
         return val
 
+    @translate_redis_exceptions
     def set_expiration(self, key: str, t: Union[int, datetime.time]) -> bool:
         """ Sets the TTL of the given key manually
         :param key: key to set TTL
@@ -90,20 +136,23 @@ def set_expiration(self, key: str, t: Union[int, datetime.time]) -> bool:
         """
         return self.redis.expire(key, t, gt=True)
 
+    @translate_redis_exceptions
     def ttl(self, key: str) -> datetime.time:
         """ Gets the TTL of the given key
         :param key: key to get TTL for
         :return: datetime value in seconds
         """
         return self.redis.ttl(key)
 
+    @translate_redis_exceptions
     def delete(self, key: str) -> int:
         """ Deletes the given key from Redis https://redis.io/commands/del/
         :param key: key to delete
         :return: number of keys removed
         """
         return self.redis.delete(self._encode_value(key))
 
+    @translate_redis_exceptions
     def hget(self, key: str, field: str) -> str:
         """ Gets the value of field from hash key https://redis.io/commands/hget/
         :param key: hash key to retrieve field value from
@@ -112,6 +161,7 @@ def hget(self, key: str, field: str) -> str:
         """
         return self._decode_value(self.redis.hget(self._encode_value(key), self._encode_value(field)))
 
+    @translate_redis_exceptions
     def hgetall(self, key: str) -> dict:
         """ Gets all values of the given hash https://redis.io/commands/hgetall/
         :param key: hash key to grab all values from
@@ -122,6 +172,7 @@ def hgetall(self, key: str) -> dict:
             encoded_vals = {self._decode_value(k): self._decode_value(v) for k, v in encoded_vals.items()}
         return encoded_vals
 
+    @translate_redis_exceptions
     def hset(self, key: str, field: str, value: Union[str, int, float]) -> int:
         """ Sets a single field on a hash key https://redis.io/commands/hset/
         :param key: hash key to set field -> value mapping on
@@ -131,6 +182,7 @@ def hset(self, key: str, field: str, value: Union[str, int, float]) -> int:
         """
         return self.redis.hset(self._encode_value(key), self._encode_value(field), self._encode_value(value))
 
+    @translate_redis_exceptions
     def hset_multiple(self, key: str, items: dict) -> int:
         """ Sets all k,v pairs in items on hash key https://redis.io/commands/hset/ (variadic form)
         :param key: hash key to store items under
@@ -140,6 +192,7 @@ def hset_multiple(self, key: str, items: dict) -> int:
         encoded_dict = {self._encode_value(k): self._encode_value(v) for k, v in items.items()}
         return self.redis.hset(key, mapping=encoded_dict)
 
+    @translate_redis_exceptions
     def dbsize(self) -> int:
         """ Returns number of keys in redis https://redis.io/commands/dbsize/ """
         return self.redis.dbsize()

pyproject.toml

@@ -1,6 +1,6 @@
 [tool.poetry]
 name = "dcicutils"
-version = "8.19.0"
+version = "8.20.0"
 description = "Utility package for interacting with the 4DN Data Portal and other 4DN resources"
 authors = ["4DN-DCIC Team <support@4dnucleome.org>"]
 license = "MIT"