AssemblyScanner cannot find concrete serialization types from unobtrusive assembly

[evt-jonny]

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

	if (assembly.GetName().Name == CoreAssemblyName)
	{
	return processed[assembly.FullName] = true;
	}
	if (ShouldScanDependencies(assembly))
	{
	foreach (var referencedAssemblyName in assembly.GetReferencedAssemblies())
	{
	var referencedAssembly = GetReferencedAssembly(referencedAssemblyName);
	var referencesCore = ScanAssembly(referencedAssembly, processed);
	if (referencesCore)
	{
	processed[assembly.FullName] = true;
	break;
	}
	}
	}

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 the MessageMetadaRegistry even if the custom IMessageConvention defines those types as message. The only types from these assemblies that will be added are those that are handled by an IHandleMessages<> (which automatically belongs to a scanned assembly by virtue of referencing NServiceBus.Core in order to implement IHandleMessages<>).

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.EnclosedMessageTypes explicitly 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

AssemblyScanner does 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
SystemJsonSerializer for sure, although I suspect it would also apply to NewtonsoftJsonSerializer.

Steps to reproduce

1. Create an unobtrusive contracts assembly
2. Add a complex interface contract type, such as:

public interface IComplexContract
{
    IComplexChild Child { get; }
}

public interface IComplexChild
{
    bool IsComplex { get; }
}

3. Add concrete implementations for your contract:

public class ConcreteContract : IComplexContract
{
    public ConcreteChild Child { get; set; }
    public IComplexContract.Child => Child;
}

public class ConcreteChild : IComplexChild
{
    public bool IsComplex => true;
}

4. Add a handler for IConcreteContract:

public class ComplexContractHandler : IHandleMessages<IComplexContract>
{
    public Task Handle(IComplexContract message)
    {
        // unreachable
        return Task.CompletedTask;
    }
}

5. Send an IComplexContract message. Even if ConcreteContract is in the NServiceBus.EnclosedMessageTypes, the AssemblyScanner will not scan the unobtrusive assembly and will therefore never register ConcreteChild. While a dynamic type will be created to deserialize the IComplexContract, the same will not be down for IComplexChild and MessageDeserializationException will be thrown saying it is unable to deserialize $.Child.

Relevant log output

