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

Should TaskExtensions.AsIEnumerator unwrap AggregateException? #19

Open
dubois opened this issue Dec 3, 2019 · 0 comments
Open

Should TaskExtensions.AsIEnumerator unwrap AggregateException? #19

dubois opened this issue Dec 3, 2019 · 0 comments

Comments

@dubois
Copy link

dubois commented Dec 3, 2019

I'm trying to incrementally convert our codebase from coroutines to async/await. Doing it incrementally means a lot of "await " and "Task.AsIEnumerator()" to convert between the two styles at the boundaries.

One issue I've found is that converting a Coroutine -> async Task is not as simple as changing the method from "IEnumerator DoSomethingCoroutine()" -> "async Task DoSomethingAsync()" and fixing up callers to append ".AsIEnumerator()", because exceptions that propagate from Task and AsIEnumerator() are wrapped with AggregateException().

I'm assuming that the main use case of Task.AsIEnumerator is to help with the transition process, so I think there's a good argument for unwrapping the AggregateException. On the other hand, I'm not sure when the AggregateException might contain 2 or more exceptions and what AsIEnumerator should do in that case.

Here's some sample code that illustrates the issue:

    class MyCustomException : Exception {}

    IEnumerator<object> CallerCoroutine() {
        // Old code is sometimes verbose, for call stacks and exception propagation
        using (var cr = DoSomethingCoroutine()) {
            while (true) {
                try {
                    if (!cr.MoveNext()) { break; }
                } catch (MyCustomException e) {
                    // ...
                }
                yield return cr.Current;
            }
        }

        // Simple automated refactor doesn't work because the exception is now AggregateException
        using (var cr = DoSomethingAsync().AsIEnumerator()) {
            // ... body is same as above ...
        }
    }

    IEnumerator<object> DoSomethingCoroutine() {
        if (UnityEngine.Random.value > .5f) { throw new MyCustomException(); }
        yield return null;
    }

    async Task<object> DoSomethingAsync() {
        if (UnityEngine.Random.value > .5f) { throw new MyCustomException(); }
        await Awaiters.NextFrame;
        return null;
    }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant