1

Edit See the title "Problem" at the end within my question to crack this question down.

Coming from nodejs where we could chain promises, in C# I'm seeing Async Tasks almost comparable. Here's my attempt.

Edit - I can't mark my uber level caller methods as async as a dll based library is calling it

Caller object

public void DoSomething(MyRequest request) 
{
    Delegate.Job1(request)
        .ContinueWith(Delegate.Job2)
        .ContinueWith(Fault, TaskContinuationOptions.OnlyOnFaulted)
        .ContinueWith(Result);
}

public void Result(Task<MyRequest> task)
{
    MyRequest request = task.Result;
    Console.Writeline(request.result1 + " " + request.result2);
}

public void Fault(Task<MyRequest> task)
{
   MyRequest request = task.Result;
   Console.Writeline(request.result);
}

Delegate Object

public async Task<MyRequest> Job1(MyRequest request) 
{
    var data = await remoteService.Service1Async();
    request.result1 = data;
    return request;
}

public async Task<MyRequest> Job2(Task<MyRequest> task)
{
    var data = await remoteService.Service2Async();
    request.result2 = data;
    return request;
}

Problem:

1) Edit (fixed, the linked dll to my project was missing it's linked dll) Task.Result (request) is coming as null in the Result method, Also Status = Faulted

2) Also is Fault Handling correct? I'm expecting Fault to be only called when an exception occurs within the Delegate methods, otherwise it should skip.

2-b) Another option is check within the Result function (delete Fault function) if Task.status = RanTocompletion and branch there for success or fault

Edit after the answer

I've a gotcha, what if I can't make my controller async.

Controller

public void ICannotBeAsync()
{
    try
    {
        var r = await caller.DoSomething(request); // though I can use ContinueWith here, ???
    }
    catch(Exception e)
    {
        //exception handling
    }
}

Caller

public async Task DoSomethingAsync(MyRequest request)
{
     request.result1 = await delegateInstance.Job1(request);
     request.result2 = await delegateInstance.Job2(request);
     Console.Writeline(request.result1 + " " + request.result2);
     return result;
}

Edit 2 - based on VMAtm Edit, please review OnlyOnFaulted (Fault) option.

Delegate.Job1(request)
    .ContinueWith(_ => Delegate.Job2(request), TaskContinuationOptions.OnlyOnRanToCompletion)
    .ContinueWith(() => {request.result = Task.Exception; Fault(request);}, TaskContinuationOptions.OnlyOnFaulted)
    .ContinueWith(Result, TaskContinuationOptions.OnlyOnRanToCompletion);

Problem -

Gave it a test, actual code below, none of the Result or Fault is getting called, although the method GetCustomersAsync returned successfuly. My understanding everything stops at Fault because it's marked to run on Fault only, execution stops there and Result handler is not called.

Customer.GetCustomersAsync(request)
    .ContinueWith(_ => { Debug.WriteLine("not executing"); Fault(request); }, TaskContinuationOptions.OnlyOnFaulted)
    .ContinueWith(_ => { Debug.WriteLine("not executing either"); Result(request); }, TaskContinuationOptions.OnlyOnRanToCompletion);

Edit 3 Building on Evk's answer.

Task<Request> task = Customer.GetCustomersAsync(request);
task.ContinueWith(_ => Job2Async(request), TaskContinuationOptions.OnlyOnRanToCompletion);
task.ContinueWith(_ => Job3Async(request), TaskContinuationOptions.OnlyOnRanToCompletion);
task.ContinueWith(_ => Result(request), TaskContinuationOptions.OnlyOnRanToCompletion);
task.ContinueWith(t => { request.Result = t.Exception; Fault(request); }, TaskContinuationOptions.OnlyOnFaulted);
user2727195
  • 7,122
  • 17
  • 70
  • 118
  • 1
    You would simply just await each task then put a try catch over it. Node JS Promises were to prevent callback hell. Async/Await in C# prevents callback hell. So there is no need to port over Node Js thinking to C# as it already deals with that scenario. – Callum Linington Feb 07 '17 at 13:48
  • I like the chaining that comes with Task based implementation, I'll be using it heavily but just not able to get it right now, I'm not sure why `Task.Result` is coming as null within my `Result` function, any fixes? – user2727195 Feb 07 '17 at 13:53
  • 4
    I would urge you to follow more the c# style, because if you have other developers, or you look at documentation for frameworks you/they may become confused because you have a kind of paradigm shift. Plus you may run into problems later with altering your implementations when the requirements change. – Callum Linington Feb 07 '17 at 14:00
  • ok, is chaining using `ContinueWith` confusing for C# developers, if then what's the norm? – user2727195 Feb 07 '17 at 14:03
  • @user2727195 Well as it is your code is wrong in the normal case, and your error handling is also very wrong. While it's *possible* to do correctly with `ContinueWith` it's rather tedious, hard to do exactly right, and *if* you do it right, obscures the important business code within lots of mechanical code. Using `await` has none of those problems. These problems also become more and more pronounced the more complex the code gets. – Servy Feb 07 '17 at 14:14

2 Answers2

1

There are multiple issues with this code:

Delegate.Job1(request)
    .ContinueWith(Delegate.Job2)
    .ContinueWith(Fault, TaskContinuationOptions.OnlyOnFaulted)
    .ContinueWith(Result);

First of all, you are continuing the execution with Delegate.Job2 even if the Delegate.Job1 failed. So you need a OnlyOnRanToCompletion value here. Similar to the Result continuation, you are continuing in all the cases, so the task with an error still goes through the chain and, as you already see, is in Faulted state with a null as a result.

So, your code, if you can't use on that level await, could be like this (also, as @Evk stated, you had to add the exception handling to all of your code, which is realy ugly):

Delegate.Job1(request)
    .ContinueWith(Delegate.Job2, TaskContinuationOptions.OnlyOnRanToCompletion)
    .ContinueWith(Fault, TaskContinuationOptions.OnlyOnFaulted)
    .ContinueWith(Result, TaskContinuationOptions.OnlyOnRanToCompletion)
    .ContinueWith(Fault, TaskContinuationOptions.OnlyOnFaulted);

However, you still have an option to use the await keyword inside your method and after that use a lambda to run it synchronously, if it is an option for you:

public async Task DoSomethingAsync(MyRequest request)
{
    try
    {
         request.result1 = await delegateInstance.Job1(request);
         request.result2 = await delegateInstance.Job2(request);
         Console.Writeline(request.result1 + " " + request.result2);
         return result;
     }
     catch(Exception e)
     {

     }
}

