Rethrowing previous exception inside ContinueWith

Question

Intro

After puzzling over my code for a while, I discovered that exceptions don't necessarily propagate through ContinueWith:

int zeroOrOne = 1;
Task.Factory.StartNew(() => 3 / zeroOrOne)
    .ContinueWith(t => t.Result * 2)
    .ContinueWith(t => Console.WriteLine(t.Result))
    .ContinueWith(_ => SetBusy(false))
    .LogExceptions();

In this example, the SetBusy line 'resets' the chain of exceptions, so the divide by zero exception isn't seen and subsequently blows up in my face with "A Task's exception(s) were not observed..."

So... I wrote myself a little extension method (with tons of different overloads, but basically all doing this):

public static Task ContinueWithEx(this Task task, Action<Task> continuation)
{
     return task.ContinueWith(t =>
     {
         if(t.IsFaulted) throw t.Exception;
         continuation(t);
     });
}

Searching around a bit more, I came across this blog post, where he proposes a similar solution, but using a TaskCompletionSource, which (paraphrased) looks like this:

public static Task ContinueWithEx(this Task task, Action<Task> continuation)
{
     var tcs = new TaskCompletionSource<object>();
     task.ContinueWith(t =>
     {
         if(t.IsFaulted) tcs.TrySetException(t.Exception);
         continuation(t);
         tcs.TrySetResult(default(object));
     });
     return tcs.Task;
}

Question

Are these two versions strictly equivalent? Or is there a subtle difference between throw t.Exception and tcs.TrySetException(t.Exception)?

Also, does the fact that there's apparently only one other person on the whole internet who's done this indicate that I'm missing the idiomatic way of doing this?

Answer 1

Since you are checking the status of the task before invoking the continuation, I don't see any compelling reason to pass the task in the continuation as argument. So instead of Action<Task> I would prefer a parameter of type Action.

There is a simpler way to implement the ContinueWithEx, that avoids both the performance impact of throwing/catching exceptions, the mess of nested AggregateExceptions, and the complexity of the TaskCompletionSource class. The trick is to return the antecedent task, and use the Unwrap method to unwrap the resulting nested Task<Task>:

public static Task ThenIfSuccessful(this Task task, Action continuation)
{
     return task.ContinueWith(t =>
     {
         if (t.IsCompletedSuccessfully)
             continuation();
         return t;
     }, TaskScheduler.Default).Unwrap();
}

And with generic Task<T>:

public static Task<T> ThenIfSuccessful<T>(this Task<T> task, Action<T> continuation)
{
     return task.ContinueWith(t =>
     {
         if (t.IsCompletedSuccessfully)
             continuation(t.Result);
         return t;
     }, TaskScheduler.Default).Unwrap();
}

Answer 2

The difference between the two is subtle. In the first example, you are throwing the exception returned from the task. This will trigger the normal exception throwing and catching in the CLR, the ContinueWith will catch and wrap it and pass it to the next task in the chain.

In the second you are calling TrySetException which will still wrap the exception and pass it to the next task in the chain, but does not trigger any try/catch logic.

The end result after one ContinueWithEx is AggregateException(AggregateException(DivideByZeroException)). The only difference I see is that the inner AggregateException has a stack trace set in the first example (because it was thrown) and no stack trace in the second example.

Neither is likely to be significantly faster than the other, but I would personally prefer the second to avoid unneeded throws.

I have done something like this where the continuation returned a result. I called it Select, handled cases of the previous task being cancelled, provided overloads to modify the exception instead of or in addition to the result, and used the ExecuteSynchronously option. When the continuation would itself return a Task, I called that Then instead based on the code from this article.

Answer 3

I used this example in one of my projects, but there were quite an mount of edge cases that were not covered. Since I needed this operation to serve as a core method, I took the time to make it a bit more product ready. Here you are the version I ended with, it met the expectations of my test battery

/// <summary>
/// https://stackoverflow.com/q/11839959/17780206 - Rethrowing previous exception inside ContinueWith
/// </summary>
public static Task<TResult> ContinueWithEx<T, TResult>(this Task<T> task, Func<Task<T>, TResult> continuation)
{
    // we want to run continuations asynchronously so that we don't block the thread, this way we can avoid deadlocks
    var tcs = new TaskCompletionSource<TResult>(TaskCreationOptions.RunContinuationsAsynchronously);

    // we use the default scheduler to avoid deadlocks, this is mostly important for UI applications
    var taskScheduler = TaskScheduler.Default;

    task.ContinueWith(t => {
        if (t.IsFaulted)
        {
            var exception = t.Exception?.InnerExceptions.Count == 1 ? t.Exception.InnerExceptions[0] : t.Exception;
            if (!tcs.TrySetException(exception!))
            {
                // this is not expected to happen, but if it does, log it
                Logger.Warning("Failed to set exception because state of the task is already set", exception);
            }
        }
        else if (t.IsCanceled)
        {
            if (!tcs.TrySetCanceled())
            {
                // this is not expected to happen, but if it does, log it
                Logger.Warning("Failed to set canceled because state of the task is already set");
            }
        }
        else
        {
            // this try catch is important to catch exceptions that might be thrown by the continuation
            // if an exception is thrown, we want to set the exception on the task completion source
            // if we would not do that, the code will freeze in such occurence because the task is never marked as completed/cancelled/failed
            try
            {
                var result = continuation(t);
                if (!tcs.TrySetResult(result))
                {
                    // this is not expected to happen, but if it does, log it
                    Logger.Warning("Failed to set result because state of the task is already set");
                }
            }
            catch (Exception ex)
            {
                if (!tcs.TrySetException(ex))
                {
                    // this is not expected to happen, but if it does, log it
                    Logger.Warning("Failed to set exception because state of the task is already set", ex);
                }
            }
        }
    }, taskScheduler);
    return tcs.Task;
}