c# - Returning a 404 from an explicitly typed ASP.NET Core API controller (not IActionResult)

Question

ASP.NET Core API controllers typically return explicit types (and do so by default if you create a new project), something like:

[Route("api/[controller]")]
public class ThingsController : Controller
{
    // GET api/things
    [HttpGet]
    public async Task<IEnumerable<Thing>> GetAsync()
    {
        //...
    }

    // GET api/things/5
    [HttpGet("{id}")]
    public async Task<Thing> GetAsync(int id)
    {
        Thing thingFromDB = await GetThingFromDBAsync();
        if(thingFromDB == null)
            return null; // This returns HTTP 204

        // Process thingFromDB, blah blah blah
        return thing;
    }

    // POST api/things
    [HttpPost]
    public void Post([FromBody]Thing thing)
    {
        //..
    }

    //... and so on...
}

The problem is that return null; - it returns an HTTP 204: success, no content.

This is then regarded by a lot of client side Javascript components as success, so there's code like:

const response = await fetch('.../api/things/5', {method: 'GET' ...});
if(response.ok)
    return await response.json(); // Error, no content!

A search online (such as this question and this answer) points to helpful return NotFound(); extension methods for the controller, but all these return IActionResult, which isn't compatible with my Task<Thing> return type. That design pattern looks like this:

// GET api/things/5
[HttpGet("{id}")]
public async Task<IActionResult> GetAsync(int id)
{
    var thingFromDB = await GetThingFromDBAsync();
    if (thingFromDB == null)
        return NotFound();

    // Process thingFromDB, blah blah blah
    return Ok(thing);
}

That works, but to use it the return type of GetAsync must be changed to Task<IActionResult> - the explicit typing is lost, and either all the return types on the controller have to change (i.e. not use explicit typing at all) or there will be a mix where some actions deal with explicit types while others. In addition unit tests now need to make assumptions about the serialisation and explicitly deserialise the content of the IActionResult where before they had a concrete type.

There are loads of ways around this, but it appears to be a confusing mishmash that could easily be designed out, so the real question is: what is the correct way intended by the ASP.NET Core designers?

It seems that the possible options are:

1. Have a weird (messy to test) mix of explicit types and IActionResult depending on expected type.
2. Forget about explicit types, they're not really supported by Core MVC, always use IActionResult (in which case why are they present at all?)
3. Write an implementation of HttpResponseException and use it like ArgumentOutOfRangeException (see this answer for an implementation). However, that does require using exceptions for program flow, which is generally a bad idea and also deprecated by the MVC Core team.
4. Write an implementation of HttpNoContentOutputFormatter that returns 404 for GET requests.
5. Something else I'm missing in how Core MVC is supposed to work?
6. Or is there a reason why 204 is correct and 404 wrong for a failed GET request?

These all involve compromises and refactoring that lose something or add what seems to be unnecessary complexity at odds with the design of MVC Core. Which compromise is the correct one and why?

Answer 1

This is addressed in ASP.NET Core 2.1 with ActionResult<T>:

public ActionResult<Thing> Get(int id) {
    Thing thing = GetThingFromDB();

    if (thing == null)
        return NotFound();

    return thing;
}

Or even:

public ActionResult<Thing> Get(int id) =>
    GetThingFromDB() ?? NotFound();

I'll update this answer with more detail once I've implemented it.

Original Answer

In ASP.NET Web API 5 there was an HttpResponseException (as pointed out by Hackerman) but it's been removed from Core and there's no middleware to handle it.

I think this change is due to .NET Core - where ASP.NET tries to do everything out of the box, ASP.NET Core only does what you specifically tell it to (which is a big part of why it's so much quicker and portable).

I can't find a an existing library that does this, so I've written it myself. First we need a custom exception to check for:

public class StatusCodeException : Exception
{
    public StatusCodeException(HttpStatusCode statusCode)
    {
        StatusCode = statusCode;
    }

    public HttpStatusCode StatusCode { get; set; }
}

Then we need a RequestDelegate handler that checks for the new exception and converts it to the HTTP response status code:

public class StatusCodeExceptionHandler
{
    private readonly RequestDelegate request;

    public StatusCodeExceptionHandler(RequestDelegate pipeline)
    {
        this.request = pipeline;
    }

    public Task Invoke(HttpContext context) => this.InvokeAsync(context); // Stops VS from nagging about async method without ...Async suffix.

    async Task InvokeAsync(HttpContext context)
    {
        try
        {
            await this.request(context);
        }
        catch (StatusCodeException exception)
        {
            context.Response.StatusCode = (int)exception.StatusCode;
            context.Response.Headers.Clear();
        }
    }
}

Then we register this middleware in our Startup.Configure:

public class Startup
{
    ...

    public void Configure(IApplicationBuilder app)
    {
        ...
        app.UseMiddleware<StatusCodeExceptionHandler>();

Finally actions can throw the HTTP status code exception, while still returning an explicit type that can easily be unit tested without conversion from IActionResult:

public Thing Get(int id) {
    Thing thing = GetThingFromDB();

    if (thing == null)
        throw new StatusCodeException(HttpStatusCode.NotFound);

    return thing;
}

This keeps the explicit types for the return values and allows easy distinction between successful empty results (return null;) and an error because something can't be found (I think of it like throwing an ArgumentOutOfRangeException).

While this is a solution to the problem it still doesn't really answer my question - the designers of the Web API build support for explicit types with the expectation that they would be used, added specific handling for return null; so that it would produce a 204 rather than a 200, and then didn't add any way to deal with 404? It seems like a lot of work to add something so basic.