Fix: Gracefully handle exceptions in PubSub message retrieval #625
Conversation
nuclearcat
force-pushed
the
fix-exception
branch
from
119f4ee to
811c478
Compare
| except aioredis.ConnectionError: | ||
| async with self._lock: | ||
| channel = self._subscriptions[sub_id]['sub'].channel | ||
| new_redis_sub = self._redis.pubsub() | ||
| await new_redis_sub.subscribe(channel) | ||
| self._subscriptions[sub_id]['redis_sub'] = new_redis_sub | ||
| sub['redis_sub'] = new_redis_sub |
There was a problem hiding this comment.
Here the method creates a new subscription on connection error. How will the client receive the new sub ID?
The client will need it to keep listening and also to unsubscribe.
I think it's better to return None instead of creating a new subscription.
The client will need to subscribe again in this case.
There was a problem hiding this comment.
We’re not creating a new subscription ID here—only a new redis_sub object bound to the same sub_id.
On ConnectionError we resubscribe to the same channel and update self._subscriptions[sub_id]['redis_sub'], so the client keeps using the original sub_id for both listening and unsubscribe(). No new ID is generated or needed.
As i recall subscription id we generate artificially in python using counter in redis named kernelci-api-pubsub-id (ID_KEY). Redis Pub/Sub has no subscription IDs or message IDs.
| sub['redis_sub'] = new_redis_sub | ||
| continue | ||
| except aioredis.RedisError: | ||
| return None # Handle any exceptions gracefully |
There was a problem hiding this comment.
I'd suggest to log something before returning None just to know afterwards what happened from the logs.
nuclearcat
force-pushed
the
fix-exception
branch
2 times, most recently
from
b60a921 to
8064d6b
Compare
This commit introduces several improvements to the Pub/Sub system to make it more resilient to connection issues and to handle exceptions more gracefully. The following changes are included: - A `health_check_interval` is added to the Redis connection to keep it alive and prevent timeouts. - Reconnection logic is implemented in the `listen` method. If the connection to Redis is lost, the client will now attempt to reconnect and re-subscribe to the channel automatically. - The exception handling in the `listen` method is made more specific, catching `ConnectionError` for reconnection and other `RedisError` exceptions for graceful exit. These changes improve the overall stability and robustness of the Pub/Sub system, especially in environments where the Redis server might be restarted or the network connection is not perfectly stable. Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
nuclearcat
force-pushed
the
fix-exception
branch
from
8064d6b to
6828035
Compare
2 participants
An exception was observed in the logs when retrieving messages from the Redis subscription. This change adds a try-except block to catch potential exceptions during the
get_messagecall and handle them gracefully by returning None.This prevents the subscription loop from crashing and improves the overall stability of the PubSub system.