The Wayback Machine - https://web.archive.org/web/20201127022544/https://github.com/dotnet/aspnetcore/issues/27165
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActionResult<T> should convert to OkObjectResult or set StatusCode to 200 #27165

Open
snappyfoo opened this issue Oct 23, 2020 · 3 comments
Open

ActionResult<T> should convert to OkObjectResult or set StatusCode to 200 #27165

snappyfoo opened this issue Oct 23, 2020 · 3 comments

Comments

@snappyfoo
Copy link

@snappyfoo snappyfoo commented Oct 23, 2020

Currently if you return T in an action that returns ActionResult<T> it will create an ObjectResult. The problem is that the StatusCode remains null rather than 200, even though the eventual response generated from it will be 200.

This makes it confusing in, for example, an action filter, where a developer may only want to handle Ok object results on the executed context, including where T is returned directly. They may:

  • Safe cast to OkObjectResult and find null
  • Better, they safe cast to ObjectResult which works but then find StatusCode is null

They may not handle the result as they should as it's not obvious that a null StatusCode would eventually wind up as a 200 in the response. It seems to be an implementation detail that could easily change.

The assumption is always that returning T will result in a 200 OK (e.g. documentation mentions how you can leave out the T type in a ProducesResponseType attribute for 200 when using ActionResult<T>).

Can this be further enforced by creating an OkObjectResult (which will set StatusCode to 200 in its constructor)? If not, perhaps just an ObjectResult as it currently does, but setting StatusCode to 200 with member initialisation?

Is there perhaps room for improvement in documentation about any of this? Happy to be pointed to anything that makes it explicit how response status codes are created from result status codes including when the result status code is null etc.

@javiercn javiercn added the area-mvc label Oct 23, 2020
@mkArtakMSFT mkArtakMSFT added this to the Next sprint planning milestone Oct 23, 2020
@msftbot
Copy link
Contributor

@msftbot msftbot bot commented Oct 23, 2020

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Oct 23, 2020

@snappyfoo setting the StatusCode would be a less dramatic change. There's a special case we would have to configure for ActionResult<ProblemDetails> since ObjectResult specializes on that type: https://github.com/dotnet/aspnetcore/blob/master/src/Mvc/Mvc.Core/src/ObjectResult.cs#L78-L88, but it should be find outside of that.

@pranavkm pranavkm modified the milestones: Next sprint planning, Backlog Nov 6, 2020
@msftbot
Copy link
Contributor

@msftbot msftbot bot commented Nov 6, 2020

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.