dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.6k stars 10.06k forks source link

Using HttpClientFactory without dependency injection #28385

Closed poke closed 3 years ago

poke commented 5 years ago

Is there any way to consume the HttpClientFactory without using dependency injection, or more precisely without using M.E.DI?

I am creating a client library for which I want to make use of HttpClientFactory but I am going to use this library also in a legacy project that does not not have a M.E.DI-compatible DI.

Ideally, I would just new up a DefaultHttpClientFactory and consume that directly but since the factory itself is internal, this is not possible.

Why is the HttpClientFactory internal and is there any way to consume it explicitly without using DI?

JHeLiu commented 4 years ago

@davidfowl Yeah, I know that, but if you're going to dynamically set a timeout, it's going to be a new HttpClient.I was looking for an easier way to use functionality, such as using(var HTTP = new HttpClient()).DI is not the preferred solution, or it has its own factory, it doesn't need DI, and if it doesn't, I'll just use static

sln162 commented 4 years ago

@JHeLiu 你可以参考我上面的回答,我一直这么处理的:在Startup.cs里Configure方法中,增加一个, clientFactory = app.ApplicationServices.GetService(); 这个clientFactory 就是一个全局的静态变量,public static IHttpClientFactory clientFactory;

有一个专门的httphelpr类,里面的方法,就用这个 var client = Startup.clientFactory.CreateClient("default");

然后,得到了HttpClient,就总结多个get/post的方法,以供全局调用。

用英文交流,你用的什么翻译,感觉你们相互沟通困难....

davidfowl commented 4 years ago

Yeah, I know that, but if you're going to dynamically set a timeout, it's going to be a new HttpClient.

No that’s absolutely false. You don’t need this library, you just need to use the right overloads to make http requests.

JHeLiu commented 4 years ago

@jmk92 我所有的请求不在控制器里面发出的,在服务类 或者共用类发出的 控制器里面就相当于一个数据请求中转而已,我在想尽量避免在其它类库里面不引用 WEB里面的东西,就纯净的 System.Net.Http ystem.Net.Http 相关的

sln162 commented 4 years ago

@JHeLiu 我提供的方式,可以在任意类中调用。如果你希望代码写在其中一个DLL中,和主DLL没有任何依赖,那不应该用HttpClientFactory,他必须用DI,唯一一种静态化的方式就是我这个。其实你可以自己封装一个httpclient,原生的有释放问题,自己控制好就行

JHeLiu commented 4 years ago

@jmk92 GG,看来只能使用静态变量来处理了?如果遇到很多个站点,假如自己的超时也不同,那不的有多个。等等,尽管如果核心地方 抽出来,比如用 key注册,但是好像每一个域名只能维护一个 httpclient 啊

sln162 commented 4 years ago

假如自己的超时也不同,那不的有多个 不是啊,你自己封装的方法中,参数可以多一个超时时间,然后代码做区分就行了。

比如用 key注册,但是好像每一个域名只能维护一个 httpclient 啊 并不是的,Startup.clientFactory.CreateClient("default");看这里面的default,你可以任意定义,每个是一个httpclient。 静态变量的方法只是把HttpClientFactory给静态化了,相当于你在哪里都能调用它了,还没到创建httpclient 的那一步,注意是,还没到创建的那一步,至于后面你想怎么用,随你

davidfowl commented 4 years ago
public static async Task<string> Get(this HttpClient httpClient, string url, int timeout = 30, Dictionary<string, string> Headers = null)
{
    if (Headers != null && Headers.Count > 0)
    {
        foreach (var item in Headers)
        {
            httpClient.DefaultRequestHeaders.TryAddWithoutValidation(item.Key, item.Value);
        }
    }

    using (var cts = new CancellationTokenSource())
    {
        cts.CancelAfter(TimeSpan.FromSeconds(timeout));
        var response = await httpClient.GetAsync(url, cts.Token);
        return await response.Content.ReadAsStringAsync(cts.Token);
    }
}
JHeLiu commented 4 years ago

假如自己的超时也不同,那不的有多个 不是啊,你自己封装的方法中,参数可以多一个超时时间,然后代码做区分就行了。

比如用 key注册,但是好像每一个域名只能维护一个 httpclient 啊 并不是的,Startup.clientFactory.CreateClient("default");看这里面的default,你可以任意定义,每个是一个httpclient。 静态变量的方法只是把HttpClientFactory给静态化了,相当于你在哪里都能调用它了,还没到创建httpclient 的那一步,注意是,还没到创建的那一步,至于后面你想怎么用,随你

