Add an IOError when send() is called on a closed connection

[adriangb]

Closes #66

[adriangb]

cc @andrewgodwin @Kludex

[carltongibson]

Can a raise a strong concern about this proposal.

We use http.disconnect in Django to allow the framework to detect client disconnects on long-running connections, and clean up properly.

We're very happy with it, and this change would be highly disruptive.

It's also not clear how this resolves the issues linked to on the ticket… If I'm running a long running task, I want to listen for http.disconnect on receive() before completing the (presumably expensive) work that leads to a send(). If http.disconnect is removed, there would be no way to cancel the work prior to getting an exception on send(). That's a strict regression.

[adriangb]

You'd still be able to detect a disconnect my catching an IOError raised when send() is called.

It's also not clear how this resolves the issues linked to on the ticket… If I'm running a long running task, I want to listen for http.disconnect on receive() before completing the (presumably expensive) work that leads to a send()

Let me see. I assume you don't care where or how you are notified of the disconnect, but you do want to be notified. The issue is that as it stands the only way to do that is to call receive(). This is problematic because if your long-running task also wants to consume the request body you're now calling receive() for two different reasons, and unless you can do that in the same place you're going to have issues like Starlette does.

Two options:

• Keep the existing events around forever as a way to get the information by calling receive()
• Raise the exact same IOError when receive() is called

I like the second option. It makes sense to me that any time you try to interact with a client that is disconnected, be it sending or receiving data, you would be notified (via an error) that the client is disconnected. It's then up the user / framework to determine which of the two interactions they use.

[andrewgodwin]

I do not believe we discussed deprecating the disconnect message at all, merely raising an error when someone tries to send() down a closed socket. @adriangb, did I interpret something wrong?

[adriangb]

@andrewgodwin you are right and did not misunderstand, we had not explicitly discussed it. I felt it made sense to make receive() symmetrical with send() in raising an IOError, thus making the disconnect message redundant and a good candidate for deprecation in the next major version of the spec. As it stands right now we'll end up with an asymmetry. But that's not a big deal, I've reverted the wording to only what we explicitly discussed.

[andrewgodwin]

OK, only one more nit now - the spec doc has two empty lines before each header and you have just one. Fix that and we can merge it in, I think.

[adriangb]

Done!

Can we do a minor spec version bump after this so that frameworks can know if the server supports this?

[andrewgodwin]

Yes, I think that would be appropriate - I'll add one in after this is merged.