Propagating multiple async exceptions (or not)

In an earlier post, I mentioned  that in the CTP, an asynchronous method will throw away anything other than the first exception in an AggregateException thrown by one of the tasks it's waiting for. Reading the TAP documentation, it seems this is partly expected behaviour and partly not. TAP claims (in a section about how "await" is achieved by the compiler):

It is possible for a Task to fault due to multiple exceptions, in which case only one of these exceptions will be propagated; however, the Task’s Exception property will return an AggregateException containing all of the errors.

Unfortunately, that appears not to be the case. Here's a test program demonstrating the difference between an async method and a somewhat-similar manually written method. The full code is slightly long, but here are the important methods:

static async Task ThrowMultipleAsync()
{
    Task t1 = DelayedThrow(500);
    Task t2 = DelayedThrow(1000);
    await TaskEx.WhenAll(t1, t2);
}

static Task ThrowMultipleManually()
{
    Task t1 = DelayedThrow(500);
    Task t2 = DelayedThrow(1000);
    return TaskEx.WhenAll(t1, t2);
}

static Task DelayedThrow(int delayMillis)
{
    return TaskEx.Run(delegate {
        Thread.Sleep(delayMillis);
        throw new Exception("Went bang after " + delayMillis);
    });
}

The difference is that the async method is generating an extra task, instead of returning the task from TaskEx.WhenAll. It's waiting for the result of WhenAll itself (via EndAwait). The results show one exception being swallowed:

Waiting for From async method
Thrown exception: 1 error(s):
Went bang after 500

Task exception: 1 error(s):
Went bang after 500


Waiting for From manual method
Thrown exception: 2 error(s):
Went bang after 500
Went bang after 1000

Task exception: 2 error(s):
Went bang after 500
Went bang after 1000

The fact that the "manual" method still shows two exceptions means we can't blame WhenAll - it must be something to do with the async code. Given the description in the TAP documentation, I'd expect (although not desire) the thrown exception to just be a single exception, but the returned task's exception should have both in there. That's clearly not the case at the moment.

Waiter! There's an exception in my soup!

I can think of one reason why we'd perhaps want to trim down the exception to a single one: if we wanted to remove the aggregation aspect entirely. Given that the async method always returns a Task (or void), I can't see how that's feasible anyway... a Task will always throw an AggregateException if its underlying operation fails. If it's already throwing an AggregateException, why restrict it to just one?

My guess is that this makes it easier to avoid the situation where one AggregateException would contain another, which would contain another, etc.

To demonstrate this, let's try to write our own awaiting mechanism, instead of using the one built into the async CTP. GetAwaiter() is an extension method, so we can just make our own extension method which has priority over the original one. I'll go into more detail about that in another post, but here's the code:

public static class TaskExtensions
{
    public static NaiveAwaiter GetAwaiter(this Task task)
    {
        return new NaiveAwaiter(task);
    }
}

public class NaiveAwaiter
{
    private readonly Task task;

    public NaiveAwaiter(Task task)
    {
        this.task = task;
    }

    public bool BeginAwait(Action continuation)
    {
        if (task.IsCompleted)
        {
            return false;
        }
        task.ContinueWith(_ => continuation());
        return true;
    }

    public void EndAwait()
    {
        task.Wait();
    }
}

Yes, it's almost the simplest implementation you could come up with. (Hey, we do check whether the task is already completed...) There no scheduler or SynchronizationContext magic... and importantly, EndAwait does nothing with any exceptions. If the task throws an AggregateException when we wait for it, that exception is propagated to the generated code responsible for the async method.

So, what happens if we run exactly the same client code with these classes present? Well, the results for the first part are different:

Waiting for From async method
Thrown exception: 1 error(s):
One or more errors occurred.

Task exception: 1 error(s):
One or more errors occurred.

We have to change the formatting somewhat to see exactly what's going on - because we now have an AggregateException containing an AggregateException. The previous formatting code simply printed out how many exceptions there were, and their messages. That wasn't an issue because we immediately got to the exceptions we were throwing. Now we've got an actual tree. Just printing out the exception itself results in huge gobbets of text which are unreadable, so here's a quick and dirty hack to provide a bit more formatting:

static string FormatAggregate(AggregateException e)
{
    StringBuilder builder = new StringBuilder();
    FormatAggregate(e, builder, 0);
    return builder.ToString();
}

static void FormatAggregate(AggregateException e, StringBuilder builder, int level)
{
    string padding = new string(' ', level);
    builder.AppendFormat("{0}AggregateException with {1} nested exception(s):", padding, e.InnerExceptions.Count);
    builder.AppendLine();
    foreach (Exception nested in e.InnerExceptions)
    {
        AggregateException nestedAggregate = nested as AggregateException;
        if (nestedAggregate != null)
        {
            FormatAggregate(nestedAggregate, builder, level + 1);
            builder.AppendLine();
        }
        else
        {
            builder.AppendFormat("{0} {1}: {2}", padding, nested.GetType().Name, nested.Message);
            builder.AppendLine();
        }
    }
}

Now we can see what's going on better:

AggregateException with 1 nested exception(s):
AggregateException with 2 nested exception(s):
  Exception: Went bang after 500
  Exception: Went bang after 1000

Hooray - we actually have all our exceptions, eventually... but they're nested. Now if we introduce another level of nesting - for example by creating an async method which just waits on the task created by ThrowMultipleAsync - we end up with something like this:

AggregateException with 1 nested exception(s):
AggregateException with 1 nested exception(s):
  AggregateException with 2 nested exception(s):
   Exception: Went bang after 500
   Exception: Went bang after 1000

You can imagine that for a deep stack trace of async methods, this could get messy really quickly.

However, I don't think that losing the information is really the answer. There's already the Flatten method in AggregateException which will flatten the tree appropriately. I'd be reasonably happy for the exceptions to be flattened at any stage, but I really don't like the behaviour of losing them.

It does get complicated by how the async language feature has to handle exceptions, however. Only one exception can ever be thrown at a time, even though a task can have multiple exceptions set on it. One option would be for the autogenerated code to handle AggregateException differently, setting all the nested exceptions separately (in the single task which has been returned) rather than either setting the AggregateException which causes nesting (as we've seen above) or relying on the awaiter picking just one exception (as is currently the case). It's definitely a decision I think the community should get involved with.

Conclusion

As we've seen, the current behaviour of async methods doesn't match the TAP documentation or what I'd personally like.

This isn't down to the language features, but it's the default behaviour of the extension methods which provide the "awaiter" for Task. That doesn't mean the language aspect can't be changed, however - some responsibility could be moved from awaiters to the generated code. I'm sure there are pros and cons each way - but I don't think losing information is the right approach.

Next up: using extension method resolution rules to add diagnostics to task awaiters.

Published Wed, Nov 3 2010 21:09 by skeet
Filed under: , ,

Comments

# Multiple exceptions yet again... this time with a resolution

I've had a wonderful day with Mads Torgersen, and amongst other things, we discussed multiple exceptions

Thursday, November 04, 2010 2:51 PM by Jon Skeet: Coding Blog

# Eduasync part 11: More sophisticated (but lossy) exception handling

(This post covers projects 13-15 in the source code .) Long-time readers of this blog may not learn much

Wednesday, June 22, 2011 3:27 AM by Jon Skeet: Coding Blog