Particular / NServiceBus

Build, version, and monitor better microservices with the most powerful service platform for .NET
https://particular.net/nservicebus/
Other
2.09k stars 648 forks source link

SingleCallChannelReceiver doesn't work with unity #1827

Closed rosieks closed 10 years ago

rosieks commented 10 years ago

SingleCallChannelReceiver doesn't work with UnityBuilder.

Logs with UnityBuilder:

2013-12-02 10:04:43.9703 DEBUG NServiceBus.Gateway.Receiving.IdempotentChannelReceiver Received message of type SingleCallSubmit for client id: 4e5cde3c-b09e-4943-8379-b5a5f88d589e 
2013-12-02 10:04:43.9863 DEBUG NServiceBus.Gateway.Channels.Http.HttpChannelReceiver Sending HTTP 200 response. 
2013-12-02 10:04:44.0143 DEBUG NServiceBus.Gateway.Channels.Http.HttpChannelReceiver Http request processing complete.

When I change builder from UnityBuilder to DefaultBuilder log looks like this:

2013-12-02 10:18:44.5661 DEBUG NServiceBus.Gateway Received message of type SingleCallSubmit for client id: 9a85fc19-dd91-411d-9f82-2f7c793236ed 
2013-12-02 10:18:44.6111 INFO NServiceBus.Gateway.Receiving.GatewayReceiver Sending message to Notifications 
2013-12-02 10:18:45.0402 DEBUG NServiceBus.Gateway.Channels.Http.HttpChannelReceiver Sending HTTP 200 response. 
2013-12-02 10:18:45.0722 DEBUG NServiceBus.Gateway.Channels.Http.HttpChannelReceiver Http request processing complete. 

It looks like this line: https://github.com/Particular/NServiceBus/blob/develop/src/NServiceBus.Core/Gateway/Receiving/GatewayReceiver.cs#L64 resolve IdempotentChannelReceiver when you use Unity and SingleCallChannelReceiver when you use default builder

chrisbednarski commented 10 years ago

There are multiple registrations for the IReceiveMessagesFromSites interface.

Is there a way to set priority which one should be resolved when building a new instance per call?

rosieks commented 10 years ago

As far as I know there is no way to set priority in unity, but priority is no problem. The real problem is implementation of UnityObjectBuilder. From what I understand when you register some interface you check if some other class was registered for that interfaces. If nothing was already registered then you register your class. Otherwise you register your class with the name (but then this class can be only resolved with ResolveAll)

Possible solution to resolve that issue is to register all types with name and store name of last registerd type in DefaultInstances (by replacing HashSet<Type> to Dictionary<Type, string>). Then you just use ResolveAll in BuildAll method and Resolve(name) in Build

chrisbednarski commented 10 years ago

You should be able to work around this issue by providing your own registration in reverse order

public class GatewayHack : INeedInitialization
{
  public void Init()
  {
    Configure.Component<SingleCallChannelReceiver>(DependencyLifecycle.InstancePerCall);
    Configure.Component<IdempotentChannelReceiver>(DependencyLifecycle.InstancePerCall);
  }
}
rosieks commented 10 years ago

I can also use old Gateway implementation by using call type Submit with AutoAsk - this is not a problem. The real problem is incompatibility of UnityObjectBuilder with DefaultBuilder because that can cause much more problems.

andreasohlund commented 10 years ago

Agreed, can you see if you can create a failing unit test for unity?

https://github.com/Particular/NServiceBus/tree/develop/src/ObjectBuilder.Tests

On Wed, Dec 4, 2013 at 6:07 PM, rosieks notifications@github.com wrote:

I can also use old Gateway implementation by using call type Submit with AutoAsk - this is not a problem. The real problem is incompatibility of UnityObjectBuilder with DefaultBuilder because that can cause much more problems.

— Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/1827#issuecomment-29824037 .

SimonCropp commented 10 years ago

