[issue43656] TracebackException or StackSummary.extract with capture_locals=True fail to catch exceptions raised by repr() on value of frame local variable in FrameSummary.__init__.

[Joe Wells]

Joe Wells <bugs-python-org at jbw.link> added the comment:

Andrei, thanks very much for the pointer to bug/issue https://bugs.python.org/issue39228.  I had not noticed the earlier comment by Irit pointing to that bug.  (Is there some way to merge bugs so that someone visiting one of the bugs will see the merged stream of comments?)

The remarks in that bug are precisely why my recommended fix has this line:

                    d[k] = '<repr failed for ' + object.__repr__(v) + '>'

instead of this:

                    d[k] = object.__repr__(v)

Does this not meet the concern expressed there?  Something that would more thoroughly meet that would be the following:

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

I use object.__repr__ instead of repr because when repr(v) has already failed it is likely that repr(e) on the exception e would also be likely to fail.  Although we could also try repr(e) first to see if it raises an exception.

Your suggested reaction to this bug (documenting Python best practice) is something that should be done regardless.  I completely agree.  Unfortunately, better documentation of how to write good Python does not address the point of this bug which is that debugging tools should not fail when used to debug buggy code.  The purpose of a debugging tool is to help with badly written code, not to work only with well written code.  Right now the capture_locals=True feature is not safe to use without wrapping it with an exception handler.  As a result of this bug, I have been forced to rewrite this:

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

to instead be this:

def format_exception_chunks(exception):
    try:
        tbe = (traceback.TracebackException
               . from_exception(exception, capture_locals=True))
        return tbe.format()
    except Exception:
        pass
    # the traceback module fails to catch exceptions that occur when
    # formatting local variables' values, so we retry without
    # requesting the local variables.
    try:
        tbe = traceback.TracebackException.from_exception(exception)
        return ('WARNING: Exceptions were raised while trying to'
                ' format exception traceback with local variable'
                ' values,\n'
                'so the exception traceback was formatted without'
                ' local variable values.\n',
                *tbe.format())
    except Exception:
        return ('WARNING: Exceptions were raised while trying to format'
                ' exception traceback,\n'
                'so no formatted traceback information is being'
                ' provided.\n',)

My argument is that it is better to fix the bug once in traceback.py instead of every user of the capture_locals=True feature having to discover the hard way (with hours of painful bug investigation) that they need to write lots of exception handling and bug workarounds around their use of the capture_locals=True feature.

----------

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue43656>
_______________________________________