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

Removing the "must not start sending till after first body event" constraint #138

Closed
pgjones opened this issue Dec 16, 2019 · 5 comments
Closed

Comments

@pgjones
Copy link
Contributor

pgjones commented Dec 16, 2019

The ASGI specification current states that,

The protocol server must not start sending the response to the client until it has received at least one Response Body event.

Hypercorn interprets this such that it waits to a response body event is received before it sends the first response line and headers, and as I understand it, Daphne does the same. Uvicorn on the other hand sends the response (line and headers) straight away.

I think this part of the specification follows from the WSGI specification,

The start_response callable must not actually transmit the response headers. Instead, it must store them for the server or gateway to transmit only after the first iteration of the application return value that yields a non-empty string, or upon the application's first invocation of the write() callable. In other words, response headers must not be sent until there is actual body data available, or until the application's returned iterable is exhausted. (The only possible exception to this rule is if the response headers explicitly include a Content-Length of zero.)

with the justification for ASGI likely also following the WSGI justification,

This delaying of response header transmission is to ensure that buffered and asynchronous applications can replace their originally intended output with error output, up until the last possible moment. For example, the application may need to change the response status from "200 OK" to "500 Internal Error", if an error occurs while the body is being generated within an application buffer.

I've been asked on Hypercorn if this part of the specification is necessary, and I'm not convinced it is. I think the WSGI justification isn't that strong in that it only supports a rare case of failure between response.start and response.body. Whereas without this feature all failures post response.start would be treated the same way. It also seems that Uvicorn works fine without this.

@njsmith
Copy link

njsmith commented Dec 16, 2019

More discussion in the quart chat:

https://gitter.im/python-quart/lobby?at=5df7ee1cc6ce6027ebd2ca6f

@pgjones pgjones changed the title Removing the "most not start sending till after first body event" constraint Removing the "must not start sending till after first body event" constraint Dec 17, 2019
@andrewgodwin
Copy link
Member

I don't think it is totally necessary - I just lifted it from WSGI, presuming they had a reason for it, but I don't think it's been very useful in the WSGI case anyway and ASGI has even less need of it.

@njsmith
Copy link

njsmith commented Jan 8, 2020

There's more detailed discussion in the link I posted earlier, but AFAICT the current behaviour is what you want.

The wsgi rationale about going back and changing your mind about headers is unconvincing, yeah.

But, the thing is, you have to have some explicit, specification-guaranteed way for the application to control whether the headers are flushed immediately, for correctness. And in most cases, you don't want them to be flushed immediately, for efficiency. (You want the headers and start of the body to go into a single network packet.) But of course, as soon as the application starts sending the body, you have to flush the headers anyway. So the current semantics sort of automatically do everything that you want, without any extra fuss.

The alternative would be to add a bunch more API surface area for buffer control, e.g. mandating that every header and body has a flush: True/False field or something. And it just doesn't seem worth it to me, given that the end result would be basically isomorphic to what we have now, just more verbose and less consistent with WSGI.

The spec could definitely do with some more elaboration to make this more obvious though, and explicitly mention that sending an empty body message is how you flush headers.

@Kludex
Copy link
Contributor

Kludex commented Sep 3, 2023

I'm just seeing this discussion, but some months ago this #387 was merged - which removes the "must not start sending till after first body event" sentence. 👀

@andrewgodwin
Copy link
Member

Yes - I think this can be considered closed out now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants