Mvc: Why no HttpResponseException?

[markrendle]

I know it's in the Web API shim, but why is there no equivalent (that I can find) in Core MVC? It was a very useful way of returning 404s from methods that return Task<Spaceship>.

I can patch one in myself easily enough with some middleware, so I wondered if there was a major performance issue or something that led to it being removed.

[davidfowl]

Just the classic, "we don't want people using exceptions for control flow" paradigm. People did things like use it from within their business logic which is all sorts of wrong. I can't speak to the performance issues because I haven't measured but throwing an exception that is meant to be caught to get some data is worse than just returning that result.

[Eilon]

Yep, for exactly the reasons @davidfowl said.

[KeithHenry]

I appreciate the case for the no control flow exceptions, but what's supposed to replace it?

Suppose I have an action on an API

// 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;
}

Ok, so I'm returning an 204 OK HTTP status, which would be fine if this were a POST and I just need to communicate success with no body.

However, shouldn't this HTTP GET action return a 404 not found? It's not successful.

Now I can switch the return type from the explicit, unit tested Task<Thing> to Task<IActionResult> and then return NotFound(); - but then does that mean we should _always_ use IActionResult? Or should we use a mix of explicit types and IActionResult depending on whether a 404 result is an option?

I've also asked this on Stackoverflow.

It seems the options are:

• Have a weird (messy to test) mix of explicit types and IActionResult depending on expected type.
• Forget about explicit types, they're not really supported by Core MVC, always use IActionResult.
• Write an implementation of HttpResponseException and use it like ArgumentOutOfRangeException.
• Write an implementation of HttpNoContentOutputFormatter that returns 404 for GET requests.
• Something else I'm missing in how Core MVC is supposed to work?
• Or is there a reason why 204 is correct and 404 wrong for a failed GET request?

[Eilon]

My suggestion would be to have the action method return IActionResult (or the async variation).

The reason that both IActionResult and object are supported is that they each suit different purposes and different styles. For some of the simplest cases, it's nice to use object, but it isn't powerful at all. For full control, there's IActionResult, which follows the well-known "command pattern."

The IActionResult pattern allows the controller to explicitly state what should happen as a result of the action: some kind of error code, a redirect, a serialized data object, a rendered view, etc.

[KeithHenry]

Cheers for the response, I get that IActionResult is more powerful - this isn't a _power_ action though, it's a trivial HTTP GET - it's extant in even a single line action...

// GET api/things/5
[HttpGet("{id}")]
public Task<Thing> GetAsync(int id) =>
    GetThingFromDBAsync(); // returns 204 success if not found in the DB

If the answer really is to just use IActionResult then the problem is that it isn't possible to create a simple explicitly typed controller - you're _always_ going to need IActionResult on GET actions (see my first bullet in the previous post). It also makes the trivial case above _more_ complicated and the serialisation something that can be hard coded in each action (though there are ways around that).

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

    // Oops, now the choice of serialiser is made by the action, rather than middleware
    return Json(thing);
}

I realise there are lots of ways to add a 404 that add complexity or override ASP Core behaviour, but none of that seems right for something as trivial as that.

[rynowak]

Json() is there for legacy reasons. Prefer Ok() or one of the other status-code like methods that take an object.

[KeithHenry]

Yes, good point. That avoids issues with hardcoded serialisers. Doesn't change the main issue of making the trivial case more complicated or leaving you with a mix of return types.

[danielearwicker]

The suggestion to use IActionResult instead of the actual returned model type seems counterproductive, as it undervalues the type information in controller methods.

For example, as TypeScript becomes ever more popular on the service-consumption side, it's a good idea for services to have type descriptions that can be harvested to automatically generate matching type declarations (and indeed this has been true for years with a large selection of other statically-typed client languages.)

I've been looking at a project called http://frhagn.github.io/Typewriter which has some examples for generating client-side proxies for controller classes, so the TS code has a completely type-safe way to make calls, pass data and return it, etc.

Regarding the advice against using exceptions for control flow, I wonder why that is applicable here? When a method returns an error status instead of a valid model, that is an exceptional case, and occurs when the caller passed parameters that make it impossible for the implementer to honour the request. This is literally what exceptions are for. The exception case can happen, but it is not expected to be the usual case. The vast majority of requests will take the mainstream path. The tiny subset that takes the exception path does not seem to be a major priority for optimisations.

