Conversation
This allows to isolate them from each other so that an exception in a nested group does not affect coroutines running outside.
patrick-kidger
left a comment
There was a problem hiding this comment.
Nit aside LGTM!
Do you want to try this in your downstream use-case + confirm that it works there, before we merge it here? Then we can be sure that this isn't missing some detail.
Actually also, I think I've thought of an even neater API:
try:
return (yield from _nest(...)), True
except BaseException as e:
return e, Falseand then the consuming code can do more complicated logic on the exception. Would that be preferable do you think?
Sorry for the delay in getting back on this. I fixed the nit and added the
So I'm guessing we can't just let the exception propagate up. I do like that the current approach kind of forces/encourages me to return something in case of an exception... I also don't think there is any limitation with the current logic, if I have |
patrick-kidger
left a comment
There was a problem hiding this comment.
So I've tried writing out a couple of test-cases and as a result, I do think the approach I have in #21 (review) is slightly neater overall. So let's go with that!
(In particular this is better for try/finally patterns, when we want to perform some cleanup regardless of success or failure, and this way we don't need to pipe the same logic into two different locations: after the isolate, and inside the cleanup.)
| # This `yield from` is load bearing! We must not allow the tinyio event loop to | ||
| # interpose itself between the exception arising out of `fn(*news)`, and the | ||
| # current stack frame. Otherwise we would get a `CancelledError` here instead. | ||
| return (yield from _nest(fn(*news), exception_group=exception_group)), True |
There was a problem hiding this comment.
I don't understand the exception_group parameter. I think the caller is already free to provide any other kwargs they like, just by closing over them in fn. (e.g. functools.partial). Why special-case this one?
There was a problem hiding this comment.
This is a parameter passed to _nest (and ultimately to tinyio.Loop().runtime()). It's not passed to fn. So unless I'm missing something closing over them in fn doesn't achieve that.
I think you are right, it does simplify things a bit as well. Changed. |
So that an exception in an isolated group of coroutines does not propagate to "outer" coroutines.
This is a follow-up of #20 against the
devbranch.