@rosieks can you upload a repro of you problem somewhere? as in a full zipped solution that illustrates the problem?

rosieks commented 10 years ago

http://sdrv.ms/1ddj6LW This is Gateway sample from NSB 4.2. I changed in it few thinks:

Everything works great with DefaultBuilder. After change it to UnityBuilder I don't receive any message from Index.htm

johnsimons commented 10 years ago

Proposed solution

  1. Register by default SingleCallChannelReceiver only
  2. Add an option for user to register IdempotentChannelReceiver instead of SingleCallChannelReceiver This could be done via appsetting flag

Thoughts?

andreasohlund commented 10 years ago

Sounds good (assuming there are mutually exclusive in terms of how we use them?)

On Thu, Jan 9, 2014 at 4:24 PM, John Simons notifications@github.comwrote:

Proposed solution

  1. Register by default SingleCallChannelReceiver only
  2. Add an option for user to register IdempotentChannelReceiverinstead of SingleCallChannelReceiver This could be done via appsetting flag

Thoughts?

— Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/1827#issuecomment-31935311 .

chrisbednarski commented 10 years ago

@johnsimons @andreasohlund , shouldn't this be the other way around (IdempotentChannelReceiver as default), since the legacy mode is on by default until v5? Also, what about containers that are not able to create instances of objects that haven't been registered at all? (SingleCallChannelReceiver requires IdempotentChannelReceiver in the constructor)

andreasohlund commented 10 years ago

Correct, what ever is the default in v4 needs to be registered by default. Can you mix atm? (channel1 == legacy, channel2=Idempotent)

On Sat, Jan 11, 2014 at 11:57 AM, chrisbednarski notifications@github.comwrote:

@johnsimons https://github.com/johnsimons @andreasohlundhttps://github.com/andreasohlund, shouldn't this be the other way around ( IdempotentChannelReceiver as default), since the legacy mode is on by default until v5?

— Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/1827#issuecomment-32092010 .

chrisbednarski commented 10 years ago

@andreasohlund, you can mix ... the mode is selected per channel

andreasohlund commented 10 years ago

oh, so then we need to register both?

On Sat, Jan 11, 2014 at 12:00 PM, chrisbednarski notifications@github.comwrote:

@andreasohlund https://github.com/andreasohlund, you can mix ... the mode is selected per channel

— Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/1827#issuecomment-32092052 .

chrisbednarski commented 10 years ago

looks that way ...

The new channel is able to receive messages from old endpoints and the SingleCallChannelReceiver constructor requires IdempotentChannelReceiver to be injected.

johnsimons commented 10 years ago

So what is the fix?

johnsimons commented 10 years ago

The new channel is able to receive messages from old endpoints

Why?

johnsimons commented 10 years ago

The only way I see us fixing this is to register a single IReceiveMessagesFromSites

chrisbednarski commented 10 years ago

@johnsimons backwards compatibility is probably the only reason. A site needs to be able to update to gateway v2 without forcing all the senders to also update to new version of gateway. Later, all the senders can also update to v2

chrisbednarski commented 10 years ago

If the issue is only with Unity, could this be handled by the object builder? Maybe adding a Microsoft.Practices.ObjectBuilder2.BuilderStrategy? I don't know anything about Unity.

chrisbednarski commented 10 years ago

What about?

Configure.Component<Func<IReceiveMessagesFromSites>>(builder =>
    () => builder.Build<SingleCallChannelReceiver>(), DependencyLifecycle.InstancePerCall);

I'll prototype it. See #1894

rosieks commented 10 years ago

@chrisbednarski In my opinion this issue is not only with Unity, but also with Windsor and Ninject.

chrisbednarski commented 10 years ago

@rosieks my suggested solution (the func) requires no changes to the builders, but thanks for pointing that out.

billism1 commented 10 years ago

I see this is still open, so...

