Joe Wells <bugs-python-...@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 <rep...@bugs.python.org>
<https://bugs.python.org/issue43656>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to