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?

Solution 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.

Solution 2

You can actually use IActionResult or Task<IActionResult> instead of Thing or Task<Thing> or even Task<IEnumerable<Thing>>. If you have an API that returns JSON then you can simply do the following:

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

    // 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); // This will be JSON by default
    }

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

Update

It seems as though the concern is that being explicit in the return of an API is somehow helpful, while it is possible to be explicit it is in fact not very useful. If you're writing unit tests that exercise the request / response pipeline you are typically going to verify the raw return (which would most likely be JSON, i.e.; a string in C#). You could simply take the returned string and convert it back to the strongly typed equivalent for comparisons using Assert.

This seems to be the only shortcoming with using IActionResult or Task<IActionResult>. If you really, really want to be explicit and still want to set the status code there are several ways to do this - but it is frowned upon as the framework already has a built-in mechanism for this, i.e.; using the IActionResult returning method wrappers in the Controller class. You could write some custom middleware to handle this however you'd like, however.

Finally, I would like to point out that if an API call returns null according to W3 a status code of 204 is actually accurate. Why on earth would you want a 404?

204

The server has fulfilled the request but does not need to return an entity-body, and might want to return updated metainformation. The response MAY include new or updated metainformation in the form of entity-headers, which if present SHOULD be associated with the requested variant.

If the client is a user agent, it SHOULD NOT change its document view from that which caused the request to be sent. This response is primarily intended to allow input for actions to take place without causing a change to the user agent's active document view, although any new or updated metainformation SHOULD be applied to the document currently in the user agent's active view.

The 204 response MUST NOT include a message-body, and thus is always terminated by the first empty line after the header fields.

I think the first sentence of the second paragraph says it best, "If the client is a user agent, it SHOULD NOT change its document view from that which caused the request to be sent". This is the case with an API. As compared to a 404:

The server has not found anything matching the Request-URI. No indication is given of whether the condition is temporary or permanent. The 410 (Gone) status code SHOULD be used if the server knows, through some internally configurable mechanism, that an old resource is permanently unavailable and has no forwarding address. This status code is commonly used when the server does not wish to reveal exactly why the request has been refused, or when no other response is applicable.

The primary difference being one is more applicable for an API and the other for the document view, i.e.; the page displayed.

Solution 3

In order to accomplish something like that(still, I think that the best approach should be using IActionResult), you can follow, where you can throw an HttpResponseException if your Thing is null:

// GET api/things/5
[HttpGet("{id}")]
public async Task<Thing> GetAsync(int id)
{
    Thing thingFromDB = await GetThingFromDBAsync();
    if(thingFromDB == null){
        throw new HttpResponseException(HttpStatusCode.NotFound); // This returns HTTP 404
    }
    // Process thingFromDB, blah blah blah
    return thing;
}

Solution 4

I too looked high and low for an answer to what to do about strongly typed responses when I wanted to return an 400 response for bad data sent into the request. My project is in ASP.NET Core Web API (.NET5.0). The solution I found was basically set the status code and return default version of the object. Here is your example with the change to set the status code to 404 and return the default object when the db object is null.

[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)
        {
            this.Response.StatusCode = Microsoft.AspNetCore.Http.StatusCodes.Status404NotFound;
            return default(Thing);
        }
        
        // Process thingFromDB, blah blah blah
        return thing;
    }

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

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

View more solutions

Comments

[Keith]

Hi David, cheers. I realise that I can switch to return IActionResult - but if this is the answer why does the ASP.NET Core API Controller support implicit type conversions at all if IActionResult is really required?

[David Pine]

If you really think about it, an API that claims to return a Thing that actually returns null is actually the non-body 204. If you want to return something that is more flexible the supported framework approach is to use IActionResult. It shouldn't be the framework's responsibility to assume anything, especially 404. But again, you have the ability to do it this way. As an alternative you could add custom middleware to set the Response.StatusCode if you'd like instead, but why go through the trouble?

[Keith]

There already is HttpNoContentOutputFormatter middleware ensuring that null gets a 204 rather than a 200. I don't want the framework to assume anything - I don't really want to override their default behaviour without understanding exactly why they did it that way. It seems that public void Post totally should return a 204 if successful, but public Anything Get(id) shouldn't return a success code for a failed action. I feel it's something I'm missing about the intended best practice.

[Keith]

Cheers (+1) - I think this is a step in the right direction, but (on testing) HttpResponseException and the middleware to handle it don't appear to be in .NET Core 1.1. The next question is: should I roll my own middleware or is there an existing (ideally MS) package that already does this?

[Keith]

It looks like this was the way to do this in ASP.NET Web API 5, but it's been dropped in Core, which makes sense given's Core's manual approach. In most of the cases where Core has dropped a default ASP behaviour there's a new optional middleware that you can add in Startup.Configure, but there doesn't appear to be one here. In lieu of that it doesn't appear to be too difficult to write one from scratch.

[Keith]

Ok, I've fleshed out an answer based on this that does work, but still doesn't answer the original question: stackoverflow.com/a/41484262/905

[Hackerman]

I think that if the route-resource is valid, but doesn't return anything, the proper response should be 204(No Content)....if the route doesn't exist should return a 404 response(not found)...makes sense, right?

[Keith]

@Hackerman I suspect that might well be an option, but plenty of client side APIs generalise (1xx on it... 2xx ok, 3xx go here, 4xx you got it wrong, 5xx we got it wrong) - 2xx implies that all is OK, when actually the user requested a resource that isn't there (like the example in my question, the Fetch API considers 204 as OK to continue). I suppose 204 could mean right resource path, wrong resource ID, but that not been my understanding. Any citation on that as the desired pattern?

