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

Reading the actual request may trigger CONNRESET on a cached connection #320

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

smurfix
Copy link

@smurfix smurfix commented Nov 19, 2015

This moves the block that reads the data up into the try: block.

I hit this issue while testing aioamqp with pyrabbit. The sequence of events is:

  • send request without auth header
  • get an error
  • … meanwhile, the server starts thinking about what you just did
    (like doing a DNS lookup for its error log)
  • re-send the request with auth header
  • … the server gets around to deciding that it wants to close your connection, and does so
  • get an ECONNRESET error reading the reply. :-(

This can also happen when you're re-using a cached connection after some time and hit the server's decision that it has waited long enough and will now close the connection.

Thus, please apply.

@smurfix
Copy link
Author

smurfix commented Nov 19, 2015

Of course I meant "Reading the response."

I fixed the commit's comment and re-uploaded.

(if the connection was cached)

thus move that code inside the try:.
Handle ECONNRESET and EPIPE correctly

and simplify the code a bit (pull up conn.close()
while we're at it
@smurfix
Copy link
Author

smurfix commented Dec 19, 2015

Any progress? this change is necessary on py3.5.

@smurfix
Copy link
Author

smurfix commented Dec 19, 2015

this is required to fix Polyconseil/aioamqp#50

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 this pull request may close these issues.

1 participant