Michael Ray Lovett
Michael Ray Lovett

Reputation: 7060

Why doesn't await on Task.WhenAll throw an AggregateException?

In this code:

private async void button1_Click(object sender, EventArgs e) {
    try {
        await Task.WhenAll(DoLongThingAsyncEx1(), DoLongThingAsyncEx2());
    }
    catch (Exception ex) {
        // Expect AggregateException, but got InvalidTimeZoneException
    }
}

Task DoLongThingAsyncEx1() {
    return Task.Run(() => { throw new InvalidTimeZoneException(); });
}

Task DoLongThingAsyncEx2() {
    return Task.Run(() => { throw new InvalidOperation();});
}

I expected WhenAll to create and throw an AggregateException, since at least one of the tasks it was awaiting on threw an exception. Instead, I'm getting back a single exception thrown by one of the tasks.

Does WhenAll not always create an AggregateException?

Upvotes: 188

Views: 107590

Answers (8)

Alexei Sosin
Alexei Sosin

Reputation: 3351

What you really need to do is:

await Task.WhenAll(DoLongThingAsyncEx1(), DoLongThingAsyncEx2())
     .ContinueWith(t => { if(t.IsFaulted) throw t.Exception!.Flatten(); }  );

Upvotes: 1

noseratio
noseratio

Reputation: 61736

Many good answers here, but I still would like to post my rant as I've just come across the same problem and conducted some research. Or skip to the TLDR version below.

The problem

Awaiting the task returned by Task.WhenAll only throws the first exception of the AggregateException stored in task.Exception, even when multiple tasks have faulted.

The current docs for Task.WhenAll say:

If any of the supplied tasks completes in a faulted state, the returned task will also complete in a Faulted state, where its exceptions will contain the aggregation of the set of unwrapped exceptions from each of the supplied tasks.

Which is correct, but it doesn't says anything about the aforementioned "unwrapping" behavior of when the returned task is awaited.

I suppose, the docs don't mention it because that behavior is not specific to Task.WhenAll.

It is simply that Task.Exception is of type AggregateException and for await continuations it always gets unwrapped as its first inner exception, by design. This is great for most cases, because usually Task.Exception consists of only one inner exception. But consider this code:

Task WhenAllWrong()
{
    var tcs = new TaskCompletionSource<DBNull>();
    tcs.TrySetException(new Exception[]
    {
        new InvalidOperationException(),
        new DivideByZeroException()
    });
    return tcs.Task;
}

var task = WhenAllWrong();    
try
{
    await task;
}
catch (Exception exception)
{
    // task.Exception is an AggregateException with 2 inner exception 
    Assert.IsTrue(task.Exception.InnerExceptions.Count == 2);
    Assert.IsInstanceOfType(task.Exception.InnerExceptions[0], typeof(InvalidOperationException));
    Assert.IsInstanceOfType(task.Exception.InnerExceptions[1], typeof(DivideByZeroException));

    // However, the exception that we caught here is 
    // the first exception from the above InnerExceptions list:
    Assert.IsInstanceOfType(exception, typeof(InvalidOperationException));
    Assert.AreSame(exception, task.Exception.InnerExceptions[0]);
}

Here, an instance of AggregateException gets unwrapped to its first inner exception InvalidOperationException in exactly the same way as we might have had it with Task.WhenAll. We could have failed to observe DivideByZeroException if we did not go through task.Exception.InnerExceptions directly.

Microsoft's Stephen Toub explains the reason behind this behavior in the related GitHub issue:

The point I was trying to make is that it was discussed in depth, years ago, when these were originally added. We originally did what you're suggesting, with the Task returned from WhenAll containing a single AggregateException that contained all the exceptions, i.e. task.Exception would return an AggregateException wrapper which contained another AggregateException which then contained the actual exceptions; then when it was awaited, the inner AggregateException would be propagated. The strong feedback we received that caused us to change the design was that a) the vast majority of such cases had fairly homogenous exceptions, such that propagating all in an aggregate wasn't that important, b) propagating the aggregate then broke expectations around catches for the specific exception types, and c) for cases where someone did want the aggregate, they could do so explicitly with the two lines like I wrote. We also had extensive discussions about what the behavior of await sould be with regards to tasks containing multiple exceptions, and this is where we landed.

