Exception with broken __getattr__ causes error rendering tracebacks

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

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

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.

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

@vsajip

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?

@terryjreedy

@terryjreedy Yes, I still maintain logging 🙂

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.

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.

@Zac-HD

Thank you for the analysis, I also believe it's not a problem with logging module, but with traceback module. I used logging as an example because it's where I observed the problem.

That's a good point that such buggy exception cause problems in other places, but in my experience it has been different because when this causes error in user exception handling code, there is a traceback clearly showing the user where the problem, so it's just a normal error, but when such error happens internally in traceback module, it's hard to understand where the problem is, especially because it looks like something is wrong with python itself and not with user code.

I read again the discussion from the pep, there was this comment where it was suggested to catch errors while handling __notes__ - handling, not accessing, so it's arguably slightly different, but this seems the general idea that "we should not break traceback printing code".

traceback module already has some protection against many cases broken __repr__ or malformed __notes__, but since python 3.11, traceback module accesses __notes__ attribute, which does not exist in Exception base class, so if the exception class defines a __getattr__ it will always be called in this getattr(exc_value, '__notes__', None). I was thinking traceback should also not blindly "trust" that this getattr from user code, for same reasons that it does not trust repr from user code.

In a sense, the question here "how much traceback module tolerate errors while producing a traceback ?".

We can maybe ask @Zac-HD ( I hope you don't mind 🙇 ) who was author of the pep and addressed a similar problem, but with a different solution in agronholm/exceptiongroup#58 .

The backport for 3.11 is not straightforward, there was first a trivial conflict with #103895 that was not backported, but the hard part is that 3.11 branch does not have #110702 and there is also another C implementation of exception formatting in Python/pythonrun.c.

I made a new pull request #114118 in which I also adapted that C code with equivalent changes to also ignore exceptions while getting notes, would it be the correct way to proceed ?

Will take a look when I get a chance. What about 3.12?

pythongh-113358: Fix rendering tracebacks with exceptions with a brok…

@vsajip

Thank you @vsajip , 3.12 seems the same as 3.11 . I made a draft pull request with the same patch as 3.11 #114173

[3.11] gh-113358: Fix rendering tracebacks with exceptions with a bro…

[3.12] pythongh-113358: Fix rendering tracebacks with exceptions with…

[3.12] gh-113358: Fix rendering tracebacks with exceptions with a bro…

[3.11] gh-113358 Fix rendering tracebacks with exceptions with a bro…

Thank you!

Good, thank you all !

pythongh-113358: Fix rendering tracebacks with exceptions with a brok…

Navigation Menu

Repository navigation

Exception with broken getattr causes error rendering tracebacks #113358

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

20 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Exception with broken __getattr__ causes error rendering tracebacks #113358

Description

Bug report

Bug description:

CPython versions tested on:

Operating systems tested on:

Linked PRs

Activity

terryjreedy commented on Dec 21, 2023

perrinjerome commented on Dec 22, 2023

perrinjerome commented on Dec 22, 2023

terryjreedy commented on Dec 22, 2023

vsajip commented on Dec 22, 2023

vsajip commented on Dec 27, 2023

vsajip commented on Jan 9, 2024

perrinjerome commented on Jan 11, 2024

20 remaining items

perrinjerome commented on Jan 17, 2024

vsajip commented on Jan 17, 2024

perrinjerome commented on Jan 17, 2024

iritkatriel commented on Jan 21, 2024

perrinjerome commented on Jan 22, 2024

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

Exception with broken getattr causes error rendering tracebacks #113358