Using MapDiscoveredCommands with multiple aggregates results in error 'No handler found for command Object '

[iappwebdev]

Describe the bug
In a ASP NET Web Application, app.MapDiscoveredCommands() is not working as expected. All correctly registered commands are found but all are failing on execution with error

{
  "type": "CommandHandlerNotFound`1",
  "title": "No handler found for command Object",
  "status": 500,
  "detail": "Eventuous.Exceptions+CommandHandlerNotFound`1[System.Object]: No handler found for command Object"
}

To Reproduce
See attached solution TestEventuous.zip with the following structure:

The solution contains Api, Application and Domain projects with commands for Car (StartEngine) and Train (OpenDoors) and the corresponding CommandServices.

[AggregateCommands<Car>]
public static class CarCommands
{
    [HttpCommand(Route = "car/start-engine")]
    public sealed record StartEngine
    {
        public required string CarId { get; init; }
    }
}

[AggregateCommands<Train>]
public static class TrainCommands
{
    [HttpCommand(Route = "train/open-doors")]
    public sealed record OpenDoors
    {
        public required string TrainId { get; init; }
    }
}

public sealed class CarService : CommandService<Car, CarState, CarId>
{
    public CarService(IAggregateStore? store, AggregateFactoryRegistry? factoryRegistry = null, StreamNameMap? streamNameMap = null, TypeMapper? typeMap = null)
        : base(store, factoryRegistry, streamNameMap, typeMap)
    {
        On<CarCommands.StartEngine>()
            .GetId(_ => new CarId(Guid.NewGuid().ToString()))
            .Act((car, _) => car.StartEngine());
    }
}

public sealed class TrainService : CommandService<Train, TrainState, TrainId>
{
    public TrainService(IAggregateStore? store, AggregateFactoryRegistry? factoryRegistry = null, StreamNameMap? streamNameMap = null, TypeMapper? typeMap = null)
        : base(store, factoryRegistry, streamNameMap, typeMap)
    {
        On<TrainCommands.OpenDoors>()
            .GetId(_ => new TrainId(Guid.NewGuid().ToString()))
            .Act((Train, _) => Train.OpenDoors());
    }
}

Running the application displays swagger with two endpoint car/start-engine and train/open-doors. Executing those endpoints yields the error described.

When changing
app.MapDiscoveredCommands(typeof(ApplicationModule).Assembly);
to
app.MapDiscoveredCommands<Car>(typeof(ApplicationModule).Assembly);
then car command is working as expected (but not train, but that is wanted, as described in the docs)

Expected behavior
All discovered commands should reach their corresponding handler.

Screenshots

[alexeyzimarev]

Hm, it should not be mapping object, it must have a proper type. I will try to reproduce it.

[iappwebdev]

Hm, it should not be mapping object, it must have a proper type. I will try to reproduce it.

I could reproduce it and can give the fix to this, I'll create a pull request.

The main issue is still there, registered and recognised handlers are not found on execution.

[alexeyzimarev]

Maybe we can start by adding a test with this scenario, so the issue would be clearly illustrated.

[iappwebdev]

Maybe we can start by adding a test with this scenario, so the issue would be clearly illustrated.

I just did this today and found the problem. In file HttpCommandMapping.cs: when using app.MapDiscoveredCommands<TAggregate>(), the handlers are registered by using a generic made version of Map:

void MapAssemblyCommands(Assembly assembly) {
    var decoratedTypes = assembly.DefinedTypes.Where(
        x => x.IsClass && x.CustomAttributes.Any(a => a.AttributeType == attributeType)
    );

    var method = typeof(RouteBuilderExtensions).GetMethod(nameof(Map), BindingFlags.Static | BindingFlags.NonPublic)!;

    foreach (var type in decoratedTypes) {
        var attr = type.GetAttribute<HttpCommandAttribute>()!;

        if (attr.AggregateType != null && attr.AggregateType != typeof(TAggregate))
            throw new InvalidOperationException(
                $"Command aggregate is {attr.AggregateType.Name} but expected to be {typeof(TAggregate).Name}"
            );

        var genericMethod = method.MakeGenericMethod(typeof(TAggregate), type, type);
        genericMethod.Invoke(null, new object?[] { builder, attr.Route, null, attr.PolicyName });
    }
}

When calling app.MapDiscoveredCommands(), handlers are registered by a non generic version