NServiceBus.MessageDeserializationException: An error occurred while attempting to extract logical messages from incoming physical message {GUID}
 ---> System.NotSupportedException: Deserialization of interface types is not supported. Type 'IComplexChild'. Path: $.Child | LineNumber: 0 | BytePositionInLine: ##.
 ---> System.NotSupportedException: Deserialization of interface types is not supported. Type 'IComplexChild'.
   --- End of inner exception stack trace ---
   at System.Text.Json.ThrowHelper.ThrowNotSupportedException(ReadStack& state, Utf8JsonReader& reader, NotSupportedException ex)
   at System.Text.Json.ThrowHelper.ThrowNotSupportedException_DeserializeNoConstructor(Type type, Utf8JsonReader& reader, ReadStack& state)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo`1.ReadJsonAndSetMember(Object obj, ReadStack& state, Utf8JsonReader& reader)
   at System.Text.Json.Serialization.Converters.ObjectDefaultConverter`1.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCore(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.Serialization.JsonConverter`1.ReadCoreAsObject(Utf8JsonReader& reader, JsonSerializerOptions options, ReadStack& state)
   at System.Text.Json.JsonSerializer.ReadCore[TValue](Utf8JsonReader& reader, JsonTypeInfo jsonTypeInfo, ReadStack& state)
   at System.Text.Json.JsonSerializer.ContinueDeserialize[TValue](ReadBufferState& bufferState, JsonReaderState& jsonReaderState, ReadStack& readStack, JsonTypeInfo jsonTypeInfo)
   at System.Text.Json.JsonSerializer.ReadFromStream[TValue](Stream utf8Json, JsonTypeInfo jsonTypeInfo)
   at System.Text.Json.JsonSerializer.Deserialize(Stream utf8Json, Type returnType, JsonSerializerOptions options)
   at NServiceBus.Serializers.SystemJson.JsonMessageSerializer.Deserialize(ReadOnlyMemory`1 body, Type type) in /_/src/NServiceBus.Core/Serializers/SystemJson/JsonMessageSerializer.cs:line 53
   at NServiceBus.Serializers.SystemJson.JsonMessageSerializer.Deserialize(ReadOnlyMemory`1 body, IList`1 messageTypes) in /_/src/NServiceBus.Core/Serializers/SystemJson/JsonMessageSerializer.cs:line 41
   at NServiceBus.DeserializeMessageConnector.Extract(IncomingMessage physicalMessage) in /_/src/NServiceBus.Core/Pipeline/Incoming/DeserializeMessageConnector.cs:line 112
   at NServiceBus.DeserializeMessageConnector.ExtractWithExceptionHandling(IncomingMessage message) in /_/src/NServiceBus.Core/Pipeline/Incoming/DeserializeMessageConnector.cs:line 46
   --- End of inner exception stack trace ---
   at NServiceBus.DeserializeMessageConnector.ExtractWithExceptionHandling(IncomingMessage message) in /_/src/NServiceBus.Core/Pipeline/Incoming/DeserializeMessageConnector.cs:line 50
   at NServiceBus.DeserializeMessageConnector.Invoke(IIncomingPhysicalMessageContext context, Func`2 stage) in /_/src/NServiceBus.Core/Pipeline/Incoming/DeserializeMessageConnector.cs:line 28
   at NServiceBus.ProcessingStatisticsBehavior.Invoke(IIncomingPhysicalMessageContext context, Func`2 next) in /_/src/NServiceBus.Core/Performance/Statistics/ProcessingStatisticsBehavior.cs:line 25
   at NServiceBus.ReceiveDiagnosticsBehavior.Invoke(IIncomingPhysicalMessageContext context, Func`2 next) in /_/src/NServiceBus.Core/OpenTelemetry/Metrics/ReceiveDiagnosticsBehavior.cs:line 33
   at NServiceBus.TransportReceiveToPhysicalMessageConnector.Invoke(ITransportReceiveContext context, Func`2 next) in /_/src/NServiceBus.Core/Pipeline/Incoming/TransportReceiveToPhysicalMessageConnector.cs:line 35
   at NServiceBus.RetryAcknowledgementBehavior.Invoke(ITransportReceiveContext context, Func`2 next) in /_/src/NServiceBus.Core/ServicePlatform/Retries/RetryAcknowledgementBehavior.cs:line 25
   at NServiceBus.MainPipelineExecutor.Invoke(MessageContext messageContext, CancellationToken cancellationToken) in /_/src/NServiceBus.Core/Pipeline/MainPipelineExecutor.cs:line 49
   at NServiceBus.MainPipelineExecutor.Invoke(MessageContext messageContext, CancellationToken cancellationToken) in /_/src/NServiceBus.Core/Pipeline/MainPipelineExecutor.cs:line 68
   at NServiceBus.Transport.AzureServiceBus.MessagePump.ProcessMessage(ServiceBusReceivedMessage message, ProcessMessageEventArgs processMessageEventArgs, String messageId, Dictionary`2 headers, BinaryData body, CancellationToken messageProcessingCancellationToken) in /_/src/Transport/Receiving/MessagePump.cs:line 285

Additional Information

Workarounds

Perhaps adding a concrete type for the complex interface to an assembly that references NServiceBus.Core to ensure it available to the DeserializeMessageConnector ?

Possible solutions

My janky solution just to prove the point was to add a delegate predicate to AssemblyScannerConfiguration to tell it the assembly was an unobtrusive package that should for sure be scanned:

