python-hyper / h11

A pure-Python, bring-your-own-I/O implementation of HTTP/1.1
https://h11.readthedocs.io/
MIT License
490 stars 62 forks source link

How to send multiple requests as a client via a keep alive connection? #100

Closed pgjones closed 4 years ago

pgjones commented 4 years ago

Currently sending a EndOfMessage means that b"" will be sent to the server - however this indicates to the server that the client is done sending. The docs seem to avoid this by allowing multiple events to be sent as one (with the b"" appended to the end and hence not present) - however the code does not. I think the docs are correct and the code should be changed (which I'll happily write a PR for), however I'm not sure enough...

send_to_server(client.send(h11.Request(...)))
send_to_server(client.send(h11.EndOfMessage()))  # Server half closes...
client.start_next_cycle()
send_to_server(client.send(h11.Request(...)))  # Issues here
njsmith commented 4 years ago

I think there's some confusion... when h11.Connection.send return b"", that means "don't send anything to the server", not "close the connection". And in fact with the standard socket API, if you do sock.send(b""), that's a no-op. The only way recv return b"" is if the sender has closed or shutdown the connection.

To make this more confusing, there are a few APIs that do treat b"" as triggering a close: this includes some versions of openssl, and, uh, h11.Connection.receive_data. (The rationale here is that receive_data generally takes the output from recv, not the input to send. For openssl idk what their excuse is; trio's wrapper has a workaround for this quirk.)

So it sounds like maybe this could be clearer in the docs? And also it sounds like you found an independent bug in the docs where it tries to send multiple events at once :-).

pgjones commented 4 years ago

That makes much more sense, and it allows me to correct the testing code in Hypercorn to also no-op. Thanks.

I think this would be good in the docs, I'll try to add something and cleanup the multiple send.

pgjones commented 4 years ago

See #101 for the fix. Having re-read the docs I think this is clear in them and I was confused.