Message287465
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. |
|
Date |
User |
Action |
Args |
2017-02-09 23:12:03 | vstinner | set | recipients:
+ vstinner, rhettinger, methane, python-dev, serhiy.storchaka, yselivanov |
2017-02-09 23:12:03 | vstinner | set | messageid: <1486681923.21.0.616763457637.issue29507@psf.upfronthosting.co.za> |
2017-02-09 23:12:03 | vstinner | link | issue29507 messages |
2017-02-09 23:12:02 | vstinner | create | |
|