void LocalMap(Type aggregateType, Type type, string? route, string? policyName) {
    var appServiceBase = typeof(ICommandService<>);
    var appServiceType = appServiceBase.MakeGenericType(aggregateType);

    var routeBuilder = builder
        .MapPost(
            GetRoute(type, route),
            async Task<IResult> (HttpContext context) => {
                var cmd = await context.Request.ReadFromJsonAsync(type, context.RequestAborted);

                if (cmd == null) throw new InvalidOperationException("Failed to deserialize the command");

                if (context.RequestServices.GetRequiredService(appServiceType) is not ICommandService
                    service) throw new InvalidOperationException("Unable to resolve the application service");

                var result = await service.Handle(cmd, context.RequestAborted);

                return result.AsResult();
            }
        )

Here, deeper at await service.Handle(cmd, context.RequestAborted); the generic CommandType information gets lost.

---

Testing

I introduced a new (duplicated) command for BookRoom with a wrapper class annotated with AggregateCommands in BookService:

[AggregateCommands<Booking>]
static class NestedCommands {
    [HttpCommand(Route = "nested-book")]
    internal record NestedBookRoom(string BookingId, string RoomId, LocalDate CheckIn, LocalDate CheckOut, float Price, string? GuestId);
}

Then I extended HttpCommandTests to test MapDiscoveredCommands()

[Fact]
public void RegisterAggregatesCommands() {
    var builder = WebApplication.CreateBuilder();

    using var app = builder.Build();

    var b = app.MapDiscoveredCommands(typeof(NestedCommands).Assembly);

    b.DataSources.First().Endpoints[0].DisplayName.Should().Be("HTTP: POST nested-book");
}

The test passed so registering commands using MapDiscoveredCommands() works correctly.

---

Next I introducded a test for executing the registered command:

[Fact]
public async Task MapDiscoveredCommand() {
    using var fixture = new ServerFixture(
        output,
        _ => { },
        app => app.MapDiscoveredCommands(typeof(NestedCommands).Assembly)
    );

    using var client = fixture.GetClient();

    var cmd = fixture.GetNestedBookRoom();

    var response = await client.PostJsonAsync("/nested-book", cmd);
    response.Should().Be(HttpStatusCode.OK);
}

The test failed with eventuous.application - EventId: [1], EventName: [CommandHandlerNotFound], Message: [Handler not found for command: 'NestedBookRoom']

Possible solution 1

I took a close look on how handlers are registered and how they are found. In CommandService, handlers are registered by AddHandlerUntyped()

void BuildHandlers() {
    lock (_handlersLock) {
        foreach (var commandType in _builders.Keys) {
            var builder = _builders[commandType];
            var handler = builder.Build();
            _handlers.AddHandlerUntyped(commandType, handler);
        }

        _initialized = true;
    }
}

Command are then searched by using

public async Task<Result<TState>> Handle<TCommand>(TCommand command, CancellationToken cancellationToken) where TCommand : class {
    if (!_initialized) BuildHandlers();

    if (!_handlers.TryGet<TCommand>(out var registeredHandler)) {
        Log.CommandHandlerNotFound<TCommand>();
...

As we saw before, the type information gets lost and TCommand is of type object when using app.MapDiscoveredCommands(). So I extended CommandHandlersMap by an additional method TryGetUntyped():

static readonly MethodInfo TryGetInternalMethod =
    typeof(HandlersMap<TAggregate, TId>).GetMethod(nameof(TryGetInternal), BindingFlags.NonPublic | BindingFlags.Instance)!;

...

internal bool TryGetUntyped(Type command, [NotNullWhen(true)] out RegisteredHandler<TAggregate, TId>? handler) {
    var parameters = new object?[] { null };
    bool tryGet = (bool)TryGetInternalMethod.MakeGenericMethod(command).Invoke(this, parameters)!;
    handler = tryGet ? (RegisteredHandler<TAggregate, TId>?)parameters[0] : null;
    return tryGet;

Now the handler was found correctly.

As I looked further in the method I realized that there are further places to adapt:

Log.CommandHandlerNotFound<TCommand>();
...
Log.CommandHandled<TCommand>();
...
Log.ErrorHandlingCommand<TCommand>(e);
...
return new ErrorResult<TState>($"Error handling command {typeof(TCommand).Name}", e);

I introduced the corresponding *Untyped methods in Eventuous.Diagnostics.ApplicationEventSource.

    [NonEvent]
    public void CommandHandlerNotFound<TCommand>() => CommandHandlerNotFound(typeof(TCommand).Name);

    [NonEvent]
    public void CommandHandlerNotFoundUntyped(Type commandType) => CommandHandlerNotFound(commandType.Name);

    [NonEvent]
    public void ErrorHandlingCommand<TCommand>(Exception e) => ErrorHandlingCommand(typeof(TCommand).Name, e.ToString());

    [NonEvent]
    public void ErrorHandlingCommandUntyped(Type commandType, Exception e) => ErrorHandlingCommand(commandType.Name, e.ToString());

    [NonEvent]
    public void CommandHandled<TCommand>() {
        if (IsEnabled(EventLevel.Verbose, EventKeywords.All)) CommandHandled(typeof(TCommand).Name);
    }

    [NonEvent]
    public void CommandHandledUntyped(Type commandType) {
        if (IsEnabled(EventLevel.Verbose, EventKeywords.All)) CommandHandled(commandType.Name);
    }

If all this are acceptable changes, the FunctionalCommandService should be adapted as well. I can create a pull request if you approve my thoughts

Possible solution 2 (?)

I did not succeed to change the LocalMap method such that the command type information gets passed to the Handle method. Maybe you have an idea?

[iappwebdev]

IMHO there should be a better solution instead of creating *Untyped methods for every generic method with TCommand...

[alexeyzimarev]

Basically, the issue has nothing to do with multiple aggregates, or any sort of edge case. Discovered commands mapping got broken when I changed the Command Service to use the generic type map instead of a dictionary. As commands discovery didn't have any tests, I overlooked the consequences as Handle<T> must have the specific T for the type map to work.

Right now, I fixed it by creating a generic call and caching it for consecutive invocations. Ideally, of course, it should be solved by creating a code generator, which would also avoid calling MapDiscoveredCommands(Assembly) as it would just create code to map discovered commands.

[alexeyzimarev]

IMHO there should be a better solution instead of creating *Untyped methods for every generic method with TCommand...

The code doesn't create untyped methods, the AddHandlerUntyped itself has no generic argument, that's why it has such a name.

[alexeyzimarev]

Your test passes now, should we consider the issue resolved?

[alexeyzimarev]

Ok, it should work with the latest alpha package

[iappwebdev]

Thank you for checking my thought and for finding the correct cause of the problem. I will check my app with the latest alpha package and let you know if it worked.