-
-
Notifications
You must be signed in to change notification settings - Fork 858
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
Client.close() can hang. (asyncio only). #634
Comments
From the code sample and stack traces, the issue seems to be that we fail to close the socket with the same server - the ConnectionResetError traceback shows that very explicitly: stream.wait_closed() hangs. To me this could seem like an asyncio bug (although very unlikely) - perhaps one way to start the investigation would be to connect to the endpoint via a raw asyncio stream (without HTTPX), then close it immediately and see if wait_closed() hangs as well? |
Thank you for your feedback @florimondmanca :) I've tried what you suggest (although I have to admit my knowledge on the subject is quite limited): import asyncio
async def main():
reader, writer = await asyncio.open_connection("login.microsoftonline.com", port=443)
print("Opened")
writer.close()
await writer.wait_closed()
print("Closed")
asyncio.run(main()) The stream is closing properly in this case. |
It seems fairly likely this might have been resolved by 0.9.4, just released. Some things to test out here...
|
Thank you @tomchristie!
So, should we suspect a bug in |
Okay, thanks, that's super helpful - what's the absolute simplest case you can reduce this to? |
I'm assuming the you wouldn't see an issue if using |
This: import asyncio
import httpx
async def main():
async with httpx.Client() as client:
response = await client.get("https://login.microsoftonline.com")
asyncio.run(main())
You're right. In this case , the issue happens when i call import asyncio
import httpx
async def main():
client = httpx.Client()
response = await client.get("https://login.microsoftonline.com")
# await client.close()
asyncio.run(main()) |
@frankie567 Can you share the debug output from running your sample program?
Also, is this specific to 3.7, ie can you reproduce (if able to) on 3.6 or 3.8? |
I can reproduce this on 3.8 but not on 3.6 |
If of any use, I've created a test script with Tox to help debug on different versions: https://github.com/frankie567/httpx-issue-634 |
@frankie567 Ah sorry, meant to point you at the |
Thanks a lot for the reproduction repo @frankie567, super helpful! Here's what
|
So, turns out that if we remove the I pushed the change in #640. @frankie567 Can you verify this solves the issue on your side?
|
@florimondmanca yes, I was debugging exactly that, it gets stuck on debug trace post for reference ->
|
@florimondmanca Yes, it solves the issue 😃 I would be curious though to understand what's so particular in the MS endpoint that prevents the stream to close properly 🤔 |
Closed via #640. Even so, it would be really useful if anyone's able to dig into this further. (Perhaps try simplifying things right down, eg. just open the connection using asyncio, then issue a |
@tomchristie I'll try things and keep you updated ; I'm really curious about this issue. |
So, I tried things but unfortunately I wasn't able to go much further in our understanding of the issue. I made a replication script using the I found that the issue happens only with SSL. By reading the debug logs, it looks like MS never send an EOF after the client has initiated a shutdown. Hence, we get stuck at Debug log with login.microsoftonline.com ❌
Debug log with www.google.fr ✅
I've also made another attempt by using the low-level socket/ssl API (which is synchronous): https://github.com/frankie567/httpx-issue-634/blob/master/debug_ssl.py Basically, it just opens the socket, unwrap and close. With
Other servers either immediately return or raise |
Thanks for going further in the investigation @frankie567! Not sure if this was asked before — does this behavior replicate with Requests or urllib3? Surely a server not sending an EOF on connection close is an issue these libraries have encountered before, might be worth looking at whether/how they’re handling it. |
Ok, so now things are becoming interesting 😅 I've dug a bit into On the other hand, in the And here is where the problem arises for MS server. Doing this with import logging
import urllib3
logging.basicConfig(level=logging.DEBUG)
logging.getLogger('urllib3').setLevel(logging.DEBUG)
http = urllib3.PoolManager()
pool = http.connection_from_host('login.microsoftonline.com', port=443, scheme='https')
conn = pool._new_conn() # urllib3.connection.HTTPSConnection -> urllib3.connection.HTTPConnection -> http.client.HTTPConnection
conn.connect()
conn.sock.unwrap() So... I don't know. My (bold) theory would be that |
Okay, that's some good investigation, thanks. My short on this would be that on asyncio we're now doing the right thing just calling It's interesting that It's possible? that there's a "technically correct, but real-world problematic" bug that's worth filing against asyncio here, but I don't have enough background to take a call on that. |
Following your comment, I had a look at They purposely have an Maybe it's worth to file a case at |
I think we can reassess that "very unlikely". 😃 |
I've been doing some async requests with HTTPX for a while now without any issue.
Summary
However, I've stumbled upon today a case I cannot explain. While doing a request to Microsoft Graph OAuth2 access token endpoint, I correctly receive a response but HTTPX client seems to hang and never return. I didn't find any other API with the same behaviour.
Reproduction
Response headers
After waiting a while, a `ConnectionResetError` is raised. Here is the stacktrace
Versions
The text was updated successfully, but these errors were encountered: