24

When awaiting a faulted task (one that has an exception set), await will rethrow the stored exception. If the stored exception is an AggregateException it will rethrow the first and discard the rest.

How can we use await and at the same time throw the original AggregateException so that we do not accidentally lose error information?

Note, that it is of course possible to think of hacky solutions for this (e.g. try-catch around the await, then call Task.Wait). I really wish to find a clean solution. What is the best-practice here?

I thought of using a custom awaiter but the built-in TaskAwaiter contains lots of magic that I'm not sure how to fully reproduce. It calls internal APIs on TPL types. I also do not want to reproduce all of that.

Here is a short repro if you want to play with it:

static void Main()
{
    Run().Wait();
}

static async Task Run()
{
    Task[] tasks = new[] { CreateTask("ex1"), CreateTask("ex2") };
    await Task.WhenAll(tasks);
}

static Task CreateTask(string message)
{
    return Task.Factory.StartNew(() => { throw new Exception(message); });
}

Only one of the two exceptions is thrown in Run.

Note, that other questions on Stack Overflow do not address this specific problem. Please be careful when suggesting duplicates.

i3arnon
  • 113,022
  • 33
  • 324
  • 344
usr
  • 168,620
  • 35
  • 240
  • 369
  • 3
    Did you see this blog entry explaining why they chose to implement exception handling for `await` like they did? http://blogs.msdn.com/b/pfxteam/archive/2011/09/28/10217876.aspx – Matthew Watson Aug 19 '13 at 13:52
  • 1
    Yes, thanks. I kind of agree with that reasoning for the general case, but my use case is not covered. – usr Aug 19 '13 at 22:40
  • "In all cases, the Task’s Exception property still returns an AggregateException that contains all of the exceptions, so you can catch whichever is thrown and go back to consult Task.Exception when needed." – Tim Sparkles May 12 '14 at 21:00
  • Relevant GitHub API proposal: [Configure an await to propagate all errors by throwing an AggregateException](https://github.com/dotnet/runtime/issues/47605) – Theodor Zoulias Feb 09 '21 at 23:39
  • Here's a solution that preserves all exceptions and propagates the cancellation status properly: https://stackoverflow.com/a/62607500/1768303 – noseratio Aug 02 '21 at 01:48
  • 1
    Here is an updated link to the blog post originally shared by Matthew Watson: https://devblogs.microsoft.com/pfxteam/task-exception-handling-in-net-4-5/ – Holistic Developer Aug 30 '22 at 01:12

6 Answers6

27

I disagree with the implication in your question title that await's behavior is undesired. It makes sense in the vast majority of scenarios. In a WhenAll situation, how often do you really need to know all of the error details, as opposed to just one?

The main difficulty with AggregateException is the exception handling, i.e., you lose the ability to catch a particular type.

That said, you can get the behavior you want with an extension method:

public static async Task WithAggregateException(this Task source)
{
  try
  {
    await source.ConfigureAwait(false);
  }
  catch
  {
    // source.Exception may be null if the task was canceled.
    if (source.Exception == null)
      throw;

    // EDI preserves the original exception's stack trace, if any.
    ExceptionDispatchInfo.Capture(source.Exception).Throw();
  }
}
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • 13
    Getting all exceptions is important for diagnostics. I don't ever want to miss nullrefs and so on. They must be logged. *Handling* an exception is a different scenario though. Btw, the solution works fine. Thanks. – usr Aug 19 '13 at 14:01
  • 2
    If you're talking about [fatal or boneheaded exceptions](http://blogs.msdn.com/b/ericlippert/archive/2008/09/10/vexing-exceptions.aspx), I recommend using the top-level handlers (`UnobservedTaskException`, Application handlers) instead of repeated `try`/`catch` blocks. – Stephen Cleary Aug 19 '13 at 14:09
  • 2
    I agree mostly. The scenario I have in mind is a top-level error handler indeed (all it does is log crashes). But the full crash info must somehow arrive there. UnobservedTaskException and Application handlers do not guarantee that in the presence of await because errors might end up observed but swallowed.; I get very nervous when I see errors swallowed. I have seen too many cases where production suffered from severe errors for month and nobody noticed. – usr Aug 19 '13 at 14:14
  • 2
    [`UnobservedTaskException` is still raised](http://blogs.msdn.com/b/pfxteam/archive/2011/09/28/10217876.aspx) for unobserved exceptions (it just doesn't crash the process anymore). The only case where you have an observed, swallowed exception is `WhenAll`, and it only happens when you have multiple exceptions, one of which is *not* swallowed. You really do have to *try* to ignore exceptions with `await`. – Stephen Cleary Aug 19 '13 at 14:21
  • @RoyiNamir I had planned adding an answer of my own detailing all the different possible approaches. I have found a few more in the meantime. But who knows when I'll get around to doing that. – usr May 10 '14 at 09:48
  • @StephenCleary ( I know what ConfigureAwait does ) but how come it helps also in throwing The aggregated exceptions ? I mean , How does this relate ? – Royi Namir May 10 '14 at 16:45
  • @RoyiNamir: It doesn't. It's just saying that the context doesn't have to be resumed on to `throw`. – Stephen Cleary May 11 '14 at 01:10
  • 1
    The reason why one would want to know all the exception details is similar to why there's [CA1031: Do not catch general exception types](http://msdn.microsoft.com/en-us/library/ms182137.aspx). Like a "catch all" clause `await` can hide exceptions without you ever finding out.. or only after lots and lots of time spent on debugging. Catch-all clauses used to be frowned upon - and i still think they are. Why should the same reasons not apply to `await Task.WhenAll`? – BatteryBackupUnit Jan 20 '15 at 09:47
  • 1
    @BatteryBackupUnit: In the vast majority of async code, `await` does the right thing by only considering a single exception. `WhenAll` is the only situation where this *may not* be the best solution. When I've used `WhenAll`, I almost never need to know all the exceptions - just that there was *an* exception. So `await` does the right thing the vast, vast majority of the time. It's different than `catch { }` because only the *additional* exceptions are ignored, not all of them. – Stephen Cleary Jan 20 '15 at 14:11
  • @StephenCleary I'm unclear whether you're advocating the current design as the best possible design or just the best one "you" (MS) could pick given pre-existing constraints (Task's throwing AggregateException..). If it's the later then i think it would be nice if one could move forward - getting the design better. I fully acknowledge that such a transition could take years to implement. If it's the former (best design possible) then i don't agree. – BatteryBackupUnit Jan 20 '15 at 14:33
  • 1
    @BatteryBackupUnit: I think it's the best design possible. (BTW, I don't work for MS and never have). Feel free to write up an alternate design and post a link, and I might change my mind. :) – Stephen Cleary Jan 20 '15 at 14:46
  • 1
    regarding MS, sorry, i think i mixed you up with Mr. Toub :/ as far as the design goes: Task should always rethrow an exception directly (no wrapping as `AggregateException` by infrastructure), always a single exception directly. Special cases like `Task.WhenAll` should throw an `AggregateException` or maybe a more specific exception like `ParallelWorkJoinException` (which would wrap 1-n exceptions - 1 for each failed Task). `await` would then not need to unwrap an exception. Same behavior as of now for 99% cases, special cases get obvious special treatment - straight forward. – BatteryBackupUnit Jan 20 '15 at 15:34
  • 2
    @BatteryBackupUnit: Task has two primary uses: one as a "future" for asynchronous code, and the other as a "unit of work" for parallel code. I think your approach would require a mutable/freezable `AggregateException` at least. Feel free to [post your idea for discussion](https://github.com/dotnet/corefx). Aside: technically, `await` doesn't unwrap the `AggregateException`, but that's an OK mental model. – Stephen Cleary Jan 20 '15 at 16:09
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/69231/discussion-between-batterybackupunit-and-stephen-cleary). – BatteryBackupUnit Jan 20 '15 at 16:17
  • 3
    Take into account that `source.Exception` could be a null reference during cancellation. – tm1 Apr 15 '19 at 10:16
  • @tm1: Excellent point, thanks! I've updated the code in the answer. – Stephen Cleary Apr 15 '19 at 22:47
  • 1
    @StephenCleary The fact that this update took nearly six years seems to be an excellent demonstration of your point that this is unnecessary in most cases. – bornfromanegg May 07 '20 at 06:28
  • 1
    The `Task.Exception` property returns a different `AggregateException` instance each time. So it should be slightly more efficient to get the exception once, and store it in a local variable. Or do `if (!source.IsFaulted)` instead of `if (source.Exception == null)`. Also using the `ExceptionDispatchInfo` is most likely redundant. The `source.Exception` is a newly created instance, that has never been thrown before ([source code](https://github.com/dotnet/runtime/blob/a94fa0652fffd7cc3220e096d3ae9c409f126571/src/libraries/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs#L1252)). – Theodor Zoulias Dec 07 '22 at 12:35
5

I know I'm late but i found this neat little trick which does what you want. Since the full set of exceptions are available with on awaited Task, calling this Task's Wait or a .Result will throw an aggregate exception.

    static void Main(string[] args)
    {
        var task = Run();
        task.Wait();
    }
    public static async Task Run()
    {

        Task[] tasks = new[] { CreateTask("ex1"), CreateTask("ex2") };
        var compositeTask = Task.WhenAll(tasks);
        try
        {
            await compositeTask.ContinueWith((antecedant) => { }, TaskContinuationOptions.ExecuteSynchronously);
            compositeTask.Wait();
        }
        catch (AggregateException aex)
        {
            foreach (var ex in aex.InnerExceptions)
            {
                Console.WriteLine(ex.Message);
            }
        }
    }

    static Task CreateTask(string message)
    {
        return Task.Factory.StartNew(() => { throw new Exception(message); });
    }
adonthy
  • 151
  • 2
  • 6
5

Here is a shorter implementation of Stephen Cleary's WithAggregateException extension method:

public static async Task WithAggregateException(this Task source)
{
    try { await source.ConfigureAwait(false); }
    catch when (source.IsCanceled) { throw; }
    catch { source.Wait(); }
}

public static async Task<T> WithAggregateException<T>(this Task<T> source)
{
    try { return await source.ConfigureAwait(false); }
    catch when (source.IsCanceled) { throw; }
    catch { return source.Result; }
}

This approach is based on a suggestion by Stephen Toub in this API proposal in GitHub.


Update: I added a special handling of the cancellation case, to prevent the awkwardness of propagating an AggregateException that contains an OperationCanceledException. Now the OperationCanceledException is propagated directly, and the Task.IsCanceled status is preserved. Kudos to @noseratio for pointing out this flaw in the comments of this answer. Of course now this implementation is not much shorter than Stephen Cleary's approach!

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
  • @ConstantLearner this is not a modified version of Stephen Cleary's answer. This is a different implementation, that also happens to be a bit shorter. Regarding the `ExceptionDispatchInfo`, it's not needed in my implementation because I don't `throw` directly any exception. The exception is thrown indirectly by calling the `Task.Wait` method. I don't think that it's needed in Stephen Cleary's implementation either. The `Task.Exception` property is just a container for `InnerExceptions`. It doesn't have a stack trace by itself. So `throw source.Exception;` should be the same. – Theodor Zoulias Dec 07 '22 at 12:22
3

Exception Handling (Task Parallel Library)

I could say more but it would just be padding. Play with it, it does work as they say. You just have to be careful.

maybe you want this

God (Jon Skeet) explains await exception handling

(personally i shy away from await, but thats just my preference)

in response to comments (too long for a comment reply)

Then use threads as your starting point for an analogous argument as the best practises there will be the source of ones for here.

Exceptions happily get swallowed unless you implement code to pass them out (for instance the async pattern that the await is preumably wrapping ... you add them to an event args object when you raise an event). When you have a scenario where you fire up an arbitrary number of threads and execute on them you have no control over order or the point at which you terminate each thread. Moreover you would never use this pattern if an error on one was relevant to another. Therefor you are strongly implying that execution of the rest is completley independent - IE you are strongly implying that exceptions on these threads have already been handled as exceptions. If you want to do something beyond handling exceptions in these threads in the threads they occur in (which is bizzarre) you should add them to a locking collection that is passed in by reference - you are no longer considering exceptions as exceptions but as a piece of information - use a concurrent bag, wrap the exception in the info you need to identify the context it came from - which would of been passed into it.

Don't conflate your use cases.

activout.se
  • 6,058
  • 4
  • 27
  • 37
John Nicholas
  • 4,778
  • 4
  • 31
  • 50
  • The first link does not deal with `await`. The 2nd one contains a valid answer. Thanks. I will let other answers come in for a few days because I want to discover the *best* solution. – usr Aug 19 '13 at 13:58
  • the thing is i dont agree with your use case. I think when you are using await you are assuming that each task handles its own exceptions - await tells you that something has broken in an operation where you cannot cancel the other tasks (ie they must be independant). Or at least i think that is assumed in its pattern. Ifyou want to handle exceptions outside of the task then you should use the top pattern. I probably should of explained that. Also if you want aggregate exception - the question - the first should be your choice. – John Nicholas Aug 21 '13 at 16:40
  • The top pattern involves blocking which is an entirely different use case than await addresses. A valid use case but not mine - I require async IO and non-blocking here.; Also, tasks do not necessarily handle their own errors. If that was the case we'd never need error propagation. Errors are *supposed* to propagate so that the normal flow of control is aborted. All I want is *all* errors, not just an arbitrary one. – usr Aug 21 '13 at 16:48
  • updated ... ofc a task handles its own errors ... a task it a unit of encapsulation around a unit of work. The work of handling its exceptions I would argue is most definatley a part of that or else you are creating a locking nightmare for yourself as 100 threads all throw exceptions and block on one piece of code - unless you make it have no state at which point u would realise that it essentially sits in the task itself anyway ... or compromise with me and pass in a lambda to handle the exception. – John Nicholas Aug 27 '13 at 16:31
  • @JohnNicholas reference to god's article is broken ( – Konstantin Chernov Nov 12 '16 at 00:00
  • https://codeblog.jonskeet.uk/2011/06/22/eduasync-part-11-more-sophisticated-but-lossy-exception-handling/ .... god im opinionated at times – John Nicholas Nov 15 '16 at 20:15
1

I don't want to give up the practice to only catch the exceptions I expect. This leads me to the following extension method:

public static async Task NoSwallow<TException>(this Task task) where TException : Exception {
    try {
        await task;
    } catch (TException) {
        var unexpectedEx = task.Exception
                               .Flatten()
                               .InnerExceptions
                               .FirstOrDefault(ex => !(ex is TException));
        if (unexpectedEx != null) {
            throw new NotImplementedException(null, unexpectedEx);
        } else {
            throw task.Exception;
        }
    }
}

The consuming code could go like this:

try {
    await Task.WhenAll(tasks).NoSwallow<MyException>();
catch (AggregateException ex) {
    HandleExceptions(ex);
}

A bone-headed exception will have the same effect as in synchronous world, even in case it is thrown concurrently with a MyException by chance. The wrapping with NotImplementedException helps to not loose the original stack trace.

tm1
  • 1,180
  • 12
  • 28
0

Extension that wraps original aggregation exception and doesn't change return type, so it can still be used with Task<T>

 public static Task<T> UnswallowExceptions<T>(this Task<T> t)
        => t.ContinueWith(t => t.IsFaulted ? throw new AggregateException("whatever", t.Exception) : t.Result);

Example:

Task<T[]> RunTasks(Task<T>[] tasks) => 
Task.WhenAll(CreateSometasks()).UnswallowExceptions();
try
{ var result = await CreateTasks(); }
catch(AggregateException ex) {  } //ex is original aggregation exception here

NOTE This method will throw if task was canceled, use another approach if cancelling is important for you

Vitaly
  • 2,064
  • 19
  • 23
  • The `ContinueWith` is a [primitive](https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html) method, and should be used with great care, especially in conjunction with async/await code. Also how about the case that the `t` is canceled? How does the `UnswallowExceptions` method handle this case? – Theodor Zoulias Mar 10 '21 at 16:16
  • It will obviously throw. I will update answer with this information – Vitaly Mar 10 '21 at 17:09
  • I don't think it will throw. It will probably return a task that can never transition to a `Canceled` state. Btw without having tested your approach, I think that it should behave similar with this shorter version: `=> t.ContinueWith(t => t.Result);` – Theodor Zoulias Mar 10 '21 at 17:40
  • I tested before updating an answer, it throws TaskCanceledException when access t.Result and than it's wrapped in AggregationException, so you lose original exceptions and catch AggregationException with only TaskCanceledException inside. A check for t.IsCanceled could be added before accessing t.Result but then you get wrong task result and not cancelled task. if you need all of that, you'd better check John Skeet's solution, it covers everything but creating TaskCompletionSource on every call was overkill for me – Vitaly Mar 11 '21 at 08:59
  • Why not use Stephen Cleary's [solution](https://stackoverflow.com/a/18315625/11178549)? It is short, and it seems to handle all cases correctly. Do you want to avoid async/await for some reason? – Theodor Zoulias Mar 11 '21 at 09:04
  • Checked your shorter version, it works similar to throw t.Exception, if only use ex.Flatten().InnerExceptions then it's even better since you don't create new Exception. But stacktrace and source is changed, not very important in this case but might not be desired – Vitaly Mar 11 '21 at 09:44