I have the following endpoint:
[HttpPost("Submit")]
public String post()
{
_ = _service.SubmitMetric("test", MetricType.Count, 60, 1);
return "done";
}
And the service implementation:
public Task<HttpResponseMessage> SubmitMetric(<params>)
{
// build payload
using (var httpClient = new HttpClient())
{
return httpClient.PostAsync(<params>);
}
}
When I run the code and call the endpoint, the HTTP POST is not triggered. However, if I change my code to:
public async Task<HttpResponseMessage> SubmitMetric(<params>)
{
// build payload
using (var httpClient = new HttpClient())
{
return await httpClient.PostAsync(<params>);
}
}
the POST is submitted as expected. Why is that happening, and what can I do if I don't really care about the HTTP response? I just want to submit it and continue my flow. Shouldn't I be able to use it without await
ing the result? For example:
public void SubmitMetric(<params>)
{
// build payload
using (var httpClient = new HttpClient())
{
httpClient.PostAsync(<params>);
}
}
CodePudding user response:
Don't do it. Await for it even though you discard the result.
Fire and forget is an anti pattern and the context that you are performing the request can be invalidated/killed before the request could be completed, terminating the connection. Just await it, and don't do anything with the result.
CodePudding user response:
httpClient
will be disposed while the POST operation is running, probably resulting in killing the socket. If you use await
, the object will remain inside the using
clause while the operation is running, and it won't be terminated before it finishes.
Note that in your current implementation, you're creating a new connection on each API request, which might eventually lead to socket exhaustion. A better approach would be injecting IHttpClientFactory
, which manage the lifetime of network connections for you, and reuses connections from the pool:
public class MyService
{
private readonly IHttpClientFactory _httpClient;
public MyService(IHttpClientFactory httpClient)
{
_httpClient = httpClient;
}
public async Task<HttpResponseMessage> SubmitMetric(/*<params>*/)
{
var httpClient = _httpClient.CreateClient();
return await httpClient.PostAsync(/*<params>*/);
}
}
Note: You need to add services.AddHttpClient()
in ConfigureServices
in your Startup.cs
to enable injection.
CodePudding user response:
There are two problems with this code. If either was fixed, there would be no problem:
- The HttpClient is used incorrectly. An HttpClient object is thread-safe and meant to be reused, not disposed. Disposing it like this leaks sockets and can result in application crashes or worse, instability. An HttpClient resolves the URL's Host to a socket and caches that socket. The OS also caches opened sockets because opening them is expensive. They're kept alive for a while even if an application closes them because some packets may still be in transit
- By not awaiting
PostAsync
execution exits theusing
block and theHttpClient
instance is disposed before the request had a chance to even start.
In any case, making a POST doesn't take long so there's no need to make the method fire-and-forget. Besides, few applications are OK with losing metrics, especially when things go wrong. That's when metrics are most useful. Which is why ASP.NET Core 6 adds built-in support for OpenTelemetry tracing and metrics. More on that at the end, but the supporting packages can be used in ASP.NET Framework as well. You may be able to replace your current service with a built-in one.
Use await - not enough
One way to fix this is to use await
but that doesn't solve the HttpClient usage problem.
public async Task<HttpResponseMessage> SubmitMetric(<params>)
{
// build payload
using (var httpClient = new HttpClient())
{
return await httpClient.PostAsync(<params>);
}
}
At the very least the HttpClient should be stored in a field. Once that's done though, there's no longer any reason to await
, provided the service itself is still around :
HttpClient httpClient = new HttpClient();
public Task<HttpResponseMessage> SubmitMetric(<params>)
{
return httpClient.PostAsync(<params>);
}
Long lived services
Which brings us to keeping the service around. In ASP.NET and ASP.NET Core each request is served by a separate thread, in a new instance of the Controller class. The request itself is used as a GC scope so anything created during a request is disposed once this concludes, including the HttpClient instance.
To keep the Metrics service around we need to either register it as Singleton
in ASP.NET Core's DI, make it a BackgroundService or ensure it's a singleton in ASP.NET Framework. We could make the field static, but that leads to the next issue.
Proper HttpClient usage
HttpClient can still cause problems if used as a singleton. The HttpClient caches sockets to specific machines. If that machine goes away, the HttpClient will still try to communicate with it causing errors. This can happen easily when the remote services uses a load balancer or fails over to a new server. To fix this, the HttpClient instance or rather the sockets, need to be recycled periodically.
That's the job of the HttpClientFactory. This class caches and recycles SocketClientHandler instances, the classes that do the actual work in an HttpClient. These are recycled periodically, eg every 10 minutes. When asked for a new HttpClient instance, it creates a new instance wrapping one of the already available handlers.
When you use services.AddHttpClient
in ASP.NET Core you're actually configuring an HttpClientFactory. When you add an HttpClient dependency in a controller, the instance will be created by the configured HttpClientFactory.
This means that the following action would work properly :
HttpClient _client;
public MyController(HttpClient client)
{
_client=client;
}
[HttpPost("Submit")]
public String post()
{
await _client.PostAsync(<params>);
return "done";
}
A scoped service with an HttpClient
dependency would also work:
MyService _service;
public MyController(MyService service)
{
_service=service;
}
HttpPost("Submit")]
public String post()
{
await _service.SubmitMetric("test", MetricType.Count, 60, 1);
return "done";
}
where MyService
is :
class MyService
{
HttpClient _client;
public MyService(HttpClient client)
{
_client=client;
}
public Task<HttpResponseMessage> SubmitMetric(<params>)
{
// build payload
return httpClient.PostAsync(<params>);
}
}
In this case there's no real need to await inside SubmitMetric
, that's taken care of by the action.
Using the built-in OpenTelemetry tracing and metrics
ASP.NET Core 6, the upcoming Long-Term-Support version, adds native support for the OpenTelemetry standard for logging, tracing and metrics. This allows using a standard API to push metrics to a lot of different observability applications like Prometheus, Jaeger, Zipking, Elastic and Splunk.
Instead of rolling one's own metrics infrastructure it's better to use the standard API. OpenTelemetry for .NET supports this in ASP.NET Framework 4.6 and later. ASP.NET Core 5 and later are instrumented to publish metrics and tracing to OpenTelemetry providers through the built-in System.Diagnostics
namespace and the Activity
class.
In fact, Controller is already instrumented so you could get rid of the metrics service, adding any Tags and Baggage to the request's current activity:
[HttpPost("Submit")]
public String post()
{
Activity.Current?.AddTag("test");
...
return "done";
}
Metrics were added in ASP.NET Core 6 Preview 5:
Meter meter = new Meter("my.library.meter.name", "v1.0");
Counter<int> _counter;
public MyController(...)
{
_counter = meter.CreateCounter<int>("Requests");
}
[HttpPost("Submit")]
public String post()
{
counter.Add(60, KeyValuePair.Create<string, object>("request", "test"));
return "done";
}