It should be possible to get a traceback out of a cancel scope

[njsmith]

With fail_after, it's a bit annoying that it can't show you where the block was cancelled. (The traceback just starts at the end of the scope.)

This will be a problem in particular for test timeouts. (Well, they probably won't use fail_after, but they'll have the same problem.)

I don't think we can capture the exceptions in cancel scope objects by default, because we don't want to pin the stacks in memory.

We could have a flag that tells it to save caught exceptions, though. Or try to do something clever with catching them ourselves inside the scope, but that's basically the same thing from the user point of view, and the cancel scope already knows how to reliably catch its own exceptions.

[nmalaguti]

What about converting the Cancelled exception caught by the cancel scope into a traceback.TracebackException which is meant to have enough information to print the stack but ensure no references are held. This could be stored on the scope and exposed for use by fail_after to set the __cause__ on the TooSlowError. It isn’t a real exception, but it should work if someone prints or logs the TooSlowError.

This would also help users that want to debug where their cancellation occurred on a given cancel scope by allowing them to get some printable info from the scope.

[nmalaguti]

Never mind. The __cause__ must derive from BaseException.

So what are the risks of storing the Cancelled error on the scope object? Make it an option on open_cancel_scope to save the error and make it available? Default true or false? Make it the users problem if they keep their scopes around with references that hold memory and don’t set keep_exception to false?

[njsmith]

I think the simplest approach is probably to have some sort of save_exceptions flag you have to enable, since fail_after knows well beforehand that it will want the exceptions, it can hide the extra fiddliness from the user, and I don't have any other use cases in mind. Capturing by default is tricky because there are sensible use cases where cancel scope objects are saved and may inadvertantly outlive the stack frame they apply to. (Like when you save off a cancel scope somewhere so another task can find it and call its cancel method.)

…

On Nov 8, 2017 07:48, "Nick Malaguti" ***@***.***> wrote: Never mind. The __cause__ must derive from BaseException. So what are the risks of storing the Cancelled error on the scope object? Make it an option on open_cancel_scope to save the error and make it available? Default true or false? Make it the users problem if they keep their scopes around with references that hold memory and don’t set keep_exception to false? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#285 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAlOaAGCxHTebNW3cOzSr95UthvZyNB_ks5s0bE_gaJpZM4O2NXc> .

[cjerdonek]

What about providing a hook for the user to transform the exception how they'd like, and with a sensible default (e.g. saving None or the default formatting of the traceback as a string)? This would also let the user store the exception as is if they wanted.

[njsmith]

Do you have a use case?

[cjerdonek]

I'm thinking of the times I ran into the same or a similar issue with asyncio. By default, asyncio doesn't seem to provide access to the full stack trace when a CancelledError or TimeoutError occurs. Stack traces just "stop" at the point where the asyncio machinery reraises it. This also came up for me in test runs, making troubleshooting more difficult, etc. What I wound up doing in one class of cases was wrapping the coroutine and logging the stack trace before it is bubbled up / handled by asyncio's machinery. But I can see cases where you might want to do something a little more than simple logging. For example, you might want to grab particular attributes off the exception in cases where you're interested in structured data, but maybe without having to store everything, to avoid possible issues of the sort you mention.

Disclaimer: I'm not using trio currently, but I'd like to at some point in the future when it's more mature. After wrestling with a number of the asyncio issues that trio was designed to solve, trio seems like the right way to go. (So thanks for all your work in developing a better way! It will be huge I hope!)

[njsmith]

@cjerdonek Trio does provide full stack traces in general AFAIK – it even has some horrible code to reconstruct more accurate tracebacks in certain cases. If you run into cases where it doesn't then please file a bug :-).

The issue here is more specific. In Trio, cancellation is done using "cancel scopes": you wrap a chunk of code in a with block, and then you can cancel specifically that block. The way this is implemented is that when the block is cancelled, then we throw in a special Cancelled exception, which unwinds the stack back to the with block, which catches the exception. Then your program can continue on, doing whatever it's going to do. Sometimes, though, the reason that you're cancelling something is that you've decided to give up and raise an exception. (Perhaps because you've gotten tired of waiting.) In that case, the first thing you do after the cancel block finishes unwinding is raise another exception. This is what the fail_after convenience function does. But right now, there's no connection between the Cancelled exception that's used to unwind the stack, and the real exception we raise afterwards. It would be nice if in this particular case, we could stick that Canceled exception on as the __context__ for the TooSlowError we raise afterwards, because maybe sometimes it'll give you a useful hint about what operation got stuck.

