Nservicebus: MessageMapper does not call default constructor

[agross]

I have several commands (classes) that initialize properties using the default ctor (without args). Since upgrading to NSB6, these ctors are no longer called and lead to NullReferenceExceptions downstream when a handler receives the message.

Example command:
```c#
public class SendEmail
{
public SendEmail()
{
Attachments = new List();
}

public IList<string> Attachments { get; }

}

Failing test (NServiceBus.Core.Tests/MessageMapper/MessageMapperTests.cs):

```c#
[Test]
public void Constructor_should_be_called()
{
    var mapper = new MessageMapper();
    var message = mapper.CreateInstance<SomeMessage>();
    Assert.IsNotNull(message.SomeList);
}

public class SomeMessage
{
    public SomeMessage()
    {
        SomeList = new List<string>();
    }

    public IList<string> SomeList { get; }
}

[agross]

I see that the test succeeds when I initialize the mapper with typeof(SomeMessage). For some reason, my NSB service does not do this, i.e. MessageMapper.typeToConstructor does not include SendEmail. I checked my DefiningCommandsAs conventions and the SendEmail type is included.

The debug log shows this:

DEBUG eBus.SerializationFeature - Message definitions:
 NServiceBus.Unicast.Messages.MessageMetadata
NServiceBus.Unicast.Messages.MessageMetadata

[agross]

I added some logging to the message conventions evaluation and it seems that the type scanner runs twice.

.Initializers.Conventions - Found command: Angebot.Contracts.Commands.ErstelleAngebotFürTraining
.Initializers.Conventions - Found command: Angebot.Contracts.Commands.Auditable
.Initializers.Conventions - Found command: Angebot.Contracts.Commands.ErstelleAngebotFürConsulting
iceBus.PersistenceStartup - Activating persistence 'InMemoryPersistence' to provide storage for 'NServiceBus.Persistence.StorageType+Sagas' storage.
iceBus.PersistenceStartup - Activating persistence 'InMemoryPersistence' to provide storage for 'NServiceBus.Persistence.StorageType+Timeouts' storage.
iceBus.PersistenceStartup - Activating persistence 'InMemoryPersistence' to provide storage for 'NServiceBus.Persistence.StorageType+Subscriptions' storage.
iceBus.PersistenceStartup - Activating persistence 'InMemoryPersistence' to provide storage for 'NServiceBus.Persistence.StorageType+Outbox' storage.
iceBus.PersistenceStartup - Activating persistence 'InMemoryPersistence' to provide storage for 'NServiceBus.Persistence.StorageType+GatewayDeduplication' storage.
iceBus.ErrorQueueSettings - Error queue retrieved from code configuration via 'EndpointConfiguration.SendFailedMessagesTo()'.
.Initializers.Conventions - Found command: Ops.Contracts.Commands.ErfasseInTrello
.Initializers.Conventions - Found command: Ops.Contracts.Commands.TrageEmailInMailingListeEin
.Initializers.Conventions - Found command: Ops.Contracts.Commands.SendeEmail
iceBus.ErrorQueueSettings - Error queue retrieved from code configuration via 'EndpointConfiguration.SendFailedMessagesTo()'.
eBus.SerializationFeature - Number of messages found: 2
eBus.SerializationFeature - Message definitions: 
 NServiceBus.Unicast.Messages.MessageMetadata
NServiceBus.Unicast.Messages.MessageMetadata

st.MessageHandlerRegistry - Associated 'Angebot.Contracts.Commands.ErstelleAngebotFürTraining' message with 'Angebot.Features.Training.Handler' handler.
st.MessageHandlerRegistry - Associated 'Angebot.Contracts.Commands.ErstelleAngebotFürConsulting' message with 'Angebot.Features.Consulting.Handler' handler.

The Found command parts are from my code.

The handler that uses SendeEmail looks like this:

public Task Handle(ErstelleAngebotFürConsulting message, IMessageHandlerContext context)
{
  return context.Send<SendeEmail>(m =>
  {
    m.Empfänger = _emailRecipient.Internal;
    ...
  });
}

I set a breakpoint on the m.Empfänger line, m is an uninitialized object at this point in time. I travelled up the call stack into NServiceBus.Core.dll!NServiceBus.MessageInterfaces.MessageMapper.Reflection.MessageMapper.CreateInstance<Ops.Contracts.Commands.SendeEmail>(System.Action<Ops.Contracts.Commands.SendeEmail> action = {Method = {System.Reflection.RuntimeMethodInfo}}) Line 101 and checked this.typesToConstructor. The list only contains types logged in the first pass of the convention scan.

I might add that the host dll does not contain a reference to the Ops.Contracts assembly (which contains SendeEmail). SendeEmail only becomes visible after Angebot.dll is loaded.

Host -> Angebot (handlers) -> Ops.Contracts (SendeEmail)

[timbussmann]

Hey @agross

Thanks a lot for provided details. It definitely looks like a bug which might be caused by a combination of unobtrusive messages and the "missing" reference to the Contracts assembly which causes the message to not be initialized correctly. I need to look a bit closer at this though.

As a workaround, can you new up the message yourself instead of using the mapper to create the instance for you, e.g.:

var message = new SendEmail{
   ...
};
return context.Send(message);

Are all messages you're using (e.g. ErstelleAngebotFürTraining) regular classes or are some of them only interfaces?

[agross]

Hi @timbussmann,

