Is catching TaskCanceledException and checking Task.Canceled a good idea?

Tim Lovell-Smith picture Tim Lovell-Smith · Jul 3, 2014 · Viewed 9k times · Source

There are some people on my team who really love coding with async Task. And sometimes they like to use CancellationToken parameters.

What I'm unsure about is whether we should as a team be using this style of code (A):

async Task<someObject> DoStuff(CancellationToken t)
{
    while (!t.IsCanceled)
    {
        try {
            Task.Delay(5000, t);
        }
        catch (AggregateException e) // or is it TaskCanceledException or OperationCanceledException? I don't know? :)
        {
        }
        // poll something, return someObject, or null
    }
    return null;
}

This obviously means the caller probably has to check the cancellation token themselves to determine whether to continue processing, and they might have to handle null retVals:

var retVal = await DoStuff(token);
if (token.IsCanceled) { ... }

However, if we adopt a second style of code (B) that relies on TaskCanceledException:

async Task<someObject> DoStuff(CancellationToken t)
{
    while(true)
    {
        Task.Delay(5000, t);
        // poll something, return someObject, or null
    }
}

The implementation code is definitely simpler - and the caller has the option of handling the exception or not, as appropriate... but I can't help worrying that callers might forget that TaskCanceledException is something they have to worry about, and processes may crash as a result of them not catching these exceptions (on foreground or background threads).

So, my overly optimistically phrased question is: which do you think is the best style that everyone should always use, and why? :)

Answer

i3arnon picture i3arnon · Jul 3, 2014

In the .Net framework itself when you pass a CancellationToken as a parameter you will get back a TaskCanceledException. I would not go against that and create my own design pattern because people who are familiar with .Net will be familiar with your code.

My guideline is this: The one that cancels the token is the one that should handle the TaskCanceledException, so If you're using a CancellationToken inside your method for your own reasons, go ahead and use a try-catch block. But if you get the token as a parameter, let the exception be thrown.