Tasks: exceptions and cancelation

Question

I need to do a long running task. I've done this by executing a Task while there's a loading box on the UI. When an exception is thrown, I want to stop the task and show a msgbox to the user. If everything goes right, I stop the loading box.

The code below works as expected, but I was wondering if I'm doing it correct here since this is the first time I'm doing something like this.

MyClassLibrary

Baseclass:

 private readonly CancellationTokenSource cancellationTokenSource = new CancellationTokenSource();
 public MyEnum Stage {get; protected set;}
 public event EventHandler<EventArgs> ProgrammingStarted;
 public event EventHandler<ProgrammingExecutedEventArgs> ProgrammingExecuted;
 protected void ProgramImage()
 {
 this.OnProgrammingStarted(new EventArgs());
 var task =
 Task.Factory.StartNew(this.ProgramImageAsync)
 .ContinueWith(
 this.TaskExceptionHandler,
 cancellationTokenSource.Token,
 TaskContinuationOptions.OnlyOnFaulted,
 TaskScheduler.FromCurrentSynchronizationContext()) //Catch exceptions here
 .ContinueWith(
 o => this.ProgramImageAsyncDone(),
 cancellationTokenSource.Token,
 TaskContinuationOptions.NotOnFaulted,
 TaskScheduler.FromCurrentSynchronizationContext()); //Run this when no exception occurred
 }
 private void ProgramImageAsync()
 {
 Thread.Sleep(5000); // Actual programming is done here
 throw new Exception("test");
 }
 private void TaskExceptionHandler(Task task)
 {
 var exception = task.Exception;
 if (exception != null && exception.InnerExceptions.Count > 0)
 {
 this.OnProgrammingExecuted(
 new ProgrammingExecutedEventArgs { Success = false, Error = exception.InnerExceptions[0] });
 this.Explanation = "An error occurrred during the programming.";
 }
 // Stop execution of further taks
 this.cancellationTokenSource.Cancel();
 }
 private void ProgramImageAsyncDone()
 {
 this.OnProgrammingExecuted(new ProgrammingExecutedEventArgs { Success = true });
 this.Explanation = ResourceGeneral.PressNextBtn_Explanation;
 this.IsStepComplete = true;
 }

Derived class:

public override AdvanceStage()
{
 switch (this.Stage)
 {
 case MyEnum.Stage1:
 this.DoSomething();
 this.Stage = MyEnum.Stage2;
 break;
 case MyEnum.Stage2:
 this.ProgramImage();
 this.Stage = MyEnum.Stage6;
 break;
 case MyEnum.Stage3:
 this.DoSomethingElse();
 this.Stage = MyEnum.Stage4;
 break;
 case MyEnum.Stage4:
 this.ProgramImage();
 this.Stage = MyEnum.Stage7;
 break;
 //And so one...
 //The setting of the next stage depends of other factors too, but this is not important now.
 }
}

UI

MyUserControl

private MyClassLibrary.MyDerivedClass myDerivedObject;
public MyUserControl() //ctor
{
 ...
 this.myDerivedObject = new MyClassLibrary.MyDerivedClass();
 this.myDerivedObject.ProgrammingStarted += this.OnProgrammingStarted;
 this.myDerivedObject.ProgrammingExecuted += this.OnProgrammingExecuted;
}
private void BtnNextStageClick(object sender, RoutedEventArgs e)
{
 try
 {
 //Do some stuff
 this.myDerivedObject.AdvanceStage();
 }
 catch (Exception ex)
 {
 MessageBox.Show("Error blabla...");
 }
}
private void OnProgrammingStarted(object sender, EventArgs e)
{
 this.ShowLoadingBox();
}
private void OnProgrammingExecuted(object sender, EventArgs e)
{
 this.RemoveLoadingBox();
 if (!e.Success)
 {
 MessageBox.Show("Error blabla " + e.Error.Message);
 }
}

As you can see, the UI doesn't know what's going to be executed in AdvanceStage(). Only the ProgramImage() is a long running task. All others should be executed sequentially. Therefore I've implemented the events so the object can inform the UI that the programming is started or finished.

• Koen: I had asked this question on SO, but maybe that wasn't the right place.

Answer 1

Since you're using Task-based async processing it's better to declare long-running method as returning Task or Task<T> object:

public async Task ProgramImageAsync()
{
 await Task.Delay(TimeSpan.FromSeconds(5)); // Actual programming is done here
 throw new DivideByZeroException("test");
}