现在就是不想用DI,它可以直接使用吗?像在.net framework 时代那样使用,我看官方例子 new HttpClient(),这个类里面的超时只能设置一次,后续设置就会报错了,是否有例子参考参考,感谢

JHeLiu commented 4 years ago
public static async Task<string> Get(this HttpClient httpClient, string url, int timeout = 30, Dictionary<string, string> Headers = null)
{
    if (Headers != null && Headers.Count > 0)
    {
        foreach (var item in Headers)
        {
            httpClient.DefaultRequestHeaders.TryAddWithoutValidation(item.Key, item.Value);
        }
    }

    using (var cts = new CancellationTokenSource())
    {
        cts.CancelAfter(TimeSpan.FromSeconds(timeout));
        var response = await httpClient.GetAsync(url, cts.Token);
        return await response.Content.ReadAsStringAsync(cts.Token);
    }
}

Thank you very much. I'll look into it

JHeLiu commented 4 years ago
public static async Task<string> Get(this HttpClient httpClient, string url, int timeout = 30, Dictionary<string, string> Headers = null)
{
    if (Headers != null && Headers.Count > 0)
    {
        foreach (var item in Headers)
        {
            httpClient.DefaultRequestHeaders.TryAddWithoutValidation(item.Key, item.Value);
        }
    }

    using (var cts = new CancellationTokenSource())
    {
        cts.CancelAfter(TimeSpan.FromSeconds(timeout));
        var response = await httpClient.GetAsync(url, cts.Token);
        return await response.Content.ReadAsStringAsync(cts.Token);
    }
}

@davidfowl response.Content.ReadAsStringAsync(cts.Token); This method has no arguments

sln162 commented 4 years ago

@JHeLiu 他给你的代码应该是.NET 5.0的,你用的可能是3.0/3.1/2.2

JHeLiu commented 4 years ago

@jmk92 是的 我的是 .NET CORE 3.1

sln162 commented 4 years ago

@@JHeLiu 代码就不贴了,我都说的很明白了,都到创建httpclient部分了,剩下的自由发挥,你试过没,试试我的方案,再说行不行,反正我封装的方法,超时是参数,任意传递,不存在你的问题。

JHeLiu commented 4 years ago

@@JHeLiu 代码就不贴了,我都说的很明白了,都到创建httpclient部分了,剩下的自由发挥,你试过没,试试我的方案,再说行不行,反正我封装的方法,超时是参数,任意传递,不存在你的问题。

嗯嗯,感觉 他的我试过了,可以,我再去监控下请求端口,3Q

Rassi commented 4 years ago

I am trying to extend a library which instantiates extensions itself without any DI support using the default constructior. In this extension I would like to use HttpClient functionality, and ensure that I am not susceptible to the following issues described in docs

I have been aware of the pitfalls of HttpClient for many years, and have previously used a static instance even though it had DNS issues. So I was very happy when HttpClientFactory was announced, since it was presented as a solution to all known problems, and likely will be updated if new issues arise. As a best practice I am trying to use HttpClientFactory whenever I need an HttpClient, and telling all of my colleagues to do the same.

Now that I'm trying to write a class library targeting netstandard, and without DI support, I am grappling with the question of how to get an HttpClient "properly". I have previously also run into making console applications and needing an HttpClient, where my default of using HttpClientFactory was not straight forward. I then ultimately end up simply instantiating an HttpClient, and hoping I won't experience any of the aforementioned issues.

I am probably in the category @rynowak stated in https://github.com/dotnet/extensions/issues/1345#issuecomment-480548175

B. I don't need that. I just want an HttpClient with good network behavior by default.

I have spent a couple of days reading this and many other issues, posts etc and I see quite a number of people wanting to use HttpClientFactory to avoid problems in a DI less environment. If possible I would like some guidance as to which solution you would think has the least drawbacks in a DI less scenario.

  1. Create my own setup logic like suggested by @rynowak, or will this influence/conflict with user code if they decides to change these values too?:

    If you are on .NET Core - you should use a single HttpClient directly and set SocketsHttpHandler.PooledConnectionTimeout here to an appropriate value.

    If you are on .NET Framework - you should use a single HttpClient and use ServicePoint to configure the similar settings.

  2. Instantiate HttpClientFactory using ServiceCollection:
    var serviceProvider = new ServiceCollection().AddHttpClient().BuildServiceProvider();
    _httpClientFactory = serviceProvider.GetService<IHttpClientFactory>();
  3. Extract DefaultHttpClientFactory and modify to my needs:

As of now I'm leaning towards the second solution, since it is the simplest and I will be able to update Microsoft.Extensions.Http in case new fixes are made to HttpClientFactory, but it still irks me that I need to pull in Microsoft.Extensions.DependencyInjection

Rassi commented 4 years ago

Also, just for clarification. @rynowak also states:

The good news for anyone interested in connection management is that .NET now has reasonable behavior on Linux (as of 2.1 and SocketsHttpHandler) but it requires configuration.

Does this mean that if I'm on Core 2.1+ I can use a single instance HttpClient and it will respect DNS changes (and not get socket exhaustion)?

davidfowl commented 4 years ago

Does this mean that if I'm on Core 2.1+ I can use a single instance HttpClient and it will respect DNS changes (and not get socket exhaustion)?

You can configure HttpClient with a timeout on both .NET Framework and .NET Core 2.1 and above with a timeout. You don't need this library to do that. I'm not sure why those projects copied that logic...

Mpdreamz commented 4 years ago

We decided to move to this strategy based on: https://github.com/dotnet/corefx/pull/29822 we'll move to the new timeouts available on the handler when we include netcoreapp3.x TFM's in the package and circumvent the copied logic all together.

hellznrg commented 4 years ago

What is so magical about the Microsoft system that this HttpClient problem exists, yet this problem doesn't exist in the node.js ecosystem?

CrossBound commented 4 years ago

I'm using AutoFac container in a long-running NT Service, so I also don't want to add all the Microsoft DI just to get the factory. I'm wanting to solve the following problems:

  1. I want to avoid the socket exhaustion problem, so a scoped dependency will not work.
  2. I want to avoid the lack of DNS refreshing, so a single static variable will not work.
  3. I need to customize the handler to use a client certificate on every request.

If my understanding is correct, it's not necessarily the HttpClient that is the trouble but the underlying HttpMessageHandler so really what I want that the IHttpClientFactory can solve is managing the lifetime of the message handler.

In my case I am solving the problem by just rolling my own client factory instead of adding the Microsoft DI container sub-system.

    public class HttpClientFactory : IDisposable
    {
        #region static members

        private static object _locker = new object();
        private static HttpClientHandler _handler = null;
        private static DateTime _expirationTime = DateTime.MinValue;

        #endregion static members

        private Action<HttpClientHandler> _configurator;
        private TimeSpan _handlerLifeTime;

        public HttpClientFactory(TimeSpan LifeTime, Action<HttpClientHandler> HandlerConfiguration)
        {
            if(LifeTime.TotalSeconds < 1)
            {
                throw new ArgumentOutOfRangeException(nameof(LifeTime)
                    , "You must provide a lifetime of at least 1 second for the handler");
            }

            _handlerLifeTime = LifeTime;
            _configurator = HandlerConfiguration;
        }

        // handler configuration is optional if all you want is the basic
        public HttpClientFactory(TimeSpan LifeTime) : this(LifeTime, null) { }

        public HttpClient GetClient()
        {
            lock (_locker)
            {
                if (_handler == null || _expirationTime <= DateTime.UtcNow)
                {
                    _handler?.Dispose(); // dispose old instance before it goes out of memory
                    _handler = new HttpClientHandler();
                    _expirationTime = DateTime.UtcNow.Add(_handlerLifeTime);

                    // configure handler
                    _configurator?.Invoke(_handler);
                }
            }

            // tell the client not to dispose the handler so we can reuse it
            return new HttpClient(_handler, false);
        }

        public void Dispose()
        {
            if(_handler != null)
            {
                _handler.Dispose();
                _handler = null;
            }
        }
    }

and simply tapping this new factory into my AutoFac container.

            var containerBuilder = new ContainerBuilder();

            // register singleton instance of my client factory
            containerBuilder.RegisterInstance(new HttpClientFactory(TimeSpan.FromMinutes(30), handler =>
            {
                // attach my client certificate
                var certificate = getClientCertificate(Configuration);
                if (certificate != null)
                {
                    handler.ClientCertificates.Add(certificate);
                }
            })).Named<HttpClientFactory>("client_certificate").SingleInstance(); 

            // now my HttpClient can be scoped
            containerBuilder.Register(context =>
            {
                var factory = context.ResolveNamed<HttpClientFactory>("client_certificate");
                return factory.GetClient();
            });

            return containerBuilder.Build();

