[1.26] Improve error message when calling urllib3.request()

[sg3-141-592]

My attempt at closing #3046 . Sharing in-case someone recognises the issue.

I created class request to wrap RequestMethods and used the suggested method of overriding the module with an instance of the request class.

Added a unit test to ensure that a warning is raised for the user when trying to call urllib3.request().

This is working in everything but Python 2.7. I'm having a nightmare trying to fix some sort of class namespace issue. Namespaces and imports appear to work differently in Python 2.7, so any imports are inaccessible from inside the class. warning, urlencode and encode_multipart_formdata are failing because these imports aren't visible, we're just getting NoneType for them.

________________ TestRequestImport.test_request_import_warning _________________
Traceback (most recent call last):
  File "/Users/runner/work/urllib3/urllib3/test/test_request.py", line 11, in test_request_import_warning
    urllib3.request(1, a=2)
  File "/Users/runner/work/urllib3/urllib3/.nox/test-2-7/lib/python2.7/site-packages/_pytest/recwarn.py", line 227, in __exit__
    if all(a is None for a in exc_info):
  File "/Users/runner/work/urllib3/urllib3/.nox/test-2-7/lib/python2.7/site-packages/_pytest/recwarn.py", line 227, in <genexpr>
    if all(a is None for a in exc_info):
TypeError: expected string or Unicode object, NoneType found

[illia-v]

@sg3-141-592 thanks for looking into this!

Shouldn't it still be the same type of error with a clearer message though?

---

BTW, this little tweak based on https://stackoverflow.com/a/48100440 seems to preserve the module type and some attributes of urllib3.request which somebody may depend on based on the Hyrum's law 🙂

diff --git a/src/urllib3/request.py b/src/urllib3/request.py
index a84c0e46..67c05bc9 100644
--- a/src/urllib3/request.py
+++ b/src/urllib3/request.py
@@ -176,7 +176,7 @@ class RequestMethods(object):
 
 if not six.PY2:
 
-    class RequestModule(object):
+    class RequestModule(sys.modules[__name__].__class__):
         def __call__(self, *args, **kwargs):
             """
             If user tries to call this module directly urllib3 v2.x style raise an error to the user
@@ -188,4 +188,4 @@ if not six.PY2:
 
         RequestMethods = RequestMethods
 
-    sys.modules[__name__] = RequestModule()
+    sys.modules[__name__].__class__ = RequestModule

1.26.x

>>> import urllib3
>>> urllib3.request
<module 'urllib3.request' from '/home/user/urllib3/src/urllib3/request.py'>
>>> urllib3.request()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: 'module' object is not callable
>>> dir(urllib3.request)
['RequestMethods', '__all__', '__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__spec__', 'absolute_import', 'encode_multipart_formdata', 'urlencode']

Your branch

>>> import urllib3
>>> urllib3.request
<urllib3.request.RequestModule object at 0x7f514416d090>
>>> urllib3.request()
/home/user/urllib3/src/urllib3/request.py:184: UserWarning: urllib3.requests() method is not supported in this release
upgrade to urllib3 v2 to use it
  warnings.warn(
>>> dir(urllib3.request)
['RequestMethods', '__call__', '__class__', '__delattr__', '__dict__', '__dir__', '__doc__', '__eq__', '__format__', '__ge__', '__getattribute__', '__getstate__', '__gt__', '__hash__', '__init__', '__init_subclass__', '__le__', '__lt__', '__module__', '__ne__', '__new__', '__reduce__', '__reduce_ex__', '__repr__', '__setattr__', '__sizeof__', '__str__', '__subclasshook__', '__weakref__']

>>> from urllib3.request import urlencode
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: cannot import name 'urlencode' from '<unknown module name>' (unknown location)

Your branch with the patch

>>> import urllib3
>>> urllib3.request
<module 'urllib3.request' from '/home/user/urllib3/src/urllib3/request.py'>
>>> urllib3.request()
/home/user/urllib3/src/urllib3/request.py:184: UserWarning: urllib3.requests() method is not supported in this release
upgrade to urllib3 v2 to use it
  warnings.warn(
>>> dir(urllib3.request)
['RequestMethods', 'RequestModule', '__all__', '__builtins__', '__cached__', '__doc__', '__file__', '__loader__', '__name__', '__package__', '__spec__', '__warningregistry__', 'absolute_import', 'encode_multipart_formdata', 'six', 'sys', 'urlencode', 'warnings']

>>> from urllib3.request import urlencode
>>> # urlencode imported successfully.
>>> from urllib3.request import foo
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: cannot import name 'foo' from 'urllib3.request' (/home/user/urllib3/src/urllib3/request.py)

This is working in everything but Python 2.7. I'm having a nightmare trying to fix some sort of class namespace issue. Namespaces and imports appear to work differently in Python 2.7, so any imports are inaccessible from inside the class. warning, urlencode and encode_multipart_formdata are failing because these imports aren't visible, we're just getting NoneType for them.

But I am not sure if the change conflicts with what you described here, could you please clarify this?

[sg3-141-592]

Great suggestion @illia-v , I can recreate what you've described in Python 3. I've added the change and added a unit test that captures the from urllib3.request import urlencode issue.

The change you've suggested isn't working with Python 2 currently. I'm seeing

  File "/workspace/.pyenv_mirror/user/current/lib/python2.7/site-packages/urllib3/request.py", line 192, in <module>
    sys.modules[__name__].__class__ = RequestModule
TypeError: __class__ assignment: only for heap types

I've had a go at updating the original module error message as suggested, instead of the current warning you've now got.

>>> urllib3.request()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/workspace/urllib3/src/urllib3/request.py", line 185, in __call__
    raise TypeError(
TypeError: TypeError: 'module' object is not callable
urllib3.requests() method is not supported in this release, upgrade to urllib3 v2 to use it

Happy to take suggestions on error message wording.

[illia-v]

The change you've suggested isn't working with Python 2 currently. I'm seeing

  File "/workspace/.pyenv_mirror/user/current/lib/python2.7/site-packages/urllib3/request.py", line 192, in <module>
    sys.modules[__name__].__class__ = RequestModule
TypeError: __class__ assignment: only for heap types

Right, it's said to work with 3.5+

[illia-v]

@sg3-141-592 thanks, LGTM!

[pquentin]

Can you please make sure to never import six directly but use our packaged version, in code and tests?

The CI only works by accident right now because one of our test dependencies depends on six.

[sg3-141-592]

Sure @pquentin , that's just been fixed.

[pquentin]

Thanks! I left a small nit.

[sg3-141-592]

Sure @pquentin , users will now see

TypeError: 'module' object is not callable
urllib3.request() method is not supported in this release, upgrade to urllib3 v2 to use it
see https://urllib3.readthedocs.io/en/stable/v2-migration-guide.html

[pquentin]

Thanks! LGTM.