Then all you need on UI side is to have an asynchronous method like this:

private async void RunLongUIPreparation()
{
 //TODO: Show loading box here 
 var processor = new YourClassContainingProgramImageAsync();
 try
 {
 await processor.ProgramImageAsync();
 //TODO: Hide loading box here
 }
 catch (DivideByZeroException exception)
 {
 //TODO: Show messagebox with warning here
 }
}

UPDATE (based on updated question): Your solution doesn't have proper separation of concerns. The class containing ProgramImage method (MyDerivedClass) knows about UI as it handles strings shown on UI (e.g. Explanation property), events (even though they are named as a business flow events) are designed specifically for UI interaction.

In order to properly separate concerns you should move all the logic related to UI to your user control, and leave only business logic in the MyDerivedClass.

In order to do that you need to decide what is the actual trigger for showing a loading box. I'll stick to assumption that the actual reason to show a loading box is not a "programming" stage, but a long-running process, i.e. if you would add a new stage that may also take a long time to complete you may also want to show the loading box there.

The next question to answer is how do you want to detect a long-running stage? I can suggest 2 options:

• show a loading box if stage hasn't completed within a certain timeout (set a timeout inside UI control);
• add a progress notification support to MyDerivedClass so that it can notify clients about its progress (using an IProgress interface specifically designed for asynchronous progress notification).

I'll use the first option in my example as it requires less changes to my previous code:

Business logic:

public async Task ProgramImageAsync()
{
 await Task.Delay(TimeSpan.FromSeconds(5)); // Actual programming is done here
 throw new DivideByZeroException("test");
}

UI:

private async void RunLongUIPreparation()
{
 var processor = new YourClassContainingProgramImageAsync();
 try
 {
 Task stageTask = processor.ProgramImageAsync();
 ShowLoadingBoxIfRunningTooLong(stageTask);
 await processor.ProgramImageAsync();
 }
 catch (DivideByZeroException exception)
 {
 //TODO: Show messagebox with warning here
 }
}
private async void ShowLoadingBoxIfRunningTooLong(Task stageTask)
{
 await Task.Delay(TimeSpan.FromMilliseconds(100));
 if (stageTask.IsCompleted)
 return;
 try
 {
 this.ShowLoadingBox();
 await stageTask;
 }
 finally
 {
 this.RemoveLoadingBox();
 }
}

• Koen: Thanks for the feedback. Never used async before, so I have 1 more question regarding this: when using Tasks as I did, the exception doesn't "bubble up" to the UI. Here it seems it does. Am I right about this?
• almaz: yes, await translates the exception from asynchronous execution/thread to the place where the task is being awaited
• Koen: I've tried to do it your way, but the structure is a bit more complex then stated in my question. The method ProgramImageis called from inside a method in the class (that method is called by the UI), depending on the state of a property. Otherwise something else is executed which doesn't need the loading box. The UI is not aware of that property, hence it's not possible for it to know whether or not it must call it async or not. Therefore I had used the 2 events to notify the UI.
• almaz: Please edit your question to show the specifics, it is hard to answer the question properly until you describe your specifics
• Koen: I know, but I didn't think the changes would have such impact. I'll do it as soon as I arrive at work tomorrow.

Answer 2

I find the structure of your continuation confusing. The ProgramImageAsyncDone() continuation will execute when the previous continuation doesn't fault. That doesn't sound like what you wanted. Though your code will actually work, because when the original task faults, you also cancel the remaining continuation.

I think a clearer way to write this would be to have both continuations on the original Task:

var task = Task.Factory.StartNew(this.ProgramImageAsync);
var scheduler = TaskScheduler.FromCurrentSynchronizationContext();
task.ContinueWith(
 this.TaskExceptionHandler,
 CancellationToken.None,
 TaskContinuationOptions.OnlyOnFaulted,
 scheduler); //Catch exceptions here
task.ContinueWith(
 o => this.ProgramImageAsyncDone(),
 CancellationToken.None,
 TaskContinuationOptions.NotOnFaulted,
 scheduler); //Run this when no exception occurred

Or even better, using a single continuation:

Task.Factory.StartNew(this.ProgramImageAsync)
 .ContinueWith(
 t =>
 {
 if (t.IsFaulted)
 this.TaskExceptionHandler(t);
 else
 this.ProgramImageAsyncDone();
 }, TaskScheduler.FromCurrentSynchronizationContext());

• Koen: Thanks for your feedback. The last one is indeed a lot clearer to read.