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

Handle 408-response connection closes. #146

Closed
tomchristie opened this issue Jul 25, 2019 · 2 comments
Closed

Handle 408-response connection closes. #146

tomchristie opened this issue Jul 25, 2019 · 2 comments
Labels
good first issue Good for newcomers

Comments

@tomchristie
Copy link
Member

PR #145 ensures that we're checking for disconnections when we reacquire connections, by using the StreamReader.at_eof interface.

However @njsmith points out, that's not always quite sufficient. #143 (comment)

There's StreamReader.at_eof, but that will fail if the HTTP server is polite and sends a 408 Request Timeout before it closes the connection. I think the best you can do is:

def asyncio_stream_is_readable(stream):
     return stream._buffer or stream._eof

or else giving up on the Stream API and using protocols/transports directly.

Either of those would be do-able.

How common is 408 usage in the wild - are there example URLs we can confirm against?

Also worth noting that there's also always going to be a race network condition where we start getting a 408 response that was about to be sent, just as we're in the process of sending out a new request.

@sethmlarson sethmlarson added the good first issue Good for newcomers label Aug 18, 2019
@sheshtawy
Copy link

I'd love to help with this if no one is

@tomchristie
Copy link
Member Author

Closing this in favour of #34.

For robustness we ought to be closing keep-alive connections after a timeout period.

That doesn't prevent 408 cases from occuring entirely, but it would substantially mitigate against it, and is the sensible case for us to prioritize here.

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

No branches or pull requests

3 participants