Joe Wells <[email protected]> added the comment:
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.
----------
_______________________________________
Python tracker <[email protected]>
<https://bugs.python.org/issue43656>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
