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
}
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.
If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With