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

Elide task.context.run() and contextvars.callable() frames from tracebacks #631

Merged
merged 4 commits into from
Aug 28, 2018
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion trio/_core/_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import os
import random
import select
import sys
import threading
from collections import deque
import collections.abc
Expand Down Expand Up @@ -1376,7 +1377,13 @@ def run_impl(runner, async_fn, args):
except StopIteration as stop_iteration:
final_result = Value(stop_iteration.value)
except BaseException as task_exc:
final_result = Error(task_exc)
# Store for later, removing uninteresting top frames:
# 1. trio._core._run.run_impl()
# 2. contextvars.Context.run() (< Python 3.7 only)
tb_next = task_exc.__traceback__.tb_next
if sys.version_info < (3, 7): # pragma: no cover
Copy link
Member

Choose a reason for hiding this comment

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

pragma: no cover is for cases where we don't even want to have test coverage. Here, not only is it useful to have tests that cover both branches of the if, we actually have those tests :-). (We test on multiple python versions and combine the coverage.) So we shouldn't use pragma: no cover here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh that's good-- I assumed the coverage testing was looking at each version run individually rather than the aggregate.

tb_next = tb_next.tb_next
final_result = Error(task_exc.with_traceback(tb_next))

if final_result is not None:
# We can't call this directly inside the except: blocks above,
Expand Down
23 changes: 23 additions & 0 deletions trio/_core/tests/test_run.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import contextvars
import functools
import platform
import re
import sys
import threading
import time
import traceback
import warnings
from contextlib import contextmanager
from math import inf
Expand Down Expand Up @@ -1897,6 +1899,27 @@ def handle(exc):
assert result == [[0, 0], [1, 1]]


async def test_run_impl_traceback_frame_removal():
async def my_child_task():
raise KeyError()

try:
# Trick: For now cancel/nursery scopes still leave a bunch of tb gunk
# behind. But if there's a MultiError, they leave it on the MultiError,
# which lets us get a clean look at the KeyError itself. Someday I
# guess this will always be a MultiError (#611), but for now we can
# force it by raising two exceptions.
async with _core.open_nursery() as nursery:
nursery.start_soon(my_child_task)
nursery.start_soon(my_child_task)
except _core.MultiError as exc:
first_exc = exc.exceptions[0]
assert isinstance(first_exc, KeyError)
tb_text = ''.join(traceback.format_tb(first_exc.__traceback__))
for r in ('/trio/_core/.* in run_impl$', '/contextvars/.* in run$'):
assert not re.search(r, tb_text, re.MULTILINE)
Copy link
Member

Choose a reason for hiding this comment

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

If you're doing it this way, then I think you can simplify to just

try:
    async with trio.open_nursery() as nursery:
        nursery.start_soon(my_child_task)
except KeyError as exc:
    ...

(Of course that will then need adjusting when we're working on #611, but everything will, so I wouldn't worry about that.)



def test_contextvar_support():
var = contextvars.ContextVar("test")
var.set("before")
Expand Down