Skip to content
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

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

Closed
The-Compiler mannequin opened this issue Nov 10, 2014 · 18 comments
Closed

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

The-Compiler mannequin opened this issue Nov 10, 2014 · 18 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@The-Compiler
Copy link
Mannequin

The-Compiler mannequin commented Nov 10, 2014

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
    Copy link
    Mannequin Author

    The-Compiler mannequin commented Nov 10, 2014

    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.

    @The-Compiler The-Compiler mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Nov 10, 2014
    @vadmium
    Copy link
    Member

    vadmium commented Nov 10, 2014

    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
    Copy link
    Member

    vadmium commented Dec 20, 2014

    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
    Copy link
    Member

    See also bpo-6294 for a related problem.

    @vadmium
    Copy link
    Member

    vadmium commented Dec 21, 2014

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

    @vadmium
    Copy link
    Member

    vadmium commented Jun 10, 2015

    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
    Copy link
    Mannequin Author

    The-Compiler mannequin commented Feb 17, 2016

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

    @vadmium
    Copy link
    Member

    vadmium commented Feb 17, 2016

    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
    Copy link
    Member

    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
    Copy link
    Member

    vadmium commented Feb 18, 2016

    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
    Copy link
    Member

    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
    Copy link
    Member

    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?

    @vstinner
    Copy link
    Member

    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.

    @vadmium
    Copy link
    Member

    vadmium commented Feb 25, 2016

    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>

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 28, 2016

    New changeset fca9f02e10e5 by Martin Panter in branch '2.7':
    Issue bpo-22836: Keep exception reports sensible despite errors
    https://hg.python.org/cpython/rev/fca9f02e10e5

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Feb 28, 2016

    New changeset cf70b1204e44 by Martin Panter in branch '3.5':
    Issue bpo-22836: Keep exception reports sensible despite errors
    https://hg.python.org/cpython/rev/cf70b1204e44

    New changeset 2b597e03f7f4 by Martin Panter in branch 'default':
    Issue bpo-22836: Merge exception reporting from 3.5
    https://hg.python.org/cpython/rev/2b597e03f7f4

    @vadmium
    Copy link
    Member

    vadmium commented Feb 28, 2016

    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

    @vadmium vadmium closed this as completed Feb 28, 2016
    @salty-horse
    Copy link
    Mannequin

    salty-horse mannequin commented Jan 15, 2019

    I encountered a similar bug and reported it as bpo-35743.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    4 participants