0

I have below C# code that runs periodically, e.g., every 1 hour. Every time it runs, it tries to make some remote calls in parallel. When any of these remote calls throws an exception, I create only one ticket, or update the previous ticket. And in the finally block, resolve the existing previous ticket if there is no exception this time. Essentially I want to make sure that no matter how many remote calls fail, I only have one ticket to investigate. And when next time all calls succeed, the ticket gets resolved automatically.

However, when the remote calls all succeed in the try block, the call in finally block tries to resolve the ticket, but ran into HTTP 412 Precondition Failed, meaning that the ticket I got in try block was somehow updated before the finally block. If it's updated in the same thread, I wouldn't try to resolve it.

I learnt from this post that Task.WhenAll will wait for all tasks to complete even in the presence of failures (faulted or canceled tasks). In case where multiple tasks throw exceptions, would the catch block run once or more? How about the finally block?

Ticket ticket = null;
try
{
    // Query to get the existing ticket if there is any.
    ticket = await QueryExistingTicketAsync();

    // Make a lot of remote calls in parallel
    var myTasks = new List<Task>();
    myTasks.Add(RemoteCallAsync("call1"));
    myTasks.Add(RemoteCallAsync("call2"));
    myTasks.Add(RemoteCallAsync("call3"));
    // ... add more Tasks for RemoteCallAsync()

    await Task.WhenAll(myTasks);
}
catch (Exception ex)
{
    if (ticket != null)
    {
        ticket.ReChecked = true;
        ticket.LastCheckTime = DateTimeOffset.Now;

        // If the previous ticket exists, meaning the last run failed as well, 
        // update the timestamp on that ticket.
        ticket = await UpdateExistingTicketAsync(ticket);
    }
    else
    {
        // If the previous ticket does not exist yet, 
        // create one ticket for investigation, this ticket.ReChecked will be true
        ticket = await CreateNewTicketAsync();
    }
}
finally
{
    // Resolve the previous ticket if it was not created/updated in catch block
    if (ticket != null && !ticket.ReChecked)
    {
        ticket.Status = "Resolved";
        await UpdateExistingTicketAsync(ticket);
    }
}
Kun Hu
  • 417
  • 5
  • 11
  • 3
    it will only run once, if you want it to run more than once put the try catch finally inside of the logic that gets run async. – Train Oct 04 '19 at 21:31
  • Your question is unclear. In the code you posted **there is only one thread**. But you write things like _"multiple threads running in try block"_. That sort of statement doesn't make any sense. Each thread has its own stack, its own exception handling, and thus its own try/catch/finally blocks. If you are unsure of the correct terminology or otherwise cannot explain in English what your question is, please fix the question so it includes a good [mcve], explain _exactly_ what that code does, how that's different from what you want, and what _specifically_ you need help with. – Peter Duniho Oct 04 '19 at 21:39
  • I aggree with @PeterDuniho, it's hard to help you the way you stated your question. Are there multiple threads entering this `try` block and then awaiting? Are `myTasks` set up to run in background threads? As a side-note, I don't know why you want to perform what seems to be business logic when an exception is thrown, but it's a code smell to me. – V0ldek Oct 04 '19 at 22:29
  • @PeterDuniho thanks for the guidance, and apologies for the confusion. I was confused by what I saw when I typed the question. I've updated the description and code example, and clarified some details hopefully. – Kun Hu Oct 06 '19 at 00:15
  • @V0ldek Added some more details of my intention, I want to be notified when the code failed due to remote call failures, so I create a 'ticket', something like an alert. And I auto-resolve the alert when the next run succeeds, as I don't want to put too much effort on the alert if the error was transient. – Kun Hu Oct 06 '19 at 00:19
  • As a minor thing, if you're not doing anything with the `Exception ex` you should omit it. As in `try { ... } catch { ... } finally { ... }` – V0ldek Oct 06 '19 at 18:50
  • @V0ldek Thanks. I didn't write out that part, but I do encode the exception info in the ticket for later investigation. – Kun Hu Oct 07 '19 at 17:06

2 Answers2

2

Short answer: even if multiple tasks throw exceptions, there will be only one exception thrown by await Task.WaitAll and thus the catch block will execute only once. The finally block always executes once per entry to the try block. Long answer follows.

Awaiting a Task.WaitAll, if any wrapped Tasks throw an exception, throws an AggregateException containing exceptions from all of the Tasks. So assuming you've reached the await Task.WhenAll(tasks) line without exceptions, here's what happens:

  1. Task.WhenAll will wait for all the Tasks to complete, regardless of whether they fault or not.
  2. If all of the Tasks completed successfully, the Task.WhenAll completes successfully, execution resumes at the await, end of the try block is reached, the finally block executes.
  3. If one or more Tasks finished with an exception, Task.WhenAll wraps all of them into an AggregateException and completes in a faulted state. The execution resumes at the await and the aforementioned AggregateException is thrown. The catch block is executed and then the finally block.

So, assuming that one thread enters the try block, in both cases when the execution resumes at the await only one thread continues the execution. The only way for you to enter the catch or finally blocks twice is to execute the method twice.

The await statement does a lot of magic under the hood, but it never causes your execution to continue on multiple threads. One thread awaits, only one thread picks it up and continues.

V0ldek
  • 9,623
  • 1
  • 26
  • 57
1

The catch and the finally blocks are running once, but can run both. The finally block is always running, while the catch block runs only in case of an exception, before the finally block.

There is another issue with your code though.

try
{
    Ticket ticket = await QueryExistingTicketAsync();
    // ...
}
catch (Exception ex)
{
    if (ticket != null)
    {

The ticket you create on try and the ticket you update on catch is not the same ticket. The ticket declared on try has local scope, and so it's not visible outside this block. You may have declared another ticket variable before the try-catch-finally block, but even in this case the compiler would complain for the double variable declaration. So in any case the code you provided should not be able to compile.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104