-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
Broken "Exception ignored in:" message on exceptions in __repr__ #67025
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
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 === Output with 2.7: === Obj === The output with 2.7 is a bit more useful, but still confusing. |
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”. |
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>
>>> |
See also bpo-6294 for a related problem. |
Patch v2 revises the unit tests so they are cleaner. Also now tests that the <repr() failed> placeholders are in the exception reports. |
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> |
I just got bitten by this again - is anything holding this back from being merged (other than lack of review-manpower)? |
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. |
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. |
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. |
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. |
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? |
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. |
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> |
New changeset fca9f02e10e5 by Martin Panter in branch '2.7': |
New changeset cf70b1204e44 by Martin Panter in branch '3.5': New changeset 2b597e03f7f4 by Martin Panter in branch 'default': |
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 |
I encountered a similar bug and reported it as bpo-35743. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: