What's your feeling about how message validation should be treated when using ShortBus

[jrnail23]

@mhinze, in the context of using ShortBus to mediate between MVC/WebApi controllers and the application layer, what's your take on how validation should be done?

As we know, some kinds of validation need to be performed on the other side of the Mediator, at or below the application layer (i.e., enforcing a unique name for an entity, enforcing domain invariants, etc.).

How would you recommend handling these kinds of things?

Would you use a composite exception to roll up multiple validation failures into one exception?

[mhinze]

I recommend taking advantage of idiomatic UI framework code. I have learned hard lessons around writing abstractions around UI framework code. This frees me up to do a lot of things, most importantly I do not care about UI frameworks very much. If they support my requirements and have nice idioms I am good. I don't worry about their design too much - UI framework is properly abstracted from my application.

So I prefer to use ASP.NET MVC's idiomatic validation for input validation. I used to get annoyed at the validation attributes and other nonsense, and would write a ton of my own code around it. Now I just give in to it and write it idiomatically. It's really a great and fresh perspective. The only downside is I might have to map an external schema (AddUserInput, for example, to an internal command like AddUserCommand), keeping the validation attributes and stuff on the AddUserInput. This extra mapping step is annoying but not overly so and there are some libraries that can help cough AutoMapper.

I do not have a formal validation subsystem inside my handlers. This is because some clients have different validation requirements. But I do provide hints that the UI can choose to interpret to provide feedback to the client. For example, if, as part of a command, an entity is not found, I throw a NotFoundException and Web API can then return a 404.

Here is an example of my custom attributes:

public sealed class NotFoundException : ApplicationException
{
    public NotFoundException(string message, string keydescriptor, object key, Exception innerException = null) : base(message, innerException)
    {
        Data.Add(keydescriptor, key);
    }

    public NotFoundException() { }
}

public sealed class BadRequestException : ApplicationException
{
    public BadRequestException(string message, Exception innerException) : base(message, innerException) { }
}

And here is how I handle it from Web API persective:

public class ExceptionTranslator : ExceptionFilterAttribute
{
    public override void OnException(HttpActionExecutedContext context)
    {
        var ex = GetException(context.Exception);

        context.Response = GetValue(ex, context.Request);
    }

    static HttpResponseMessage GetValue(Exception ex, HttpRequestMessage request)
    {
        if (ex is NotFoundException)
        {
            return request.CreateResponse(HttpStatusCode.NotFound, ex.Message);
        }
        if (ex is BadRequestException)
        {
            return request.CreateResponse(HttpStatusCode.BadRequest, ex.Message);
        }

        // TODO Use context to provide more logging metadata
        // (injected through ControllerActivator)
        Log.Error(ex, "Internal Server Error");
        return new HttpResponseMessage(HttpStatusCode.InternalServerError);
    }

    Exception GetException(Exception exception)
    {
        var aggex = exception as AggregateException;
        return aggex != null ? aggex.InnerException : exception;
    }
}

One thing to keep in mind is that validation is a query, so finding duplicate usernames could result in a new query and query handler. This is acceptable, it is a function of the application.

The key thing is that I want to not care about my UI framework. I want to use it idiomatically. For me I feel a relief when this happens, I really hate frameworks over UI frameworks.

[jrnail23]

@mhinze Thanks for this. It definitely makes sense to treat input validation as a UI concern, and you probably just saved me a couple of cycles.

[jpcoder2]

@mhinze I'm trying to understand how exceptions are supposed to be handled within ShortBus. Your code above seems to be trying to catch exceptions at the Web app level but as far as I can tell your ShortBus Mediator catches the exception thrown in a handler and then just passes it back as a Response property. I don't see this filter would ever see it. Am I missing something?

[mhinze]

Inasmuch as this approach is doing "flow control via exceptions" I am unhappy with it. To directly answer your question, in the base controller I test the response.

if (response.HasException())
{
   throw response.Exception;
}

[mhinze]

See also: https://gist.github.com/mhinze/7551259

[jpcoder2]

Doesn't this loose valuable stack trace information? Personally I don't see a reason for the mediator to catch exceptions. Other than that, I really like ShortBus.

[jpcoder2]

Maybe an option to disable exception handling? At least it would still be backwards compatible then.

[jpcoder2]

Ok, now that I tried your suggestion it looks like it isn't throwing away all of the stack trace info. You at least get the trace info from the Handler itself and not starting at the BaseController level where it was thrown. Did .NET not used to only give you the stack from the point of the throw Exception? Maybe that was just in 2.0. Anyway, I still stand by that it seems odd for ShortBus to be catching the exceptions. I would make some changes and send you a pull request but I don't know where I would put the configuration option to allow turning this feature off since you don't seem to already have a place for options that I can tell.

[mrchief]

if (response.HasException())
{
   throw response.Exception;
}

@mhinze Why catch the exception in the first place? Isn't that frowned upon?

Also, what would be the best place to log exception: BaseController? Or inside Mediator.cs if one doesn't go with a BaseController approach or via command decorators?