AssemblyScanner cannot find concrete serialization types from unobtrusive assembly #6967
Comments
|
Hi @evt-jonny We don't believe this is caused by assembly scanning. The receiver is able to handle the message as long as it has access to the message assembly containing the concrete message type. I pushed a sample showing this to mikeminutillo/SupportRepro#6 (the concrete message type is not picked up by the assembly scanner as the Messages assembly has no direct or indirect reference to NServiceBus) This works because the If the concrete message type cannot be loaded, NServiceBus will generate a proxy for the interface type found in the class IComplexContract__impl : IComplexContract
{
public IComplexChild Child { get; }
}When the deserializer gets the message body, it will look like this: {
"Child": { "IsComplex": true }
}The serializer has no way of knowing how to deserialize that payload into the target type. This could work if the serializer was configured to emit type annotations. |
|
You're right that it will try to find the type dynamically, but this requires an exact assembly qualified name match. In our case, the contract assembly is a project reference and does not implicitly have a file version at all, but even if it did doesn't this make it impossible to send messages using any other assembly version than what the production handler code is currently using? If the contract package is updated but does not break old message types sent by external systems, we have not seen any need up until now to immediately upgrade those systems. But if we are relying on the assembly name matching, we would have to update every service sending any type from the unobtrusive contracts package every time that package changed, and then coordinate the release of every system together. Perhaps it's possible to use binding redirects or something (I'm not sure if those work in reverse like they would need to with |
@evt-jonny we have guidance about how to version contract assemblies https://docs.particular.net/nservicebus/messaging/sharing-contracts#versioning
To extend what @mikeminutillo said we also have this documentation page that further describes the message type detection https://docs.particular.net/nservicebus/messaging/message-type-detection which explains that ultimately it will fall back to load the type by fullname. |
The documentation accurately says this (emphasis mine):
Our issue is that there is no loaded/registered type that matches the type name, because the types from the containing assembly are not loaded because they do not reference In the above code snippet:
|
|
@evt-jonny It would be helpful if you could provide a repo that has a sample that demonstrates your scenario. That would make it easier for all of us to better understand the problem and determine what we could do to address it. |
|
I can provide that, but it will take me a bit. In the meantime, I believe #6968 is a better solution to our issue than last-minute type loading from the fully qualified name, and I would love to get feedback on that. As @danielmarbach points out, we could also work around our issue by having stable assembly versions, but the cat is pretty much out of the bag for us since we have production systems referencing contracts packages that don't do this. |
|
@bording @danielmarbach @mikeminutillo Jonny is needing to step away to take care of personal business and I am going to cover things on our end until he returns. Here is a repro of what we are seeing: I believe what Jonny was thinking based on stepping through the code that the AssemblyScanner is where we see this behavior change. For his PR #6968, I created a PR into his repo to add tests to cover what his expected behavior change would be: We have around 12 services using NSB v7 that we are in the process of moving to NSB v8 (at the same time Az Fns --> ASP.NET Core, Az Fn --> AKS, and switching to unobtrusive mode). Needless to say, we like to make large increments of change all in one go. So, we have a subset of services running in NSB v8, now with unobtrusive mode and trying to move one more over and that's where we are now with this discussion. The service we are migrating now is the subscriber of the event in question and owns/publishes the contract to our internal NuGet feed. There's a number of ways to work around the behavior difference. We didn't follow the best practice of keeping Assembly version at a fixed 1.0.0, which we could work towards. We were also considering dropping all assembly info from all of our |
|
I have been able to reproduce this problem exactly. If the concrete type is picked up by the assembly scanner, then it gets added to the Message Metadata Registry and NServiceBus can infer the type at runtime based on type name only (i.e. the assembly details can be different). If it is not picked up by the assembly scanner, then NServiceBus cannot infer the type and falls back on the generated interface proxy (which the serializer cannot deserialize into). With unobtrusive messages, there is no need for a reference to NServiceBus so the assembly scanner will not scan the message types. If you add a reference to NServiceBus, but do not use it, the compiler optimizes it out. So to make this work we would need a way to tell the message metadata registry about the message types that aren't picked up by assembly scanning. Or we need a way of including an assembly that does not reference NServiceBus. |
|
@bbrandt One thing you may be able to do in the short-term is to create an NServiceBus behavior to rewrite the class FixEnclosedMessageTypesBehavior : Behavior<IIncomingPhysicalMessageContext>
{
readonly Dictionary<string, string> typesToFix;
readonly ConcurrentDictionary<string, string> transformedEnclosedMessageTypes = new();
public FixEnclosedMessageTypesBehavior(Dictionary<string, string> typesToFix)
{
this.typesToFix = typesToFix;
}
public override Task Invoke(IIncomingPhysicalMessageContext context, Func<Task> next)
{
if (context.Message.Headers.TryGetValue(Headers.EnclosedMessageTypes, out var enclosedMessageTypes))
{
var transformedHeader = transformedEnclosedMessageTypes.GetOrAdd(
enclosedMessageTypes,
static (key, types) =>
{
var messageTypes = key.Split(';');
for (var i = 0; i < messageTypes.Length; i++)
{
var messageType = messageTypes[i].Split(',')[0];
if(types.TryGetValue(messageType, out var fixedType))
{
messageTypes[i] = fixedType;
}
}
return string.Join(";", messageTypes);
}, typesToFix);
context.Message.Headers[Headers.EnclosedMessageTypes] = transformedHeader;
}
return next();
}
}
class FixEnclosedMessageTypesFeature : Feature
{
protected override void Setup(FeatureConfigurationContext context)
{
var config = context.Settings.Get<FixEnclosedMessageTypesConfiguration>();
context.Pipeline.Register(
new FixEnclosedMessageTypesBehavior(config.types),
"Fixes enclosed message types header for types which have moved"
);
}
}
public class FixEnclosedMessageTypesConfiguration
{
internal readonly Dictionary<string, string> types = [];
public void Add(Type type) => types.Add(
type.FullName ?? throw new Exception($"{type.Name} does not have a valid FullName"),
type.AssemblyQualifiedName ?? throw new Exception($"{type.Name} does not have a valid AssemblyQualifiedName")
);
}
public static class FixEnclosedMessageTypesConfigurationExtensions
{
public static FixEnclosedMessageTypesConfiguration FixEnclosedMessageTypes(
this EndpointConfiguration endpointConfiguration
)
{
endpointConfiguration.EnableFeature<FixEnclosedMessageTypesFeature>();
return endpointConfiguration.GetSettings().GetOrCreate<FixEnclosedMessageTypesConfiguration>();
}
}And to use it: endpointConfig.FixEnclosedMessageTypes().Add(typeof(ConcreteContract));This is going to intercept the incoming message and rewrite the enclosed message types header for types who's name match to the concrete type configured. That way when NServiceBus tries to figure out the type and load it at runtime, it will work. |
|
Thanks, @mikeminutillo. I've worked this code into my repro and wrote some tests to validate the behavior. Thanks for your help. Repro Commit Stream
|
|
We are discussing internally how to handle this situation. This is caused by a very specific combination of features, specifically you need to be using:
Please let me know if the workaround provided resolves the issue in this specific case. |
|
Thanks Mike, The links to the repo are just there to demonstrate that your workaround works for my repro case as well. |
|
Responding to @mikeminutillo's comment on my PR:
@mikeminutillo totally agree, and I expressed this same concern in my original issue post. That's also why I made this an opt-in setting in the PR. Another solution that might be better would be to do something like Where Then the NSB implementer could override that. This would work perfectly for our use case, because the So if we could inject an AssemblyConvention, it would basically be the same regex, less the specifics around Events vs Commands vs Messages. |
|
@evt-jonny @bbrandt given that the workaround has you unstuck, we have accepted this issue into our backlog and will figure out how to resolve it in a future release. Thank you both for your in-depth analysis and pull requests. |
Describe the bug
Description
AssemblyScanner only adds types from assemblies that reference NServiceBus.Core either explicitly or via other referenced assemblies:
NServiceBus/src/NServiceBus.Core/Hosting/Helpers/AssemblyScanner.cs
Lines 222 to 239 in f66e5f7
This means, as far as I can tell, that an assembly containing message contracts but using unobtrusive mode (and therefore not referencing
NServiceBus.Core) will not be scanned and types from it will never be added to theMessageMetadaRegistryeven if the customIMessageConventiondefines those types as message. The only types from these assemblies that will be added are those that are handled by anIHandleMessages<>(which automatically belongs to a scanned assembly by virtue of referencingNServiceBus.Corein order to implementIHandleMessages<>).In the case that the handled message contracts from the unobtrusive assembly are simple interfaces, this is perhaps fine because concrete types can be created dynamically in MessageMapper.cs, but any interface that contains--as a property--another interface will not be deserializable.
This is true even when the
NServiceBus.EnclosedMessageTypesexplicitly defines the concrete/implementation type and when that concrete type lives in the same assembly as the interface:https://github.com/Particular/NServiceBus/blob/f66e5f74ee49084ae8c74c91890e41d2dce7ecec/src/NServiceBus.Core/Pipeline/Incoming/DeserializeMessageConnector.cs#L70C9-L103C10
Expected behavior
There needs to be some way for a user to ensure that unobtrusive assemblies are scanned for concrete types, especially if the concrete types are in the same assembly as the handled type.
Actual behavior
AssemblyScannerdoes not scan the unobtrusive assembly and does not find these types, and therefore it is impossible to send messages that are handled by a complex interface contract type.Versions
8.1.6
SystemJsonSerializerfor sure, although I suspect it would also apply toNewtonsoftJsonSerializer.Steps to reproduce
ConcreteContractis in theNServiceBus.EnclosedMessageTypes, theAssemblyScannerwill not scan the unobtrusive assembly and will therefore never registerConcreteChild. While a dynamic type will be created to deserialize theIComplexContract, the same will not be down forIComplexChildandMessageDeserializationExceptionwill be thrown saying it is unable to deserialize$.Child.Relevant log output
Additional Information
Workarounds
Perhaps adding a concrete type for the complex interface to an assembly that references
NServiceBus.Coreto ensure it available to theDeserializeMessageConnector?Possible solutions
My janky solution just to prove the point was to add a delegate predicate to
AssemblyScannerConfigurationto tell it the assembly was an unobtrusive package that should for sure be scanned:And then passing this delegate through to the
AssemblyScanner.ScanAssembly():A nicer solution that is essentially the same would be to add something in the same vein as
IMessageConventionrather than a raw delegate.But the better solution, IMHO, would be to automatically scan assemblies that contain any types defined by the
IMessageConventionto be messages. A naive solution to this would add a lot of overhead in scanning all the types from all those assemblies, but there are already some inefficiencies there due to loading every referenced assembly just to tell whether it's referencingNServiceBus.CoreNServiceBus/src/NServiceBus.Core/Hosting/Helpers/AssemblyScanner.cs
Lines 227 to 239 in f66e5f7
Either that, or expose the
AssemblyScanningComponent.Configuration.UserProvidedTypesoutside of acceptance tests and let the user do the assembly scanning themselves.Additional information
The text was updated successfully, but these errors were encountered: