HiveBrain v1.2.0
Get Started
← Back to all entries
debugcsharpMinor

Is this the wrong way to handle AggregateException with Tasks

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisaggregateexceptionthehandlewithwaytaskswrong

Problem

I'm seeing a lot of code like this at my new site

try
        {
            Task.Factory.StartNew(() => 
            {
            ...
            });
        }
        catch (AggregateException ex)
        {
            ex.Handle((x) =>
            {
                Log.Write(x);
                return true;
            });
        }


From what I've read, as Wait(), Result or Exception are not called the AggregateException will not have been marshalled. Also the Task may not even of started yet.
I'm thinking of using Joe Albahari's extension method that uses a continuation as these are mainly fire and forget Tasks

public static void PrintExceptions (this Task task)
{
       task.ContinueWith (t => { var ignore = t.Exception; },
       TaskContinuationOptions.OnlyOnFaulted);
}


Am I on the right track?

Solution

You are quite right. These exception handling blocks will never catch any task exceptions. The code should be modified to check the Task.IsFaulted flag in the continuation and check the Task.Exception property for the actual exception.

In fact, should an exception occur it will remain unhandled and your application will crash unless you attach an exception handler to TaskScheduler.UnobservedTaskException.

You should definitely check the "Exception Handling (Task Parallel Library)" article in MSDN for the proper ways to handle Task exceptions.

You should also take a look at the Async CTP for C#. It has a production license even though it is technically a CTP and it will make coding with Tasks a lot easier and safer, as you can see from the snippet I found here:

private async void FetchData()
{
    using (var wc = new WebClient())
    {
        try
        {
            string content = await wc.DownloadStringTaskAsync(
                new Uri("http://www.interact-sw.co.uk/oops/"));

            textBox1.Text = content;
        }
        catch (WebException x)
        {
            textBox1.Text = "Error: " + x.Message;
        }
    }
}


You can use the CTP just by adding a reference to the appropriate DLL. The resulting code will certainly be safer that what you posted here, CTP or not.

Code Snippets

private async void FetchData()
{
    using (var wc = new WebClient())
    {
        try
        {
            string content = await wc.DownloadStringTaskAsync(
                new Uri("http://www.interact-sw.co.uk/oops/"));

            textBox1.Text = content;
        }
        catch (WebException x)
        {
            textBox1.Text = "Error: " + x.Message;
        }
    }
}

Context

StackExchange Code Review Q#8494, answer score: 3

Revisions (0)

No revisions yet.