7
\$\begingroup\$

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?

\$\endgroup\$
3
  • 1
    \$\begingroup\$ That looks wrong to me. You should inspect the code base for other, subtler, worse mistakes or misconceptions. Threading is hard, and I have yet to see correctly multi-threaded corporate code. \$\endgroup\$
    – SLaks
    Jan 30 '12 at 2:08
  • \$\begingroup\$ What can be worse than never checking for exceptions? That's what this code does. \$\endgroup\$ Feb 1 '12 at 16:30
  • 1
    \$\begingroup\$ This is bad code and should never be written. blog.stephencleary.com/2013/08/startnew-is-dangerous.html \$\endgroup\$ Dec 10 '15 at 22:26
3
\$\begingroup\$

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.

\$\endgroup\$
0
2
\$\begingroup\$

I agree with what has been said before. Here is what I think

As far as I was aware, there are certain methods/properties that cause a Task to have its Exceptions "Observed", these are "Result/Wait()"

As already stated you would also need to handle TaskScheduler.UnobservedTaskException is 100% correct as far my knowledge goes.

\$\endgroup\$
1
  • \$\begingroup\$ This is much better as a (terse) comment \$\endgroup\$
    – jpaugh
    May 15 '17 at 19:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.