In this specific situation, we know ahead of time that we want to do this, and we actually want the Cancelled object itself, not any kind of preprocessed replacement (__context__ has to be a real exception object). So a flag asking the cancel scope to hold onto the Cancelled object seems sufficient.

[njsmith]

I thought of another complication: there's some subtle interaction between MultiError and this feature. Specifically, there are two fairly common cases to think about:

• The cancel scope gets a MultiError containing multiple Cancelled exceptions, and wants to catch them all. (This is the usual thing that happens if you cancel a block that contains a nursery with multiple running tasks in it.)

• The cancel scope gets a MultiError containing a mix of Cancelled exceptions and other things, and wants to catch (some of) the Cancelled exceptions and let the rest propagate.

Right now, MultiError.catch maps a handler over all the exceptions inside a MultiError, and lets you examine each one before deciding whether to catch it or not. Then it discards all the ones that were caught, and if any are left it re-raises a new exception built out of them. (With some cleverness to keep tracebacks correct etc.)

So the simple way to do this would be to have our handler check the flag, and if it's set, then for each primitive Cancelled exception it catches, it stashes it in some set or something. But the problem with this is that you lose the tree structure and traceback that the original MultiError had -- you just get a flat list of exceptions with part of their tracebacks missing. Not really something that you want to attach as a __context__. In the second case above, this is perhaps the best you can manage... I guess we could have some mechanism for MultiError.catch to also extract the caught exceptions and reconstruct them into a proper MultiError?

Another option, a bit hackier but maybe simpler if it works, would be to say oh well, we actually only care about capturing these exceptions when we're in the first case above. So what we could do is have an explicit check where we save the original MultiError, run the MultiError.catch as usual, and then if it swallows everything we know that the original MultiError consisted solely of our Cancelled exceptions and we just save that instead. But this does lead to some weird inconsistencies – we set the "did we catch an exception?" flag based on whether we caught at least one Cancelled (i.e., it's true in both of the cases above), but if we did things this way then we'd only actually have an exception to point to as being the one we caught in the first case, not the second.

[oremanj]

Funnily enough, it appears that implicit exception chaining is actually doing what we want here; there's some discussion at https://gitter.im/python-trio/general?at=5ad5753a27c509a7741cca44.

In [4]: async def afn():
   ...:     try:
   ...:         with trio.fail_after(1):
   ...:             await trio.sleep(5)
   ...:     except trio.TooSlowError as exc:
   ...:         return exc
   ...:

In [5]: exc = trio.run(afn)

In [6]: exc
Out[6]: trio.TooSlowError()

In [7]: exc.__context__
Out[7]: trio.Cancelled()

Simpler illustrative example:

In [8]: class suppress:
   ...:     def __enter__(self):
   ...:         pass
   ...:     def __exit__(self, *exc_info):
   ...:         return True
   ...:

In [9]: from contextlib import contextmanager

In [10]: @contextmanager
    ...: def chain():
    ...:     with suppress():
    ...:         yield
    ...:     raise ValueError
    ...:

In [11]: with chain():
    ...:     raise KeyError
    ...:
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
<ipython-input-10-bf8693b31045> in chain()
      3     with suppress():
----> 4         yield
      5     raise ValueError

<ipython-input-11-6f32b66051a3> in <module>()
      1 with chain():
----> 2     raise KeyError

KeyError:

During handling of the above exception, another exception occurred:

ValueError                                Traceback (most recent call last)
<ipython-input-11-6f32b66051a3> in <module>()
      1 with chain():
----> 2     raise KeyError

/usr/local/Cellar/python/3.6.5/Frameworks/Python.framework/Versions/3.6/lib/python3.6/contextlib.py in __exit__(self, type, value, traceback)
     97                 value = type()
     98             try:
---> 99                 self.gen.throw(type, value, traceback)
    100             except StopIteration as exc:
    101                 # Suppress StopIteration *unless* it's the same exception that

<ipython-input-10-bf8693b31045> in chain()
      3     with suppress():
      4         yield
----> 5     raise ValueError

ValueError:

This is pretty subtle, though, so probably we still want the explicit chaining or a comment in fail_after. It's also unlikely that the implicit chaining will do the right thing with MultiErrors; my guess would be that the entire MultiError gets attached rather than just the Cancelled part, but I haven't verified this.