Particular / NServiceBus.AzureServiceBus

The Azure ServiceBus Transport
https://docs.particular.net/nservicebus/azure-service-bus/
Other
13 stars 17 forks source link

The method 'OnMessage' or 'OnMessageAsync' has already been called #368

Closed ottarhenanger closed 8 years ago

ottarhenanger commented 8 years ago

Hi, I've upgraded to version 7.0.0 (Release) of NServiceBus.Azure.Transports.WindowsAzureServiceBus.

Endpoint is configured as following:

var transport = config.UseTransport<AzureServiceBusTransport>(); transport.ConnectionString(serviceBusConnectionString); transport.UseTopology<ForwardingTopology>(); transport.Sanitization().UseStrategy<Sha1Sanitization>();

When trying to start endpoint, the exception below is raised.

The method 'OnMessage' or 'OnMessageAsync' has already been called. at Microsoft.ServiceBus.Messaging.MessageReceiver.OnMessage(MessageReceivePump pump) at NServiceBus.Transport.AzureServiceBus.MessageReceiverAdapter.OnMessage(Func2 callback, OnMessageOptions options) in C:\Build\src\Transport\Connectivity\MessageReceiverAdapter.cs:line 37 at NServiceBus.Transport.AzureServiceBus.MessageReceiverNotifier.Start() in C:\Build\src\Transport\Receiving\MessageReceiverNotifier.cs:line 136 at NServiceBus.Transport.AzureServiceBus.TopologyOperator.StartNotifiersFor(IEnumerable1 entities) in C:\Build\src\Transport\Topology\TopologyOperator.cs:line 108 at NServiceBus.Transport.AzureServiceBus.TopologyOperator.Start(TopologySection topologySection, Int32 maximumConcurrency) in C:\Build\src\Transport\Topology\TopologyOperator.cs:line 41 at NServiceBus.TransportReceiver.Start() in C:\Build\src\NServiceBus.Core\Transports\TransportReceiver.cs:line 47 at NServiceBus.StartableEndpoint.StartReceivers(List1 receivers) in C:\Build\src\NServiceBus.Core\StartableEndpoint.cs:line 90 at NServiceBus.StartableEndpoint.d1.MoveNext() in C:\Build\src\NServiceBus.Core\StartableEndpoint.cs:line 53 --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at NServiceBus.Endpoint.d1.MoveNext() in C:\Build\src\NServiceBus.Core\Endpoint.cs:line 28 --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter1.GetResult() at Telenor.Management.Api.Configuration.ServiceBusConfig.Configure() in D:\Development\TCS\TCSAPI\src\Telenor.Management.Api\Configuration\ServiceBusConfig.cs:line 26 at Telenor.Management.Api.Startup.ConfigureApp(IAppBuilder app) in D:\Development\TCS\TCSAPI\src\Telenor.Management.Api\Startup.cs:line 23 at Telenor.Management.Api.OwinCommunicationListener.<OpenAsync>b__14_0(IAppBuilder appBuilder) in D:\Development\TCS\TCSAPI\src\Telenor.Management.Api\OwinCommunicationListener.cs:line 83 at Microsoft.Owin.Hosting.Engine.HostingEngine.Start(StartContext context) at Microsoft.Owin.Hosting.WebApp.Start(String url, Action1 startup) at Telenor.Management.Api.OwinCommunicationListener.OpenAsync(CancellationToken cancellationToken) in D:\Development\TCS\TCSAPI\src\Telenor.Management.Api\OwinCommunicationListener.cs:line 95 at Microsoft.ServiceFabric.Services.Runtime.StatelessServiceInstanceAdapter.d__10.MoveNext()`

ottarhenanger commented 8 years ago

After downgrading to version 7.0.0-rc0005 the problem is solved. From my packages.config:

<package id="NServiceBus" version="6.0.0" targetFramework="net461" /> <package id="NServiceBus.Autofac" version="6.0.0" targetFramework="net461" /> <package id="NServiceBus.Azure.Transports.WindowsAzureServiceBus" version="7.0.0-rc0005" targetFramework="net461" /> <package id="NServiceBus.Azure.Transports.WindowsAzureStorageQueues" version="7.0.0" targetFramework="net461" /> <package id="NServiceBus.Persistence.AzureStorage" version="1.0.0" targetFramework="net461" />

yvesgoeleven commented 8 years ago

@ottarhenanger

I'm unable to reproduce this situation here, using your config mentioned above, any chance you can send us a repro?

SeanFeldman commented 8 years ago

@ottarhenanger I've tried to repro this issue, and couldn't. See my spike here.

ottarhenanger commented 8 years ago

@SeanFeldman I recreated the problem in my spike here.

The problem occurs when having more than one endpoint listeners to a self-hosted Web API project. In my project I need the service to listen both to HTTP and HTTPS. In the spike this is simulated with 2 http endpoints.

To run my spike you need latest version of Azure Service Fabric SDK installed on PC.

SeanFeldman commented 8 years ago

Thank you @ottarhenanger The detail of co-hosting multiple endpoints using the same transport is probably the issue we're having. Let us review it and get back to you.

SeanFeldman commented 8 years ago

@ottarhenanger looking at the code I see that you're starting two instances of OwinCommunicationListener, one on port 8574 and another one on port 8673. When you start each instance, you're going through the same Autofac container, which means you're re-running the passed in Action<IAppBuilder> startup code. Startup code is the one that registers the NSB endpoint. By doing so, you're creating under the same AppDomain multiple endpoints, but they all share the same container. That causes the problem.

This SF service allows listening on 2 different ports, and it can be scaled out (if needed), but this is the same service / NSB endpoint and should only be initialized once.

I couldn't commit the change to your repo (no rights), so I'll just paste the changes here:

ExternalApi/Startup.cs

// Configure NService bus
// ServiceBusConfig.Configure();  <| == do not register NSB with web startup as it will be called multiple times by the communication listener

ExternalApi/Program.cs

context =>
{
  ServiceRuntime.RegisterServiceAsync("ExternalApiType", // <|== why to go through the container here?
  // var externalApiService = DependencyConfig.GetConfiguredContainer().Resolve<ExternalApiService(new TypedParameter(typeof(StatelessServiceContext), context)); 

  var externalApiService = new ExternalApiService(context);
  ServiceBusConfig.Configure(); // <|== will be registered once
  return externalApiService;
}).GetAwaiter().GetResult();

If you'd like me to commit the code, please add me as a collaborator and I'll push the change.

ottarhenanger commented 8 years ago

I noticed that same yesterday and have refactored my code accordingly. What puzzled me was that this was working in 7.0.0-rc0005 but not in release version. Thanks for your assistance.

yvesgoeleven commented 8 years ago

@ottarhenanger I can explain that, we fixed a bug in RC6 that would have hidden this exception. In rc5 we were assiging a pool of receivers, but only used one (wasting resources). When you started the same endpoint again, the code would just go to the next receiver and start using it. But after we fixed our bug, all receivers are already in use, so if you start it again they complain about already having a callback attached.