Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Safe way to implement a "Fire and Forget" method on ASP.NET Core

I am trying to implement a simple logging library which will be used across multiple projects. The job of library is to send HTTP requests to ElasticSearch. The main point of this library is that it must not wait for the response. Also, I don't care about any error/exceptions. It must send the request to ElasticSearch, and immediately return. I don't want to make interfaces with return type Task, I want them to stay void.

Below is my sample code. Is it a correct and safe implementation of "Fire and Forget"? Is it ok if I use Task.Run() in a high load library? Or should I avoid using Task.Run() in my case? Also, if I don't use await with Task.Run(), will I block the thread? This code is in the library:

public enum LogLevel
{
    Trace = 1,
    Debug = 2,
    Info = 3,
    Warn = 4,
    Error = 5,
    Fatal = 6
}

public interface ILogger
{
    void Info(string action, string message);
}

public class Logger : ILogger
{
    private static readonly HttpClient _httpClient = new HttpClient(new HttpClientHandler { Proxy = null, UseProxy = false });
    private static IConfigurationRoot _configuration;

    public Logger(IConfigurationRoot configuration)
    {
        _configuration = configuration;
    }

    public void Info(string action, string message)
    {
        Task.Run(() => Post(action, message, LogLevel.Info));
        /*Post(action, message, LogLevel.Info);*/ // Or should I just use it like this?
    }

    private async Task Post(string action, string message, LogLevel logLevel)
    {
        // Here I have some logic

        var jsonData = JsonConvert.SerializeObject(log);
        var content = new StringContent(jsonData, Encoding.UTF8, "application/json");

        var response = await _httpClient.PostAsync(_configuration.GetValue<string>("ElasticLogger:Url"), content);
        // No work here, the end of the method
    }
}

This is how I register logger inside ConfigureServices method in Startup class of my web api:

public void ConfigureServices(IServiceCollection services)
{
     // ......

     services.AddSingleton<ILogger, Logger>();

     // .....
}

This code is in a method inside my web api:

public void ExecuteOperation(ExecOperationRequest request)
{
    // Here some business logic

    _logger.Info("ExecuteOperation", "START"); // Log

   // Here also some business logic

    _logger.Info("ExecuteOperation", "END"); // Log
}
like image 503
Nomad Avatar asked Jun 08 '18 08:06

Nomad


1 Answers

Re : Unawaited call to an async method vs Task.Run()

Since there's only a small amount of CPU bound work in Post (i.e. creating json payload), there's no benefit of another Task.Run - the overhead of scheduling a new Task on the Threadpool will outweigh any benefit IMO. i.e.

Post(action, message, LogLevel.Info);*/ // Or should I just use it like this?

is the better of the two approaches. You'll likely want to suppress the compiler warning associated within unawaited Tasks and leave a comment for the next dev to come across the code.

But as per Stephen Cleary's definitive answer, fire and forget in ASP.Net is almost never a good idea. Preferable would be to offload work, e.g. via a queue, to a Windows Service, Azure Web Job et al.

There are additional dangers - if the unawaited Task throws, you'll want to observe the exception.

Also, note that any work done after the Post (e.g. if you work with response) that this is still a continuation Task which needs to be scheduled on the Threadpool - if you fire off high volumes of your Post method, you'll wind up with a lot of thread contention when they complete.

Re : Also, if I don't use await with Task.Run(), will I block thread?

await doesn't require a thread. await is syntactic sugar to ask the compiler to rewrite your code asynchronously. Task.Run() will schedule a second task on the ThreadPool, which will only do a tiny amount of work before it hits the PostAsync method, which is why the recommendation is not to use it.

The amount of caller thread usage/block on the unawaited call from Info to Post depends on what kind of work is done before the Task is returned. In your case the Json serialization work would be done on the caller's thread (I've labelled #1), however the execution time should be negligible in comparison to the HTTP call duration. So although not awaited by method Info, any code after the HTTP call will still need to be scheduled when the Http call completes, and will be scheduled on any available thread (#2).

public void Info(string action, string message)
{
#pragma warning disable 4014 // Deliberate fire and forget
    Post(action, message, LogLevel.Info); // Unawaited Task, thread #1
#pragma warning restore 4014
}

private async Task Post(string action, string message, LogLevel logLevel)
{
    var jsonData = JsonConvert.SerializeObject(log); // #1
    var content = new StringContent(jsonData, Encoding.UTF8, "application/json"); // #1

    var response = await httpClient.PostAsync(...), content);

    // Work here will be scheduled on any available Thread, after PostAsync completes #2
}

Re: Exception Handling

try..catch blocks work with async code - await will check for a faulted Task and raise an exception:

 public async Task Post()
 {
     try
     {
         // ... other serialization code here ...
         await HttpPostAsync();
     }
     catch (Exception ex)
     {
         // Do you have a logger of last resort?
         Trace.WriteLine(ex.Message);
     }
 }

Although the above will meet the criteria for observing the exception, it is still a good idea to register an UnobservedTaskException handler at the global level.

This will help you detect and identify where you've failed to observe an exception:

TaskScheduler.UnobservedTaskException += (sender, eventArgs) =>
{
    eventArgs.SetObserved();
    ((AggregateException)eventArgs.Exception).Handle(ex =>
    {
        // Arriving here is BAD - means we've forgotten an exception handler around await
        // Or haven't checked for `.IsFaulted` on `.ContinueWith`
        Trace.WriteLine($"Unobserved Exception {ex.Message}");
        return true;
    });
};

Note that the above handler is only triggered when the Task is collected by the GC, which can be some time after the occurrence of the exception.

like image 56
StuartLC Avatar answered Sep 29 '22 14:09

StuartLC