Aspnetcore: 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:
   C# [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:
c# 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.

[jsauve]

I think a part of the consideration is how one designs their controller methods. I tend to:

• Keep all my controllers very light by consuming injected services that do the heavy lifting.
• Always have a try-catch block in each controller method.
• The catch calls a method in my base controller that handles all the exceptions from the service layer and returns an appropriate result type based on whatever might have gone wrong in the service layer.
• The success portion of the try makes minimal calls and always ends by returning Ok() or Ok(result).

Using a consistent pattern like this makes the controllers easy to read, and keeps the API surface clean. But when I run across something like this where Ok(result) returns a NoContentResult 204 instead of an OkObjectResult 200 (as the documentation says it does), it all of a sudden introduces some bloat in the controller, hence encapsulating a solution in the base controller.

I've gone so far as to add this in my base controller:

protected ActionResult OkOrNoContent(object value = null)
{
    if (value == null)
        return base.NoContent();

    return base.Ok(value);
}

/// <summary>
/// Use OkOrNoContent() instead.
/// </summary>
/// <returns><see cref="OkResult"/></returns>
[Obsolete("Use OkOrNoContent() instead.", true)]
#pragma warning disable CS0809
public override OkResult Ok()
#pragma warning restore CS0809
{
    return base.Ok();
}

/// <summary>
/// Use OkOrNoContent(value) instead.
/// </summary>
/// <param name="value"></param>
/// <returns><see cref="OkResult"/></returns>
[Obsolete("Use OkOrNoContent(value) instead.", true)]
#pragma warning disable CS0809
public override OkObjectResult Ok(object value)
#pragma warning restore CS0809
{
    return base.Ok(value);
}

...so that the rest of my team can avoid the pitfall. Our Postman tests helped reveal the problem.

But I just saw the OutputFormatter solution above, so I may try that instead.

[AlexejTimonin]

@rynowak Can I take a stab at this?

[syndicatedshannon]

Wondering what it 'should' return.

A 204 (No Content) status code if the action has been enacted and no further information is to be supplied.
A 200 (OK) status code if the action has been enacted and the response message includes a representation describing the status.

• https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/DELETE

10.2.5 204 No Content
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.

https://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html

The MDN doc is more opinionated, but if I follow it, then from void/Task-returning controller methods, I'd expect to return 204s. However, they don't, and I don't see an easy way to configure it.

[cremor]

The MDN doc is more opinionated, but if I follow it, then from void/Task-returning controller methods, I'd expect to return 204s. However, they don't, and I don't see an easy way to configure it.

I agree. For Web API 2 (the old one, before .NET Core was a thing) it was even documented that void returns 204. But it seems like this was changed in .NET Core so that void/Task now returns 200.

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

[msftbot[bot]]

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.