classification
Title: Broken "Exception ignored in:" message on exceptions in __repr__
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: martin.panter Nosy List: Arfrever, The Compiler, martin.panter, python-dev, r.david.murray, salty-horse, serhiy.storchaka, vstinner
Priority: normal Keywords: patch

Created on 2014-11-10 10:22 by The Compiler, last changed 2019-01-15 10:23 by salty-horse. This issue is now closed.

Files
File name Uploaded Description Edit
repr_exception.py The Compiler, 2014-11-10 10:22 Script demonstrating the issue
unraisable-continue.patch martin.panter, 2014-12-20 04:18 review
unraisable-continue.v2.patch martin.panter, 2014-12-21 23:34 review
unraisable-continue.v3.patch martin.panter, 2015-06-10 02:02 review
unraisable-continue.v4.patch martin.panter, 2016-02-18 06:55 review
unraisable-continue.v5.patch martin.panter, 2016-02-25 13:04 review
Messages (18)
msg230950 - (view) Author: Florian Bruhin (The Compiler) * Date: 2014-11-10 10:22
When there's an unraisable exception (e.g. in __del__), and there's an exception in __repr__ as well, PyErr_WriteUnraisable returns after writing "Exception ignored in:" immediately.

I'd expect it to fall back to the default __repr__ instead.

See the attached example script.

Output with 3.4:

=== Obj ===
Exception ignored in: <bound method Obj.__del__ of <__main__.Obj object at 0x7fd842deb4a8>>
Traceback (most recent call last):
  File "test.py", line 4, in __del__
    raise Exception('in del')
Exception: in del
=== BrokenObj ===
Exception ignored in: (no newline)

Output with 2.7:

=== Obj ===
Exception Exception: Exception('in del',) in <bound method Obj.__del__ of <__main__.Obj object at 0x7fa824dbfa50>> ignored
=== BrokenObj ===
Exception Exception: Exception('in del',) in  ignored

The output with 2.7 is a bit more useful, but still confusing.
msg230955 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2014-11-10 11:17
This is one that has often bugged me. When your repr() implementation is broken, it is quite confusing figuring out what is going wrong. Falling back to object.__repr__() is one option, however I would probably be happy with a simple “exception in repr()” message, and a proper newline.

Another way that I have come across this is:

$ python -c 'import sys; sys.stdout.detach()'
Exception ignored in: [no newline]

The workaround there is to set sys.stdout = None. In that case I think repr(sys.stdout) is trying to say “ValueError: underlying buffer has been detached”.
msg232961 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2014-12-20 04:18
Here is a patch that substitutes an explanation if the repr() fails. Output now looks like this, terminated with a newline:

=== BrokenObj ===
Exception ignored in: <repr() failed>
Traceback (most recent call last):
  File "<stdin>", line 3, in __del__
Exception: in del
$ ./python -c 'import sys; sys.stdout.detach()'
Exception ignored in: <repr() failed>
ValueError: underlying buffer has been detached

I also made it work sensibly if printing the exception message fails:

>>> class Exception(Exception):
...     def __str__(self): raise Exception("Exception is broken")
... 
>>> f = BrokenObj(); del f
Exception ignored in: <repr() failed>
Traceback (most recent call last):
  File "<stdin>", line 3, in __del__
__main__.Exception: <str() failed>
>>> raise Exception()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
__main__.Exception: <str() failed>
>>>
msg233001 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-12-21 18:24
See also issue 6294 for a related problem.
msg233005 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2014-12-21 23:34
Patch v2 revises the unit tests so they are cleaner. Also now tests that the <repr() failed> placeholders are in the exception reports.
msg245104 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-06-10 02:02
Updated the patch to address an oversight in the new test cases.

I think Issue 6294 is probably the same underlying issue. The patch there could be used as the basis for a Python 2 patch. My patches here are for Python 3, and I suspect the code is significantly different in each version, because the error messages are different.

Comparison (for bikeshedding etc) with the message produced by the “traceback” module when str() fails:

>>> try:
...     raise BrokenStrException("message")
... except BrokenStrException:
...     print_exc()
...     raise
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
BrokenStrException: <unprintable BrokenStrException object>
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
__main__.BrokenStrException: <str() failed>
msg260379 - (view) Author: Florian Bruhin (The Compiler) * Date: 2016-02-17 08:14
I just got bitten by this again - is anything holding this back from being merged (other than lack of review-manpower)?
msg260385 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-17 10:11
Yes a review would help. Also, I suspect this will require a separate patch for Python 2. When I get a chance I will have another look at this and see if I am comfortable committing it.
msg260395 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-02-17 11:47
This may be a part of more general issue. Should repr() fail at all? Wouldn't be better to fall back to the default __repr__ instead? repr() is typically used for debugging. Failing repr() can be a part of larger __repr__, thus raising an exception in subobject's repr() leads to the loss of information about full object.
msg260434 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-18 06:55
Thanks for the review; here is an updated patch.

