Potentially simplify client middleware default args to () instead of `None`

[bdraco]

It might simplify the code checks and optimise slightly if we disallow None here and just default to ().

Originally posted by @Dreamsorcerer in #9732 (comment)

[bdraco]

How should we handle the case were we want to override the middleware in the client request to disable it if the default is also a ()?

[Dreamsorcerer]

How should we handle the case were we want to override the middleware in the client request to disable it if the default is also a ()?

I guess the request method would default to None. The code handling middlewares would still never receive None, as it'd receive the session middlewares if the request middlewares is None.

[bdraco]

This does work with an empty tuple for ClientSession, but after looking at the broader implications, I think it's best to keep the current approach for now because we still need to check if middlewares is None in

                    effective_middlewares = (
                        self._middlewares if middlewares is None else middlewares
                    )

to know if we should suppress the ClientSession middleware.

Since it doesn’t simplify the logic and might introduce a bit of ambiguity, I’d lean toward leaving it as-is. But happy to revisit if you think there’s a cleaner way to handle it.

[Dreamsorcerer]

That check is on the .request() middlewares. But, effective_middlewares is then processed later on using a for loop, which is currently guarded with another None check due to the session middlewares None possibility.

i.e. If we disallow None at the session level, then effective_middlewares can never be None, which means all code after that point can avoid any extra checks.

[Dreamsorcerer]

e.g. This check could be removed:

aiohttp/aiohttp/client.py

Line 695 in 5dcb36a

if effective_middlewares:

[bdraco]

e.g. This check could be removed:

aiohttp/aiohttp/client.py

Line 695 in 5dcb36a

if effective_middlewares:

Doesn't that mean we push everybody though build_client_middlewares and make them pay the cost of that function even if they have no middleware?

[Dreamsorcerer]

Is there a notable cost? Once the caching is sorted, the only code that would be executed is:

for middleware in reversed(()):
    ... # not executed
return handler

[bdraco]

Its not significant but any additional calls in the client request path do add up. How about we change it to () for ClientSession and leave the existing code in place than change it over once we have the cache in place in 4.0 so its not a breaking change later?

[Dreamsorcerer]

Yeah, I'm not too bothered about backporting it.