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

Throws errors with socks module #1147

Closed
infodox opened this issue Jan 27, 2013 · 10 comments
Closed

Throws errors with socks module #1147

infodox opened this issue Jan 27, 2013 · 10 comments

Comments

@infodox
Copy link

infodox commented Jan 27, 2013

Hey,
I have been trying to get requests to play nice with the "Socks" module (from http://code.google.com/p/socksipy-branch/). urllib3 and httplib work fine with it, but the requests wrapper does not. Here is an example.

Python 2.7.3 (default, Sep 26 2012, 21:51:14) 
[GCC 4.7.2] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import socks
>>> import requests
>>> socks.setdefaultproxy(socks.PROXY_TYPE_SOCKS5, 'localhost', 9050)
>>> socks.wrapmodule(requests)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "socks.py", line 107, in wrapmodule
    module.socket.socket = socksocket
AttributeError: 'module' object has no attribute 'socket'
>>> 

Hopefully there is a workaround for this :) Socks works with almost every other module, so it makes no sense for it to not work with requests.

@kennethreitz
Copy link
Contributor

Very interesting.

I wonder if this is a bug with socks or not.

@sigmavirus24
Copy link
Contributor

It looks like wrapmodule (from the trace back) is trying to monkey patch the socket stdlib. We don't use it directly, so if it isn't expecting our use of urllib3 then it makes sense it would fail.

@sigmavirus24
Copy link
Contributor

Yep. wrapmodule is expecting that requests have the socket library in the top-level namespace: https://code.google.com/p/socksipy-branch/source/browse/trunk/socks.py#109

The author should probably use inspect to see where (if anywhere) a package or module is using the socket library. I'm of the opinion that this is an issue with socksipy module rather than with requests.

@kennethreitz
Copy link
Contributor

+1. I'd be happy to reasonably accomidate, but we can't do so easily in this case.

@sigmavirus24
Copy link
Contributor

I wonder if there is a way to import it from urllib3 so it would properly work.

@kennethreitz
Copy link
Contributor

considering the popularity of requests, I think adding special support for it in socks would be a good idea :)

@sigmavirus24
Copy link
Contributor

Hm, are you sure this works with urllib3? I can understand why it would work for httplib. The import is right there, but I see no import of socket that would be reachable from simply import urllib3.

And yeah @kennethreitz I think it would be great too, that's why I'm exploring it.

@kennethreitz
Copy link
Contributor

I mean for him to do the work instead of us :)

@sigmavirus24
Copy link
Contributor

Yeah @kennethreitz that would always be the ideal case. :)

@infodox, in a clean virtualenv I just installed urllib3 (1.5 from pypi) and downloaded the socks.py file.

This is my interactive session:

>>> import socks
>>> import urllib3
>>> socks.wrapmodule(urllib3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "socks.py", line 111, in wrapmodule
    raise GeneralProxyError((4, "no proxy specified"))
socks.GeneralProxyError: (4, 'no proxy specified')
>>> socks._defaultproxy = True
>>> socks.wrapmodule(urllib3)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "socks.py", line 109, in wrapmodule
    module.socket.socket = socksocket
AttributeError: 'module' object has no attribute 'socket'

Are you sure it works fine on urllib3?

It might work if you did: socks.wrapmodule(urllib3.connectionpool) and socks.wrapmodule(urllib3.util) but it definitely doesn't work using only urllib3.

As for us, I think we could import socket from urllib3.connectionpool and it would work since that is the first import in __init__.py, I'm just not sure if this would pollute the requests namespace.

@sigmavirus24
Copy link
Contributor

@infodox, having given it some thought, I honestly think that socks.py could be written better. In short, if they want to patch the socket module, they're probably going to want to do so for the entire process, not just for one module. They should be replacing the socket entry in sys.modules, not trying to rely on a module having socket in their namespace. As such, I'm +1 on closing this unless @kennethreitz wants to add a reference to the socket stdlib to requests, i.e., in requests/__init__.py putting import socket.

@kennethreitz kennethreitz reopened this Feb 10, 2013
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants