Skip to content

Wrong type for hdrs in urllib.error.HTTPError #10092

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

Open
bdrung opened this issue Apr 27, 2023 · 4 comments
Open

Wrong type for hdrs in urllib.error.HTTPError #10092

bdrung opened this issue Apr 27, 2023 · 4 comments
Labels
stubs: false positive Type checkers report false errors

Comments

@bdrung
Copy link

bdrung commented Apr 27, 2023

Example code code.py:

import urllib.error
raise urllib.error.HTTPError("https://example.com/", 502, "Bad Gateway", hdrs={}, fp=None)

mypy complains:

code.py:2: error: Argument "hdrs" to "HTTPError" has incompatible type "Dict[<nothing>, <nothing>]"; expected "Message"  [arg-type]

Looking at the Python stdlib code, hdrs is always a dict[str, str].

@srittau
Copy link
Collaborator

srittau commented Apr 27, 2023

Where do you see that in the stdlib code? The only instances I see where HTTPError is instantiated are in urllib/request.py, which just passes the headers on from the various handlers. Those handlers get passed HTTPMessage objects - at least according to our stubs. Those are not dicts.

@bdrung
Copy link
Author

bdrung commented Apr 27, 2023

All those functions in urllib.requests raises urllib.error.HTTPError with hdrs=headers:

  • HTTPRedirectHandler.redirect_request
  • HTTPRedirectHandler.http_error_302
  • AbstractDigestAuthHandler.http_error_auth_reqed
  • URLopener.http_error_default
  • FancyURLopener.redirect_internal

urllib.requests.Request sets:

self.headers = {}

and then adds headers with:

    def add_header(self, key, val):
        # useful for something like authentication
        self.headers[key.capitalize()] = val

@srittau
Copy link
Collaborator

srittau commented Apr 27, 2023

I'm not sure why our stubs say the handler methods expect a Message. The documentation says: "... hdrs will be a mapping object ..." and I couldn't find any relevant urllib code that uses (HTTP)Message. (urlretrieve() and noheaders() return a Message, but they are not called from within urllib.)

So I think the best option is to indeed change HTTPError.hdrs and the arguments to the methods to MutableMapping or Mapping. Any PRs to improve the situation are welcome!

@srittau srittau added the stubs: false positive Type checkers report false errors label Apr 27, 2023
@aminalaee
Copy link
Contributor

aminalaee commented Sep 20, 2023

Checking the history, the PR which added this is: #5854

And trying a sample code, looks like (HTTP)Message is the correct type, which also implements some Mapping methods, but it's not the other way around.

import urllib.error
import urllib.request

try:
    urllib.request.urlopen('https://jigsaw.w3.org/HTTP/Basic/')
except urllib.error.HTTPError as ex:
    print(type(ex.headers))  # <class 'http.client.HTTPMessage'>

Which is according to the docs:
https://docs.python.org/3/library/urllib.error.html#urllib.error.HTTPError.headers
https://docs.python.org/3/library/http.client.html#httpmessage-objects

@srittau Do you still think the change is required?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stubs: false positive Type checkers report false errors
Projects
None yet
Development

No branches or pull requests

3 participants