rebus-org / Rebus.AzureServiceBus

:bus: Azure Service Bus transport for Rebus
https://mookid.dk/category/rebus
Other
33 stars 20 forks source link

Topic not found exception is swallowed #32

Closed thecontrarycat closed 5 years ago

thecontrarycat commented 5 years ago

The code for sending to a topic is currently configured to swallow MessagingEntityNotFoundException with the comment "if the topic does not exist, it's allright". Why is this deemed to be ok?

My domain events are all important, and losing some because the publisher started before the subscriber has registered itself with ASB is not a desired scenario. I would like the publisher to receive an error if it tries to publish an event that does not have an associated topic (and therefore will cannot be delivered).

 if (destinationQueue.StartsWith(MagicSubscriptionPrefix))
                        {
                            var topicName = _nameFormatter.FormatTopicName(destinationQueue.Substring(MagicSubscriptionPrefix.Length));

                            foreach (var batch in messages.Batch(DefaultOutgoingBatchSize))
                            {
                                var list = batch.Select(GetMessage).ToList();

                                try
                                {
                                    await GetTopicClient(topicName).SendAsync(list).ConfigureAwait(false);
                                }
                                catch (MessagingEntityNotFoundException)
                                {
                                    // if the topic does not exist, it's allright
                                }
                                catch (Exception exception)
                                {
                                    throw new RebusApplicationException(exception, $"Could not publish to topic '{topicName}'");
                                }
                            }
                        }
mookid8000 commented 5 years ago

(...) Why is this deemed to be ok?

Because Rebus does not know or care how many subscribers there is. So, as Rebus does not distinguish between whether you have 4 or 3 subscribers, it does not distinguish between whether you have 1 or 0 subscribers either.

No matter what Rebus does, you need to take care to start your subscribers first*, if you want to be sure you receive published messages.

This rule holds for when you go from 3 to 4 subscribers, and – as you have found out – it holds the same way when you go from 0 to 1 subscribers.

I hope you can see that the consistency of this behavior (which also happens to be consistent with other Rebus transports, like e.g. RabbitMQ) is the way users get the smallest number of surprises along the way.


(*) Please note: Once a subscriber HAS been started, and it has not explicitly unsubscribed, it will stay subscribed to that topic. This means that published messages will be received in the subscriber's input queue from that point in time and on, no matter if the subscriber is running when the messages are published.

thecontrarycat commented 5 years ago

Thanks for the explanation. I agree that being consistent across transports is a solid goal.

However, as I am looking to implement Rebus in a microservices environment I cannot agree that going from 0 to 1 is the same as going from 4 to 5.

When you go from 4 to 5 you cannot (and for coupling reasons do not want to) guarantee that the published event will go to all expected subscribers. However, when the topic does not even exist you absolutely do know that the published event will not go to any expected subscriber.

This greatly concerns me as I do not want to be in the situation where I am potentially deploying 10s of hosts and have to orchestrate their deployment so that their subscriptions have all occurred prior to any messages being sent.

Will you accept a pull request to add this as an option? I have the code written already ;-)

mookid8000 commented 5 years ago

Will you accept a pull request to add this as an option?

Sorry, but no. 😐In this case, it's a very conscious decision that it works that way.

Also, it's generally (across different types of message brokers) not possible for a publisher to know how many subscribers there are. It's just Azure Service Bus that has this slightly clunky model, where you have to construct topics and bindings (i.e. "subscriptions") out of "service bus entities", and therefore throws the MessagingEntityNotFoundException if a non-existent topic is used.

(...) when the topic does not even exist you absolutely do know that the published event will not go to any expected subscriber.

I can see how that may be true.

But, for the reasons stated (consistent behavior for any number of subscribers n in [n ; n+1]), I do not believe it's the right thing to do.

And there would be no way for me to replicate that behaviour with other transports, e.g. with RabbitMQ.

Thank you very much for offering a PR to fix this, though! πŸ™I hope you don't take my opinion as dismissive or anything, I truly appreciate both a) a well-written suggestion, and especially b) a PR.

