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

Fix traceback reporting for exceptions with __cause__ cycles. #3805

Merged
merged 1 commit into from Aug 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/3804.bugfix.rst
@@ -0,0 +1 @@
Fix traceback reporting for exceptions with ``__cause__`` cycles.
4 changes: 3 additions & 1 deletion src/_pytest/_code/code.py
Expand Up @@ -719,7 +719,9 @@ def repr_excinfo(self, excinfo):
repr_chain = []
e = excinfo.value
descr = None
while e is not None:
seen = set()
while e is not None and id(e) not in seen:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i believe for exceptions its fine to use the direct object banking on the object identity hashing by default,
and that way this wouldn't incur the same cost as it does with id() on pypy

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't really want to make assumptions about exceptions since any user space exception could appear here, even one with an exceptionally bad __hash__ + __eq__. I considered using object.__hash__(e) instead of id(e) but I figure they're probably the same.

The "performance hit" in pypy is almost certainly negligible and this isn't a performance critical path (formatting failed test tracebacks).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the rebuttal

seen.add(id(e))
if excinfo:
reprtraceback = self.repr_traceback(excinfo)
reprcrash = excinfo._getreprcrash()
Expand Down
45 changes: 45 additions & 0 deletions testing/code/test_excinfo.py
Expand Up @@ -4,6 +4,7 @@
import operator
import os
import sys
import textwrap
import _pytest
import py
import pytest
Expand Down Expand Up @@ -1265,6 +1266,50 @@ def g():
]
)

@pytest.mark.skipif("sys.version_info[0] < 3")
def test_exc_chain_repr_cycle(self, importasmod):
mod = importasmod(
"""
class Err(Exception):
pass
def fail():
return 0 / 0
def reraise():
try:
fail()
except ZeroDivisionError as e:
raise Err() from e
def unreraise():
try:
reraise()
except Err as e:
raise e.__cause__
"""
)
excinfo = pytest.raises(ZeroDivisionError, mod.unreraise)
r = excinfo.getrepr(style="short")
tw = TWMock()
r.toterminal(tw)
out = "\n".join(line for line in tw.lines if isinstance(line, str))
expected_out = textwrap.dedent(
"""\
:13: in unreraise
reraise()
:10: in reraise
raise Err() from e
E test_exc_chain_repr_cycle0.mod.Err

During handling of the above exception, another exception occurred:
:15: in unreraise
raise e.__cause__
:8: in reraise
fail()
:5: in fail
return 0 / 0
E ZeroDivisionError: division by zero"""
)
assert out == expected_out


@pytest.mark.parametrize("style", ["short", "long"])
@pytest.mark.parametrize("encoding", [None, "utf8", "utf16"])
Expand Down