Author vstinner
Recipients methane, python-dev, rhettinger, serhiy.storchaka, vstinner, yselivanov
Date 2017-02-09.23:12:02
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1486681923.21.0.616763457637.issue29507@psf.upfronthosting.co.za>
In-reply-to
Content
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.
History
Date User Action Args
2017-02-09 23:12:03vstinnersetrecipients: + vstinner, rhettinger, methane, python-dev, serhiy.storchaka, yselivanov
2017-02-09 23:12:03vstinnersetmessageid: <1486681923.21.0.616763457637.issue29507@psf.upfronthosting.co.za>
2017-02-09 23:12:03vstinnerlinkissue29507 messages
2017-02-09 23:12:02vstinnercreate