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

ServerDisconnectedError is trigger happy #394

Closed
digitaldavenyc opened this issue Jun 4, 2015 · 15 comments
Closed

ServerDisconnectedError is trigger happy #394

digitaldavenyc opened this issue Jun 4, 2015 · 15 comments
Assignees

Comments

@digitaldavenyc
Copy link

I'm getting this error a lot when using the aiohttp library when making http requests. It's firing on over 50% of the domains I visit. The documentation is extremely light on what this error actually is why it is firing at all. Can someone clarify what it is?

@digitaldavenyc
Copy link
Author

I figured out what the issue was, if the session connection is closed before you try to read the results of the requests ServerDisconnectedError fires. This must've been a functional change is an update, I did not receive this error using and older version of aiohttp

Is it necessary to close the connection on all requests?

@asvetlov
Copy link
Member

asvetlov commented Jun 5, 2015

ServerDisconnectedError is caused when server side closes connection in non-expected manner.

Response should be release explicitly (yield from resp.release()) to avoid resource warning -- it's best practice. After Python 3.5 release I'll add context manager for that.

@digitaldavenyc
Copy link
Author

Thanks! Was wondering what the best practice was for opening and closing connections.

Calling response and connector close() before calling read() was also causing a ServerDisconnectedError. I modified the code so that didn't happen and it resolved the issue. Does this make any sense?

@asvetlov
Copy link
Member

asvetlov commented Jun 5, 2015

I think it's a bug.
If you did not see it in older aiohttp versions it's regression.
We improved disconnection handling and probably made this error.
I suggest the following behavior: after direct connector/client session closing all requests and responses produced by connector should be closed silently.

@fafhrd91 that do you think about?

@digitaldavenyc
Copy link
Author

Running yield from resp.release() actually introduced another error. I've since removed it and reverted to not closing any connections and my application is performing well.

I probably should have noted that this was being used for aiohttp.request GET requests and not serving content.

@asvetlov asvetlov self-assigned this Jun 14, 2015
@asvetlov
Copy link
Member

Running yield from resp.release() actually introduced another error.

Please elaborate.

@digitaldavenyc
Copy link
Author

I upgraded to the latest aiohttp and this is no longer an issue.

@fafhrd91
Copy link
Member

Good

@wvxvw
Copy link

wvxvw commented Mar 20, 2018

@asvetlov how do you imagine a combination of response.raise_for_status() and response.release()?

@asvetlov
Copy link
Member

async with client.get(url) as resp:
    resp.raise_for_status()

@wvxvw
Copy link

wvxvw commented Mar 20, 2018

@asvetlov are you even serious? What if response must outlive the function in which it is created?

@asvetlov
Copy link
Member

asvetlov commented Mar 20, 2018

I'm deadly serious.
What prevent you from something like:

async with client.get(url) as resp:
    process_response(resp)  # do all your dirty work here, even resp.raise_for_status() call

@wvxvw
Copy link

wvxvw commented Mar 20, 2018

If I could chose everything about my code, I wouldn't chose Python, and surely not asyncio, and none of its minions. The reason I'm here is that I cannot chose. One thing I cannot chose is what to do with response generated in this way. It must outlive the scope of the context manager it is in.

Sorry for the tone: most things I'm upset about aren't your fault. The retard who conceived the idea of context managers is probably the most guilty one, but Python core developers must share in this responsibility, obviously. You are only guilty of thinking that it is a good idea to use one.

Now, technically, you probably realize there is a problem. You probably also suspect that there isn't a good solution, (because, what you offer is certainly not a solution) that that was a mistake, but the change to API is too extreme. You'd rather live with bad API than try to deal with change. I can understand this too.

@wvxvw
Copy link

wvxvw commented Mar 20, 2018

In addition to above, if response outlives the session, then aiohttp hangs forever, not even errors. I cannot really think of a worse interface.

@aio-libs aio-libs locked as too heated and limited conversation to collaborators Mar 21, 2018
@asvetlov
Copy link
Member

Please keep your emotions for other places.
Sure, Python and especially asyncio maybe not fit well to people with alternative brain organization.
If you hate aiohttp, asyncio and Python -- please don't use it.
Save your and our time.
Otherwise please keep constructive tone in conversation.

The thread is frozen, it falled into counterproductive series of blames.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants