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

Big response for async clients #226

Closed
wants to merge 4 commits into from
Closed

Conversation

mindflayer
Copy link
Owner

No description provided.

@coveralls
Copy link

coveralls commented Feb 18, 2024

Coverage Status

coverage: 97.608% (-1.4%) from 99.046%
when pulling 345d04a on fix/httpx-long-response
into d154be2 on main.

Copy link

sonarcloud bot commented Feb 18, 2024

Quality Gate Passed Quality Gate passed

Issues
2 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@@ -205,7 +207,7 @@ def __exit__(self, exc_type, exc_val, exc_tb):
@property
def fd(self):
if self._fd is None:
self._fd = MocketSocketCore()
self._fd = MocketSocketCore(w_fd=self.write_fd)
Copy link
Collaborator

@ento ento Feb 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fd and fileno get called against different instances of MocketSocket, so the file descriptors opened in fileno aren't set to self._write_fd/self.read_fd when this line gets called.

The first instance of MocketSocket is created when socket.socket(..) is called: https://github.com/python/cpython/blob/v3.11.6/Lib/asyncio/base_events.py#L950
The second is inside FakeSSLContext.wrap_bio(..): https://github.com/mindflayer/python-mocket/blob/3.12.4/mocket/mocket.py#L137

I wondered if returning a simple proxy object from FakeSSLContext.wrap_bio(..) would work, and that seems to have fixed the aiohttp-related test failures. But it still gets stuck when trying to write a big response to the file descriptor returned by os.pipe().. I'm not used to socket-level programming, but I guess, in the real world, outgoing data gets fed in chunks to avoid choking the socket. I wonder if we should use a temporary file in the filesystem, like how httpretty seems to do.

Call trace of MocketSocket.__init__, fd, and fileno

This was produced by adding hunter calls like this: fix/httpx-long-response...debug-httpx-long-response#diff-2f0c58c6d1983a74d8fcc87ac26dc7ea0add9c921d0512782dab3b2b75d93c19
Ran pip install hunter and invoked

pytest tests/tests38/test_http_aiohttp.py::AioHttpsEntryTestCase::test_no_verify -s 2>hunter.log

Removing 2>hunter.log will let it write to the terminal with pretty colors.