And please, do use your customized ASB transport implementation for your own projects, as I'm sure it provides value to you. πŸ™‚

thecontrarycat commented 5 years ago

Interestingly, it looks like your RabbitMQ transport has inconsistent behaviour when the exchange doesn't exist.

As it gets the Model and then for-loops over the messages to send, if an early message causes the Model's channel to be closed (because it was sent to non-existent exchange) then sending of later messages will fail (because the channel is now closed).

You're right that in the case of a one-off event going to a non-existent exchange that it is not treated as a failure, however.

Personally, I still don't understand why sending to an exchange/topic that doesn't exist is treated differently to sending to a queue that doesn't exist. Either way, it is a configuration error because the producer does not know the correct way to route a message. The non-existence of the destination is a different failure state (one of configuration) to the non-existence of a subscriber on an existing topic or exchange (because this is how pub/sub is supposed to work).

mookid8000 commented 5 years ago

(...) I still don't understand why sending to an exchange/topic that doesn't exist is treated differently to sending to a queue that doesn't exist (...)

It's actually a quite fundamental distinction in Rebus that publishers (those that await bus.Publish(...)) do not and should not care whether someone is listening, whereas senders (those that await bus.Send(...)) by definition care that their intended recipient gets the message.

This assumption is the underlying principle of the design decisions made around these things.

thecontrarycat commented 5 years ago

Absolutely, and I agree that the Publishers should never have an awareness of the presence (or otherwise) of Subscribers.

Where I take issue is that when using Brokered Messaging the actual subscriptions are hidden behind an endpoint (the topic or exchange), and messages can't be published at all if that endpoint doesn't exist.

There is a functional difference between "the broker has accepted your message and will forward it to any subscribers" and "the broker rejected your request as invalid". The latter meaning "your infrastructure hasn't been configured to allow publishing this event yet", which to me is just as much of an error condition as attempting to use the wrong connection string.

If it helps, both the official RabbitMQ and Azure Service Bus clients treat an attempt to publish to an invalid address as an error condition. IMO, Rebus should do the same, as the current behaviour is non-obvious and will lead to lost messages in any non-trivial multi-process installation.

mookid8000 commented 5 years ago

But... if you publish a message with RabbitMQ on a topic exchange, using a routing key, to which no queue is bound, the message will be delivered to no-one, and thus effectively disappear. Wouldn't you say that this is pretty much the analogue to publishing on a non-existent topic in Azure Service Bus?

thecontrarycat commented 5 years ago

For me, I'd equate the RabbitMQ topic exchange and ASB topic as the same thing

No topic exchange = channel closed No topic = exception thrown

Exchange but no subs = send success Topic but no subs = send success

It's ok if you think differently, and maybe your thinking is coming from the perspective of Rebus handling all of the infrastructure instantiation (as it creates queues, topics and subscriptions itself), but for me I'd like confirmation that my infrastructure is in place when I send. The number of subscribers is - as you say - not important.

mookid8000 commented 5 years ago

I'd equate the RabbitMQ topic exchange and ASB topic as the same thing

Well, I guess it depends on how you're used to using RabbitMQ). Rebus simply uses two exchanges, of types "direct" and "topic" respectively, where all direct bindings (meant for await bus.Send(..)) are made in the direct exchange, and all multicast bindings (means for await bus.Publish(...)) are made in the topic exchange.

This way, it's the bindings that come and go as Rebus endpoints come/go/subscribe/unsubscribe.

(...) maybe your thinking is coming from the perspective of Rebus handling all of the infrastructure instantiation (...)

It is! πŸ™‚ My mission with Rebus is to make your messaging

  1. easy
  2. pleasant
  3. platform-independent

A very important property of platform-independence, is that platform-specific implementations have uniform behavior across platforms when possible.

(...) I'd like confirmation that my infrastructure is in place when I send

I definitely understand where you're coming from. πŸ™‚

I want you to know that I really appreciate your comments in here 😁 It's good to be challenged on these things. πŸ‘Š