pardahlman / RawRabbit

A modern .NET framework for communication over RabbitMq
MIT License
746 stars 144 forks source link

HandleRetryAsync: Watch out when working with ExchangeBindings #314

Closed fgieseke closed 6 years ago

fgieseke commented 6 years ago

The retry queue is configured with ... {QueueArgument.DeadLetterExchange, deliveryArgs.Exchange}

Now assume you have an infrastructure as follows: image

If a message was sent to Exchange A and processing fails in a component subscribing the queue bound to Exchange D then the message gets enqueue in the retry queue but with DeadLetterExchange = Exchange A! (this is where the message came from!) So after TTL exceeds the message is rerouted to Exchange A and all other exchanges will get this message again!

The exchangename for the DeadLetterExchange has to be the exchange the subscriber bound it's queue to.

pardahlman commented 6 years ago

Hi - thanks for reporting this. I'm trying to follow your diagram: are exchange B, C and D bound to exchange A (using exchange to exchange bindings)? How did the message end up on Exchange D (is A a topic exchange so that the message is routed to all services?) I think if the message is only delivered to D and you do a retry it is't wrong that it is re-published to A.

fgieseke commented 6 years ago

Hi Yes, its an exchange to exchange binding and all exchanges receive the message.

pardahlman commented 6 years ago

I've been thinking about this scenario and it makes sense. I've updated the middleware so that it tries to use the ConsumerConfiguration when deciding which dead letter exchange to use, only using the delivery args as a fallback in the unlikely event that the configuration is not present (this can happen if you create a custom operation based on existing middleware).

Thanks for reporting this!

pardahlman commented 6 years ago

Closing this for now, as it will be part of the upcoming release. Feel free to reopen this if the problem is still present in rc4