If we just fall back the the default, it hides the fact that that there is a problem in the custom __repr__() method. Another option may be to both indicate there was an error, _and_ a fall back.
msg260765 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-02-24 08:16
Surely changing the behavior of repr() can be made only in the default branch. this issue needs the patch for all versions.

I don't know whether this is the best solution, but the patch looks good enough to me at this moment. If you have no doubts feel free to commit it Martin.
msg260778 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-02-24 09:54
I reviewed unraisable-continue.v4.patch, comments on Rietveld.

Would it be possible to include at least the name of the exception type in the error message? It's less likely than writing Py_TYPE(obj)->tp_name failed, than error in exc.__str() no?
msg260779 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2016-02-24 09:57
I like the idea of enhance code writing exception reports, I'm strongly opposed to modify repr() to ignore exceptions. I would to be noticed when repr() fails badly.

I like how the logging module catchs any formating error and logs an error when the formatting fails.
msg260855 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-25 13:04
Here is a new patch with Victor’s suggestions. The reports now look like

>>> raise BrokenStrException()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
__main__.BrokenStrException: <exception str() failed>
>>> o = VeryBroken()
>>> del o
Exception ignored in: <object repr() failed>
Traceback (most recent call last):
  File "<stdin>", line 9, in __del__
__main__.BrokenStrException: <exception str() failed>
msg260953 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-02-28 01:19
New changeset fca9f02e10e5 by Martin Panter in branch '2.7':
Issue #22836: Keep exception reports sensible despite errors
https://hg.python.org/cpython/rev/fca9f02e10e5
msg260963 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-02-28 03:45
New changeset cf70b1204e44 by Martin Panter in branch '3.5':
Issue #22836: Keep exception reports sensible despite errors
https://hg.python.org/cpython/rev/cf70b1204e44

New changeset 2b597e03f7f4 by Martin Panter in branch 'default':
Issue #22836: Merge exception reporting from 3.5
https://hg.python.org/cpython/rev/2b597e03f7f4
msg260964 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-02-28 04:17
FTR Python 2’s exception report in __del__() is a bit different, here is what it now looks like:

>>> o = VeryBroken()
>>> del o
Exception __main__.BrokenStrException: <exception repr() failed> in <object repr() failed> ignored
msg333663 - (view) Author: Ori Avtalion (salty-horse) Date: 2019-01-15 10:23
I encountered a similar bug and reported it as issue 35743.
History
Date User Action Args
2019-01-15 10:23:48salty-horsesetnosy: + salty-horse
messages: + msg333663
2016-02-28 04:18:54martin.panterlinkissue6294 superseder
2016-02-28 04:17:57martin.pantersetstatus: open -> closed
resolution: fixed
messages: + msg260964

stage: commit review -> resolved
2016-02-28 03:45:04python-devsetmessages: + msg260963
2016-02-28 01:19:43python-devsetnosy: + python-dev
messages: + msg260953
2016-02-25 13:04:03martin.pantersetfiles: + unraisable-continue.v5.patch

messages: + msg260855
2016-02-24 09:57:15vstinnersetmessages: + msg260779
2016-02-24 09:54:28vstinnersetmessages: + msg260778
2016-02-24 08:16:53serhiy.storchakasetassignee: martin.panter
messages: + msg260765
stage: patch review -> commit review
2016-02-18 06:55:28martin.pantersetfiles: + unraisable-continue.v4.patch

messages: + msg260434
versions: - Python 3.4
2016-02-17 11:47:07serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg260395
2016-02-17 10:11:35martin.pantersetmessages: + msg260385
2016-02-17 08:14:14The Compilersetmessages: + msg260379
2015-06-10 02:02:06martin.pantersetfiles: + unraisable-continue.v3.patch

stage: patch review
messages: + msg245104
versions: + Python 3.5, Python 3.6
2014-12-21 23:34:16martin.pantersetfiles: + unraisable-continue.v2.patch

messages: + msg233005
2014-12-21 18:24:26r.david.murraysetmessages: + msg233001
2014-12-20 04:18:38martin.pantersetfiles: + unraisable-continue.patch
keywords: + patch
messages: + msg232961
2014-11-14 02:08:12Arfreversetnosy: + Arfrever
2014-11-10 15:49:38r.david.murraysetnosy: + r.david.murray
2014-11-10 11:21:35The Compilersetnosy: + vstinner
2014-11-10 11:17:39martin.pantersetnosy: + martin.panter
messages: + msg230955
2014-11-10 10:22:49The Compilercreate