This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Author jbw
Recipients andrei.avk, iritkatriel, jbw, kj, moi90, serhiy.storchaka
Date 2021-10-20.13:12:39
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1634735559.56.0.934063833715.issue43656@roundup.psfhosted.org>
In-reply-to
Content
Here are my thoughts inspired by Andrei's insightful comments.

1. In response to the major issue Andrei raises, I agree that it is desirable that repr would never raise an exception.  The reasons Andrei mentions seem quite correct to me.  However, I think the only practical way to make that change would be the change the code of the repr builtin.  (Expecting the authors of every class in all Python code ever written to make sure that their __repr__ method will never raise an exception is unrealistic.)

The bug that is the topic of the issue in this bug report can be fixed by merely handling exceptions raised by one particular call to repr in the code of FrameSummary.__init__.  That change can only affect code that uses the traceback module to get nicer tracebacks, and only if the capture_locals=True feature is requested

Changing the implementation of the repr builtin could conceivably cause unexpected problems for lots of deployed 3rd party code during normal use, because repr is widely used in deployed code, and hence more care and thought is needed.  In contrast, changing FrameSummary.__init__ as I propose in this bug report will only affect code using the capture_locals=True feature of the traceback module, and is likely to make such code more reliable because right now that feature is failure-prone due to this bug.

So I propose that making repr never raise exceptions should be a different bug.  This bug does not need to wait for that bug to be fixed.

2. In response to a minor point of Andrei, I would like to mention that I encountered this because I had given students a coursework template containing this code:

import traceback, sys

def format_exception_chunks(exception):
    return (traceback.TracebackException.from_exception(exception, capture_locals=True).format())

def print_exception(_ignored_type, exception, _ignored_traceback):
    for chunk in format_exception_chunks(exception):
        print(chunk, file=sys.stderr, end="")

# This change to sys.excepthook adds local variables to stack traces.
sys.excepthook = print_exception

This had the unfortunate effect that when a class constructor decided that it did not like its arguments, the students were overwhelmed by a baffling cascade of exception tracebacks.  So while this was indeed “for convenience of interactive debugging”, it had the actual effect of making it nearly impossible for these beginner Python programmers to do any debugging at all.  The overall effect of this bug is that it makes it potentially unwise to routinely rely on the capture_locals=True feature.  A feature that could be useful for beginners actually makes it worse for them.

3. In response to Andrei's suggested change to my minimal example to reproduce the bug, I have a few comments.  First, his minimal example is just as good as mine for reproducing the bug.  Second, my example is inspired by the two classes I mention whose constructors trigger the bug: both of them do it by leaving the incompletely initialized object missing an attribute that repr depends on, so they both raise a missing attribute exception when an attempt is made to print the incompletely initialized object.  I suspect this is a quite common pattern in deployed Python code.  I suspect that people have not made efforts to avoid this pattern because it only triggers a bug when exception tracebacks are investigated, because the only reference to the incompletely initialized object is in the stack frame of the constructor method (either __new__ or __init__) which in turn is only referenced from the traceback.  Third, my example deliberately has as little code as possible in the __repr__ method to convey the key issue.  Fourth, one of these two examples (mine or Andrei's) should go into the unit tests when the bug is fixed.

4. I notice that the code of unittest.util.safe_repr gives an example of how to use object.__repr__(obj) to get something formatted for an object whose normal __repr__ method has raised an exception.  So I alter my recommended fix to the following:

        if locals:
            d = {}
            self.locals = d
            for k, v in locals.items():
                try:
                    d[k] = repr(v)
                except Exception:
                    d[k] = '<repr failed for ' + object.__repr__(v) + '>'
        else:
            self.locals = None

5. Can someone please change the Stage of this issue to something other than “resolved” and the Resolution of this issue to something other than “not a bug”?

I hope these comments are helpful and this bug gets somehow fixed.
History
Date User Action Args
2021-10-20 13:12:39jbwsetrecipients: + jbw, serhiy.storchaka, iritkatriel, moi90, kj, andrei.avk
2021-10-20 13:12:39jbwsetmessageid: <1634735559.56.0.934063833715.issue43656@roundup.psfhosted.org>
2021-10-20 13:12:39jbwlinkissue43656 messages
2021-10-20 13:12:39jbwcreate