Broken "Exception ignored in:" message on exceptions in __repr__

[The-Compiler]

BPO	22836
Nosy	@salty-horse, @vstinner, @bitdancer, @vadmium, @serhiy-storchaka, @The-Compiler
Files	repr_exception.py: Script demonstrating the issue unraisable-continue.patch unraisable-continue.v2.patch unraisable-continue.v3.patch unraisable-continue.v4.patch unraisable-continue.v5.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/vadmium'
closed_at = <Date 2016-02-28.04:17:57.421>
created_at = <Date 2014-11-10.10:22:49.203>
labels = ['interpreter-core', 'type-bug']
title = 'Broken "Exception ignored in:" message on exceptions in __repr__'
updated_at = <Date 2019-01-15.10:23:48.960>
user = 'https://github.com/The-Compiler'

bugs.python.org fields:

activity = <Date 2019-01-15.10:23:48.960>
actor = 'salty-horse'
assignee = 'martin.panter'
closed = True
closed_date = <Date 2016-02-28.04:17:57.421>
closer = 'martin.panter'
components = ['Interpreter Core']
creation = <Date 2014-11-10.10:22:49.203>
creator = 'The Compiler'
dependencies = []
files = ['37166', '37511', '37524', '39670', '41948', '42029']
hgrepos = []
issue_num = 22836
keywords = ['patch']
message_count = 18.0
messages = ['230950', '230955', '232961', '233001', '233005', '245104', '260379', '260385', '260395', '260434', '260765', '260778', '260779', '260855', '260953', '260963', '260964', '333663']
nosy_count = 8.0
nosy_names = ['salty-horse', 'vstinner', 'Arfrever', 'r.david.murray', 'python-dev', 'martin.panter', 'serhiy.storchaka', 'The Compiler']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue22836'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6']

[The-Compiler]

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.

[vadmium]

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”.

[vadmium]

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>
>>>

[bitdancer]

See also bpo-6294 for a related problem.

[vadmium]

Patch v2 revises the unit tests so they are cleaner. Also now tests that the <repr() failed> placeholders are in the exception reports.

[vadmium]

Updated the patch to address an oversight in the new test cases.

I think bpo-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>

[The-Compiler]

I just got bitten by this again - is anything holding this back from being merged (other than lack of review-manpower)?

[vadmium]

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.

[serhiy-storchaka]

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.

[vadmium]

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.

[serhiy-storchaka]

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.

[vstinner]

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?