references:

https://aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/ https://www.stevejgordon.co.uk/httpclient-creation-and-disposal-internals-should-i-dispose-of-httpclient

davidfowl commented 4 years ago

What is so magical about the Microsoft system that this HttpClient problem exists, yet this problem doesn't exist in the node.js ecosystem?

The main difference is the lifetime of the HttpClient. There's lots of state associated with the HttpClient, yet the client is disposable. That results in misuse where people create too many HttpClients which can result in port/connection exhaustion.

rynowak commented 4 years ago

@CrossBound - take a look at this comment - specifically option B. https://github.com/dotnet/extensions/issues/1345#issuecomment-480548175 - If you're on a supported version of .NET you can configure a long-lived HttpClient to do the right thing.

pkaminsky commented 4 years ago

HttpClient has longstanding problems where, historically, you have to choose between insufficient pooling on one hand, or being trapped in old DNS on the other. And stuff like that.

For solving those problems, and for what I've imagined the IHttpClientFactory to do, I don't get why that would ever be dependent upon DI. The excuse in this thread is that the API is "opinionated." MmmmmmmmmmmmKay. But is that a value-added opinion? Does the limitation help anything? This rather jingoistic "opinion" seems, at least on the surface, to violate much simpler and MUCH more agreed upon design principles, such as not creating unnecessary hard-dependencies between two components--in this case, the DI component and the HttpClientFactory component. The .NET team's absurd trend of making all features depend on the dependency injection container is self-contradictory, and--as evinced by threads like this one--counterproductive. You were supposed to remove dependencies, not create a god-class dependency! This is not the first time I've had a problem and ended up on a disappointing thread like this, where a platform dev says it's just impossible without all the hosting and DI configuration.

Want an opinion? IHttpClientFactory sucks.

davidfowl commented 4 years ago

I hear you but those Dns problems are solved by tweaking either service point manager (on .NET Framework) or by tweaking the setting non SocketsHttpHandler on .NET Core. If that doesn't work then we'd need to do fix those issues in the platform (at least on .NET Core). If you have evidence that this doesn't work, then we'd love to hear it. The HttpClientFactory is meant to be a configuration API, not a lifetime management API. In fact, we're likely going to remove the handler pooling and instead set the appropriate settings based on the platform to avoid this confusion.

pkaminsky commented 4 years ago

If you can do that--get HttpClient to work "right" out of the box with default configuration--make sure to publish it with much fanfare and I will sing your praises

SittenSpynne commented 4 years ago

I hear you but those Dns problems are solved by tweaking either service point manager (on .NET Framework) or by tweaking the setting non SocketsHttpHandler on .NET Core. If that doesn't work then we'd need to do fix those issues in the platform (at least on .NET Core). If you have evidence that this doesn't work, then we'd love to hear it. The HttpClientFactory is meant to be a configuration API, not a lifetime management API. In fact, we're likely going to remove the handler pooling and instead set the appropriate settings based on the platform to avoid this confusion.

The default behavior of the HttpClient shouldn't require navigating gotchas that I have to spend all day reading blog articles to solve. Once I've read 3 blog articles I have 5 different solutions, 2 of them tell me "using HttpClientFactory is the easiest way to get it right". The other 3 warn me if I'm not an experienced networking engineer, I might do more harm than good. None of these issues are in the HttpClient documentation. It's the Tomb of Horrors of pits of failure.

I've been using .NET since 2003 and I still worry every time I use HttpClient that I'm missing some ancestral knowledge because I didn't follow the right blog post. It'd be nice if MS retooled it to be right by default and let me tweak it if I have special needs and understand what those needs are. It's possible it works better in .NET Core? I have to configure it differently there. What's that mean for Xamarin? That's sort of .NET Core but actually Mono, right? What's that going to mean for MAUI in .NET 5? Will I have to learn four ways to configure HttpClient?

No other language or runtime I use makes it this hard to write an application that needs to make HTTP requests.

ATofler-Quest commented 4 years ago

If you are on .NET Core - you should use a single HttpClient directly and set SocketsHttpHandler.PooledConnectionTimeout here to an appropriate value.

@rynowak did you meant PooledConnectionLifetime instead of PooledConnectionTimeout? Because your link goes to PooledConnectionLifetime property.

rashadrivera commented 4 years ago

Is there any way to consume the HttpClientFactory without using dependency injection, or more precisely without using M.E.DI?