...
        /// <summary>
        /// Defines an additional path for assembly scanning.
        /// </summary>
        public string? AdditionalAssemblyScanningPath { get; set; }

        /// <summary>
        /// Register assembly types based on a predicate.
        /// </summary>
        public Func<string, bool>? AssemblyContainsMessages { get; set; }

        /// <summary>
        /// A list of <see cref="Assembly" />s to ignore in the assembly scanning.
        /// </summary>
        /// <param name="assemblies">The file name of the assembly.</param>
        public void ExcludeAssemblies(params string[] assemblies)
        {
...

And then passing this delegate through to the AssemblyScanner.ScanAssembly():

...
            processed[assembly.FullName] = false;

            var assemblyName = assembly.GetName().Name;
            if (assemblyName == CoreAssemblyName
                || AssemblyContainsMessages?.Invoke(assemblyName) == true)
            {
                return processed[assembly.FullName] = true;
            }

            if (ShouldScanDependencies(assembly))
            {
...

A nicer solution that is essentially the same would be to add something in the same vein as IMessageConvention rather than a raw delegate.

But the better solution, IMHO, would be to automatically scan assemblies that contain any types defined by the IMessageConvention to 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 referencing NServiceBus.Core

NServiceBus/src/NServiceBus.Core/Hosting/Helpers/AssemblyScanner.cs

Lines 227 to 239 in f66e5f7

	if (ShouldScanDependencies(assembly))
	{
	foreach (var referencedAssemblyName in assembly.GetReferencedAssemblies())
	{
	var referencedAssembly = GetReferencedAssembly(referencedAssemblyName);
	var referencesCore = ScanAssembly(referencedAssembly, processed);
	if (referencesCore)
	{
	processed[assembly.FullName] = true;
	break;
	}
	}
	}

Either that, or expose the AssemblyScanningComponent.Configuration.UserProvidedTypes outside of acceptance tests and let the user do the assembly scanning themselves.

Additional information

[mikeminutillo]

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 EnclosedMessageTypes header contains the concrete message type name, and NServiceBus uses Type.GetType to create the instance of the message type to be deserialized.

If the concrete message type cannot be loaded, NServiceBus will generate a proxy for the interface type found in the EnclosedMessageTypes header. That is handled by the ConcreteProxyCreator. In the example you have given, it would look something like this:

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.

[evt-jonny]

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 Type.GetType()) but this still seems like an extremely brittle fallback solution, especially when I am already loading types from the assembly that contains the concrete types I need.

[danielmarbach]

In our case, the contract assembly is a project reference and does not implicitly have a file version at all

@evt-jonny we have guidance about how to version contract assemblies

https://docs.particular.net/nservicebus/messaging/sharing-contracts#versioning

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?

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.

[evt-jonny]

@danielmarbach

that ultimately it will fall back to load the type by fullname.

The documentation accurately says this (emphasis mine):

In cases when the assembly qualified type is not known by the endpoint, NServiceBus will fall back to any loaded type that matches the contained FullName, even when the type resides in a different assembly.

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 NServiceBus.Core. AssemblyScanner.ScanAssembly() returns false for the relevant assembly and therefore those types are not available to the MessageMetadataRegistry to match to the type in the NServiceBus.EnclosedMessageTypes:

NServiceBus/src/NServiceBus.Core/Unicast/Messages/MessageMetadataRegistry.cs

Lines 56 to 90 in 2a5e715

	public MessageMetadata GetMessageMetadata(string messageTypeIdentifier)
	{
	Guard.ThrowIfNullOrEmpty(messageTypeIdentifier);
	var cacheHit = cachedTypes.TryGetValue(messageTypeIdentifier, out var messageType);
	if (!cacheHit)
	{
	messageType = GetType(messageTypeIdentifier);
	if (messageType == null)
	{
	foreach (var item in messages.Values)
	{
	var messageTypeFullName = GetMessageTypeNameWithoutAssembly(messageTypeIdentifier);
	if (item.MessageType.FullName == messageTypeIdentifier ||
	item.MessageType.FullName == messageTypeFullName)
	{
	Logger.DebugFormat("Message type: '{0}' was mapped to '{1}'", messageTypeIdentifier, item.MessageType.AssemblyQualifiedName);
	cachedTypes[messageTypeIdentifier] = item.MessageType;
	return item;
	}
	}
	Logger.DebugFormat("Message type: '{0}' No match on known messages", messageTypeIdentifier);
	}
	cachedTypes[messageTypeIdentifier] = messageType;
	}
	if (messageType == null)
	{
	return null;
	}

In the above code snippet:

• on line 60, cachedTypes only contains types from scanned assemblies that reference NServiceBus.Core, so does not include our type
• on line 64, we cannot find the fully qualified type based on the version as already discussed
• on line 68, we are only looking through the already-cached MessageMetadata, which again only includes types from assemblies that reference NServiceBus.Core
• so we hit line 87 and return null for the concrete type

[bording]

@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.

[evt-jonny]

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.

[bbrandt]

@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:
mikeminutillo/SupportRepro#7

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:
evt-jonny#1

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 EnclosedMessageTypes headers, to switch at some point from SQL Filters to Correlation Filters in ASB, but all these are going to take us some time to work out, while coordinating our multiple teams.

[mikeminutillo]

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.

[mikeminutillo]

@bbrandt One thing you may be able to do in the short-term is to create an NServiceBus behavior to rewrite the EnclosedMessageTypesHeader to something that can be loaded using Type.GetType(..). This is probably best done on the receiver, if you control it. Something like this:

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.

[bbrandt]

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

1. Started with a copy of:
   https://github.com/Eventellect/docs.particular.net/tree/master/samples/versioning/Core_8

2. Add Complex Child interface and class - both subscribers are able to consume the event even though it does not match the version

3. Switched to Unobtrusive mode - No other changes. It no longer works.

NServiceBus.MessageDeserializationException: An error occurred while attempting to extract logical messages from incoming physical message 9ec5500e-fc24-4922-a255-b12c003c2c0b
---> System.NotSupportedException: Deserialization of interface types is not supported. Type 'Contracts.IComplexChild'. Path: $.Child | LineNumber: 0 | BytePositionInLine: 23.

4. Workaround the behavior change of unobtrusive messages using EnclosedMessageTypesMappingBehavior

[mikeminutillo]

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:

1. Unobtrusive message conventions
2. Interface message contracts with non-concrete properties
3. A serializer which does not emit type annotations
4. Different versions of the concrete contract assembly

• If the interface message contract has purely concrete properties, we will generate a proxy for it at runtime that the serializer deserialize into
• If the configured serializer emits type annotations then the deserializer on the receiving side can figure out the type to deserialize into
• If both sender and receiver use the same version of the concrete contract assembly then the deserializer can rely on the EnclosedMessageTypes header as Type.GetType(..) will work

Please let me know if the workaround provided resolves the issue in this specific case.

[bbrandt]

Thanks Mike,
I had intended my last message to communicate that the software team applied your workaround and is working great.

The links to the repo are just there to demonstrate that your workaround works for my repro case as well.

[evt-jonny]

Responding to @mikeminutillo's comment on my PR:

This change has the potential to slow down endpoint creation significantly as we would need to perform type-scanning on all assemblies.

@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 IMessageConventions, maybe something like:

public interface IAssemblyConvention
{
    bool ShouldScanTypesForAssembly(Assembly assembly);
}

Where ShouldCanTypesForAssembly() replaces the code in AssemblyScanner that checks whether the Assembly is NServiceBus.Core. Then the default implementation would just be:

public class AssemblyConvention : IAssemblyConvention
{
    public bool ShouldScanTypesForAssembly(Assembly assembly) => AssemblyIsNServiceBusCore(assembly);
}

Then the NSB implementer could override that. This would work perfectly for our use case, because the MessageConventions we use are literally just string matches against the namespace, i.e. IsEvent(Type type) => Regex.IsMatch(type.FullName, "^Eventellect\.([^.]+\.)\.Contracts\.Events").

So if we could inject an AssemblyConvention, it would basically be the same regex, less the specifics around Events vs Commands vs Messages.