This appears to cause an issue when running the gateway and configuring NServiceBus to use the Ninject Builder. When doing this, I am seeing a Ninject exception with the following message:

"Error activating IReceiveMessagesFromSites More than one matching bindings are available. Activation path: 1) Request for IReceiveMessagesFromSites

Suggestions: 1) Ensure that you have defined a binding for IReceiveMessagesFromSites only once."

I have tested with NServiceBus v4.3.0 and 4.4.2. My workaround is to turn off the gateway (on by default for Master), as I do not need the gateway running on the Master node anyway.

SimonCropp commented 10 years ago

@billism1 for completeness can you include your stack trace for that error?

johnsimons commented 10 years ago

Stacktrace:

System.AggregateException : Test run failed due to one or more exception
  ----> System.AggregateException : One or more errors occurred.
  ----> NServiceBus.AcceptanceTesting.Support.ScenarioException : Endpoint failed to start
  ----> Ninject.ActivationException : Error activating IReceiveMessagesFromSites
More than one matching bindings are available.
Activation path:
  1) Request for IReceiveMessagesFromSites

Suggestions:
  1) Ensure that you have defined a binding for IReceiveMessagesFromSites only once.

   at NServiceBus.AcceptanceTesting.Support.ScenarioRunner.Run(IList`1 runDescriptors, IList`1 behaviorDescriptors, IList`1 shoulds, Func`2 done, Int32 limitTestParallelismTo, Action`1 reports) in ScenarioRunner.cs: line 86
   at NServiceBus.AcceptanceTesting.ScenarioWithContext`1.Run(Nullable`1 testExecutionTimeout) in Scenario.cs: line 90
   at NServiceBus.AcceptanceTests.Gateway.When_doing_request_response_between_sites.Callback_should_be_fired() in When_doing_request_response_between_sites.cs: line 27
--AggregateException
   at System.Threading.Tasks.Task.WaitAll(Task[] tasks, Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.WaitAll(Task[] tasks, Int32 millisecondsTimeout)
   at System.Threading.Tasks.Task.WaitAll(Task[] tasks, TimeSpan timeout)
   at NServiceBus.AcceptanceTesting.Support.ScenarioRunner.StartEndpoints(IEnumerable`1 endpoints) in ScenarioRunner.cs: line 269
   at NServiceBus.AcceptanceTesting.Support.ScenarioRunner.PerformScenarios(RunDescriptor runDescriptor, IEnumerable`1 runners, Func`1 done) in ScenarioRunner.cs: line 236
   at NServiceBus.AcceptanceTesting.Support.ScenarioRunner.PerformTestRun(IList`1 behaviorDescriptors, IList`1 shoulds, RunDescriptor runDescriptor, Func`2 done) in ScenarioRunner.cs: line 160
--ScenarioException
   at NServiceBus.AcceptanceTesting.Support.ScenarioRunner.<>c__DisplayClass22.<StartEndpoints>b__20() in ScenarioRunner.cs: line 274
   at System.Threading.Tasks.Task.Execute()
--ActivationException
   at Ninject.KernelBase.Resolve(IRequest request) in c:\Projects\Ninject\ninject\src\Ninject\KernelBase.cs: line 342
   at Ninject.ResolutionExtensions.Get(IResolutionRoot root, Type service, IParameter[] parameters) in c:\Projects\Ninject\ninject\src\Ninject\Syntax\ResolutionExtensions.cs: line 151
   at NServiceBus.ObjectBuilder.Common.CommonObjectBuilder.Build() in CommonObjectBuilder.cs: line 159
   at NServiceBus.Gateway.Receiving.GatewayReceiver.Start() in GatewayReceiver.cs: line 66
   at NServiceBus.Satellites.SatelliteLauncher.StartSatellite(SatelliteContext context) in SatelliteLauncher.cs: line 121
   at NServiceBus.Satellites.SatelliteLauncher.<>c__DisplayClass4.<Start>b__2(Int32 index) in SatelliteLauncher.cs: line 61
   at System.Threading.Tasks.Parallel.<>c__DisplayClassf`1.<ForWorker>b__c()
   at System.Threading.Tasks.Task.InnerInvokeWithArg(Task childTask)
   at System.Threading.Tasks.Task.<>c__DisplayClass11.<ExecuteSelfReplicating>b__10(Object param0)
chrisbednarski commented 10 years ago

Is there any issue with registering a Func<IReceiveMessagesFromSites>? There would be two registrations for IReceiveMessagesFromSites, but only a single Func<IReceiveMessagesFromSites>. The func only needs to be registered as long as the legacy IdempotentChannelReceiver remains. It could later be reverted to a single registration of SingleCallChannelReceiver once IdempotentChannelReceiver has been removed.

billism1 commented 10 years ago

My stacktrace:

--- System.AggregateException:
One or more errors occurred.
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.Wait()
   at System.Threading.Tasks.Parallel.ForWorker[TLocal](Int32 fromInclusive, Int32 toExclusive, ParallelOptions parallelOptions, Action`1 body, Action`2 bodyWithState, Func`4 bodyWithLocal, Func`1 localInit, Action`1 localFinally)
   at System.Threading.Tasks.Parallel.For(Int32 fromInclusive, Int32 toExclusive, Action`1 body)
   at NServiceBus.Satellites.SatelliteLauncher.Start() in c:\BuildAgent\work\31f8c64a6e8a2d7c\src\NServiceBus.Core\Satellites\SatelliteLauncher.cs:line 34
   at NServiceBus.Unicast.UnicastBus.Start(Action startupAction) in c:\BuildAgent\work\31f8c64a6e8a2d7c\src\NServiceBus.Core\Unicast\UnicastBus.cs:line 817
   at NServiceBus.Unicast.UnicastBus.Start() in c:\BuildAgent\work\31f8c64a6e8a2d7c\src\NServiceBus.Core\Unicast\UnicastBus.cs:line 769
   at NServiceBus.Hosting.GenericHost.Start() in c:\BuildAgent\work\31f8c64a6e8a2d7c\src\NServiceBus.Core\Hosting\GenericHost.cs:line 72
   at NServiceBus.Hosting.Windows.WindowsHost.Start() in c:\BuildAgent\work\31f8c64a6e8a2d7c\src\NServiceBus.Hosting.Windows\WindowsHost.cs:line 53
   at NServiceBus.Hosting.Windows.Program.<>c__DisplayClassd.<Main>b__5(WindowsHost service) in c:\BuildAgent\work\31f8c64a6e8a2d7c\src\NServiceBus.Hosting.Windows\Program.cs:line 81
   at Topshelf.Internal.ControllerDelegates`1.StartActionObject(Object obj) in c:\Projects\TopShelfForNSB\src\Topshelf\Internal\ControllerDelegates.cs:line 18
   at Topshelf.Internal.IsolatedServiceControllerWrapper`1.<>c__DisplayClass2.<set_StartAction>b__1(TService service) in c:\Projects\TopShelfForNSB\src\Topshelf\Internal\IsolatedServiceControllerWrapper.cs:line 65
   at Topshelf.Internal.ServiceController`1.<.cctor>b__1(ServiceController`1 sc) in c:\Projects\TopShelfForNSB\src\Topshelf\Internal\ServiceController.cs:line 35
   at Magnum.StateMachine.LambdaAction`1.Execute(T instance, Event event, Object parameter) in :line 0
   at Magnum.StateMachine.EventActionList`1.Execute(T stateMachine, Event event, Object parameter) in :line 0   at NServiceBus.Hosting.Windows.Program.Main(String[] args) in c:\BuildAgent\work\31f8c64a6e8a2d7c\src\NServiceBus.Hosting.Windows\Program.cs:line 152

--- Ninject.ActivationException:
Error activating IReceiveMessagesFromSites
More than one matching bindings are available.
Activation path:
  1) Request for IReceiveMessagesFromSites