[Hackerman]

Take a look at this article racksburg.com/choosing-an-http-status-code ...I think that the flow chart for status codes 2xx/3xx explains it very well...you can look at the others too :)\

[Keith]

@Hackerman That still points to 404 being correct in this context.

[Hackerman]

Nope...when they say Does the resource even exist, they are meaning the http resource, not the content of that response...for example the request to your api endpoint is a good one and I was able to reach it, yes, then you have a 2XX response...when we talk about a restful api, we talk about http resources, like a more natural htpp way..so if one user was able to hit your api endpoint then, an http response 404 doesn't make sense..but if the server response is an empty one, then the proper response should be 204, like, yeah, you hit the right endpoint but I have an empty answer right now.

[Keith]

This uses Json to return the IActionResult - that works but hard codes the serialiser. Really you want to be able to change the serializer (say to XML) globally without changing every method. Best practice appears to be to use return Ok(thing);

[Keith]

@Hackerman I'm really not sure on that - it seems like trying to convey information about the API in the API. If we're doing that shouldn't we also implement 405 (rather than 404) for valid controllers without the requested action and so on? In REST the thing being returned is the resource, not the API itself. I think that's why the convention is for names to be plural (rather than singular like they'd be in a DB) - api/things/5 is the resource that expects to be a single thing.

[Keith]

Cheers :-) This appears to be the proposed practice (github.com/aspnet/Mvc/issues/4311#issuecomment-271052290) but if it is then my point from the first comment still stands. I've +1 but someone else has silently downvoted (which is helpful).

[David Pine]

@Keith, my answer is the answer for this question. The first comment you're referring to is technically a separate question than this one. Are you more curious about testing now or what?

[Keith]

Cheers David, but it isn't - that comment is a direct quote from my question. Also from my question I am not looking for return NotFound(); as an answer - these only work with IActionResult ... that would appear to make explicit return types pointless That's what I'm really asking, not "How do I use NotFound()" but "How should I return 404 with explicit typing" - your answer appears to be "don't use explicit typing", but if it is then it's missing the critical detail as to why explicit typing is the default (or even supported at all)

[Hackerman]

Http resources, valid http resources....remember that REST doesn't cares about the logic behind it....I miss sometimes that SO doesn't haves a board where you can write or draw things xD...

[Keith]

@Hackerman yes, that's kind of why I think api/missing and api/valid/missing should both return a 404, not 404 for the former and 204 for the latter.

[Keith]

Cheers for the update :-) That's why I thought they added HttpNoContentOutputFormatter - 204 is correct when no change should be seen in the client's view, it's perfect for anything that returns void or Task (like most POST actions). However, for api/things/5 - there is a clear change for the client and they should see some kind of not found message.

[Hackerman]

If you have defined a valid route in your application for api/valid/{$id} for example, and you try to hit that endpoint with api/valid/21 you have a 2xx response; it doesn't matter if that id with the value of 21 exist or not in your database or if produces a content result or not, the resource exist(your http resource api endpoint)...if 21 produces a result then you have a 200 with for example a json result...if not and returns an empty result, then a 204...Is a little bit hard to understand, it took me a while when I start with this too :)

[Keith]

@Hackerman but isn't that embedding information about the REST service in those codes - whether the route is valid with a missing ID, or the controller valid without an implementation for the HTTP verb, or the route to the action invalid itself, or even the wrong type (api/valid/abc when ID is an int will return a 404) - isn't that trying to expose details of the implementation of the service?

[Hackerman]

Let's do the opposite...show me on which case you should return a 204 status code?

[Keith]

@Hackerman POST JSON thing to api/things/5 hits public void Post or public async Task PostAsync and it successfully applies the changes with nothing to report but success. The MVC Core designers have added HttpNoContentOutputFormatter to force that to return 204 rather than 200.

[Hackerman]

And if 5 is invalid or doesn't exist...or the thing with the id is marked with a soft delete on the parent table....then based on what your previous comments in that case it should returns a 404 status code?

[Keith]

@Hackerman That would vary by the API - POST can carry out upsert style actions, but if you wanted strict POST updates and PUT additions then yeah, a missing ID would be 404 while a successful update would be a 204.

[Keith]

We've been talking about GET api/things/5 returning a 404 if not found. What about POST api/things/5 - if that succeeds it should return a 204 (which is what public void Post or public async Task PostAsync does), but if it failed to update because there is no thing 5 then shouldn't it return a 404 too? Surely it shouldn't return a success status when it's failed?

[Hackerman]

This should be the right answer...and about the status codes, there are different implementations(some uses one code, some uses another) based on what you want to accomplish and how...if you want to use a 404 on some cases and as long as your api is well documented, it shouldn't be a problem.

[Sebastian P.R. Gingter]

This shouldn't be the right answer. If you want to /get/{id} and there is no element of that id on the server, a 404 - not found is the correct answer to that. And regarding being explicitly typed - the type information of the method is actually used in tools like swagger who rely on the controllers declaration to be able to generate the correct swagger file. So IActionResult is missing a lot information in that case.

[David Pine]

Good thing this wasn't accepted as the right answer. Thanks for commenting. :)

[ruffin]

"I'll update this answer with more detail once I've implemented it." Getting close to implemented? /ducks and runs ;^D Interested in a revisit, though, honestly. What about a POST? (not rocket science, but it'd be nice to have in one place)