AssemblyScanner cannot find concrete serialization types from unobtrusive assembly about nservicebus HOT 14 OPEN

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.

from nservicebus.

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.

from nservicebus.

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.

from nservicebus.

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.

from nservicebus.

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

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

from nservicebus.

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

from nservicebus.

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.

from nservicebus.

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

from nservicebus.

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

from nservicebus.

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

from nservicebus.

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.

from nservicebus.

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.

from nservicebus.

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.

from nservicebus.

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

from nservicebus.