0

I have following code to handle my TaskContinuations. I am bit confused because I have below OnlyOnFaulted block which I expect will be entered if the task throws an unhandled exception.

However, unhandled exception, handled exception that is rethrown using throw, or cancellation will land in the OnlyOnCanceled block.

GetDataAsync(id).ContinueWith((antecedant) =>
{
    // do something when async method completed
    }, TaskContinuationOptions.OnlyOnRanToCompletion)
    .ContinueWith((antecedant) =>
    {
        var error = antecedant.Exception.Flatten(); //so when is this called if everything is cought by OnCancelled below?
    }, TaskContinuationOptions.OnlyOnFaulted)
    .ContinueWith((antecedant) =>
    {
        // this is fired if method throws an exception or if CancellationToken cancelled it or if unhandled exception cought
        var error = "Task has been cancelled";
    }, TaskContinuationOptions.OnlyOnCanceled);

I would expect that re-thrown errors and cancellations will land in the OnlyOnCanceled block whereas unhandled exception will land in the OnlyOnFaulted block

Note that I cannot just await GetDataAsync because this is called in a method called from View's c-tor. I explained that in this post NetworkStream ReadAsync and WriteAsync hang infinitelly when using CancellationTokenSource - Deadlock Caused by Task.Result (or Task.Wait)

UPDATE

Instead using code above, I am using Task.Run like below. I am decorating the lambda passed into Task.Run with async to provide "Async all the way" as recommended by Jon Goldberger at https://blog.xamarin.com/getting-started-with-async-await/

Task.Run(async() =>  
{
    try
    {
        IList<MyModel> models = await GetDataAsync(id);
        foreach (var model in models)
        {
            MyModelsObservableCollection.Add(model);
        }
    } catch (OperationCancelledException oce) {}
    } catch (Exception ex) {}

});

This felt a better solution since I can wrap the code inside Task.Run with try...catch block and the exception handling is behaving as I would expect.

I am definitely planning to give a try to suggestion offered by Stephen Cleary at https://msdn.microsoft.com/en-us/magazine/dn605875.aspx as it seam to be a cleaner solution.

pixel
  • 9,653
  • 16
  • 82
  • 149
  • 2
    You should really use `await` to add continuations, rather than `ContinueWith`, in basically all cases, precisely because writing code like this is *much* harder to do correctly. Save yourself the effort. – Servy Mar 04 '19 at 21:18
  • 1
    That's not a reason to not use `await` at all. You should *absolutely* be using `await` to add these continuations, given your description of the problems. If you actually want to fire and forget this operation (which is unlikely, it's very unlikely that there'd be something to do in the constructor where you don't want to know when its done or return an object from the constructor when it hasn't completed some operation used to construct it (and if it's not an operation used to construct it then you shouldn't be calling it from the constructor)) then you can fire and forget an async method. – Servy Mar 04 '19 at 22:07

1 Answers1

0

As I said in my other answer, you should use await, and, since this is a constructor for a ViewModel, you should synchronously initialize to a "Loading..." state and asynchronously update that ViewModel to a "Display Data" state.

To answer this question directly, the problem is that ContinueWith returns a task representing the continuation, not the antecedent. To simplify the code in your question:

GetDataAsync(id)
    .ContinueWith(A(), TaskContinuationOptions.OnlyOnRanToCompletion);
    .ContinueWith(B(), TaskContinuationOptions.OnlyOnFaulted)
    .ContinueWith(C(), TaskContinuationOptions.OnlyOnCanceled);

A() will be called if GetDataAsync(id) runs to completion. B() will be called if A() faults (note: not if GetDataAsync(id) faults). C() will be called if B() is canceled (note: not if GetDataAsync(id) is canceled).

There are a couple of other problems with your usage of ContinueWith: it's missing some flags (e.g., DenyChildAttach), and it's using the current TaskScheduler, which can cause surprising behavior. ContinueWith is an advanced, low-level method; use await instead.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • Thanks Stephen. I will give it a try once I get a chance and try yours NotifyTaskCompletion as suggested in your linked article. I read that article couple of days ago and liked that solution but for now, I am using the solution in my UPDATE section above as it is not causing any issues for me and allows me to move on adding more required functionality in my project (time cranch). I made a note to get back to that and reflect on NotifyTaskCompletion for that reason, thanks for that article. Meanwhile, If you can comment on the UPDATE section above, that would be highly appreciated. Thanks – pixel Mar 06 '19 at 21:42
  • ... but to comment on your comment on ContinueWith, originally I tried to pass TaskScheduler.FromCurrentSynchronizationContext() to my ContinueWith above and run into issues, it was complaining about something when used together with passed in TaskContinuationOption. I dont remember what exactly was the problem but I do rememer I had it only if I add TaskScheduler like : "}, TaskContinuationOptions.OnlyOnRanToCompletion, TaskScheduler.FromCurrentSynchronizationContext()". I havent had time to investigate it but I remember that passing only continuation worked fine (aside from your points) – pixel Mar 06 '19 at 21:56