How to correctly implement Result object program flow with MediatR?

Question

Instead of using exceptions for program flow, I'm attempting to use a custom Result object based on the ideas discussed here in MediatR. I have a very simple example here....

public class Result 
{
    public HttpStatusCode StatusCode { get; set; }
    public string Error { get; set; }

    public Result(HttpStatusCode statusCode)
    {
        StatusCode = statusCode;
    }

    public Result(HttpStatusCode statusCode, string error)
    {
        StatusCode = statusCode;
        Error = error;
    }
}

public class Result<TContent> : Result
{
    public TContent Content { get; set; }

    public Result(TContent content) : base(HttpStatusCode.OK)
    {
        Content = content;
    }
}

The idea being that any failure will use the non-generic version, and success will use the generic version.

I'm having trouble with the following design issues...

1. If a response can potentially be generic or non-generic, what do I specify as the return type for the controller method?

For example...

[HttpGet("{id}")]
public async Task<ActionResult<Result<string>>> Get(Guid id)
{
    return await _mediator.Send(new GetBlobLink.Query() { TestCaseId = id });
}

This won't work if I'm just returning a validation failure of type Result

2. How do I constrain a mediatr pipeline behaviour to potentially handle a generic or non-generic Result response?

For example, if the result is a success, I want to return just the Content from the generic version of Result. If it's a failure, I want to return the result object.

This is what I've come up with to start but it feels extremely 'smelly'

public class RestErrorBehaviour<TRequest, TResponse> : IPipelineBehavior<TRequest, TResponse>
    where TRequest : IRequest<TResponse>
{
    public IHttpContextAccessor _httpContextAccessor { get; set; }
    public RestErrorBehaviour(IHttpContextAccessor httpContextAccessor)
    {
        _httpContextAccessor = httpContextAccessor;
    }

    public async Task<TResponse> Handle(TRequest request, CancellationToken cancellationToken, RequestHandlerDelegate<TResponse> next)
    {
        var response = await next();
        
        if (response.GetType().GetGenericTypeDefinition() == typeof(Result<>))
        {
            _httpContextAccessor.HttpContext.Response.StatusCode = 200;

            //How do I return whatever the value of Content on the response is here?
        }
        if (response is Result)
        {
            _httpContextAccessor.HttpContext.Response.StatusCode = 400;

            return response;
        } 
        
        return response;
    }
}

3. Serializing might be a challenge. What if there is a use case for needing the full Result<> object returned for a successful request - I don't necessarily always want to display "error": null.

Am I going down a rabbit hole? Is there a better way to do this?

The reasons for attempting something like this

• Skinny controllers
• Nicely formatted json responses to API requests
• Avoid exception control flow for validation (And therefore avoiding needing to use .net core middleware to handle and format the request exceptions).

Many thanks,

Answer 1

Am I going down a rabbit hole? Is there a better way to do this?

Yes and yes.

There are several issues here. Some are technical difficulties, others stem from taking the wrong approach. I want to address those issues individually before solving them, so the solution (and the reasoning behind it) makes more sense.

---

1. Type abuse

You're abusing your Result type (and its generic variant) by trying to use it (the type itself) as data:

if (response.GetType().GetGenericTypeDefinition() == typeof(Result<>))

This is not how a result object is supposed to be treated. The result object contains the information inside of it. It should not be divined based on its type.

I'll get back to this point, but first I want to address your direct question. Both issues will be resolved further down this answer.

---

2. Empty results

Note: while not every default value is null (e.g. structs, primitives), for the rest of this answer I'm going to refer to it as a null value to keep things simple.

You're struggling with a problem in trying to accommodate both the base and generic Result types, which you've noticed is not easy to accomplish. It's important to realize that this problem is one of your own making: you're struggling to manage the two separate types that you intentionally created.

When you've downcast your object to a base type, and then think to yourself "boy I would really want to know what the derived type was and what it contains", then you're dealing with polymorphism abuse. The rule of thumb here is that when you need to know/access the derived type, then you shouldn't have downcast it into its base type.

The first thing to address when dealing with a problem of your own making, is to re-evaluate whether you should've been doing this (the thing that causes the issue) in the first place.

The only real reason to separate Result and Result<T> is so you can avoid having a Result<T> with an uninitialized T value (usually null). So let's re-evaluate that decision: should we have avoided null values here to begin with?

My answer is no. null is a meaningful value here, it shows the absence of a value. Therefore we shouldn't avoid it, and therefore our basis for separating the Result and Result<T> types becomes (pun intended) null and void.

The end conclusion here is that you can safely remove Result and refactor any code that uses it to actually use a Result<T> with an uninitialized T, but I'll get to the concrete solution further on.

---

3. Setting the response value

_httpContextAccessor.HttpContext.Response.StatusCode = 200;

//How do I return whatever the value of Content on the response is here?

It's unclear to me why you are trying to set the value of the response in your Mediatr pipeline behavior. The value that is returned is decided by the controller action. This is already in your code:

