Creating `SSLContext` at import time makes mocking "impossible"

[mindflayer]

Is your feature request related to a problem?

Hi there, I am the author of Mocket.

Mocket is probably the only tool around able to mock non-blocking sockets, and it's also listed under https://docs.aiohttp.org/en/stable/third_party.html.

With aiohttp==3.10.6 you moved the creation of SSLContext at import time.
This makes mocking HTTPS calls impossible if applying an agnostic approach which works for every client (mocking socket and ssl).
To fix that on my side I'd have to change Mocket for mocking aiohttp internals: _SSL_CONTEXT_VERIFIED and _SSL_CONTEXT_UNVERIFIED.

Describe the solution you'd like

Just an hint to understand my point. I leave to you folks the decision about the possible change.

Change _make_ssl_context (in connector.py) to make it fail (e.g. add 1 / 0 as its first line).

>>> import aiohttp
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/tmp/aiohttp/__init__.py", line 6, in <module>
    from .client import (
  File "/tmp/aiohttp/client.py", line 85, in <module>
    from .connector import (
  File "/tmp/aiohttp/connector.py", line 757, in <module>
    _SSL_CONTEXT_VERIFIED = _make_ssl_context(True)
                            ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/employee/repos/python-mocket/aiohttp/connector.py", line 737, in _make_ssl_context
    1 / 0
    ~~^~~
ZeroDivisionError: division by zero

Describe alternatives you've considered

I don't like the idea of stopping supporting aiohttp, but I don't want Mocket to adapt to clients' internals, because it's exactly the opposite approach (mocking socket/ssl VS mocking clients).

Related component

Client

Additional context

No response

Code of Conduct

• I agree to follow the aio-libs Code of Conduct

[Dreamsorcerer]

I'm not overly clear what the issue is. Are you saying you don't mock aiohttp, but mock the ssl calls? So, in this case you're trying to mock the creation of the SSLContext?

In that case, I'm not sure this worked correctly on previous releases, as the previous function used a cache decorator. Therefore, I'm assuming if you created a ClientSession before mocket, then even if you created another ClientSession with mocket it would still get the previous (unmocked) context. Likewise, if you created one with mocket first, then created one outside of the mock scope, the second one would still have the mocked context.

As the context will always be the same, and it's a blocking, expensive call, we wouldn't want to recreate the context every time.

[mindflayer]

Why not simply making them into @property? Something like:

class TCPConnector(BaseConnector):
    _SSL_CONTEXT_VERIFIED = None
    ...

    @property
    def ssl_context_verified(self):
        if not self._SSL_CONTEXT_VERIFIED:
            self._SSL_CONTEXT_VERIFIED = ...
        return self._SSL_CONTEXT_VERIFIED
...

In this case you only create them if you actually need to do it, making importing the client faster as a bonus.

[mindflayer]

I'm not overly clear what the issue is. Are you saying you don't mock aiohttp, but mock the ssl calls? So, in this case you're trying to mock the creation of the SSLContext?

Correct, I both mock socket and ssl modules and that's enough for mocking "any" client.

[Dreamsorcerer]

That wouldn't be cached, it'd be per connector. Again, the previous implementation used a cached function to create them. We moved them to import time to avoid blocking calls at runtime, but the more important thing here is that the contexts are reused across sessions/connectors. Homeassistant can use a lot of sessions, so that's one particular area helping to shape these decisions. @bdraco can provide some additional background.

If we simply revert the change, then I think the behaviour of your library would still be incorrect..

[mindflayer]

If we simply revert the change, then I think the behaviour of your library would still be incorrect..

Before v .6 everything worked perfectly. Mocket is currently used for mocking aiohttp calls.

[Dreamsorcerer]

But, did you actually test the scenarios I described?

Because if they work, then I don't understand how...

[mindflayer]

Mocket is normally used in situations like:

import aiohttp
import pytest
from mocket import mocketize

@mocketize
def test_a():
    ...

@mocketize
def test_b():
    ...

That's the use-case I am trying to fix, and it worked before. To make those tests work after the change, I have to move import aiohttp under the tests using it.

[Dreamsorcerer]

Yes, I understand that, but each of those decorators is meant to be a context, right?

So, doesn't it break (on previous releases) if we do this?

def test_a():
    """Should not be mocked."""
    async with ClientSession() as sess:
        ....

@mocketize
def test_b():
    """Should be mocked."""
    async with ClientSession() as sess:
        ...

My expectation is that both of those tests will use the same context, the mocked one if test_b runs first or the real one if test_a runs first.

[mindflayer]

Even when in control, Mocket lets the real traffic pass, so nothing breaks. In your non-decorated test, you'd have real socket/context instances doing their normal job.

[Dreamsorcerer]

OK, so not a real problem if test_b runs first. What if test_a runs first, won't the mocked test fail?

[mindflayer]

Let me write down a real example mimicking what you have in mind and come back to you.

[mindflayer]

You were right, and now that I read again the previous implementation I understand why.
It was simply something that we never noticed. So, previously that use-case was broken (normally during tests you don't want to hit the network), v .6 broke any attempt to mock calls.

[bdraco]

We have the following high level requirements with creating the SSLContexts (probably a few more but these are the top ones):

1. We need to avoid all blocking I/O in the event loop to load ssl certificates from disk.
2. We need to support multiple different event loops: Regression in aiohttp 3.10.3 when running multiple loops in multiple threads #9202
3. We need to create the SSLContexts only once
4. We should not incur additional overhead to check if the SSLContext needs to be created

Currently, the only place I'm aware of where we don't have to worry about blocking I/O or if multiple event loops are running is at import time. If you have a better solution that meets the above high level requirements, we would certainly be open to implementing it or accepting a PR.