ControllerBase.Ok(object value) responds with 204 status code when value is null

[pwoosam]

Describe the bug

Returning Ok(null) from a controller action results in a 204 status code, rather than a 200. I'm not sure if this in intended, but it's not what I would expect.

To Reproduce

Steps to reproduce the behavior:

1. Using this version of ASP.NET Core '2.2'
2. Create a controller with the following method:

[HttpGet]
public async Task<IActionResult> Test(bool x)
{
    if (x)
    {
        return Ok(null);
    }
    else
    {
        return Ok(new { });
    }
}

3. Make a HTTP Get request to the endpoint where x in the query string is true. The response status code will be 204.
4. Make a HTTP Get request to the endpoint where x in the query string is false. The response status code will be 200 as expected.

Expected behavior

Response status code should always be 200 when returning Ok() or Ok(object value), regardless of if value is null.

[AlexejTimonin]

This behaviour is done by Microsoft.AspNetCore.Mvc.Formatters.HttpNoContentOutputFormatter, the default configuration of this formatter will return 204 NO CONTENT if the body is null. To opt-out add this configuration in your Startup.ConfigureServices method:

services.AddMvc(options => 
{
    var noContentFormatter = options.OutputFormatters.OfType<HttpNoContentOutputFormatter>().FirstOrDefault();
    if (noContentFormatter != null)
    {
        noContentFormatter.TreatNullValueAsNoContent = false;
    }
});

The behaviour is indeed confusing.

[cremor]

This is especially confusing because an action method that returns void results in a 200 OK without any body. Not very consistent.

[mkArtakMSFT]

Thanks for contacting us, @pwoosam.
@pranavkm, can you please look into this? Let's try to fix this, if the fix is straight forward.

[AlexejTimonin]

I do not think this is a bug, it works as intended (although confusing). It has been like this for a couple of years now. Any change to this behaviour would be a big breaking change at this point ;)

[pwoosam]

I'm not sure how many people are relying on the behavior of Microsoft.AspNetCore.Mvc.Formatters.HttpNoContentOutputFormatter since its behavior is not obvious and unexpected.

Is there a reason that the HttpNoContentOutputFormatter.TreatNullValueAsNoContent is true by default? Does this also affect other IActionResult implementations? Are there any projects that are known to depend on this behavior?
This seems like something we should opt into if we want it, rather than having to opt out of it.

I believe that this behavior should at least be disabled when using Ok(object value), since Ok should guarantee a 200 status code (provided serialization is successful).

Setting the status code to 204 when using return Ok(null) seems contradictory, especially when return Ok() is still a 200 with no content in the response body.

I understand what Microsoft.AspNetCore.Mvc.Formatters.HttpNoContentOutputFormatter is trying to accomplish, but changing an Ok IActionResult to NoContent doesn't seem to be intentional.

If this is a big breaking change, then right now is the perfect time to do it since .net core is moving to its next major version.

[paulirwin]

The XML comment for Ok(object value) clearly states that 200 OK is expected:

Creates an OkObjectResult object that produces an Microsoft.AspNetCore.Http.StatusCodes.Status200OK response.

Worse, if you inspect the code for OkObjectResult, it appears like it should be using a 200 status code no matter what. That this behavior happens in the pipeline makes it difficult to diagnose. It also makes testing GET requests in the browser via the address bar difficult, as when 204 is returned, it doesn't change pages.

Returning 204 No Content when the code explicitly is requesting a 200 OK response is confusing, unexpected, inconsistent with the documentation, and inconsistent with common wisdom that 204 is primarily for PUT/DELETE requests when in this case 204 would be returned even if the method is GET. I'm in favor of making this a breaking change to revert it to the expected behavior and not add the HttpNoContentOutputFormatter by default. Users may always add the formatter manually, use code like if (value == null) return NoContent(), and/or a new OkOrNoContent method/result could be added.

[jsauve]

Ran into this myself today. Passed a null to Ok(), and it returned a 204. I mean, I get it, but could the XML Intellisense docs please be updated to make this clear??