[HttpGet("{id}")]
public async Task<ActionResult<Result<string>>> Get(Guid id)
{
    return await _mediator.Send(new GetBlobLink.Query() { TestCaseId = id });
}

This is where both the type of the returned value and the returned value itself are being set.

The Mediatr pipeline is part of your business logic, it is not part of your REST API. I think you've muddied that line by (a) putting the HTTP status code in your Mediatr result object and (b) having your Mediatr pipeline set the HTTP response itself.

Instead, it's your controller which should decide what the HTTP response object is. When using result classes, the usual approach is something along the lines of:

public async Task<IActionResult> Get(Guid id)
{
    var result = await GetResult(id);

    if(result.IsSuccess)
        return Ok(result.Value);
    else
        // return a bad result
}

Note that I didn't define what that bad result should be. The correct way to handle a bad result is contextual. It may be a general server error, a permissions issue, not finding the referenced resource, ... It generally requires you to understand the reason for the bad result, which requires parsing the result object.

This is too contextual to define reusably - which is exactly why your controller action should be the one deciding the correct "bad" response.

Also note that I used IActionResult. This allows you to return any result you want, which is often beneficial as it allows you to return a different "good" and "bad" result, e.g.:

if(result.IsSuccess)
    return Ok(result.Value);             // type: ActionResult<Foo>
else
    return BadRequest(result.Errors);    // type: ActionResult<string[]>

---

My proposed solution

• Remove Result entirely, instantiate bad results as Result<T> with an uninitialized T.
• Don't use HTTP status codes here. Your result class should only contain business logic, not presentation logic.
• Give your result class a boolean property to indicate success, instead of trying to divine it via the base/generic result type.
• Unrelated to your posted question, but good tip: allow for multiple error messages to be returned. It can be contextually relevant to do so.
• In order to enforce that a bad result does not contain a value, and a good result must contain a value, hide the constructor and rely on more descriptive static methods

public class Result<T>
{
    public bool IsSuccess { get; private set; }
    public T Value { get; private set; }
    public string[] Errors { get; private set; }

    private Result() { }

    public static Result<T> Success(T value) => new Result<T>() { IsSuccess = true, Value = value };
    public static Result<T> Failure(params string[] errors) => new Result<T>() { IsSuccess = false, Errors = errors };
}

• Let your Mediatr requests return their specific Result<MyFoo> type, even when there is no actual value to return

// inside your Mediatr request

return Result<BlobLink>.Success(myBlobLink);

return Result<BlobLink>.Failure("This is an error message");

• Remove the Mediatr pipeline behavior that tries to set the response. It doesn't belong there.
• Let the controller action decide what to return, based on the result it receives.
• Use IActionResult, to allow your code to return anything it wants to - since you specifically want to return different things based on your received result.

[HttpGet("{id}")]
public async Task<IActionResult> Get(Guid id)
{
    Result<BlobLink> result = await _mediator.Send(new GetBlobLink.Query() { TestCaseId = id });

    if(result.IsSuccess && result.Value != null)
        return Ok(result.Value);
    else if(result.IsSuccess && result.Value == null)
        return NotFound();
    else
        return BadRequest(result.Errors);
}

This is just a basic example of how you can have multiple http response statuses based on the returned object. Depending on the specific controller action, the relevant return statements may change.

Note that this example takes any failed result as a bad request and not an internal server error. I generally set up my REST API to catch all unhandled exceptions using exception filters, which end up returning internal server errors (unless there is a more pertinent http status code for a certain exception type).

How (and if) to distinguish internal server errors from bad request errors isn't the focus of your question. I showed you one way of doing it for the sake of having a concrete example, others approaches still exist.

---

Your concerns

• Skinny controllers

Skinny controllers are nice, but there are reasonable lines to draw here. The code you wrote to accommodate even skinnier controllers introduced more problems than having a slightly less skinny controller would've been.

If you wanted to, you could avoid the if success check in every controller action by passing it into a generalized ActionResult HandleResult<T>(Result<T> result) method which does it for you (e.g. in a base MyController : ApiController class from which all your api controllers derive); but beware that it may become very difficult for your reusable code to return contextually relevant error statuses.

This is the tradeoff: reusability is great but it comes at the cost of making it harder to handle specific cases and customized responses.

• Nicely formatted json responses to API requests

Your ActionResult and its contents will always be serialized, so that's not really an issue to worry about.

• Avoid exception control flow for validation (And therefore avoiding needing to use .net core middleware to handle and format the request exceptions).

You don't need to throw exceptions here, your result object specifically exists to return bad results without having to resort to exceptions. Simply check the success status of the result object and return the appropriate HTTP response that corresponds to the status of the result object.

That being said, exceptions are always possible. It's nigh impossible to guarantee that no exception will ever be thrown. Avoiding exceptions where possible is good, but don't skip out on actually handling any exception that may be thrown - even if you don't expect any.