search

rebus-org / Rebus.AzureServiceBus

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

Empty list of published message causes NullRefException in MS.Azure.SB #27

Closed mikanyg closed 5 years ago

mikanyg commented 5 years ago

I have been upgrading to latest versions of the following packages:

and in a test I run into into a NullRefenceException coming from Microsoft.Azure.ServiceBus.Core.MessageSender. Similar issues seems to be fixed in this PR https://github.com/Azure/azure-service-bus-dotnet/pull/642 for the Microsoft.Azure.Services package, which is not yet released. But I wonder why it occurs in the first place when Rebus calls the TopicClient.SendAsync(list).

I have tracked the calls down to this line https://github.com/rebus-org/Rebus.AzureServiceBus/blob/616c122d8b5b0f73cd166138b7fa8c6e97e7fb50/Rebus.AzureServiceBus/AzureServiceBus/AzureServiceBusTransport.cs#L480 where list seems to be null.

The exception message from Rebus is: "Could not publish to topic 'mymessageassembly_mymessage'"

The inner stacktrace from Microsoft.Azure.ServiceBus is:

at Microsoft.Azure.ServiceBus.Core.MessageSender.<OnSendAsync>d__52.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Azure.ServiceBus.RetryPolicy.<RunOperation>d__19.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.Azure.ServiceBus.RetryPolicy.<RunOperation>d__19.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.Azure.ServiceBus.Core.MessageSender.<SendAsync>d__39.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Rebus.AzureServiceBus.AzureServiceBusTransport.<>c__DisplayClass29_0.<<GetOutgoingMessages>b__3>d.MoveNext()

My test is calling a service that uses a one way client that publishes a message and I expect to receive the message, which the test subscribes to.

Any thoughts on the matter would be helpfull.

mookid8000 commented 5 years ago

I'm wondering what made you believe that it was the list instance that was null?

I know it's a NullReferenceException, but when I look at the code that leads up to making the call to SendAsync, I think it's hard to see how list could be null.

Check it out (source can be found here):

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

    try
    {
        await GetTopicClient(topicName).SendAsync(list).ConfigureAwait(false);
    }
    // (...)
mikanyg commented 5 years ago

I agree that its likely not null, but could it be empty? This issue https://github.com/Azure/azure-service-bus-dotnet/issues/550 indicates that a NullRefException occurs if its also empty, which might be the case.

The package upgrades is part of a move to go full .NET Standard for some shared bootstrapping code so we are going from Rebus.ASB v4.0.1 which used the WindowsAzure.ServiceBus package.

Could it be a missing configuration due to major version differences, something I missed?

mookid8000 commented 5 years ago

I agree that its likely not null, but could it be empty? (...)

I don't think it can. These two lines will, because of GroupBy, result in absolutely no work being done, if there are no outgoing messages in the messagesToSend queue.

And then, the implementation of Batch ensures that no lists are yielded, unless they contain something.

Could it be a missing configuration due to major version differences, something I missed?

Rebus' new Azure Service Bus transport does not require any extra configurations, compared to the previous version. Although (and this might me a long shot): Are you passing a connection string when you call .Transport(t => t.UseAzureServiceBus(connectionString))?

Could you try and include the full exception details, and not just the stack trace from the inner exception?

mikanyg commented 5 years ago

Yes we are passing in a connection string to t.UseAzureServiceBusAsOneWayClient(connString)

The bootstrapping code is all the same, it just references newer package versions and is build as .NET Standard 2.0 rather than .NET framework class library (v4.6.2)

I'll dump the full exception details soon

mikanyg commented 5 years ago

This is the exception details (VS dialog): 

Rebus.Exceptions.RebusApplicationException
  HResult=0x80131500
  Message=Could not publish to topic 'flexp_customersitemgmt_messages_customercreated__flexp_customersitemgmt_messages'
  Source=Rebus.AzureServiceBus
  StackTrace:
   at Rebus.AzureServiceBus.AzureServiceBusTransport.<>c__DisplayClass29_0.<<GetOutgoingMessages>b__3>d.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Rebus.AzureServiceBus.AzureServiceBusTransport.<>c__DisplayClass29_1.<<GetOutgoingMessages>b__1>d.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Rebus.Transport.TransactionContext.<Invoke>d__24.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Rebus.Transport.TransactionContext.<RaiseCommitted>d__21.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Rebus.Transport.TransactionContext.<Complete>d__19.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Rebus.Bus.RebusBus.<InnerSend>d__42.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Rebus.Bus.RebusBus.<InnerPublish>d__33.MoveNext()
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.GetResult()
   at FlexP.CustomerSiteMgmt.Infrastructure.Messaging.MessagePublisher.<Publish>d__2.MoveNext() in C:\Src\Repos\Orsted\CustomerSiteMgmt\src\FlexP.CustomerSiteMgmt.Infrastructure\Messaging\MessagePublisher.cs:line 18

Inner Exception 1:
NullReferenceException: Object reference not set to an instance of an object.

We use amqp protocol for transport in our connectionstring if that could be an issue?

mookid8000 commented 5 years ago

We use amqp protocol for transport in our connectionstring if that could be an issue?

Haven't heard that that should be an issue. Could you try with the sb scheme and see if it makes a difference?

mikanyg commented 5 years ago

Hmm, found the problem package, but can't really figure out what went wrong.

As part of package upgrades I also upgraded Microsoft.Azure.Amqp to version 2.4.0 which the Microsoft.Azure.ServiceBus has dependency on. Somehow I got this version on my machine package cache, but it's currently not on nuget.org. Looking at the release list https://github.com/Azure/azure-amqp/releases the next one coming out is 2.4.1 and in this issue https://github.com/Azure/azure-amqp/issues/135 it looks released.

But downgrading to v2.3.7 all my test goes through without any problems. So it's not rebus related with the NullReferenceException.

Don't know root cause, but staying on v2.3.7 will do the trick for now.

mookid8000 commented 5 years ago

Damn, that was tricky! 😁 good you figured it out 👍

SeanFeldman commented 5 years ago

I hope amqp 2.4.1 doesn't end up like 2.4.0