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

aiohttp 3.10.0 may have broken ProxyConnector implementation? #43

Closed
vEpiphyte opened this issue Jul 31, 2024 · 10 comments
Closed

aiohttp 3.10.0 may have broken ProxyConnector implementation? #43

vEpiphyte opened this issue Jul 31, 2024 · 10 comments

Comments

@vEpiphyte
Copy link

The happyeyeballs support ( added here aio-libs/aiohttp@1700e9d aio-libs/aiohttp#7954 aio-libs/aiohttp#8005 ) changed the call signature of the TCPConnector._wrap_create_connection function.

Using a proxy like this in aiohttp 3.9.5 would result in a timeout when connecting.

proxy='socks5://user:pass@127.0.0.1:1'  # Example proxy value that would previously timeout connecting
connector = aiohttp_socks.ProxyConnector.from_url(proxy)
async with aiohttp.ClientSession(connector=connector) as sess:
    async with sess.request('GET', url) as resp:
        # do stuff with response

In aiohttp 3.10.0 this now raises a TypeError:

Traceback (most recent call last):
  File "/some/path/file.py", line 1729, in foo
    async with sess.request('GET', url) as resp:
  File "/some/venv/lib/python3.11/site-packages/aiohttp/client.py", line 1344, in __aenter__
    self._resp = await self._coro
                 ^^^^^^^^^^^^^^^^
  File "/some/venv/lib/python3.11/site-packages/aiohttp/client.py", line 648, in _request
    conn = await self._connector.connect(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/some/venv/lib/python3.11/site-packages/aiohttp/connector.py", line 546, in connect
    proto = await self._create_connection(req, traces, timeout)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/some/venv/lib/python3.11/site-packages/aiohttp/connector.py", line 954, in _create_connection
    _, proto = await self._create_direct_connection(req, traces, timeout)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/some/venv/lib/python3.11/site-packages/aiohttp/connector.py", line 1282, in _create_direct_connection
    transp, proto = await self._wrap_create_connection(
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: ProxyConnector._wrap_create_connection() missing 2 required positional arguments: 'host' and 'port'
@romis2012
Copy link
Owner

Fixed in v0.9.0

@vEpiphyte
Copy link
Author

@romis2012 Thanks! I missed that - I thought I had installed the latest aiohttp-socks when I was testing. I'll go ahead and close this.

@thecockatiel
Copy link

Hi @romis2012
is there any way we get support for older versions automatically?
or are you open to such PRs ?

@romis2012
Copy link
Owner

@thecockatiel
aiohttp-socks<0.9 is compatible with aiohttp<3.10.0 and
aiohttp-socks>=0.9 is compatible with aiohttp>=3.10.0

@thecockatiel
Copy link

why not make all versions compatible ?
it's pretty easy
I'll send a PR soon, maybe you like how small the needed change is

@romis2012
Copy link
Owner

why not make all versions compatible ?

Because there's no need for that.

I'll send a PR soon, maybe you like how small the needed change is

The PR will be rejected. Don't waste your time.

@thecockatiel
Copy link

Well, here's a bit of context, but it's okay if you still don't like it and want to reject:
I am working on a project, which aims to be able to run on debian bullseye without the need to install packages outside of the ones the repository provides, which means the aiohttp version there will be old
aiohttp-socks is missing there, so I have included the project in my source directly
I work on different machines and the one I use have modern packages
so just switching to the debian breaks the project

hence this suggestion

It was a very small change, but It's okay if you don't want it. thanks for creating this useful lib :)

@romis2012
Copy link
Owner

@thecockatiel
just use aiohttp-socks==0.8.*

@romis2012
Copy link
Owner

@thecockatiel
sorry, but we don't have the ability to support all possible versions of aiohttp

@thecockatiel
Copy link

It's okay
I mean all versions is unrealistic, but supporting more versions is nice, but it's just "nice"

as I have said before, thanks for creating this useful lib,
even if this change is not good enough to make it in the official repo, the patch certainly helps me as I have copied the files in my project

so no worries

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

No branches or pull requests

3 participants