UPDATE

Having setup your API to return swagger.json, and then running it locally, this powershell script is kind of awesome! It generates a folder called generated containing TypeScript classes for calling your API.

Invoke-WebRequest http://localhost:5000/swagger/v1/swagger.json -OutFile downloaded-swagger.json

docker run --rm -v ${PWD}:/local swaggerapi/swagger-codegen-cli generate -i /local/downloaded-swagger.json -l typescript-fetch -o /local/generated

(You need docker, and grant it access to your drive C or wherever your source is. On Linux/MacOs in a shell script use wget to download the file and pwd in backticks instead of ${PWD}.)

[Eirenarch]

@danielearwicker the suggestion in this case is to use attributes like for example SwaggerResponse attributes to document the return type.

I personally find this to be an extreme overkill and a risk of getting the implementation and attributes out of sync. I am implementing my own HttpResponseException and I refuse to feel guilty about it :)

[vhe1]

Exactly.
It's not just saying "I don't like exceptions", it's saying exceptions are worse than no return types.
Sorry, that's a really bad tradeoff.

[FredKellyAsBunny]

We can definitely implement our own HttpResponseException, and in lieu of not having one out of the box, I agree with the approach that @Eirenarch took. However, the decision to remove HttpResponseException on the basis that you didn't want people to misuse it seems short-sighted.

[KeithHenry]

I think this might be fixed by the changes coming in ASP.NET Core 2.1: https://blogs.msdn.microsoft.com/webdev/2018/02/02/asp-net-core-2-1-roadmap/

That adds ActionResult<T>, which lets you return either the explicit type or the NotFound that produces a 404.

[slawomir-brzezinski-at-interxion]

https://github.com/aspnet/Mvc/issues/4311#issuecomment-198324107: Just the classic, "we don't want people using exceptions for control flow"

Exceptions are a tool for control flow. Specifically, they can be used for clearly distinguishing, in the body of the method, the most important flow (essentially, the cause for the method to exist) from errors. The 'positive' flow(s) will be marked by return statement. This, BTW, makes the method signature and name simpler, while its responsibility quicker to grasp.

_MFowler. In one article about the tradeoff:_
_> I need to stress here, that I'm not advocating getting rid of exceptions throughout your code base. Exceptions are a very useful technique for handling exceptional behavior and getting it away from the main flow of logic._

Also, performance of exceptions is not an issue, if someone is not expecting 'invalid traffic en masse' (then, it's very easy - the feature is optional - you can just not use it).

[yanrez]

Would aspnet support controllers returning tuples? Could be a way to handle this problem - keep return value strictly typed, and augment it with error information if necessary without going back to HttpResponseException, or actually including HttpResponseException into tuple returned by controller.

[zijianhuang]

It is disappointing that davidfowl has such idea. I suppose HttpResponseException has given high level abstraction for application programming, to make Web API look like an API, and wrap the technical details of IActionResult into an exception. Removing this exception from the framework is really counter productive against application programming.

[zijianhuang]

KeithHenry had made a good post at http://stackoverflow.com/questions/41464540/returning-a-404-from-an-explicitly-typed-asp-net-core-api-controller
I guess I may be using it as workaround. Really this solution based on .net core 2.1 still breaks the abstraction of explicit/strongly typed Web API.

I see there may be trade-off between abstraction and runtime performance. However, can .NET core compiler or JIT compiler compile HttpResponseException into ActionResult and respective returns of helper classes?

[NetTecture]

I think you all are lookig at this WAY too easy - or: too simplistic.

If controller actions get more complex, there are way more progblems that may happen further down the line. I work on complex controllers for OData and I want to ahve the query logic distributed into different methods in the same controller. Helper methods that return or manipulate an IQueryable.

Now, my problem is not 404. My problem is... 403. Forbidden. There are some operations that only admins are allowed to do on SOME objects. Very often it makes sense to isolate some of the logic into helper methods. Return object or null for 404. Good so far. How do I return that a security check blows? Which is exactly what I am fighting with at the moment. Back to HttpStatusException. And remember, we still talk of UI level helper methods, often in the same controller (yes, I have controllers building linq statements using half a dozen helper methods - the resulting SQL fills a page or two). And some of them do permission checks. Performance? Heck, those controllers do often 60 or more database calls. I do not really care about the overhead of a single exception.