✨ Remove body from requests with a status code that don't allow it

[tiangolo]

✨ Remove body from requests with a status code that don't allow it

This will solve/prevent errors in Uvicorn caused by Starlette sending invalid responses.

Closes #1764, also probably solves/related to #1635

[jhominal]

I do not think that I like filtering the request body in that way.

What I mean, the Response object has contradictory instructions, and I do not think Starlette should decide which of the status code or the content specified by the user should be sacrificed. Personally, I would be more comfortable with Starlette raising an exception in that case - that keeps Starlette at the conceptual HTTP level, where I feel it was decided we would let it stay.

For the related idea of adding a lot of typing overloads using typing.Literal on the status code to signal that content cannot be set in that case, I do not know that it would help beyond very simple code patterns.

About the underlying issue in FastAPI

I know that you want to solve a FastAPI issue where an exception is raised if someone writes a minimal endpoint that returns a 204 status code - but in my view, that issue is the manifestation of an impedance mismatch between:

• python functions - where return and return None are indistinguishable;
• and HTTP responses - where "HTTP response has JSON content null" is distinct from "no HTTP response content";

As it is FastAPI that owns the process of transforming regular Python return values into JSON responses, I think that it is FastAPI that should own the handling of that case.

My suggestion would be for FastAPI to replace the default for that transformation from JSONResponse into a FastAPI-specific subclass of starlette.responses.Response (e.g. call it ImplicitResponse or ImplicitJSONResponse), that would:

• behave like JSONResponse if the status code allows a response body;
• set an empty response body if the status code does not allow a response body and the value returned by the function was None;
• raise an error, or filter the content (probably with at least a warning), if the status code does not allow a response body and the value returned by the function was not None;

That approach also allows more future flexibility for FastAPI, such as adding a flag on the endpoint on whether None should be translated as "null JSON response" or as "HTTP response without content", even when the status code allows for a response body. (E.g. I personally had the case where I wanted to return an empty response body with a 202 status code.)

[tiangolo]

Thanks for the feedback @jhominal.

On the FastAPI side, I already solved it and merged the PR there.

I'm gonna wait to hear more feedback here, hopefully @tomchristie will have the chance to chime in.

[Kludex]

I'll check this later, but if I recall correctly, it was a conscious decision.

Maybe the reason is in one of those links:

• Implement EmptyResponse class #1270
• Add EmptyResponse class #1178
• Don't set headers for responses with 1xx, 204 and 304 status code #1397
• JSONResponse Wrong Content-Length Value #1282

I'll be back on this later.

[tiangolo]

@Kludex found the previous conversation where it seems this was decided: https://gitter.im/encode/community?at=61d82b1a82a4667b2563bfc3

I'm gonna wait and see if @tomchristie has any other opinion. Or if it should be an exception for the user to know that they created an invalid response object (that Uvicorn will reject) or something like that.

[florimondmanca]

I read that discussion. I'm kind of on the fence, because the HTTP/1.1 spec says this about 204 No Content:

A 204 response is terminated by the first empty line after the header fields because it cannot contain a message body.

On the other hand, I agree with @jhominal that this seems something that's much more possible in FastAPI than Starlette.

Starlette does the right thing by default, because it uses b"" as the default body (whereas FastAPI passes that anything the view returns through JSONResponse by default). So if you write return PlainTextResponse(status_code=204) then there's no body returned.

To put oneself in the situation akin to the one which FastAPI shoot themselves with quite frequently (myself included), you'd have to explicitly write return PlainTextResponse(b"Some content", status_code=204), which, if knowing about the HTTP spec, should look suspicious.

As an illustration, I've never found myself in a "204 with body" when using solely Starlette, whereas this happened sometimes in FastAPI due to the issue which got fixed by fastapi/fastapi#5145. (I also think it might have been more structured to tweak the view -> JSONResponse transformation code into a FastAPI-specific response implementation that takes care of the impendance mismatch which @jhominal described. But I don't know the FastAPI codebase enough to really tell.)

Isn't the FastAPI fix enough? Does FastAPI need this change in Starlette to work as expected?

[tomchristie]

I'd mostly likely agree with @florimondmanca's assessment here.

Starlette forces the user to be explicit about the content and the status code, and yes there are some broken things that a user could choose to do with that, but they'd have to actively and explicitly do so.

[tiangolo]

Thanks for the feedback everyone! Yep, I get it and it makes sense.

I already solved the problem on the FastAPI side, so having it here in Starlette or not won't affect that. 🚀 But I just felt the "duty" to bring it up here and handle it here (if there was anything to handle).

Knowing that the conclusion and decision is to leave it as it is and that it's intentional, I'll close this PR (and the issue) now.

Thanks everyone for the feedback and discussion! 🍰 ☕