NServiceBus.ObjectBuilder.Common.IContainer build is throwing exception when it shouldn't
Original issue: GG question 1 and question 2
Who's affected
- Any user using transport with native support for timeouts (ASB)
Symptoms
"NServiceBus.Timeout.Core.IPersistTimeouts is not registered in the container when starting bus" exception is thrown when visual studio is configured to break on all exceptions.
Cause analysis
NServiceBus Core is executing TimeoutPersistenceVersionCheck and that code is trying to resolve IPersistTimeouts implementation from the current container. If no implementation is registered, an exception is thrown by IContainer and swallowed by the core.
When Visual Studio is configured to break on all exceptions, exception thrown by the container (UnityObjectBuilder in this case) for .Build(Type) is raised and intercepted by visual studio before TimeoutPersistenceVersionCheck swallows it, leading to a false alert.
It appears an exception is used to communicate between container and core for a presence of IPersistTimeouts implementation. Perhaps it should be done via TryBuild() that doesn't throw an exception, but rather returns a boolean to indicate success or a failure. Alternatively, having an Exists(Type).
@Particular/nservicebus-maintainers fyi
SeanFeldman
All 9 comments
Currently I don't see a way around that exceptions since we need to check for it's existence and the IBuilder interface doesn't have an HasComponent method. Since that exception is handled in the core, I wouldn't consider this as a bug. Users can safely ignore this exception.
We had other scenarios on exceptions when breaking on all exceptions in vs, and while I agree that we should avoid using exceptions as much as possible, when using this options you could as well break on handled framework or library exceptions which shouldn't bother the user.
My vote goes for closing this issue and not tagging it as a bug.
Note that this check is no longer present in v6.
timbussmann
on 19 May 2016
If addressed in v6, closing is ok. We just need to avoid communicating things internally via exceptions :)
SeanFeldman
on 19 May 2016
If addressed in v6, closing is ok. We just need to avoid communicating things internally via exceptions :)
this was related to a critical bug fix and to make sure users use valid combinations of core + persitence packages. A necessary evil. Closing this then :+1:
timbussmann
on 19 May 2016
I agree with @timbussmann even adding something like a TryBuild would be overkill and not worth the effort. Let's close this
danielmarbach
on 19 May 2016
In NH we use builder.BuildAll<SchemaUpdater>().FirstOrDefault()
andreasohlund
on 19 May 2016
Let me change it for you but it will still work
On Thursday, May 19, 2016, Andreas Öhlund [email protected] wrote:
In NH we use builder.BuildAll
().FirstOrDefault() —
You are receiving this because you are on a team that was mentioned.
Reply to this email directly or view it on GitHub
https://github.com/Particular/NServiceBus/issues/3750#issuecomment-220298557
danielmarbach
on 19 May 2016
@andreasohlund do you think this is worth a patch release for the core?
timbussmann
on 19 May 2016
No
andreasohlund
on 19 May 2016
if it is fixed in 6 i dont think it is worth patching
SimonCropp
on 19 May 2016
Related issues
SimonCropp
·
6Comments
andreasohlund
·
9Comments
timbussmann
·
10Comments
marinkobabic
·
8Comments
timbussmann
·
7Comments
Most helpful comment
if it is fixed in 6 i dont think it is worth patching