Elegantly handle task cancellation

Question

When using tasks for large/long running workloads that I need to be able to cancel I often use a template similar to this for the action the task executes:

public void DoWork(CancellationToken cancelToken)
{
    try
    {
        //do work
        cancelToken.ThrowIfCancellationRequested();
        //more work
    }
    catch (OperationCanceledException)
    {
        throw;
    }
    catch (Exception ex)
    {
        Log.Exception(ex);
        throw;
    }
}

The OperationCanceledException should not be logged as an error but must not be swallowed if the task is to transition into the cancelled state. Any other exceptions do not need to be dealt with beyond the scope of this method.

This always felt a bit clunky, and visual studio by default will break on the throw for OperationCanceledException (though I have 'break on User-unhandled' turned off now for OperationCanceledException because of my use of this pattern).

UPDATE: It's 2021 and C#9 gives me the syntax I always wanted:

public void DoWork(CancellationToken cancelToken)
{
    try
    {
        //do work
        cancelToken.ThrowIfCancellationRequested();
        //more work
    }
    catch (Exception ex) when (ex is not OperationCanceledException)
    {
        Log.Exception(ex);
        throw;
    }
}

Ideally I think I'd like to be able to do something like this:

public void DoWork(CancellationToken cancelToken)
{
    try
    {
        //do work
        cancelToken.ThrowIfCancellationRequested();
        //more work
    }
    catch (Exception ex) exclude (OperationCanceledException)
    {
        Log.Exception(ex);
        throw;
    }
}

i.e. have some sort of exclusion list applied to the catch but without language support that is not currently possible (@eric-lippert: c# vNext feature :)).

Another way would be through a continuation:

public void StartWork()
{
    Task.Factory.StartNew(() => DoWork(cancellationSource.Token), cancellationSource.Token)
        .ContinueWith(t => Log.Exception(t.Exception.InnerException), TaskContinuationOptions.OnlyOnFaulted | TaskContinuationOptions.ExecuteSynchronously);
}

public void DoWork(CancellationToken cancelToken)
{
    //do work
    cancelToken.ThrowIfCancellationRequested();
    //more work
}

but I don't really like that as the exception technically could have more than a single inner exception and you don't have as much context while logging the exception as you would in the first example (if I was doing more than just logging it).

I understand this is a bit of a question of style, but wondering if anyone has any better suggestions?

Do I just have to stick with example 1?

Answer 1

So, what's the problem? Just throw away catch (OperationCanceledException) block, and set proper continuations:

var cts = new CancellationTokenSource();
var task = Task.Factory.StartNew(() =>
    {
        var i = 0;
        try
        {
            while (true)
            {
                Thread.Sleep(1000);

                cts.Token.ThrowIfCancellationRequested();

                i++;

                if (i > 5)
                    throw new InvalidOperationException();
            }
        }
        catch
        {
            Console.WriteLine("i = {0}", i);
            throw;
        }
    }, cts.Token);

task.ContinueWith(t => 
        Console.WriteLine("{0} with {1}: {2}", 
            t.Status, 
            t.Exception.InnerExceptions[0].GetType(), 
            t.Exception.InnerExceptions[0].Message
        ), 
        TaskContinuationOptions.OnlyOnFaulted);

task.ContinueWith(t => 
        Console.WriteLine(t.Status), 
        TaskContinuationOptions.OnlyOnCanceled);

Console.ReadLine();

cts.Cancel();

Console.ReadLine();

TPL distinguishes cancellation and fault. Hence, cancellation (i.e. throwing OperationCancelledException within task body) is not a fault.

The main point: do not handle exceptions within task body without re-throwing them.

Answer 2

Here is how you elegantly handle Task cancellation:

Handling "fire-and-forget" Tasks

var cts = new CancellationTokenSource( 5000 );  // auto-cancel in 5 sec.
Task.Run( () => {
    cts.Token.ThrowIfCancellationRequested();

    // do background work

    cts.Token.ThrowIfCancellationRequested();

    // more work

}, cts.Token ).ContinueWith( task => {
    if ( !task.IsCanceled && task.IsFaulted )   // suppress cancel exception
        Logger.Log( task.Exception );           // log others
} );

Handling await Task completion / cancellation

var cts = new CancellationTokenSource( 5000 ); // auto-cancel in 5 sec.
var taskToCancel = Task.Delay( 10000, cts.Token );  

// do work

try { await taskToCancel; }           // await cancellation
catch ( OperationCanceledException ) {}    // suppress cancel exception, re-throw others

Answer 3

You could do something like this:

public void DoWork(CancellationToken cancelToken)
{
    try
    {
        //do work
        cancelToken.ThrowIfCancellationRequested();
        //more work
    }
    catch (OperationCanceledException) when (cancelToken.IsCancellationRequested)
    {
        throw;
    }
    catch (Exception ex)
    {
        Log.Exception(ex);
        throw;
    }
}

Answer 4

C# 6.0 has a solution for this..Filtering exception

int denom;

try
{
     denom = 0;
    int x = 5 / denom;
}

// Catch /0 on all days but Saturday

catch (DivideByZeroException xx) when (DateTime.Now.DayOfWeek != DayOfWeek.Saturday)
{
     Console.WriteLine(xx);
}

Answer 5

According to this MSDN blog post, you should catch OperationCanceledException, e.g.

async Task UserSubmitClickAsync(CancellationToken cancellationToken)
{
   try
   {
      await SendResultAsync(cancellationToken);
   }
   catch (OperationCanceledException) // includes TaskCanceledException
   {
      MessageBox.Show(“Your submission was canceled.”);
   }
}

If your cancelable method is in between other cancelable operations, you may need to perform clean up when canceled. When doing so, you can use the above catch block, but be sure to rethrow properly:

async Task SendResultAsync(CancellationToken cancellationToken)
{
   try
   {
      await httpClient.SendAsync(form, cancellationToken);
   }
   catch (OperationCanceledException)
   {
      // perform your cleanup
      form.Dispose();

      // rethrow exception so caller knows you’ve canceled.
      // DON’T “throw ex;” because that stomps on 
      // the Exception.StackTrace property.
      throw; 
   }
}

Answer 6

I am not entirely sure of what you are trying to achieve here but I think the following pattern might help

public void DoWork(CancellationToken cancelToken)
{
    try
    {
        //do work
        cancelToken.ThrowIfCancellationRequested();
        //more work
    }
    catch (OperationCanceledException) {}
    catch (Exception ex)
    {
        Log.Exception(ex);
    }
}

You might have observed that I have removed the throw statement from here. This will not throw the exception but will simply ignore it.

Let me know if you intend to do something else.

There is yet another way which is quite close to what you have exhibited in your code

    catch (Exception ex)
    {
        if (!ex.GetType().Equals(<Type of Exception you don't want to raise>)
        {
            Log.Exception(ex);

        }
    }