I am creating a client library for which I want to make use of HttpClientFactory but I am going to use this library also in a legacy project that does not not have a M.E.DI-compatible DI.

Ideally, I would just new up a DefaultHttpClientFactory and consume that directly but since the factory itself is internal, this is not possible.

Why is the HttpClientFactory internal and is there any way to consume it explicitly without using DI?

What does "M.E.DI" mean?

alexeyzimarev commented 4 years ago

What does "M.E.DI" mean?

Microsoft.Extensions.DependencyInjection

ericsampson commented 4 years ago

davidfowl wrote:
The HttpClientFactory is meant to be a configuration API, not a lifetime management API. In fact, we're likely going to remove the handler pooling and instead set the appropriate settings based on the platform to avoid this confusion.

Re that last part, the only thing I'd say is that I'd really hope that the equivalent of PooledConnectionLifetime be defaulted to something finite (like the effectively 2 minute default in HttpClientFactory) rather than infinite.

I recognize that it's probably not possible to change the plain HttpClient's sockethttphandler default PooledConnectionLifetime to be finite rather than the current infinite, for backcompat reasons? But it's a real discoverability/"pit of failure" issue, because there's lots of material out there (example below) that make it sound like the 2.1+ HttpClient solves these issues inherently, without being clear that it doesn't do it by default - you have to set the PooledConnectionLifetime explicitly. It's a lot easier to get an entire company of devs to understand/use "new HttpClient() does the right thing now" than trying to communicate "don't forget to do new HttpClient(new SocketsHttpHandler() { PooledConnectionLifetime = TimeSpan.FromMinutes(1) }) every single place every time! kthx"

HttpClient in .NET Core (since 2.1) performs connection pooling and lifetime management of those connections. This supports the use of a single HttpClient instance which reduces the chances of socket exhaustion whilst ensuring connections re-connect periodically to reflect DNS changes.

bugproof commented 3 years ago

HttpClient is the most confusing class of all. I still don't know if I'm using it correctly in my console apps. Every time when I have to deal with it I have 1000 voices in my head screaming I could be doing something wrong. I can't count the times I googled how to correctly use it. It's impossible to learn. I guess the safest option now is just adding DI to your console apps and be done with it. I'm not even sure what should I derive from anymore if I want to make my own HttpClientHandler correctly. I usually just created my class based on HttpMessageHandler but if this class is broken then I don't know anymore. The pain is real.

https://docs.microsoft.com/en-us/aspnet/core/fundamentals/http-requests?view=aspnetcore-3.1#alternatives-to-ihttpclientfactory

Create an instance of SocketsHttpHandler when the app starts and use it for the life of the app.

What if I want to add some functionality to SocketsHttpHandler ? It's sealed. (One workaround someone mentioned is deriving from DelegatingHandler and passing the execution down to SocketHttpHandler). It still gives me headaches.

ghost commented 3 years ago

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!

yanxiaodi commented 3 years ago

This issue should not be closed. I don't see the proper solution after 2 years.

Gladskih commented 3 years ago

That WebRequest-HttpWebRequest / HttpClient problem makes me think I've chosen wrong platform to learn and work with 10 years ago and all my life gone wrong.

daiplusplus commented 3 years ago

@rynowak please re-open this issue, @msftbot closed this when it shouldn't have.

natalie-o-perret commented 3 years ago

@bugproof @yanxiaodi @silkfire @Gladskih @Jehoel

According to the official Microsoft Docs, here:

Alternatives to IHttpClientFactory

Using IHttpClientFactory in a DI-enabled app avoids:

  • Resource exhaustion problems by pooling HttpMessageHandler instances.
  • Stale DNS problems by cycling HttpMessageHandler instances at regular intervals.

There are alternative ways to solve the preceding problems using a long-lived SocketsHttpHandler instance.

  • Create an instance of SocketsHttpHandlerwhen the app starts and use it for the life of the app.
  • Configure PooledConnectionLifetime to an appropriate value based on DNS refresh times.
  • Create HttpClient instances using new HttpClient(handler, disposeHandler: false) as needed.

The preceding approaches solve the resource management problems that IHttpClientFactory solves in a similar way.

  • The SocketsHttpHandler shares connections across HttpClient instances. This sharing prevents socket exhaustion.
  • The SocketsHttpHandler cycles connections according to PooledConnectionLifetime to avoid stale DNS problems.

You can use HttpClient without relying on HttpClientFactory and DI, by using SocketsHttpHandler:

