STINNER Victor added the comment:

Oh, I was too lazy to run the full test suite, I only ran a subset and I was 
bitten by buildbots :-) test_unraisable() of test_exceptions fails. IHMO the 
BrokenRepr subtest on this test function is really implementation specific.

To fix buildbots, I removed the BrokenRepr unit test, but kept the other cases 
on test_unraisable(): change be663c9a9e24. See my commit message for the full 
rationale.

In fact, the patch changed the error message logged when a destructor fails. 
Example:
---
class Obj:
    def __del__(self):
        raise Exception("broken del")

    def __repr__(self):
        return "<useful repr>"

obj = Obj()
del obj
---

Before, contains "<useful repr>":
---
Exception ignored in: <bound method Obj.__del__ of <useful repr>>
Traceback (most recent call last):
  File "x.py", line 3, in __del__
    raise Exception("broken del")
Exception: broken del
---

After, "<useful repr>" is gone:
---
Exception ignored in: <function Obj.__del__ at 0x7f10294c3110>
Traceback (most recent call last):
  File "x.py", line 3, in __del__
    raise Exception("broken del")
Exception: broken del
---


There is an advantage. The error message is now better when repr(obj) fails. 
Example:
---
class Obj:
    def __del__(self):
        raise Exception("broken del")

    def __repr__(self):
        raise Excepiton("broken repr")

obj = Obj()
del obj
---

Before, raw "<object repr() failed>" with no information on the type:
---
Exception ignored in: <object repr() failed>
Traceback (most recent call last):
  File "x.py", line 3, in __del__
    raise Exception("broken del")
Exception: broken del
---

After, the error message includes the type:
---
Exception ignored in: <function Obj.__del__ at 0x7f162f873110>
Traceback (most recent call last):
  File "x.py", line 3, in __del__
    raise Exception("broken del")
Exception: broken del
---


Technically, slot_tp_finalize() can call lookup_maybe() to get a bound method 
if the unbound method failed. The question is if it's worth it? In general, I 
dislike calling too much code to log an exception, since it's likely to raise a 
new exception. It's exactly the case here: logging an exception raises a new 
exception (in repr())!

Simpler option: revert the change in slot_tp_finalize() and document that's 
it's deliberate to get a bound method to get a better error message.


The question is a tradeoff between performance and correctness.

----------

_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue29507>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

Reply via email to