One other important thing to note, this unwrapping behavior is shallow. I.e., it will only unwrap the first exception from AggregateException.InnerExceptions and leave it there, even if it happens to be an instance of another AggregateException. This may add yet another layer of confusion. For example, let's change WhenAllWrong like this:

async Task WhenAllWrong()
{
    await Task.FromException(new AggregateException(
        new InvalidOperationException(),
        new DivideByZeroException()));
}

var task = WhenAllWrong();

try
{
    await task;
}
catch (Exception exception)
{
    // now, task.Exception is an AggregateException with 1 inner exception, 
    // which is itself an instance of AggregateException
    Assert.IsTrue(task.Exception.InnerExceptions.Count == 1);
    Assert.IsInstanceOfType(task.Exception.InnerExceptions[0], typeof(AggregateException));

    // And now the exception that we caught here is that inner AggregateException, 
    // which is also the same object we have thrown from WhenAllWrong:
    var aggregate = exception as AggregateException;
    Assert.IsNotNull(aggregate);
    Assert.AreSame(exception, task.Exception.InnerExceptions[0]);
    Assert.IsInstanceOfType(aggregate.InnerExceptions[0], typeof(InvalidOperationException));
    Assert.IsInstanceOfType(aggregate.InnerExceptions[1], typeof(DivideByZeroException));
}

A solution (TLDR)

So, back to await Task.WhenAll(...), what I personally wanted is to be able to:

  • Get a single exception if only one has been thrown;
  • Get an AggregateException if more than one exception has been thrown collectively by one or more tasks;
  • Avoid having to save the Task only for checking its Task.Exception;
  • Propagate the cancellation status properly (Task.IsCanceled), as something like this would not do that: Task t = Task.WhenAll(...); try { await t; } catch { throw t.Exception; }.

I've put together the following extension for that:

public static class TaskExt 
{
    /// <summary>
    /// A workaround for getting all of AggregateException.InnerExceptions with try/await/catch
    /// </summary>
    public static Task WithAggregatedExceptions(this Task @this)
    {
        // using AggregateException.Flatten as a bonus
        return @this.ContinueWith(
            continuationFunction: anteTask =>
                anteTask.IsFaulted &&
                anteTask.Exception is AggregateException ex &&
                (ex.InnerExceptions.Count > 1 || ex.InnerException is AggregateException) ?
                Task.FromException(ex.Flatten()) : anteTask,
            cancellationToken: CancellationToken.None,
            TaskContinuationOptions.ExecuteSynchronously,
            scheduler: TaskScheduler.Default).Unwrap();
    }    
}

Now, the following works the way I want it:

try
{
    await Task.WhenAll(
        Task.FromException(new InvalidOperationException()),
        Task.FromException(new DivideByZeroException()))
        .WithAggregatedExceptions();
}
catch (OperationCanceledException) 
{
    Trace.WriteLine("Canceled");
}
catch (AggregateException exception)
{
    Trace.WriteLine("2 or more exceptions");
    // Now the exception that we caught here is an AggregateException, 
    // with two inner exceptions:
    var aggregate = exception as AggregateException;
    Assert.IsNotNull(aggregate);
    Assert.IsInstanceOfType(aggregate.InnerExceptions[0], typeof(InvalidOperationException));
    Assert.IsInstanceOfType(aggregate.InnerExceptions[1], typeof(DivideByZeroException));
}
catch (Exception exception)
{
    Trace.WriteLine($"Just a single exception: ${exception.Message}");
}

Upvotes: 69

Richiban
Richiban

Reputation: 5930

