davidfowl / AspNetCoreDiagnosticScenarios

This repository has examples of broken patterns in ASP.NET Core applications
7.65k stars 736 forks source link

IHttpClientFactory Question #104

Closed nquandt closed 9 months ago

nquandt commented 9 months ago

https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/713b756bfc8c2d53dc35791cacbbc770c48fd482/Scenarios/Controllers/HttpClientController.cs#L31C13-L31C13

David, I figure you are a busy guy.. but I've noticed a discrepancy between .NET documentation and figured I'd get the best answer asking.

When it comes to the IHttpClientFactory.CreateClient is it best to utilize using given that it is an IDisposable? I know if I used typed clients I get disposal through DI.. Is it okay to inject clients into singletons, given that clients are supposed to be short lived?

Further.. should someone always using an IDisposable OR properly dispose via .Dispose() in the event of a short lived asset?

Am I correct in understanding that "typically" IDispoables are meant for releasing unmanaged resources that might not be correctly disposed by GC... ?

or does GC call .Dispose()?

I feel that a lot of disposal magic happens in DI, and so when working strictly inside a method that has some disposable assets I find myself getting a bit confused on what to using

Example of my typical flow (using httpclient as an example)

public async Task<Something> DoAsync()
{
   using var client = _factory.CreateClient("MaybeName");
   using var requestMessage = new HttpRequestMessage(....);

   using var response = await client.SendAsync(requestMessage);

   DoSomethingWithResponse...;

   return Something;
}

Any guidance is appreciated

kapsiR commented 9 months ago

David, I figure you are a busy guy.. but I've noticed a discrepancy between .NET documentation and figured I'd get the best answer asking.

What's the discrepancy?

When it comes to the IHttpClientFactory.CreateClient is it best to utilize using given that it is an IDisposable?

Yes. Read the lifetime management section of the docs.

Is it okay to inject clients into singletons, given that clients are supposed to be short lived?

It depends. Do you talk about HttpClient or IHttpClientFactory?
Read Recommended use of the Guidelines for using HttpClient:

To summarize recommended HttpClient use in terms of lifetime management, you should use either long-lived clients and set PooledConnectionLifetime (.NET Core and .NET 5+) or short-lived clients created by IHttpClientFactory.

Further.. should someone always using an IDisposable OR properly dispose via .Dispose() in the event of a short lived asset?

Am I correct in understanding that "typically" IDispoables are meant for releasing unmanaged resources that might not be correctly disposed by GC... ?

or does GC call .Dispose()?

I feel that a lot of disposal magic happens in DI, and so when working strictly inside a method that has some disposable assets I find myself getting a bit confused on what to using

Read Using objects that implement IDisposable, TL;DR:

nquandt commented 9 months ago

@kapsiR Thank you for your reply! I realize discrepancy is the incorrect word. I meant that it wasn't explicitly called out anywhere that CreateClient should be disposed and so that left me a bit confused.. Reading the Using objects article is what I was missing, and helps me understand that yes I should be using using with the CreateClient.

I appreciate you writing this out for me!