`urllib.error.HTTPError(..., fp=None)` raises a `KeyError` instead of an `AttributeError` on attribute access

[vxgmichel]

Bug 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 specially

Linked PRs

• gh-98778: Update HTTPError to initialize properly even if fp is None #99966
• [3.11] gh-98778: Update HTTPError to initialize properly even if fp is None (gh-99966) #100096
• [3.10] gh-98778: Update HTTPError to initialize properly even if fp is None (gh-99966) #100097

[serhiy-storchaka]

The problem is that HTTPError is a subclass of tempfile._TemporaryFileWrapper, which is not properly initialized if fp is None.

cpython/Lib/urllib/error.py

Lines 49 to 50 in 3e07f82

	if fp is not None:
	self.__super_init(fp, hdrs, url, code)

cc @orsenthil

[vxgmichel]

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 specially