Fix: Gracefully handle exceptions in PubSub message retrieval #625
+19
−4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 <[email protected]>
nuclearcat
force-pushed
the
fix-exception
branch
from
119f4ee to
811c478
Compare
JenySadadia
reviewed
Aug 12, 2025
Comment on lines
+145
to
+151
| 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.
JenySadadia
reviewed
Aug 12, 2025
| 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.