Exception with broken __getattr__ causes error rendering tracebacks

[perrinjerome]

Bug report

Bug description:

Running this code:

import logging

class X(Exception):
    def __getattr__(self, a):
        raise KeyError(a)  # !!!

try:
    raise X()
except X:
    logging.getLogger().exception('')

print('ok')

does not print "ok" but instead:

# python3.11 repro.py 
--- Logging error ---
Traceback (most recent call last):
  File "/workspaces/cpython/repro.py", line 8, in <module>
    raise X()
object address  : 0x7f3007c67e80
object refcount : 2
object type     : 0x7f3015bf0b20
object type name: KeyError
object repr     : KeyError('__notes__')
lost sys.stderr

CPython versions tested on:

3.11, CPython main branch

Operating systems tested on:

Linux

Linked PRs

• gh-113358: Fix rendering tracebacks with exceptions with a broken __getattr__ #113359
• [3.12] gh-113358: Fix rendering tracebacks with exceptions with a broken __getattr__ (GH-113359) #114105
• [3.11] gh-113358: Fix rendering tracebacks with exceptions with a broken __getattr__ (GH-113359) #114109
• [3.11] gh-113358: Fix rendering tracebacks with exceptions with a broken __getattr__ (GH-113359) #114118
• [3.12] gh-113358: Fix rendering tracebacks with exceptions with a broken __getattr__ (GH-113359) #114173
• [3.11] gh-113358 Fix rendering tracebacks with exceptions with a broken __getattr__ : Normalize exception  #114379

[terryjreedy]

To me, this looks like a bug in your code, not in CPython.

[perrinjerome]

In this example, the user code is (of course) wrong, but it's not good that after such an error python is not able to output a proper traceback.

This example causes an error during the logging of error, which is unexpected from the logging system, as mentioned in https://docs.python.org/3.13/library/logging.html#logging.Handler.handleError :

(...) exceptions get silently ignored. This is what is mostly wanted for a logging system - most users will not care about errors in the logging system, they are more interested in application errors.

this documentation is for a different context, but it indicates the general idea that the logging module is trying to prevent its internal errors from propagating.

The traceback module is also generally careful about custom exceptions with broken behaviors and has several try / except, for example when printing an exception, or printing the notes attached to an exception (this is done by using the _safe_string utility function)

I guess it's not so uncommon to have exceptions with a __getattr__ that may raise something else than an AttributeError when __notes__ is accessed. urllib.error.HTTPError was a case from the standard library that was reported in #98778 . The fix has been to fix the underlying error, but in that issue it was also suggested to ignore exceptions while getting __notes__ in #98778 (comment)

Another case where this behavior is problematic is when such an exception is raised in an unittest, this crashes the testrunner :

import unittest

class X(Exception):
    def __getattr__(self, a):
        raise KeyError(a)

class T(unittest.TestCase):
    def test(self):
        raise X()

# python3.11 -m unittest repro.py 
Traceback (most recent call last):
  File "/usr/lib64/python3.11/unittest/case.py", line 57, in testPartExecutor
    yield
  File "/usr/lib64/python3.11/unittest/case.py", line 623, in run
    self._callTestMethod(testMethod)
  File "/usr/lib64/python3.11/unittest/case.py", line 579, in _callTestMethod
    if method() is not None:
       ^^^^^^^^
  File "/workspaces/cpython/repro.py", line 9, in test
    raise X()
object address  : 0x7f070f738d60
object refcount : 2
object type     : 0x7f071da27b20
object type name: KeyError
object repr     : KeyError('__notes__')
lost sys.stderr

I don't see explicit reference in the docs that unittest guarantees it would properly report all exceptions, but as a user I expect the testrunner to handle this and properly report an error.

[perrinjerome]

I forgot to mention that this is a new behaviour from python 3.11, in older versions there was no error.

[terryjreedy]

Thanks for providing more info on the issue. The above satisfies my question here and on the PR. I will leave it to others, though, to decide what to do.
@vsajip Still involved with logging?

[vsajip]

@terryjreedy Yes, I still maintain logging 🙂

[vsajip]

OK, it's not really a logging issue, but I'll review. The reasoning behind the attached change is that swallowing exceptions isn't ideal, but can be tolerated here since if an exception class is broken such that its __notes__ can't be retrieved, with this change it's as if there were no notes, and traceback printing can proceed. The exception swallowing would only affect the case where notes were added and couldn't be retrieved because of the broken __notes__ attribute access; since the notes can't be retrieved, they couldn't be put into the traceback anyway.

However, one thing still bugs me - if the exception class has a broken __getattr__, aren't a whole bunch of other things going to break too? For example, some exception handling user code that doesn't print a traceback, but accesses some other exception attribute such as an error code? This change won't do anything for that case - that would require handling in some other way.

Note that as far as logging is concerned, logging can be configured to swallow exceptions in production environments by setting logging.raiseExceptions to False (documented here). With that change, the script in the OP prints ok as expected. The default setting of logging.raiseExceptions is True as that makes sense in development/staging environments.

Regarding the mention of failures in test runners, I think that's a matter for refinements in test runners.

Summary: this change would perhaps be OK if we only wanted to fix traceback printing when exception attribute access is broken, but it seems not worth doing in isolation because in practice, broken attribute access in exceptions would lead to a host of other problems/failures during exception handling, which are not addressed by this change.

[vsajip]

I've marked this as pending again - if no feedback is received soon to the contrary, I'll close this issue and the PR as not planned, based on my analysis above.