I wound up just putting a method in my base controller to deal with it:

protected ActionResult OkOrNoContent(object @object)
{
    if (@object == null)
        return NoContent();

        return Ok(@object);
}

[rynowak]

We should fix this. It's bad if we're not honouring user expectations in this case.

[AlexejTimonin]

@rynowak Can I take a stab at this?

[pranavkm]

@syndicatedshannon \ @cremor this sounds different to the issue at hand. Please file a separate issue.

@AlexejTimonin could you outline your proposal here before you send a PR? Thanks.

[syndicatedshannon]

this sounds different to the issue at hand. Please file a separate issue.

Sure. Thanks for the feedback.

[syndicatedshannon]

Done. Now, back to my original comment, I'm not sure the entire issue as reported here is accurate:

Response status code should always be 200 when returning Ok() or Ok(object value), regardless of if value is null.

After doing more research to open issue #16925 , I'm pretty confident that Ok() should return 204; as should Ok(null), unless the value null or an empty serialized object (e.g. {}) is transmitted in the response, or perhaps for HTML responses, where null might be represented by an empty but present body.

[paulirwin]

@syndicatedshannon In an API method that returns IActionResult, if you write code that is calling a method called Ok, one expects that should return a status code of 200 OK in all cases by default. This seems like a reasonable expectation with no surprises. You as a developer are being explicit in requesting a specific status code. If you want to return 204, you may call NoContent(), or you may opt in to a global configuration setting that returns 204 for any Ok calls with no parameter or null. But to make that the default is surprising and not at all what the developer intends.

That said, for void methods, as well as IActionResult<T> where null is returned, I believe 204 is acceptable because you aren't explicitly requesting a specific status code. But calling a method called Ok is the developer saying "I want to return a 200 OK status code, even if the body is empty or it serializes to null".

I am not aware of any other status code methods that return status codes other than what they are named. i.e. does NotFound() ever return anything other than a 404? You wouldn't expect it to. Edit: Assuming of course that no other middleware (i.e. error pages) rewrites the status code.

[syndicatedshannon]

@paulirwin Totally makes sense. Conventionally, what should the payload be on a JSON-serialized 200 Ok()?

[paulirwin]

@syndicatedshannon Based on the principle of least surprise, my expectation would be that it would be an empty body if you call Ok() with no parameter. Passing null would serialize it to the JSON string null.

Edit: in fact, that's what the XML documentation says will happen for the Ok overload without a parameter:

Creates a OkResult object that produces an empty Status200OK response.

[syndicatedshannon]

Also, I misunderstood:

may opt in to a global configuration setting that returns 204 for any Ok calls with no parameter or null

I thought TreatNullValueAsNoContent was deprecated. My apologizes for the confusion.

[ghost]

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.

[jeremyVignelles]

I agree that Ok(null) that returns 204 is somewhat unintuitive, but I wonder how this could be fixed, since the output formatter seems to be at a higher level.
I fear that it can't differenciate between an explicit call to return Ok(null) and return null;
That formatter also has its own set of issues, like its inability to be understood by the ApiExplorer (see #28060), and it also causes a lot of issues in NSwag due to having to handle multiple return codes as a single nullable value : RicoSuter/NSwag#1259

I'd like to have this behavior removed by default, but that's probably not easily doable now (breaking change)

[brunolins16]

As @jeremyVignelles stated the currently the OutputFormatterCanWriteContext, context provided to the formatters, does not have any information that allows the differentiation from null or OKObjectResult.

We can change the api to provide additional information to the context, however, the current workaround seems a reasonable approach #8847 (comment), and I don't know if changing the default behavior will help in general (besides the fact that I agree that the API is confusing).

@pranavkm do you have any different thoughts?

[ghost]

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

[jeremyVignelles]

So now it has been clear that it won't be fixed, I guess the only thing that can be done is to fix the ApiExplorer to support that behavior : #28060