15

Waiting for a non-essential metrics to be sent makes no sense to me, as it adds latency (waiting for the server's response) on each call to the back-end dotnet core service, which might happen multiple times per client call. Still, logging an error on failure is important (I do not need to throw it though, as the metric's failure should not impact the service).

I found multiple ways of doing so. Here is the method to be called in a FireAndForget manner:

public async Task FireAndForget()
{
    try{ await sendTheMetric(); } // Try catch on the await, to log exceptions
    catch(Exception e){ logger.debug(e) }
}

Method 1: Remove the await.

FireAndForget(); // No await in front of the call

Method 2: Seems similar to method 1 to me, as we do not await the Task.Factory.StartNew call.

Task.Factory.StartNew(async () => await MyAsyncTask());

Method 3: Enqueue on the ThreadPool as a workItem.

ThreadPool.QueueUserWorkItem(async o => await FireAndForget());

I have a hard time finding which one I should use for a Fire and forget call to send a non-essential metric. My goal is to not add latency to each calls to my service each time a metric is sent. Logging an error when the metrics fails to be sent is important, but it should never re-throw. The thread context is not important for the execution of the task. The task should always, or at least almost always, be completed.

Which one is the best practice for for requirements? Or are they all the same?

Note: I did not include async void, as it seems risky (if an exception occurs, it might crash, as no Task will wrap it).

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
David Gourde
  • 3,709
  • 2
  • 31
  • 65
  • 2
    i'd use a `LoggingService` that buffers your metrics in a queue, instantly returning to your businesslogic, and cleanly processing them in a worker thread in the background (so when something goes wrong, you can handle it completely encapsulated in one central point). but your question does not have a definite answer and is very opinion-based, i'm afraid. – Franz Gleichmann May 15 '20 at 13:32
  • I think ''writing logging'' should not be queued on a threadpool. It should always be in order which it was produced. Like Franz says, the loggingservice should handle it. (like queueing) etc. – Jeroen van Langen May 15 '20 at 13:44
  • I usually await these sought of tasks so that the exceptions don’t go uncaught. Otherwise `UnobservedTaskException` Would crash my program. However in your case you are catching exceptions with in the task, so it makes sense to discard it. But when your logging to the file and you want it to be thread safe, you have to wait the task, catch the exception and log it – zafar May 15 '20 at 14:00
  • How about adding the Func to a BlockingCollection that is than read by a separate thread and processed? – Magnus May 15 '20 at 14:07

2 Answers2

25

Hope this doesn't muddy the water but there is a 4th option that (from what i have read) is becoming a more accepted option.

Starting with C#7.0 you have the option of using a discard for this sort of instance.

_ = FireAndForget();

This essentially indicates that the task is not needed and thus no return value is expected. Seems to be recommended over Method 1 as it means you are explicitly indicating that it is "fire and forget" (which is less ambiguous and does not look like a coding mistake).

Paul Marshall
  • 523
  • 4
  • 11
  • I personally don't like the syntax, but I think this is best practice at the moment and code analysers like that of resharper even show a warning if you don't discard the result – Patrick Beynio May 15 '20 at 13:49
  • 1
    Found few others suggesting this, but there is an interesting implementation at https://www.meziantou.net/fire-and-forget-a-task-in-dotnet.htm – NiKiZe Sep 20 '21 at 21:26
  • Seems to be confirmed by docs as well: https://learn.microsoft.com/en-us/dotnet/csharp/fundamentals/functional/discards#a-standalone-discard – Phate01 Jun 17 '22 at 12:56
2

Method2 is also seems a good one with little change

      Task.Factory.StartNew(async () => await MyAsyncTask())
        .ContinueWith(t => {
            switch (t.Status)
            {
                case TaskStatus.Created:
                case TaskStatus.WaitingForActivation:
                case TaskStatus.WaitingToRun:
                case TaskStatus.Running:
                case TaskStatus.WaitingForChildrenToComplete:
                case TaskStatus.RanToCompletion:
                    break;
                case TaskStatus.Canceled:
                case TaskStatus.Faulted:
                    //Log error message if any 
                    break;
                default:
                    break;
            }
        });

In this way, you can still fire and forget also log an error message if any.

neelesh bodgal
  • 632
  • 5
  • 14