GeezerVison is showing github.com — `urllib.error.HTTPError(..., fp=None)` raises a `KeyError` instead of an `AttributeError` on attribute access · Issue #98778 · python/cpython
urllib.error.HTTPError(..., fp=None) raises a KeyError instead of an AttributeError on attribute access #98778
DescriptionBug report
The exception urllib.error.HTTPError(..., fp=None) raises a KeyError instead of an AttributeError when accessing an attribute that does not exist.
>>> from urllib.error import HTTPError
>>> x = HTTPError("url", 405, "METHOD NOT ALLOWED", None, None)
>>> assert getattr(x, "__notes__", ()) == ()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/home/vinmic/repos/cpython/Lib/tempfile.py", line 477, in __getattr__
file = self.__dict__['file']
~~~~~~~~~~~~~^^^^^^^^
KeyError: 'file'Your environment
Python 3.12.0a1+ (heads/main:bded5edd9a, Oct 27 2022, 19:23:58) [GCC 11.3.0] on linux
This bug should be reproducible for all python3 versions on all systems.
Context
I found this error while running a code similar to this:
from logging import getLogger
from urllib.error import HTTPError
try:
raise HTTPError("url", 405, "METHOD NOT ALLOWED", None, None)
except Exception:
getLogger().exception("Ooops")Instead of having the exception logged, I ended up with the following trace:
--- Logging error ---
Traceback (most recent call last):
File "/home/vinmic/repos/cpython/../test_trio/test_trio.py", line 6, in <module>
raise HTTPError("url", 405, "METHOD NOT ALLOWED", None, None)
urllib.error.HTTPError: HTTP Error 405: METHOD NOT ALLOWED
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/vinmic/repos/cpython/Lib/logging/__init__.py", line 1160, in emit
msg = self.format(record)
^^^^^^^^^^^^^^^^^^^
File "/home/vinmic/repos/cpython/Lib/logging/__init__.py", line 999, in format
return fmt.format(record)
^^^^^^^^^^^^^^^^^^
File "/home/vinmic/repos/cpython/Lib/logging/__init__.py", line 711, in format
record.exc_text = self.formatException(record.exc_info)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/vinmic/repos/cpython/Lib/logging/__init__.py", line 661, in formatException
traceback.print_exception(ei[0], ei[1], tb, None, sio)
File "/home/vinmic/repos/cpython/Lib/traceback.py", line 124, in print_exception
te = TracebackException(type(value), value, tb, limit=limit, compact=True)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/vinmic/repos/cpython/Lib/traceback.py", line 697, in __init__
self.__notes__ = getattr(exc_value, '__notes__', None)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/vinmic/repos/cpython/Lib/tempfile.py", line 477, in __getattr__
file = self.__dict__['file']
~~~~~~~~~~~~~^^^^^^^^
KeyError: 'file'
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/vinmic/repos/cpython/../test_trio/test_trio.py", line 8, in <module>
getLogger().exception("Ooops")
File "/home/vinmic/repos/cpython/Lib/logging/__init__.py", line 1574, in exception
self.error(msg, *args, exc_info=exc_info, **kwargs)
File "/home/vinmic/repos/cpython/Lib/logging/__init__.py", line 1568, in error
self._log(ERROR, msg, args, **kwargs)
File "/home/vinmic/repos/cpython/Lib/logging/__init__.py", line 1684, in _log
self.handle(record)
File "/home/vinmic/repos/cpython/Lib/logging/__init__.py", line 1700, in handle
self.callHandlers(record)
File "/home/vinmic/repos/cpython/Lib/logging/__init__.py", line 1770, in callHandlers
lastResort.handle(record)
File "/home/vinmic/repos/cpython/Lib/logging/__init__.py", line 1028, in handle
self.emit(record)
File "/home/vinmic/repos/cpython/Lib/logging/__init__.py", line 1168, in emit
self.handleError(record)
File "/home/vinmic/repos/cpython/Lib/logging/__init__.py", line 1082, in handleError
traceback.print_exception(t, v, tb, None, sys.stderr)
File "/home/vinmic/repos/cpython/Lib/traceback.py", line 124, in print_exception
te = TracebackException(type(value), value, tb, limit=limit, compact=True)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/vinmic/repos/cpython/Lib/traceback.py", line 763, in __init__
context = TracebackException(
^^^^^^^^^^^^^^^^^^^
File "/home/vinmic/repos/cpython/Lib/traceback.py", line 697, in __init__
self.__notes__ = getattr(exc_value, '__notes__', None)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/vinmic/repos/cpython/Lib/tempfile.py", line 477, in __getattr__
file = self.__dict__['file']
~~~~~~~~~~~~~^^^^^^^^
KeyError: 'file'
Note however that the exception is logged properly on python 3.9 (without the exceptiongroup module imported). So maybe the following patch should be applied on top of HTTPError being fixed in order to make tracebacks more robust:
diff --git a/Lib/traceback.py b/Lib/traceback.py
index 6270100348..a9c15d59be 100644
--- a/Lib/traceback.py
+++ b/Lib/traceback.py
@@ -694,7 +694,10 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None,
# Capture now to permit freeing resources: only complication is in the
# unofficial API _format_final_exc_line
self._str = _safe_string(exc_value, 'exception')
- self.__notes__ = getattr(exc_value, '__notes__', None)
+ try:
+ self.__notes__ = getattr(exc_value, '__notes__', None)
+ except Exception:
+ self.__notes__ = None
if exc_type and issubclass(exc_type, SyntaxError):
# Handle SyntaxError's speciallyLinked PRs
Activityserhiy-storchaka commented on Oct 28, 2022 The problem is that HTTPError is a subclass of tempfile._TemporaryFileWrapper, which is not properly initialized if fp is None.
|
if fp is not None: |
|
self.__super_init(fp, hdrs, url, code) |
cc @orsenthil added 3 commits that reference this issue on Dec 3, 2022pythongh-98778: Update HTTPError to initialize properly even if fp is… pythongh-98778: Update HTTPError to initialize properly even if fp is… gh-98778: Update HTTPError to initialize properly even if fp is None (g… added 2 commits that reference this issue on Dec 8, 2022gh-98778: Update HTTPError to initialize properly even if fp is None (g… gh-98778: Update HTTPError to initialize properly even if fp is None (g… vxgmichel commented on Dec 8, 2022 ContributorAuthor More actions Great! Thanks for the fix :)
Do you think it would be interesting to make traceback more robust and protect it against those kinds of buggy exceptions? I suggested the following patch in my original comment:
diff --git a/Lib/traceback.py b/Lib/traceback.py
index 6270100348..a9c15d59be 100644
--- a/Lib/traceback.py
+++ b/Lib/traceback.py
@@ -694,7 +694,10 @@ def __init__(self, exc_type, exc_value, exc_traceback, *, limit=None,
# Capture now to permit freeing resources: only complication is in the
# unofficial API _format_final_exc_line
self._str = _safe_string(exc_value, 'exception')
- self.__notes__ = getattr(exc_value, '__notes__', None)
+ try:
+ self.__notes__ = getattr(exc_value, '__notes__', None)
+ except Exception:
+ self.__notes__ = None
if exc_type and issubclass(exc_type, SyntaxError):
# Handle SyntaxError's speciallycorona10 commented on Dec 8, 2022 @vxgmichel
I think that you can open a new issue if you want.
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|