Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HttpClient Socket Exhaustion #8

Open
vella-nicholas opened this issue Mar 20, 2019 · 5 comments
Open

HttpClient Socket Exhaustion #8

vella-nicholas opened this issue Mar 20, 2019 · 5 comments
Assignees

Comments

@vella-nicholas
Copy link

A new instance of HttpClient is created with every CustomerIO instance. I am concerned on HttpClient Socket Exhaustion. Do you have any opinion on this, please?

@vegardlarsen
Copy link
Member

HttpClient exhaustion is a real issue that you should examine if you are hitting. It may very well be that our architecture is wrong here; I wrote CustomerIOSharp before I was aware of this issue.

But I think you can get away with having your CustomerIo object be a singleton, and therefore avoid the issue entirely. As long as your ICustomerFactory can tolerate being long-lived, that should work just fine?

@vella-nicholas
Copy link
Author

Socket Exhaustion can now be avoided using HttpClientFactory, I just needed to make sure I did not miss anything in this implementation.

My take on this would be to wire up HttpClientFactory in the composition root and if this is packaged provide extension methods for IServiceCollection like services.UseCustomerIo();.

To conclude, I still think this is a great API and will save anyone a lot of development time!

@vegardlarsen
Copy link
Member

I would be happy to accept a pull request for this.

It could be interesting to get a separate CustomerIOSharp.AspNetCore package as well, so you can do what you outline? That should probably be its own separate pull request, though.

@vella-nicholas
Copy link
Author

If we go forward with this, I would be more than happy to fork the project and issue a PR!

@vegardlarsen
Copy link
Member

Go for it! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants