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

better reporting for python 3.3+ chained exceptions #138

Closed
pytestbot opened this Issue Apr 30, 2012 · 21 comments

Comments

Projects
None yet
6 participants
@pytestbot
Copy link

pytestbot commented Apr 30, 2012

Originally reported by: Anonymous


With Python 3, py.test doesn't propery display exception chains. The following script

#!python
def test_chained_exception():
    try:
        raise ValueError(17)
    except Exception as exc:
        raise ValueError(23) from exc

if __name__ == "__main__":
    test_chained_exception()

when run with Python 3.2 properly outputs the exception chain:

$ python3.2 ~/bug.py
Traceback (most recent call last):
  File "/Users/walter/bug.py", line 3, in test_chained_exception
    raise ValueError(17)
ValueError: 17

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/Users/walter/bug.py", line 8, in <module>
    test_chained_exception()
  File "/Users/walter/bug.py", line 5, in test_chained_exception
    raise ValueError(23) from exc
ValueError: 23

but when run unter py.test only the outermost exception is displayed:

$ python3.2 -mpytest bug.py
================ test session starts ================
platform darwin -- Python 3.2.2 -- pytest-2.1.2
collected 1 items 

bug.py F

====================== FAILURES =====================
_______________ test_chained_exception ______________

    def test_chained_exception():
        try:
            raise ValueError(17)
        except Exception as exc:
>           raise ValueError(23) from exc
E     ValueError: 23

bug.py:5: ValueError
============= 1 failed in 0.01 seconds ==============

@pytestbot

This comment has been minimized.

Copy link
Author

pytestbot commented Sep 7, 2013

Original comment by holger krekel (BitBucket: hpk42, GitHub: hpk42):


Issue #353 was marked as a duplicate of this issue.

@pfctdayelise

This comment has been minimized.

Copy link
Member

pfctdayelise commented Jul 25, 2015

Pytest.raises also does not support this, in that if the inner and outer exceptions are different, and you use pytest.raises(SpecificError), it only matches the outer one. I guess that is correct?? but as mentioned the assert output could be nicer.

Example:

import pytest

def values():
    raise ValueError

def attributes():
    try:
        values()
    except ValueError:
        raise AttributeError


def test_attributesValueError():
    with pytest.raises(ValueError):
        attributes()

def test_attributesAttributeError():
    with pytest.raises(AttributeError):
        attributes()

test_attributesValueError fails. (tested on py27 and py34)

@The-Compiler

This comment has been minimized.

Copy link
Member

The-Compiler commented Jul 25, 2015

@pfctdayelise that's indeed the behaviour I'd expect - it's essentially the same thing try/except does as well. After all, the handled exception is never really raised.

@RonnyPfannschmidt RonnyPfannschmidt changed the title support 3.3 chained exceptions better reporting for python 3.3+ chained exceptions Oct 10, 2015

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member

RonnyPfannschmidt commented Oct 10, 2015

this is blocked by #103

@roolebo

This comment has been minimized.

Copy link
Contributor

roolebo commented Feb 13, 2016

What is the status of the issue? As far as I understand #103 has been resolved.

@RonnyPfannschmidt

This comment has been minimized.

Copy link
Member

RonnyPfannschmidt commented Feb 13, 2016

yes, nobody found time to work on this one however - we wouldn't mind ahelping hand

@roolebo

This comment has been minimized.

Copy link
Contributor

roolebo commented Feb 14, 2016

Alright, I'd be glad to chime in. But I need some time to dig into FormattedExcinfo

@nicoddemus

This comment has been minimized.

Copy link
Member

nicoddemus commented Feb 14, 2016

@roolebo, Thanks! Feel free to ask here or in the mailing list for help.

@roolebo

This comment has been minimized.

Copy link
Contributor

roolebo commented Mar 8, 2016

I've been actively looking into the issue for the last couple days. If someone badly needs full traceback with chained exceptions, please use native traceback for now:py.test --tb=native

@nicoddemus

This comment has been minimized.

Copy link
Member

nicoddemus commented Mar 8, 2016

@roolebo nice, thanks!

@roolebo

This comment has been minimized.

Copy link
Contributor

roolebo commented Mar 13, 2016

@nicoddemus is there a way to see output of print statements for debug purpose? I use tox to run tests: python runtox.py -e py35

The current idea to address the issue is to add new repr for exception chains to maintain compatibility with older versions of Python:

    def repr_excinfo(self, excinfo):
        if sys.version_info[0] < 3:
            reprtraceback = self.repr_traceback(excinfo)
            reprcrash = excinfo._getreprcrash()

            return ReprExceptionInfo(reprtraceback, reprcrash)
        else:
            repr_chain = []
            e = excinfo.value
            descr = None
            while e is not None:
                excinfo = ExceptionInfo((type(e), e, e.__traceback__))
                reprtraceback = self.repr_traceback(excinfo)
                reprcrash = excinfo._getreprcrash()
                repr_chain += (reprtraceback, reprcrash, descr)
                if e.__cause__ is not None:
                    e = e.__cause__
                    descr = 'The above exception was the direct cause of the following exception:'
                elif e.__context__ is not None:
                    e = e.__context__
                    descr = 'During handling of the above exception, another exception occurred:'
                else:
                    e = None
            return ExceptionChainRepr(repr_chain.reverse())

class ExceptionChainRepr(TerminalRepr):
    def __init__(self, chain):
        self.chain = chain
        # reprcrash of the outermost (the newest) exception in the chain
        self.reprcrash = chain[-1][1]

    def toterminal(self, tw):
        for element in self.chain:
            self.element[0].toterminal(tw)
            if self.element is not None:
                tw.line(self.element[2])
@nicoddemus

This comment has been minimized.

Copy link
Member

nicoddemus commented Mar 13, 2016

Hey @roolebo, you can pass extra parameters to py.test in tox by appending -- <args> to the command line:

tox -e py35 -- --no-capture # or -s
@roolebo

This comment has been minimized.

Copy link
Contributor

roolebo commented Mar 14, 2016

@nicoddemus Thanks! That helped a lot.

I'd like to discuss the visual appearance of an exception chain. I made a test case:

class Err(Exception):
    pass
def a():
    raise ValueError()
def b():
    raise TypeError()

def test_issue():
    try:
        a()
    except Exception as e:
        raise Err from e
    finally:
        b()

py.test with my fix makes the following report:

test_3134.py F

===================================== FAILURES =====================================
____________________________________ test_issue ____________________________________

    def test_issue():
        try:
>           a()

test_3134.py:10:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

    def a():
>       raise ValueError()
E       ValueError

test_3134.py:4: ValueError

####################################################################################
The above exception was the direct cause of the following exception:
####################################################################################

    def test_issue():
        try:
            a()
        except Exception as e:
>           raise Err from e
E           test_3134.Err

test_3134.py:12: Err

####################################################################################
During handling of the above exception, another exception occurred:
####################################################################################

    def test_issue():
        try:
            a()
        except Exception as e:
            raise Err from e
        finally:
>           b()

test_3134.py:14:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

    def b():
>       raise TypeError()
E       TypeError

test_3134.py:6: TypeError
============================= 1 failed in 0.01 seconds =============================

Is it okay to have such bold separators between elements of an exception chain? Do you have any other advices on visual appearance of the delimiter (colors, etc)?

@nicoddemus

This comment has been minimized.

Copy link
Member

nicoddemus commented Mar 14, 2016

TBH I find that separator a little too disruptive... I would use Python's example and just use empty lines as separator. As far as color goes, I would use something other than red, perhaps Cyan (light blue)?

@roolebo

This comment has been minimized.

Copy link
Contributor

roolebo commented Mar 14, 2016

@nicoddemus I tried first without separator and color, the border between exceptions wasn't visually clear. Yeah, I agree, a color might be more than enough. I tried both cyan and yellow, and the latter looks much better to me:
yellow-exc-sep
cyan-exc-sep

@nicoddemus

This comment has been minimized.

Copy link
Member

nicoddemus commented Mar 14, 2016

Thanks for the screenshots. LGTM! 😁

Make sure to add those screenshots to your PR!

@roolebo

This comment has been minimized.

Copy link
Contributor

roolebo commented Mar 14, 2016

@nicoddemus Thanks. I still have 7 tests failing :) Most of them are concerned about lack of reprtraceback property and addsection method. Are those actually needed on the ExceptionChainRepr? I rather consider to change the tests than adopt ExceptionChainRepr to existing tests.

@nicoddemus

This comment has been minimized.

Copy link
Member

nicoddemus commented Mar 14, 2016

Yes, because other code (plugins and conftests) depend on that API. I know I do in pytest-qt, and I'm pretty sure pytest-catchlog depends as well.

@roolebo

This comment has been minimized.

Copy link
Contributor

roolebo commented Mar 15, 2016

@nicoddemus Alright, thanks for the details. I made common base class with addsection method for both ReprExceptionInfo and ExceptionChainRepr. What about reprtraceback? Should I store reprtraceback of the outermost(newest) exception?

@nicoddemus

This comment has been minimized.

Copy link
Member

nicoddemus commented Mar 15, 2016

Hmm at this point I think it is more productive for us to discuss details over the code itself, so I think it would be a good time to open a PR now.

Btw, the naming seems inconsistent... ideally it should be eitherReprExceptionInfo and ReprExceptionChain or ExceptionInfoRepr and ExceptionChainRepr... or it might not even be two classes, perhaps it is simple to put all code in the same class? But as I said, I think it is better to continue this discussion over the code. 😁

roolebo added a commit to roolebo/pytest that referenced this issue Mar 19, 2016

@roolebo roolebo referenced this issue Mar 19, 2016

Closed

Close #138 #1467

RonnyPfannschmidt added a commit that referenced this issue Apr 3, 2016

Merge pull request #1486 from roolebo/fix-issue-138
Fix issue #138 - support chained exceptions

fkohlgrueber pushed a commit to fkohlgrueber/pytest that referenced this issue Oct 27, 2018

Merge pull request pytest-dev#138 from ambv/star-expr
Parse complex expressions in parameters after * and **
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.