Skip to content

Conversation

@Gsantomaggio
Copy link
Member

@Gsantomaggio Gsantomaggio commented Oct 23, 2025

This PR improves shutdown and cleanup logic for producers and consumers, focusing on graceful termination and better error handling during close operations.

  • Adds cancellation token checks to prevent processing loops from continuing after shutdown
  • Ensures channel writers are properly completed to unblock waiting consumers
  • Demotes timeout error logs to debug level to reduce log noise during normal shutdown scenarios
  • Consolidates ignore logic for cleaner handling of already-closed connections

Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
DeleteEntityFromTheServer when the socket is closed

Signed-off-by: Gabriele Santomaggio <[email protected]>
DeleteEntityFromTheServer when the socket is closed

Signed-off-by: Gabriele Santomaggio <[email protected]>
DeleteEntityFromTheServer when the socket is closed

Signed-off-by: Gabriele Santomaggio <[email protected]>
DeleteEntityFromTheServer when the socket is closed

Signed-off-by: Gabriele Santomaggio <[email protected]>
@Gsantomaggio Gsantomaggio requested a review from Copilot October 30, 2025 08:48
@Gsantomaggio Gsantomaggio marked this pull request as ready for review October 30, 2025 08:48
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves shutdown and cleanup logic for RabbitMQ Stream producers and consumers, focusing on graceful termination and better error handling during close operations.

  • Adds cancellation token checks to prevent processing loops from continuing after shutdown
  • Ensures channel writers are properly completed to unblock waiting consumers
  • Demotes timeout error logs to debug level to reduce log noise during normal shutdown scenarios
  • Consolidates ignore logic for cleaner handling of already-closed connections

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
RabbitMQ.Stream.Client/RawProducer.cs Added cancellation token check in ProcessBuffer loop, complete channel writer on close, and consolidate ignore logic for DeletePublisher
RabbitMQ.Stream.Client/RawConsumer.cs Changed synchronous Wait() to async ConfigureAwait(false) calls, added finally block to complete channel writer, and demoted timeout error to debug level
RabbitMQ.Stream.Client/Client.cs Demoted timeout error to debug level during connection close
RabbitMQ.Stream.Client/AbstractEntity.cs Consolidated ignore logic for DeleteEntityFromTheServer to include already-closed clients
Comments suppressed due to low confidence (2)

RabbitMQ.Stream.Client/RawConsumer.cs:565

  • Poor error handling: empty catch block.
                    catch { }

RabbitMQ.Stream.Client/RawConsumer.cs:565

  • Generic catch clause.
                    catch { }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +379 to +381
catch
{
// ignored
Copy link

Copilot AI Oct 30, 2025

Choose a reason for hiding this comment

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

Generic catch clause.

Suggested change
catch
{
// ignored
catch (Exception e)
{
Logger.LogError(e, "{DumpEntityConfiguration} Error while completing message buffer in Close()", DumpEntityConfiguration());

Copilot uses AI. Check for mistakes.
@Gsantomaggio Gsantomaggio added this to the 1.9.3 milestone Oct 30, 2025
@Gsantomaggio Gsantomaggio changed the title small improvements Improves shutdown and cleanup logic for producers and consumers Oct 30, 2025
@Gsantomaggio Gsantomaggio merged commit 6392986 into main Oct 30, 2025
2 checks passed
@Gsantomaggio Gsantomaggio deleted the feat/imporvements branch October 30, 2025 08:58
@Gsantomaggio
Copy link
Member Author

@jonnepmyra, can you please try it?

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