// Used throughout your application (i.e. app lifetime)
using var httpSocketHandler = new SocketsHttpHandler();
using var httpClient = new HttpClient(httpSocketHandler);
using var response = await httpClient.GetAsync(@"https://www.google.com");
ericsampson commented 3 years ago

@mary-perret-1986 that code snippet would need to be tweaked slightly to achieve the stated goal.

natalie-o-perret commented 3 years ago

@mary-perret-1986 that code snippet would need to be tweaked slightly to achieve the stated goal.

@ericsampson Would you mind elaborating on this?

daiplusplus commented 3 years ago

@mary-perret-1986 SocketsHttpHandler is not available in .NET Standard: it's only available in .NET Core 2.1 or later. If we're making redistributable libraries that use HttpClient then that makes things difficult - or if we're targeting .NET Framework then we're SOL. What solution would you suggest in that case?

natalie-o-perret commented 3 years ago

@mary-perret-1986 SocketsHttpHandler is not available in .NET Standard: it's only available in .NET Core 2.1 or later. If we're making redistributable libraries that use HttpClient then that makes things difficult - or if we're targeting .NET Framework then we're SOL. What solution would you suggest in that case?

As unrealistic as it may sound, I would suggest you to backport whatever you can to make it compliant with the version of the framework you're forced to use.

(I thought we were talking about .NET Core 3.x and .NET 5+, hence the reason I didn't really get why there was an issue for you).

silkfire commented 3 years ago

@mary-perret-1986 Doesn't it make more sense to make the default empty constructor of SocketsHttpHandler either private or internal and the only way to access the class for consumers be through a static SocketsHttpHandler.Instance? This would make sure that only one instance is ever created through the lifetime of your app.

bugproof commented 3 years ago

@mary-perret-1986 that code snippet would need to be tweaked slightly to achieve the stated goal.

@ericsampson Would you mind elaborating on this?

See? There's always something about HttpClient that will surprise you. You can't just use it with confidence. There should be only 1 instance of HttpClient in your app but then what's the point of BaseAddress? It's badly designed to be used as a singleton. HttpClientFactory solves these problems for you but it only works with DI because it's internal.

Ideally, the constructor of HttpClient should be internal or private so people will never ever instantiate it directly but through HttpClientFactory to promote good design but that would break many existing HttpClient wrappers. It's too late to fix the API I guess.

daiplusplus commented 3 years ago

@bugproof

There should be only 1 instance of HttpClient in your app but then what's the point of BaseAddress?

No-one is saying that HttpClient should be an application-wide singleton. I think you're misinterpreting @rynowak's advice (repeated below; the bold-for-relevance is mine)

  • If you are on .NET Core - you should use a single HttpClient directly and set SocketsHttpHandler.PooledConnectionTimeout here to an appropriate value.
  • If you are on .NET Framework - you should use a single HttpClient and use ServicePoint to configure the similar settings.

My understanding is Ryan meant each separate consumer of a HttpClient should have its own single instance - not that there should be an application-wide singleton. This makes sense given that HttpClient is mutable, and generally speaking instances of mutable classes should not be shared by unrelated consumers (also, I wish .NET's metadata system had C++-style const-correctness annotations so we could determine mutability through intellisense and without needing to fire-up ILSpy...)

silkfire commented 3 years ago

@Jehoel @stevejgordon This article mentions that SocketsHttpHandler handles connection cycling and lifetime management automatically just like DefaultHttpClientFactory does, but having peeked at the source code I can't see anything static residing on the class itself. So two instances wouldn't be sharing data between each other.

Sigh, I just wish Microsoft could make the DefaultHttpClientFactory API public.

davidfowl commented 3 years ago

I don't understand why you think you need it. Are you managing different settings per handler? If not then you don't need it.

silkfire commented 3 years ago

I would believe that creating multiple HttpClients - one for every consuming service - would each use their own instance of SocketsHttpHandler, possibly leading to socket exhaustion. They would use their own respective pool, if I've understood this correctly. What I'm trying to say is I don't want apps to always have keep track of a singleton SocketsHttpHandler.

davidfowl commented 3 years ago

Then use the same singleton instance?

silkfire commented 3 years ago

If this is how SocketsHttpHandler is to be used I'd rather it be a static instance than a singleton...

davidfowl commented 3 years ago

Sure store it in your own static. There are obviously much more advanced use cases but the default should be a single instance used by your app until further notice