asgiref icon indicating copy to clipboard operation
asgiref copied to clipboard

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

Open pgjones opened this issue 4 years ago • 3 comments

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.

pgjones avatar Dec 16 '19 15:12 pgjones

More discussion in the quart chat:

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

njsmith avatar Dec 16 '19 21:12 njsmith

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.

andrewgodwin avatar Jan 08 '20 17:01 andrewgodwin

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.

njsmith avatar Jan 08 '20 21:01 njsmith

I'm just seeing this discussion, but some months ago this https://github.com/django/asgiref/pull/387 was merged - which removes the "must not start sending till after first body event" sentence. 👀

Kludex avatar Sep 03 '23 06:09 Kludex

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

andrewgodwin avatar Sep 06 '23 03:09 andrewgodwin