Skip to content

Conversation

adenchen123
Copy link
Contributor

@adenchen123 adenchen123 commented Oct 16, 2025

User description

This reverts commit b8636a7.


PR Type

Bug fix


Description

  • Added RedisTimeoutException handling in RedisSubscriber to prevent crashes

  • Logs warning message and continues to next cycle on timeout

  • Improves resilience of Redis subscription mechanism


Diagram Walkthrough

flowchart LR
  A["RedisSubscriber"] --> B["Catch RedisTimeoutException"]
  B --> C["Log Warning"]
  C --> D["Continue Next Cycle"]
Loading

File Walkthrough

Relevant files
Error handling
RedisSubscriber.cs
Handle Redis timeout exceptions gracefully                             

src/Infrastructure/BotSharp.Core/Infrastructures/Events/RedisSubscriber.cs

  • Added RedisTimeoutException catch block before generic exception
    handler
  • Logs warning message with channel information on timeout
  • Allows subscription loop to continue without delay on timeout
+4/-0     

Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
- [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true -->

</details></td></tr>
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent high CPU usage on timeout

Add a Task.Delay within the RedisTimeoutException catch block to prevent a
high-CPU busy-wait loop during connection timeouts.

src/Infrastructure/BotSharp.Core/Infrastructures/Events/RedisSubscriber.cs [84-87]

 catch (RedisTimeoutException)
 {
-    _logger.LogWarning($"Redis timeout for channel {channel}, will retry in next cycle");
+    _logger.LogWarning($"Redis timeout for channel {channel}, will retry in 5 seconds.");
+    await Task.Delay(5000);
 }
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: The suggestion correctly identifies that the new catch block for RedisTimeoutException is missing a delay, which can cause a high-CPU busy-wait loop if Redis is unavailable, potentially making the application unresponsive.

High
Learned
best practice
Check connection and log context

Include context details and check connection state to aid diagnostics and ensure
safe continuation after timeouts.

src/Infrastructure/BotSharp.Core/Infrastructures/Events/RedisSubscriber.cs [84-87]

-catch (RedisTimeoutException)
+catch (RedisTimeoutException ex)
 {
-    _logger.LogWarning($"Redis timeout for channel {channel}, will retry in next cycle");
+    if (!_redis.IsConnected)
+    {
+        _logger.LogWarning(ex, $"Redis timeout for channel '{channel}' (group '{group}'), connection lost; attempting to reconnect and will retry next cycle.");
+    }
+    else
+    {
+        _logger.LogWarning(ex, $"Redis timeout for channel '{channel}' (group '{group}'); will retry next cycle.");
+    }
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Validate async/I/O usage and connection state before performing Redis operations to avoid misuse and transient failures.

Low
Add input null/empty guards

Validate that inputs like channel, group, and received are not null or empty
before use, and short-circuit with safe defaults or throw early with clear
messages.

src/Infrastructure/BotSharp.Core/Infrastructures/Events/RedisSubscriber.cs [28-32]

 public async Task SubscribeAsync(string channel, string group, int? port, bool priorityEnabled, 
         Func<string, string, Task> received, 
         CancellationToken? stoppingToken = null)
     {
+        if (string.IsNullOrWhiteSpace(channel)) throw new ArgumentException("channel cannot be null or empty", nameof(channel));
+        if (string.IsNullOrWhiteSpace(group)) throw new ArgumentException("group cannot be null or empty", nameof(group));
+        if (received is null) throw new ArgumentNullException(nameof(received));
         var db = _redis.GetDatabase();
 ...
             await HandleGroupMessage(db, channel, group, consumer, received, $"{channel}-Error");

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Ensure nullability guards before property access and default to safe fallbacks when inputs can be missing.

Low
  • More

@JackJiang1234
Copy link
Contributor

reviewed

@yileicn yileicn merged commit 9520aa0 into SciSharp:master Oct 16, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants