unity-editor-coroutines icon indicating copy to clipboard operation
unity-editor-coroutines copied to clipboard

Nested Coroutine Exception Inconsistency

Open mvi opened this issue 6 years ago • 6 comments

Calling a nested coroutine with an exception behaves differently to the built in coroutine support, consider the following:

IEnumerator Foo()
{
   Debug.Log(" Foo 1");
   yield return this.StartCoroutine(Bar());
   Debug.Log(" Foo 2");
}

IEnumerator Bar()
{
   Debug.Log(" Bar 1");
   yield return new WaitForSecondsRealtime(0.5f);
   throw new Exception();
   yield return new WaitForSecondsRealtime(0.5f);
   Debug.Log(" Bar 2");
}

Calling StartCoroutine(Foo()) from a MonoBehaviour at runtime prints:

Foo 1
Bar 1
Exception: Exception of type 'System.Exception' was thrown.

Calling the same coroutine from an editor window prints:

Foo 1
Bar 1
Exception: Exception of type 'System.Exception' was thrown.
Foo 2

This is particularly an issue when chaining coroutines together, such as a series of network related requests, e.g.

Download Resource A
Delete Resource B
Upload Resource A to B

In the above example, the first coroutine failing should stop execution (to be consistent with Unity). However because the parent coroutine continues despite the first request throwing an exception, B is deleted without its replacement being ready!

mvi avatar Sep 06 '18 11:09 mvi

Hi. Good point, never thought about exceptions (how stupid..) It should be rather easy. Just catch any exception, stop the coroutine and rethrow it. I can do it when I have some time, or you want to make a PR for it?

marijnz avatar Sep 07 '18 02:09 marijnz

Hi @marijnz I suppose the complication is stopping the entire coroutine stack, I'm not sure how to do it. I can have a look, but if you have time you would likely be able to solve it in a better way

mvi avatar Sep 07 '18 15:09 mvi

Hey, saw the PR, you didn't get to creating the proper one?

marijnz avatar Sep 12 '18 13:09 marijnz

Been moving house so not had time yet.

My approach was to add an errored flag to EditorCoroutine then cleanup the entire stack of coroutines if there was an exception (via a try-catch-rethrow as you suggested). This seems to work but I've now noticed that there are similar issues if you get an exception in a CustomYieldException, such as in keepWaiting property. I'm wondering if it would be better to take a more unified approach?

mvi avatar Sep 14 '18 10:09 mvi

Hey, I guess you mean YieldCustomYieldInstruction. What do you mean with an exception in the keepWaiting property? That happens without your changes as well?

On a side-note, it's been a while since I've written this code. Looking at it now, I wonder if certain parts have to be so complicated. A short while ago I wrote a similar coroutine runner (with less functionality) which was way simpler. But let's not rewrite, as the code has proven to generally work :)

marijnz avatar Sep 17 '18 08:09 marijnz

For example if you have a custom CustomYieldInstruction (such as a wrapper for a network request) that has an exception at some point during keepWaiting flow (such as when the response is deserialised), then right now the exception is thrown but the parent coroutine keeps running (which I believe is inconsistent with Unity's own coroutines)

mvi avatar Sep 17 '18 09:09 mvi