Skip to content

feat: improve error logging and streamline consumer prefetch handling… - #31

Merged
samerzughul merged 1 commit into
mainfrom
muhannad/new-request-context-values
Jan 25, 2026
Merged

feat: improve error logging and streamline consumer prefetch handling…#31
samerzughul merged 1 commit into
mainfrom
muhannad/new-request-context-values

Conversation

@mmalkhatib

@mmalkhatib mmalkhatib commented Jan 25, 2026

Copy link
Copy Markdown
Contributor

… in ConsumersService

Summary by CodeRabbit

  • Bug Fixes
    • Corrected consumer configuration updates to properly apply quality-of-service settings after property changes.
    • Improved error logging for consumer cancellation failures.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Jan 25, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

Updates error logging level for consumer cancellation failures and consolidates consumer QoS reconfiguration logic by removing a separate Prefetch-specific branch in favor of unified property updates.

Changes

Cohort / File(s) Summary
ConsumersService QoS Consolidation
SW.Bus/ConsumersService.cs
Elevated consumer cancellation failure logging from Warning to Error; unified BasicQoS reapplication with general property reconfiguration by removing separate Prefetch handling branch

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A rabbit hops through queues with glee,
Consolidating paths, keeping logs carefree,
No more branching—just one clear way,
QoS updates shine bright today! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly reflects the main changes: improved error logging (Warning to Error level) and streamlined consumer prefetch handling by removing conditional branching.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
SW.Bus/ConsumersService.cs (1)

222-249: Prefetch‑only updates are now ignored.

Line 222–225 only detect args/priority changes; Line 240 applies BasicQos only inside that branch. If only QueuePrefetch changes, it falls into Line 246–249 and never updates QoS, so the channel keeps the old prefetch.

🐛 Suggested fix (apply QoS when prefetch changes)
-                    var argsChanged = !DictionariesEqual(existingDef.ConsumerArgs, def.ConsumerArgs);
-                    var priorityChanged = existingDef.ConsumerPriority != def.ConsumerPriority;
+                    var argsChanged = !DictionariesEqual(existingDef.ConsumerArgs, def.ConsumerArgs);
+                    var priorityChanged = existingDef.ConsumerPriority != def.ConsumerPriority;
+                    var prefetchChanged = existingDef.QueuePrefetch != def.QueuePrefetch;
 
-                    if (priorityChanged || argsChanged)
+                    if (priorityChanged || argsChanged)
                     {
                         logger.LogInformation($"Configuration changed for {def.QueueName}. Restarting consumer.");
                         try
                         {
                             existingModel.BasicCancel(existingDef.ConsumerTag);
                         }
                         catch (Exception ex)
                         {
                             logger.LogError(ex, $"Failed to cancel consumer {def.QueueName}");
                         }
 
                         // Update properties (including Args) BEFORE re-consuming
                         existingDef.UpdateConsumerProps(def);
 
                         existingModel.BasicQos(0, def.QueuePrefetch, false);
                         
                         existingDef.ConsumerTag = existingModel.BasicConsume(def.QueueName,
                             false, "", existingDef.ConsumerArgs, existingDef.ConsumerObject);
                         
                     }
                     else 
                     {
                         existingDef.UpdateConsumerProps(def);
+                        if (prefetchChanged)
+                            existingModel.BasicQos(0, def.QueuePrefetch, false);
                     }

@samerzughul
samerzughul merged commit d924e47 into main Jan 25, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants