[1.26] Improve error message when calling urllib3.request() by sg3-141-592 · Pull Request #3058 · urllib3/urllib3

sg3-141-592 · 2023-06-02T21:04:39Z

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 · 2023-06-07T20:05:49Z

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 · 2023-06-08T16:01:53Z

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 · 2023-06-10T05:03:41Z

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 · 2023-06-10T09:52:37Z

Sure @pquentin , that's just been fixed.

pquentin

Thanks! I left a small nit.

sg3-141-592 · 2023-06-12T14:16:36Z

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.

sg3-141-592 force-pushed the improving-error-message-requests branch from c0eb262 to 6c65276 Compare June 2, 2023 21:08

sg3-141-592 changed the title ~~Improving error message when importing requests directory~~ Improving error message when importing requests directly 1.26.x Jun 2, 2023

sg3-141-592 force-pushed the improving-error-message-requests branch 2 times, most recently from 64c64d2 to 28277f9 Compare June 3, 2023 12:18

Improving error message when trying to import requests

21e7aa2

sg3-141-592 force-pushed the improving-error-message-requests branch from 28277f9 to 21e7aa2 Compare June 3, 2023 15:20

sg3-141-592 marked this pull request as ready for review June 3, 2023 15:48

sg3-141-592 changed the title ~~Improving error message when importing requests directly 1.26.x~~ WIP: Improving error message when importing requests directly 1.26.x Jun 3, 2023

sg3-141-592 added 2 commits June 3, 2023 19:28

Setting warnings for Python 3 only

8139a15

linting fix

7d3969e

sg3-141-592 force-pushed the improving-error-message-requests branch from f56512a to 7d3969e Compare June 3, 2023 19:38

sg3-141-592 changed the title ~~WIP: Improving error message when importing requests directly 1.26.x~~ Improving error message when importing requests directly 1.26.x Jun 5, 2023

illia-v reviewed Jun 7, 2023

View reviewed changes

sg3-141-592 force-pushed the improving-error-message-requests branch from 001a028 to 7798537 Compare June 7, 2023 20:00

Updating in response to review

5aa78bb

sg3-141-592 force-pushed the improving-error-message-requests branch from 7798537 to 5aa78bb Compare June 7, 2023 20:07

illia-v requested changes Jun 8, 2023

View reviewed changes

Comment thread src/urllib3/request.py

Comment thread src/urllib3/request.py Outdated

Comment thread test/test_request.py Outdated

Comment thread src/urllib3/request.py Outdated

Update in response to review

36e5d2b

sg3-141-592 force-pushed the improving-error-message-requests branch from aa2c9ea to 36e5d2b Compare June 8, 2023 20:45

sg3-141-592 requested a review from illia-v June 8, 2023 20:55

illia-v previously approved these changes Jun 9, 2023

View reviewed changes

sg3-141-592 dismissed illia-v’s stale review via e2e2824 June 10, 2023 09:27

Updating test to use locally packaged six

d9dd714

sg3-141-592 force-pushed the improving-error-message-requests branch from e2e2824 to d9dd714 Compare June 10, 2023 09:32

pquentin reviewed Jun 10, 2023

View reviewed changes

Comment thread src/urllib3/request.py

pquentin previously approved these changes Jun 10, 2023

View reviewed changes

Comment thread src/urllib3/request.py Outdated

Added link to migration guide

abfa32b

sg3-141-592 dismissed pquentin’s stale review via abfa32b June 10, 2023 18:33

pquentin approved these changes Jun 12, 2023

View reviewed changes

pquentin changed the title ~~Improving error message when importing requests directly 1.26.x~~ [1.26] Improving error message when calling urllib3.request() Jun 12, 2023

pquentin changed the title ~~[1.26] Improving error message when calling urllib3.request()~~ [1.26] Improve error message when calling urllib3.request() Jun 12, 2023

pquentin merged commit 57181d6 into urllib3:1.26.x Jun 12, 2023

data-sync-user mentioned this pull request Feb 9, 2025

Bump urllib3 from 1.26.14 to 1.26.18 mozilla/opmon#170

Open

Navigation Menu

Repository navigation

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

[1.26] Improve error message when calling urllib3.request()#3058
pquentin merged 7 commits into
urllib3:1.26.xfrom
sg3-141-592:improving-error-message-requests

sg3-141-592 commented Jun 2, 2023 •

edited

Loading

Uh oh!

illia-v left a comment

Uh oh!

sg3-141-592 commented Jun 7, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

illia-v commented Jun 8, 2023

Uh oh!

illia-v left a comment

Uh oh!

pquentin commented Jun 10, 2023

Uh oh!

sg3-141-592 commented Jun 10, 2023

Uh oh!

Uh oh!

pquentin left a comment

Uh oh!

Uh oh!

sg3-141-592 commented Jun 12, 2023

Uh oh!

pquentin left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

sg3-141-592 commented Jun 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

illia-v left a comment

Choose a reason for hiding this comment

Uh oh!

sg3-141-592 commented Jun 7, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

illia-v commented Jun 8, 2023

Uh oh!

illia-v left a comment

Choose a reason for hiding this comment

Uh oh!

pquentin commented Jun 10, 2023

Uh oh!

sg3-141-592 commented Jun 10, 2023

Uh oh!

Uh oh!

pquentin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sg3-141-592 commented Jun 12, 2023

Uh oh!

pquentin left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sg3-141-592 commented Jun 2, 2023 •

edited

Loading