0

I need to execute a task like this (I do not have control of that task's source code)

public Task DoSomething()

Sometimes that task is failed to complete. And this method will not raise any exception. In this case, I want to execute that task again until it is completed. In order to do so, I try like this :

Task task;
do {
    task = Task.Run(async () => await DoSomething();
    await Task.Delay(300);
}
while (!task.IsCompleted);

Is it correct ? Is there any better way ?

Nava
  • 21
  • 3
  • 2
    Use the `await` operator. – Dai Oct 13 '21 at 12:33
  • 4
    No, this is definitely not correct, you start a new task when the previous one didn't finish in 300 ms. – CodeCaster Oct 13 '21 at 12:33
  • 3
    > “Is it correct?” - **no** – Dai Oct 13 '21 at 12:33
  • 2
    There's a huge difference between "hasn't completed yet" and "faulted" - right now, you could be starting hundreds of `DoSomething()` calls *in parallel* (one new one every 300ms), which sounds like a way to get catastrophically worse and worse performance, and more long-running operations. What is `DoSomething()` here? – Marc Gravell Oct 13 '21 at 12:34
  • Try: `await DoSomething();` – Felipe Oriani Oct 13 '21 at 12:42
  • You can/should replace all your code shown with this one line: `await DoSomething();`. That will suspend execution until the Task returned by DoSomething completes, an error will result in an Exception being thrown. – Igor Oct 13 '21 at 12:43
  • You really should make sure, if you are going to retry, you have a strategy for eventually admitting defeat. Sometimes that intermittent failure isn't so intermittent and your code would just silently be retrying without anyone knowing where it's stuck. – Damien_The_Unbeliever Oct 13 '21 at 13:10
  • As a side note, the `DoSomething()` is not a task. It is an asynchronous method. `Task` is what this method returns when it is invoked. – Theodor Zoulias Oct 13 '21 at 13:41
  • The problem is, sometimes, DoSomething has never completed. I would like to retry DoSomething until it completes once. But I don't know how to achieve it. DoSomething won't return any exception. – Nava Oct 14 '21 at 10:32
  • Somewhat related: [How to use Task.WhenAny and implement retry](https://stackoverflow.com/questions/43763982/how-to-use-task-whenany-and-implement-retry) – Theodor Zoulias Oct 14 '21 at 17:48

2 Answers2

0

You should invoke async method directly with await.

In case of error an exception should be throw.

Your code should become something like this.

bool repeat = false;
do {
    try 
    { 
        await DoSomething(); 
        repeat = false;
    }
    catch
    {
        repeat = true;
    }
    if (repeat)
        await Task.Delay(300); 
}
while (repeat); // better with some kind of timeout or attempts counter...

Moreover, IsCompleted include also Faulted and Cancelled. If you want to use your approach you need to use new property IsCompletedSuccessfully or exclude IsFault and IsCancelled (if you use an old .NET fwk).

GibbOne
  • 629
  • 6
  • 10
  • No exception will e thrown in DoSomething(), I have tried this way already. I can not edit source code of DoSomething() to throw exception. So I can not use this way. – Nava Oct 13 '21 at 22:44
0

You, probably, want something like this:

  // Trying to complete task again and again... 
  // you may want to give some number of attempt to complete, say 
  // for (int attempt = 1; attempt <= 3; ++i) {
  while (true) {
    // Let task 300 ms to completet before it'll be canceled
    using (CancellationTokenSource cts = new CancellationTokenSource(300)) {
      try {
        await DoSomething(cts.Token);

        // Task Succeeded, break the loop
        break;
      }
      catch (TaskCanceledException) {
        // Task Cancelled, keep on looping
        ;
      }
    }
  }

Note, that you should have some support in DoSomething:

   async Task DoSomething(CancellationToken token)

instead of

   async Task DoSomething()

since we should know how to cancel it. If you don't have

   async Task DoSomething(CancellationToken token)

or alike signature, (with CancellationToken), the bitter truth is that you can't cancel. You can try wait (and forget the task if it's not completed):

   // Since we are going to forget running tasks, let's
   // restrict number of attempts 
   for (int attempt = 1; attempt <= 3; ++attempt) {
     Task task = DoSomething();

     if (await Task.WhenAny(task, Task.Delay(300)) == task) {
       // task has been completed before timeout
       break;
     } 

     // we forget task (alas, we can't cancel it) and try again
   }
Dmitry Bychenko
  • 180,369
  • 20
  • 160
  • 215
  • I dont understand this: the DoSomething() doesn't accept any parameter and the OP says he has no control over its source code. In your answer's code example, you put a cancelation token into DoSomething() – user2190492 Oct 13 '21 at 13:09
  • @user2190492: Thank you! So the problem is if there is `async Task DoSomething(CancellationToken token)` overload and if it is we can use the cancellation; if not - all we can do it to *forget* the previous attempt, and run `DoSomething()` once again – Dmitry Bychenko Oct 13 '21 at 13:24
  • Thanks for this comment. that task can't cancel. But I will use "try wait and forget" way you post. – Nava Oct 13 '21 at 22:52