Proper way to handle C# AggregateException

Question

I have a question about when its safe to handle an aggregate exception when using WhenAll(). It seems like the natural place would be inside the catch block, because if the catch block never fires, it would imply there are no exceptions to handle. But I see a lot of code that has an empty catch block and checks for the existence of an AggregateException before handling any found exceptions (including on MS website).


    public async Task MyMethod() {

      var tasks = new List<Task>();
      for (var i = 0; i < 10; i++) {
        tasks.Add(DoSthAsync());
      }

      var masterTask = Task.WhenAll(tasks);
      try {
        var results = await masterTask;
      } catch {
        // Safe to access masterTask here and handle aggregate exceptions? Have all tasks completed?
        foreach (var ex in masterTask.Exception.innerExceptions) {
          HandleException(ex);
        }
      }

      // Or necessary to check for and handle aggregate exceptions here?
      if (masterTask.Exception != null) {
        foreach (var ex in masterTask.Exception.innerExceptions) {
          HandleException(ex);
        }
      }
    }

    public async Task DoSthAsync() {
      // ...
    }

score 2 · Answer 1 · answered May 12 '19 at 01:00

It seems like the natural place would be inside the catch block

Yup, that would work fine. Task.WhenAll returns a task that completes when all the tasks have completed. So in your case, by the time your code enters the catch block, masterTask has completed, and this means all the tasks have completed.

score 1 · Accepted Answer · answered May 12 '19 at 08:55

The code that you posted works because Task.WhenAll returns a task that only completes when all sub tasks have completed.

Why does the code do it that way? Why no catch (Exception ex)? This is because await only throws the first inner exception. If you need access to multiple exceptions this code pattern is a good way to do it. You also could do catch (AggregateException ex) and use that object. It's the same object.

I personally avoid using catch like that. Essentially it's using exceptions for control flow. This makes debugging harder and can lead to verbose code.

I like this:

var whenAllTask = Task.WhenAll(...);

await whenAllTask.ContinueWith(_ => { }); //never throws

if (whenAllTask.Exception != null) ... //handle exceptions

I made the .ContinueWith(_ => { }) bit into a WhenCompleted extension method so that the code looks clean.

Then you wondered whether the second way to check for exceptions is a good idea:

  // Or necessary to check for and handle aggregate exceptions here?
  if (masterTask.Exception != null) {
    foreach (var ex in masterTask.Exception.innerExceptions) {
      HandleException(ex);
    }
  }

You can certainly do that. Essentially it's the same thing. Use what is more convenient in the particular situation.

Proper way to handle C# AggregateException

2 Answers2