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

Clarification about response.start/response.body #388

Closed
kristjanvalur opened this issue May 2, 2023 · 9 comments · Fixed by #387
Closed

Clarification about response.start/response.body #388

kristjanvalur opened this issue May 2, 2023 · 9 comments · Fixed by #387

Comments

@kristjanvalur
Copy link
Contributor

kristjanvalur commented May 2, 2023

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

It would be good to have a rationale for this rule. Particularly because it doesn't directly affect the ASGI protocol as such but more the HTTP on the wire. And many server implementations and server-like packages (such as test clients) do not currently follow this rule.

I guess it is intended to allow the server to catch errors which happen during body processing and avoid sending the response code until the last possible moment, so that it can properly communicate a 500 error. But this is just my guess. Having a clarification from the authors of the spec would be useful to help guide implementors.

@Kludex
Copy link
Contributor

Kludex commented May 2, 2023

@andrewgodwin
Copy link
Member

It was intended for compatability with WSGI, which has a similar callout in its spec, which there is to (I believe) allow for error capture in that gap.

I don't think it's needed now, though!

@kristjanvalur
Copy link
Contributor Author

Would there be any interest in a separate PR, adjusting the wording? Something like
"A server implementation may consider withholding transmission of the response headers until a the first body is received, so that it has the option to return a 5xx error response should an error arise during the processing of the body. Otherwise, the only option left to the server is typically to close the connection."

I guess that was the original reason for this, and the reason I got interested in this topic in the first place, having seen a number of internal server errors just being hidden during testing.

@andrewgodwin
Copy link
Member

Yes, if we want to outline the reasoning but not make it a requirement, that would be a nice addition I think.

@kristjanvalur
Copy link
Contributor Author

I just noticed that the "Response Body" section contains this: "Protocol servers must flush any data passed to them into the send buffer before returning from a send call." This, seems to be in the same vein as the original message. Not sure why this restriction is put in place at all, since how a server sends its html probably should not be part of the ASGI spec.

@andrewgodwin
Copy link
Member

That was likely also something inherited from the WSGI spec; I suspect from the darker days of the web where buffers were very important and specific. I doubt that it's as relevant today.

@kristjanvalur
Copy link
Contributor Author

Well, unless this is something that needs to be in place to support, e.g. long polling, where the application needs to know that the body segment it transmits is subsequently transmitted, and not held on to indefinitely...

@andrewgodwin
Copy link
Member

Sure, but even if the server can guarantee its buffers are flushed, there are no such guarantees from anything else along the network path!

@kristjanvalur
Copy link
Contributor Author

Well, its sort of the assumption, when we get down to TCP. There are innumerable RFCs about how that should behave. Remember Nagle's algorithm and how it is commonly turned off? and pure IP is never buffered unnecessarily. So, in practice, its only software which can cause unnecessarily delays.

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

Successfully merging a pull request may close this issue.

3 participants