[...]pace/python-mocket/mocket/mocket.py:108:__init__ <= aiohttp/connector.py:922:_make_ssl_context <= aiohttp/connector.py:961:_get_ssl_context <= aiohttp/connector.py:1155:_create_direct_connection <= aiohttp/connector.py:911:_create_connection <= aiohttp/connector.py:544:connect
[...]pace/python-mocket/mocket/mocket.py:108   call      => __init__(self=<mocket.mocket.FakeSSLContext object at 0x7f9b2cd0e550>, sock=<ssl._SSLMethod object at 0x7f9b2f32a0c0>, server_hostname=None, _context=None, *args=(), **kwargs={})
[...]pace/python-mocket/mocket/mocket.py:184:__init__ <= asyncio/base_events.py:950:_connect_sock <= asyncio/base_events.py:1069:create_connection <= aiohttp/connector.py:993:_wrap_create_connection <= aiohttp/connector.py:1205:_create_direct_connection <= aiohttp/connector.py:911:_create_connection
[...]pace/python-mocket/mocket/mocket.py:184   call         => __init__(self=<mocket.mocket.MocketSocket object at 0x7f9b2cd41790>, family=2, type=<socket.SocketKind object at 0x7f9b2fdd2f00>, proto=6, **kwargs={})
[...]pace/python-mocket/mocket/mocket.py:269:fileno <= asyncio/selector_events.py:640:_sock_connect <= asyncio/selector_events.py:632:sock_connect <= asyncio/base_events.py:973:_connect_sock <= asyncio/base_events.py:1069:create_connection <= aiohttp/connector.py:993:_wrap_create_connection
[...]pace/python-mocket/mocket/mocket.py:269   call            => fileno(self=<mocket.mocket.MocketSocket object at 0x7f9b2cd41790>)
[...]pace/python-mocket/mocket/mocket.py:184:__init__ <= mocket/mocket.py:137:wrap_bio <= asyncio/sslproto.py:327:__init__ <= asyncio/selector_events.py:71:_make_ssl_transport <= asyncio/base_events.py:1136:_create_connection_transport <= asyncio/base_events.py:1112:create_connection
[...]pace/python-mocket/mocket/mocket.py:184   call               => __init__(self=<mocket.mocket.MocketSocket object at 0x7f9b2cd41d50>, family=<socket.AddressFamily object at 0x7f9b2fdd18c0>, type=<socket.SocketKind object at 0x7f9b2fdd2f00>, proto=0, **kwargs={})
[...]pace/python-mocket/mocket/mocket.py:269:fileno <= asyncio/selector_events.py:777:__init__ <= asyncio/selector_events.py:925:__init__ <= asyncio/selector_events.py:77:_make_ssl_transport <= asyncio/base_events.py:1136:_create_connection_transport <= asyncio/base_events.py:1112:create_connection
[...]pace/python-mocket/mocket/mocket.py:269   call                  => fileno(self=<mocket.mocket.MocketSocket object at 0x7f9b2cd41790>)
[...]pace/python-mocket/mocket/mocket.py:207:fd <= mocket/mocket.py:307:read <= asyncio/sslproto.py:779:_do_read__copied <= asyncio/sslproto.py:734:_do_read <= asyncio/sslproto.py:602:_on_handshake_complete <= asyncio/sslproto.py:563:_do_handshake
[...]pace/python-mocket/mocket/mocket.py:207   call                     => fd(self=<mocket.mocket.MocketSocket object at 0x7f9b2cd41d50>)
[...]space/python-mocket/mocket/utils.py:16:__init__ <= mocket/mocket.py:210:fd <= mocket/mocket.py:307:read <= asyncio/sslproto.py:779:_do_read__copied <= asyncio/sslproto.py:734:_do_read <= asyncio/sslproto.py:602:_on_handshake_complete
[...]space/python-mocket/mocket/utils.py:16    call                        => __init__(self=<mocket.utils.MocketSocketCore object at 0x7f9b2e4f7240>, initial_bytes=None, w_fd=None)
[...]pace/python-mocket/mocket/mocket.py:266:write <= asyncio/sslproto.py:700:_do_write <= asyncio/sslproto.py:691:_write_appdata <= asyncio/sslproto.py:218:write <= aiohttp/http_writer.py:76:_write <= aiohttp/http_writer.py:130:write_headers
[...]pace/python-mocket/mocket/mocket.py:266   call                           => write(self=<mocket.mocket.MocketSocket object at 0x7f9b2cd41d50>, data=b'GET /anything/ HTTP/1.1\r\nHost: httpbin.localhost\r\nAccept: */*\r\nAccept-Encoding: gzip, deflate\r\nUser-Agent: Python/3.11 aiohttp/3.9.1\r\n\r\n')
[...]ce/python-mocket/mocket/mockhttp.py:45:__init__ <= mocket/mocket.py:645:collect <= mocket/mockhttp.py:203:collect <= mocket/mocket.py:292:sendall <= mocket/mocket.py:430:send <= mocket/mocket.py:267:write
[...]ce/python-mocket/mocket/mockhttp.py:45    call                              => __init__(self=<mocket.mockhttp.Request object at 0x7f9b2cd887d0>, data=b'GET /anything/ HTTP/1.1\r\nHost: httpbin.localhost\r\nAccept: */*\r\nAccept-Encoding: gzip, deflate\r\nUser-Agent: Python/3.11 aiohttp/3.9.1\r\n\r\n')
[...]ce/python-mocket/mocket/mockhttp.py:23:__init__ <= mocket/mockhttp.py:46:__init__ <= mocket/mocket.py:645:collect <= mocket/mockhttp.py:203:collect <= mocket/mocket.py:292:sendall <= mocket/mocket.py:430:send
[...]ce/python-mocket/mocket/mockhttp.py:23    call                                 => __init__(self=<mocket.mockhttp.Protocol object at 0x7f9b2cd88ad0>)
[...]pace/python-mocket/mocket/mocket.py:207:fd <= mocket/mocket.py:301:sendall <= mocket/mocket.py:430:send <= mocket/mocket.py:267:write <= asyncio/sslproto.py:700:_do_write <= asyncio/sslproto.py:691:_write_appdata
[...]pace/python-mocket/mocket/mocket.py:207   call                                    => fd(self=<mocket.mocket.MocketSocket object at 0x7f9b2cd41d50>)
[...]pace/python-mocket/mocket/mocket.py:207:fd <= mocket/mocket.py:302:sendall <= mocket/mocket.py:430:send <= mocket/mocket.py:267:write <= asyncio/sslproto.py:700:_do_write <= asyncio/sslproto.py:691:_write_appdata
[...]pace/python-mocket/mocket/mocket.py:207   call                                       => fd(self=<mocket.mocket.MocketSocket object at 0x7f9b2cd41d50>)
[...]space/python-mocket/mocket/utils.py:20:write <= mocket/mocket.py:302:sendall <= mocket/mocket.py:430:send <= mocket/mocket.py:267:write <= asyncio/sslproto.py:700:_do_write <= asyncio/sslproto.py:691:_write_appdata
[...]space/python-mocket/mocket/utils.py:20    call                                          => write(self=<mocket.utils.MocketSocketCore object at 0x7f9b2e4f7240>, content=b'HTTP/1.1 404 Not Found\r\nStatus: 404\r\nDate: Mon, 19 Feb 2024 01:56:52 GMT\r\nServer: Python/Mocket\r\nConnection: close\r\nContent-length: 0\r\nContent-type: text/plain; charset=utf-8\r\n\r\n')
mocket.utils MocketSocketCore.write write_fd <class 'NoneType'>
[...]pace/python-mocket/mocket/mocket.py:207:fd <= mocket/mocket.py:303:sendall <= mocket/mocket.py:430:send <= mocket/mocket.py:267:write <= asyncio/sslproto.py:700:_do_write <= asyncio/sslproto.py:691:_write_appdata
[...]pace/python-mocket/mocket/mocket.py:207   call                                             => fd(self=<mocket.mocket.MocketSocket object at 0x7f9b2cd41d50>)
[...]pace/python-mocket/mocket/mocket.py:207:fd <= mocket/mocket.py:304:sendall <= mocket/mocket.py:430:send <= mocket/mocket.py:267:write <= asyncio/sslproto.py:700:_do_write <= asyncio/sslproto.py:691:_write_appdata
[...]pace/python-mocket/mocket/mocket.py:207   call                                                => fd(self=<mocket.mocket.MocketSocket object at 0x7f9b2cd41d50>)
[...]pace/python-mocket/mocket/mocket.py:207:fd <= mocket/mocket.py:307:read <= asyncio/sslproto.py:779:_do_read__copied <= asyncio/sslproto.py:734:_do_read <= asyncio/sslproto.py:638:_do_flush <= asyncio/sslproto.py:625:_start_shutdown
[...]pace/python-mocket/mocket/mocket.py:207   call                                                   => fd(self=<mocket.mocket.MocketSocket object at 0x7f9b2cd41d50>)
[...]pace/python-mocket/mocket/mocket.py:207:fd <= mocket/mocket.py:307:read <= asyncio/sslproto.py:779:_do_read__copied <= asyncio/sslproto.py:734:_do_read <= asyncio/sslproto.py:638:_do_flush <= asyncio/sslproto.py:625:_start_shutdown
[...]pace/python-mocket/mocket/mocket.py:207   call                                                      => fd(self=<mocket.mocket.MocketSocket object at 0x7f9b2cd41d50>)

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fd and fileno get called against different instances of MocketSocket, so the file descriptors opened in fileno aren't set to self._write_fd/self.read_fd when this line gets called.

The big problem I have is that different clients behave in a complete different way. So, I managed to make it work for one or the other, but not both. I'll have a deeper look at what you found out and let you know if I made some progress.
Thanks!

Copy link

sonarcloud bot commented May 13, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@mindflayer
Copy link
Owner Author

mindflayer commented May 13, 2024

...
If the implementation is hard to explain, it's a bad idea.
If the implementation is easy to explain, it may be a good idea.
...

I've just started to tackle this problem again from scratch.

@mindflayer mindflayer closed this May 13, 2024
@mindflayer mindflayer deleted the fix/httpx-long-response branch May 13, 2024 09:05
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.

None yet

3 participants