6

When using the Mediatr pattern I find quite challenging to return meaningful errors to an API controller. Let's take the OrdersController.CancelOrder method as an example (src).

In this example, they "only" return Ok() and BadRequest(). In that case how will they return errors like "This orderid does not exist" (404) OR "this order has been shipped" (400) (...).

We could introduce a new class called Result holding both the returned values (if any) and potentially error messages. In that case, all your commands, queries should return Result<YourModel>. We could also add the code directly inside the controller. I cannot make up my mind both solutions have pros and cons.

What do you think about that?

Thx Seb

1 Answers1

4

That's exactly how I tend to do it using Mediatr.
Return a wrapper class.

If we take the eShopOnContainers example CancelOrder example, I would have the command, return a CancelOrderCommandResult

public class CancelOrderCommand : IRequest<CancelOrderCommandResult>
{ }

The CancelOrderCommandResult could be something along these lines:

public class CancelOrderCommandResult
{
    public CancelOrderCommandResult(IEnumerable<Error> errors)
    {
        Success = false;
        Errors = errors;
    }

    public CancelOrderCommandResult(bool success)
    {
        Success = success;
    }

    public bool Success {get; set;}

    public IEnumerable<Error> Errors {get; set;}
}

I've omitted the Error class, but it could just be POCO containing the error information, error code etc...

Our handler then becomes

public class CancelOrderCommandHandler : IRequestHandler<CancelOrderCommand, CancelOrderCommandResult>
{
    private readonly IOrderRepository _orderRepository;

    public CancelOrderCommandHandler(IOrderRepository orderRepository)
    {
        _orderRepository = orderRepository;
    }

    public async Task<bool> Handle(CancelOrderCommand command, CancellationToken cancellationToken)
    {
        var orderToUpdate = await _orderRepository.GetAsync(command.OrderNumber);

        if(orderToUpdate == null)
        {
            return false;
        }

        try 
        {
            orderToUpdate.SetCancelledStatus();
            await _orderRepository.UnitOfWork.SaveEntitiesAsync();

            //iff success, return true
            return new CancelOrderCommandResult(true);
        }
        catch (Exception ex)
        {
            var errors = MapErrorsFromException(ex);
            return new CancelOrderCommandResult(errors)
        }
    }
}

Again, MapErrorsFromException is omitted for brevity, but you could even inject this as a dependency.

In your controller, when you call _mediator.Send you now get back the CancelOrderCommandResult - and if .Success is true, return a 200 as before.

Otherwise, you have a collection of errors - with which you can make some decisions about what to return - a 400, 500, etc...

Alex
  • 37,502
  • 51
  • 204
  • 332
  • Interesting. Do you use this Result class for your queries as well? For example, _send(new GetOrderById()) returns NotFound or something else. –  Jun 06 '18 at 05:51
  • 3
    Yes, that'd be a good way to go. An alternative to this is to throw exceptions in your handler, but that goes against `don't use exceptions for flow control` a bit. – Alex Jun 06 '18 at 06:34
  • Would it be wise to return Task from the command? That way you can do return NotFound(); etc... – axelrotter Oct 21 '20 at 08:57
  • 1
    @axelrotter No, because then you couple ASP.Net with your application layer. Every part of the system should take care of its own responsibilities. – Idan Jul 29 '22 at 13:57
  • Well then I don't see any way around making your own abstraction layer (middleware) that translates your exceptions into http errors. You can use base classes to identify which exceptions you get. – axelrotter Jul 31 '22 at 17:39