I know this is a question that's already answered but the chosen answer doesn't really solve the OP's problem, so I thought I would post this.

This solution gives you the aggregate exception (i.e. all the exceptions that were thrown by the various tasks) and doesn't block (workflow is still asynchronous).

async Task Main()
{
    var task = Task.WhenAll(A(), B());

    try
    {
        var results = await task;
        Console.WriteLine(results);
    }
    catch (Exception)
    {
        if (task.Exception != null)
        {
            throw task.Exception;
        }
    }
}

public async Task<int> A()
{
    await Task.Delay(100);
    throw new Exception("A");
}

public async Task<int> B()
{
    await Task.Delay(100);
    throw new Exception("B");
}

The key is to save a reference to the aggregate task before you await it, then you can access its Exception property which holds your AggregateException (even if only one task threw an exception).

Hope this is still useful. I know I had this problem today.

Upvotes: 143

Alexey Kulikov
Alexey Kulikov

Reputation: 1147

This works for me

private async Task WhenAllWithExceptions(params Task[] tasks)
{
    var result = await Task.WhenAll(tasks);
    if (result.IsFaulted)
    {
                throw result.Exception;
    }
}

Upvotes: -4

jgauffin
jgauffin

Reputation: 101166

You can traverse all tasks to see if more than one have thrown an exception:

private async Task Example()
{
    var tasks = new [] { DoLongThingAsyncEx1(), DoLongThingAsyncEx2() };

    try 
    {
        await Task.WhenAll(tasks);
    }
    catch (Exception ex) 
    {
        var exceptions = tasks.Where(t => t.Exception != null)
                              .Select(t => t.Exception);
    }
}

private Task DoLongThingAsyncEx1()
{
    return Task.Run(() => { throw new InvalidTimeZoneException(); });
}

private Task DoLongThingAsyncEx2()
{
    return Task.Run(() => { throw new InvalidOperationException(); });
}

Upvotes: 51

Daniel Šmon
Daniel Šmon

Reputation: 171

Just thought I'd expand on @Richiban's answer to say that you can also handle the AggregateException in the catch block by referencing it from the task. E.g:

async Task Main()
{
    var task = Task.WhenAll(A(), B());

    try
    {
        var results = await task;
        Console.WriteLine(results);
    }
    catch (Exception ex)
    {
        // This doesn't fire until both tasks
        // are complete. I.e. so after 10 seconds
        // as per the second delay

        // The ex in this instance is the first
        // exception thrown, i.e. "A".
        var firstExceptionThrown = ex;

        // This aggregate contains both "A" and "B".
        var aggregateException = task.Exception;
    }
}

public async Task<int> A()
{
    await Task.Delay(100);
    throw new Exception("A");
}

public async Task<int> B()
{
    // Extra delay to make it clear that the await
    // waits for all tasks to complete, including
    // waiting for this exception.
    await Task.Delay(10000);
    throw new Exception("B");
}

Upvotes: 17

decyclone
decyclone

Reputation: 30840

I don't exactly remember where, but I read somewhere that with new async/await keywords, they unwrap the AggregateException into the actual exception.

So, in catch block, you get the actual exception and not the aggregated one. This helps us write more natural and intuitive code.

This was also needed for easier conversion of existing code into using async/await where the a lot of code expects specific exceptions and not aggregated exceptions.

-- Edit --

Got it:

An Async Primer by Bill Wagner

Bill Wagner said: (in When Exceptions Happen)

...When you use await, the code generated by the compiler unwraps the AggregateException and throws the underlying exception. By leveraging await, you avoid the extra work to handle the AggregateException type used by Task.Result, Task.Wait, and other Wait methods defined in the Task class. That’s another reason to use await instead of the underlying Task methods....

Upvotes: 104

Mohit Datta
Mohit Datta

Reputation: 151

You're thinking of Task.WaitAll - it throws an AggregateException.

WhenAll just throws the first exception of the list of exceptions it encounters.

Upvotes: 11

Related Questions