Suggestions:
  1) Ensure that you have defined a binding for IReceiveMessagesFromSites only once.
   at System.Threading.Tasks.Task.Wait(Int32 millisecondsTimeout, CancellationToken cancellationToken)
   at System.Threading.Tasks.Task.Wait()
   at System.Threading.Tasks.Parallel.ForWorker[TLocal](Int32 fromInclusive, Int32 toExclusive, ParallelOptions parallelOptions, Action`1 body, Action`2 bodyWithState, Func`4 bodyWithLocal, Func`1 localInit, Action`1 localFinally)
   at System.Threading.Tasks.Parallel.For(Int32 fromInclusive, Int32 toExclusive, Action`1 body)
   at NServiceBus.Satellites.SatelliteLauncher.Start() in c:\BuildAgent\work\31f8c64a6e8a2d7c\src\NServiceBus.Core\Satellites\SatelliteLauncher.cs:line 34
   at NServiceBus.Unicast.UnicastBus.Start(Action startupAction) in c:\BuildAgent\work\31f8c64a6e8a2d7c\src\NServiceBus.Core\Unicast\UnicastBus.cs:line 817
   at NServiceBus.Unicast.UnicastBus.Start() in c:\BuildAgent\work\31f8c64a6e8a2d7c\src\NServiceBus.Core\Unicast\UnicastBus.cs:line 769
   at NServiceBus.Hosting.GenericHost.Start() in c:\BuildAgent\work\31f8c64a6e8a2d7c\src\NServiceBus.Core\Hosting\GenericHost.cs:line 72
   at NServiceBus.Hosting.Windows.WindowsHost.Start() in c:\BuildAgent\work\31f8c64a6e8a2d7c\src\NServiceBus.Hosting.Windows\WindowsHost.cs:line 53
   at NServiceBus.Hosting.Windows.Program.<>c__DisplayClassd.<Main>b__5(WindowsHost service) in c:\BuildAgent\work\31f8c64a6e8a2d7c\src\NServiceBus.Hosting.Windows\Program.cs:line 81
   at Topshelf.Internal.ControllerDelegates`1.StartActionObject(Object obj) in c:\Projects\TopShelfForNSB\src\Topshelf\Internal\ControllerDelegates.cs:line 18
   at Topshelf.Internal.IsolatedServiceControllerWrapper`1.<>c__DisplayClass2.<set_StartAction>b__1(TService service) in c:\Projects\TopShelfForNSB\src\Topshelf\Internal\IsolatedServiceControllerWrapper.cs:line 65
   at Topshelf.Internal.ServiceController`1.<.cctor>b__1(ServiceController`1 sc) in c:\Projects\TopShelfForNSB\src\Topshelf\Internal\ServiceController.cs:line 35
   at Magnum.StateMachine.LambdaAction`1.Execute(T instance, Event event, Object parameter) in :line 0
   at Magnum.StateMachine.EventActionList`1.Execute(T stateMachine, Event event, Object parameter) in :line 0   at NServiceBus.Hosting.Windows.Program.Main(String[] args) in c:\BuildAgent\work\31f8c64a6e8a2d7c\src\NServiceBus.Hosting.Windows\Program.cs:line 152

Note, you can reproduce by adding the Ninject & NServiceBus.Ninject references and then modifying the "EndpointConfig" class in the ScaleOut example, like so (and then running the "Orders.Handler" process with the Master config):

    using NServiceBus;
    using Ninject;

    class EndpointConfig : IConfigureThisEndpoint, AsA_Publisher, IWantCustomInitialization
    {
        private static readonly IKernel Kernel = new StandardKernel();

        public void Init()
        {
            Configure.With()
                     .NinjectBuilder(Kernel);
        }
    }
johnsimons commented 10 years ago

Fixed in #1894