public void ICannotBeAsync()
{
    var task = Task.Run(() => caller.DoSomethingAsync(request);
    // calling the .Result property will block current thread
    Console.WriteLine(task.Result);
}

Exception handling could be done on either levels, so it's up to you where to introduce it. If something goes wrong during the execution, Result property will raise an AggregateException as a wrapper to inner exceptions happened during the call. Also you can use a Wait method for a task, wrapped into a try/catch clause, and check the task state after that, and deal with it as you need (it has IsFaulted, IsCompleted, IsCanceled boolean properties).

Also, it's highly recommended to use some cancellation logic for your task-oriented tasks to be able to cancel unnecessary work. You can start with this MSDN article.

Update, based on your other questions:

If you still want to use the ContinueWith instead of the await, and want to change the signatures of the Job1, Job2 methods, you should change your code like this:

Delegate.Job1(request)
    .ContinueWith(_ => Delegate.Job2(request), TaskContinuationOptions.OnlyOnRanToCompletion)
    .ContinueWith(Result, TaskContinuationOptions.OnlyOnRanToCompletion)
    .ContinueWith(Fault, TaskContinuationOptions.OnlyOnFaulted);

The reason for this is that the ContinueWith method accepts a Func<Task, Task>, because you need, in general, inspect the task for it's status and/or it's result.

As for the question about not blocking the caller, you can try out the TaskCompletionSource<TResult> class, something like this:

public void ICannotBeAsync()
{
    var source = new TaskCompletionSource<TResult>();
    var task = Task.Run(() => caller.DoSomethingAsync(request, source);
    while (!source.IsCompleted && !source.IsFaulted)
    {
        // yeild the execution to other threads for now, while the result isn't available
        Thread.Yeild();
    }
}

public async Task DoSomethingAsync(MyRequest request, TaskCompletionSource<TResult> source)
{
     request.result1 = await delegateInstance.Job1(request);
     request.result2 = await delegateInstance.Job2(request);
     Console.Writeline(request.result1 + " " + request.result2);
     source.SetResult(result);
}
VMAtm
  • 27,943
  • 17
  • 79
  • 125
  • good, what can I do about `Job1` and `Job2` method signatures, there's inconsistency, is it possible to have both methods receive `request` instead of `Task`, I tried with lambda `(prevTask) => Delegate.Job2(request)`, any ideas – user2727195 Feb 07 '17 at 17:52
  • @user2727195 In what case? For the `await` usage or for continuation? – VMAtm Feb 07 '17 at 19:11
  • @user2727195 Why don't you want to introduce the `await`, and run `DoSomethingAsync` inside lambda? – VMAtm Feb 07 '17 at 19:13
  • `Task.Run(() => caller.DoSomethingAsync(request)` needs async lambda, right? – user2727195 Feb 07 '17 at 19:14
  • @user2727195 no, you can run it synchronously – VMAtm Feb 07 '17 at 19:16
  • your question, "Why don't you want to introduce the " because my controller doesn't want to couple itself with the caller, controller submits the requests to the caller (kinda asynchronously, it's an observer object with data) and wait inside a callback method (observer pattern) for results. – user2727195 Feb 07 '17 at 19:21
  • I don't see how this can relate to the await or `ContinueWith` usage. I'll update the answer with additional thoughts, – VMAtm Feb 07 '17 at 19:28
  • also can you please help with this, http://stackoverflow.com/questions/42097864/system-identitymodel-tokens-jwt-code-examples, it's kinda urgent – user2727195 Feb 07 '17 at 19:32
  • @user2727195 I've added some new information. Hope this helps, and good luck with your project. – VMAtm Feb 07 '17 at 21:46
  • several reasons I'm sticking to the `ContinueWith`, first I'm following a Design pattern called 'FormalRequest` hard to explain here, second `.ContinueWith` looked similar to JS promises which worked really solid with sequence of async jobs, a bit of difference here in c# but works (I see Fault comes before and you have to specify continuation options for each), 3rd I don't want to have try/catch in the `caller`, that's the job of `ContinueWith` to determine the right method `result/fault` to call, – user2727195 Feb 08 '17 at 06:20
  • 4th this pattern requires caller not to be async, the uber controller doesn't want to attach itself to the caller and submits observer pattern based request, 5th cosmetics, I liked promises so Tasks but will go deeper in async/await after this although I don't like having try catch at the caller level, the delegate/helper objects should throw exception and it's the responsible of the chained sequence to call result/fault but I'll try. – user2727195 Feb 08 '17 at 06:22
  • now a gotcha, I'm editing my question titled Edit2, I need to know what Exception caused the triggered to shuttle it to view for client reporting, and this info is available with `Task.Exception` only, it's a big of eye soar, any advice. – user2727195 Feb 08 '17 at 06:23
  • I've also made the chain to call `result` and `fault` with the `request` instead of `Task` for consistency across all delegates, helpers and caller itself. The `caller` needs to be tidy, neat and clean with each method of the caller to be a sequence of chains only while all the dirty work goes into delegates and helpers including `try/catch` – user2727195 Feb 08 '17 at 06:51
  • By the way proposed solution isn't working, I tried with actual code, please see the very last section of my question titled "Problem" – user2727195 Feb 08 '17 at 07:30
  • @user2727195 You''l need then add the error handler to **every** task inside your code. If you do not use the `try/catch` - get the task from `async` method and inspect it status and exception if any, in caller, as I did in the last piece of code. – VMAtm Feb 08 '17 at 16:27
0

A lot of things has been said here, so I only answer to the last "Problem" section:

Customer.GetCustomersAsync(request)
    .ContinueWith(_ => { Debug.WriteLine("not executing"); Fault(request); }, TaskContinuationOptions.OnlyOnFaulted)
    .ContinueWith(_ => { Debug.WriteLine("not executing either"); Result(request); }, TaskContinuationOptions.OnlyOnRanToCompletion);

Problem here (and in original example too) is that the following happens:

  1. You continue GetCustomersAsync with "only on faulted" continuation.
  2. Then you continue that continuation, not GetCustomersAsync, with the next continuation, which can run only on completion.

In result, both continations can execute only when GetCustomersAsync fails. To fix:

var request = Customer.GetCustomersAsync(request);
request.ContinueWith(_ => { Debug.WriteLine("not executing"); Fault(request); }, TaskContinuationOptions.OnlyOnFaulted);
request.ContinueWith(_ => { Debug.WriteLine("not executing either"); Result(request); }, TaskContinuationOptions.OnlyOnRanToCompletion);

Note that even if you cannot change signature of some method and it absolutely must return void, you still can mark it as async:

public async void DoSomethingAsync(MyRequest request)
{
     try {
         await Customer.GetCustomersAsync(request);
         Result(request);      
     }
     catch (Exception ex) {
         Fault(request);
     }
}
Evk
  • 98,527
  • 8
  • 141
  • 191
  • interesting approach, now if we expand this, let's say I place 5 more `chained ContinueWith async operations` having option `OnlyOnRanToCompletion` at line #2 of your code, and push `Fault` (line 2) and `Result` (line 3) down, will it work as expected, i.e. tasks will continue on executing all the way to result if success, and if fail it will jump straight to fault skipping any intermediate operations. – user2727195 Feb 08 '17 at 08:39
  • please review Edit 3 based on your answer, within fault handler I've assigned the result from `t.Exception` to let uber caller or requesting clients know what went wrong. – user2727195 Feb 08 '17 at 09:01
  • Looks fine, but ensure proper error handling: if Job2Async or Job3Async will throw an exception, it will be unobserved and may crash down whole process. If there is try-catch block inside Job2Async and Job3Async then you should be fine. Of course async-await is much cleaner and much less error prone, but if you absolutely must to use ContinueWIth - should be enough. – Evk Feb 08 '17 at 09:13
  • oh no, Job2Async and Job3Async are supposed to throw Exceptions and not to handle them, it's the uber hierarchy that catches all the exception and reports nicely to the client, and I'm relying on the Fault handler specified at the end of the chain to be the fault handler of the `GetCustomersAsync`, `Job2Async` and `Job3Async`. Meaning I've to ditch this route??? – user2727195 Feb 08 '17 at 09:22
  • If you refuse to use async\await for whatever reason then you have to manually handle exceptions from Job2Async and Job3Async (you can use the same on fault continuations for them). – Evk Feb 08 '17 at 09:27
  • Repeating/Duplicating Fault continuation after each AsyncJob is erroneous. When you said I can mark a void method as async that opened the horizon actually, I'm trying your other example with `await`. – user2727195 Feb 08 '17 at 09:33
  • Also, to reduce number of continuations you can just use one continuation without options (without OnFault or OnRanToCompletion). In that continuation just check task Exception and Result property (or IsCompleted and IsFaulted) and decided what to do based on that. In your last example that will reduce number of continuations from 4 to 1. – Evk Feb 08 '17 at 09:34
  • I'm happy with `async await` now, the turning point was when you said it can keep my method void, I can't change my contract with uber callers, the request I'm receiving is basically an observer pattern and within result/fault I notify the uber with result or exception. I did try your suggested route for checking IsCompleted and isFaulted but it introduced noise in my code. await and async is clean and even better than `ContinueWith` and I'm happy to use while keeping my method void. Thanks for your wonderful help. – user2727195 Feb 08 '17 at 09:47
  • You are welcome. No doubt async\await is better than ContinueWith, so stick with it whenever possible. – Evk Feb 08 '17 at 09:48
  • @user2727195 in general `async void` is used only for a async event handlers and should be avoided, however if you can't change the signature, this is an option. You maybe will need to add `ConfigureAwait(false)` to your `await` statements. – VMAtm Feb 08 '17 at 17:31