Thanks for the workaround! It would work for the egress SendeEmail (it's a class). Only "ingress" messages are interfaces (these are the first three that are logged) and they won't be affected because interfaces don't have ctors.

Let me know if you need more info, or even access to the git repo.

[agross]

I'd appreciate if you could fix this debug log entry:

DEBUG eBus.SerializationFeature - Message definitions:
 NServiceBus.Unicast.Messages.MessageMetadata
NServiceBus.Unicast.Messages.MessageMetadata

It looks like it just logs NServiceBus.Unicast.Messages.MessageMetadata.ToString().

[timbussmann]

@agross thanks for the hint, will look at the log output.

Just to verify: I assume you configured the conventions to recognize SendEmail as a message type?

Also, you mentioned:

SendeEmail only becomes visible after Angebot.dll is loaded.

Is this just the reference hierarchy or is there some special logic which indeed means that Angebot.dll is not in bin folder when starting the endpoint?

[agross]

I assume you configured the conventions to recognize SendEmail as a message type?

As a command type:

using System;

using NServiceBus;
using NServiceBus.Logging;

namespace Angebot.Host.Initializers
{
  class Conventions : INeedInitialization
  {
    static ILog Logger = LogManager.GetLogger<Conventions>();

    public void Customize(EndpointConfiguration configuration)
    {
      configuration
        .Conventions()
        .DefiningCommandsAs(type => Log(() => !type.IsNested && InNamespace(type, ".Commands"),
                                        type,
                                        "command"))
        .DefiningMessagesAs(type => Log(() => !type.IsNested && InNamespace(type, ".Messages"),
                                        type,
                                        "message"));
    }

    static bool Log(Func<bool> filter, Type type, string category)
    {
      var match = filter();
      if (match)
      {
        Logger.Debug($"Found {category}: {type}");
      }

      return match;
    }

    static bool InNamespace(Type type, string @namespace)
    {
      return type.Namespace != null && type.Namespace.Contains(@namespace);
    }
  }
}

Is this just the reference hierarchy or is there some special logic which indeed means that Angebot.dll is not in bin folder when starting the endpoint?

Just references. The bin folder is completely populated before the host is started.

[agross]

To be more specific about the references:

The Host assembly (defining conventions) only hosts the Angebot assembly (handlers), which in turn uses Ops to send emails (IT/Ops as per Udi's course). So the host only references Angebot. I just checked it, not even this is technically required to be the case (compiles without the reference). As far as I understood NSB uses assembly scanning internally and doesn't rely on references.

("Angebot" translates to "quote" for the non-native speaker :)

[timbussmann]

@agross thanks again for the information 👍

so there is a bug in the CreateInstance I can confirm which causes message to not be instantiated via ctor in case they are not initialized. Most code paths of the message mapper are able to initialize types on-demand, but CreateInstance is not, which I think we should fix. I want to add a note that we usually recommend to use the "workaround" when working with message classes, as the mapper is mostly just overhead (as no proxy classes need to be generated). But this is just a recommendation and doesn't influence the fact, that the behavior is incorrect as it fallbacks to use FormatterServices.GetUninitializedObject (which is why your ctor is not invoked). Tbh, I think this call is entirely wrong in your scenario, but there are some weird edge cases which seem to require that. A PR to fix this will be pushed shortly.

On the other hand, as you correctly noticed, if the mapper is initialized with the type before, it works as expected. Also, given your logs, it looks like SendEmail is indeed recognized as a command by the assembly scanning process, so I'm double checking why the mapper isn't initialized with this type in the first place.

[timbussmann]

@agross regarding the Found command: Ops.Contracts.Commands.SendeEmail log entry, could you disable auto-subscribe endpointConfiguration.DisableFeature<AutoSubscribe>(); and check whether that log entry still appears?

[agross]

@timbussmann AutoSubscribe is already disabled.

[timbussmann]

@agross ah, good to know, thanks. I double checked the assembly scanner and the scanner will indeed not scan your message contract assembly (which should be no issue). The log output from the commands-convention can come from various sources during initialization.
I have a fix available in #4751 and we will discuss whether this will be backported later today. From your perspective, how much pain does this issue cause?

[agross]

I upgraded from 5 to 6, only then did I notice the issue. So I'm not sure if any of you yourself previous supported versions are affected.

Given my codebase is rather small it's not a big issued for me to use the workaround, but I can imagine it would be for bigger projects.

Thank you for providing a fix quickly, looking forward to test the next release!

[timbussmann]

Closing this issue as the bug is fixed in 6.3.2, 6.2.2, 6.1.6 and 6.0.9.

Who's affected

You are affected if all of the following criteria apply:

• You are using conventions to define unobtrusive messages types
• Your message types are located in an assembly which doesn't reference NServiceBus.Core
• Your message type performs initialization in the constructor
• You send message classes using the context.Send<Message>(m => { ... }), context.Publish<Message>(m => { ... }), session.Send<Message>(m => { ... }) or session.Publish<Message>(m => { ... }) APIs

Symptoms

The message objects which are created when sending messages using context.Send<Message>(m => { ... }), context.Publish<Message>(m => { ... }), session.Send<Message>(m => { ... }) or session.Publish<Message>(m => { ... }) may not have had their default constructor called, potentially leaving fields and properties uninitialized or with invalid default values.

@agross thanks a lot for raising this issue an providing so much valuable details to help us track that bug down 👍