Skip to content

Conversation

@kbot7
Copy link
Contributor

@kbot7 kbot7 commented Sep 15, 2018

Modified SpotifyWebClient to use a single HttpClient per class lifetime. This will allow the HttpClient to properly pool connections, and avoids SocketException errors under load.

From the Microsoft documentation:

HttpClient is intended to be instantiated once and reused throughout the life of an application. The following conditions can result in SocketException errors:

  • Creating a new HttpClient instance per request.
  • Server under heavy load.

Source: https://docs.microsoft.com/en-us/aspnet/web-api/overview/advanced/calling-a-web-api-from-a-net-client

In depth explanation:
https://aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/


public SpotifyWebClient()
{
HttpClientHandler clientHandler = CreateClientHandler(ProxyConfig);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line won't work, since ProxyConfig will always be null. I guess adding it to the constructor with default to null would work...? It also doesnt make sense keeping it as a property, does it?

@JohnnyCrazy
Copy link
Owner

Thanks, nice catch :)

Added one comment, I think we need to change the ProxyConfig handling

@kbot7
Copy link
Contributor Author

kbot7 commented Sep 15, 2018

Happy to help. I've added the changes requested. ProxyConfig is now in the constructor of SpotifyWebClient

@JohnnyCrazy JohnnyCrazy merged commit fa0f3e2 into JohnnyCrazy:master Sep 17, 2018
@JohnnyCrazy
Copy link
Owner

👍

@kbot7 kbot7 deleted the http-client-reuse branch September 21, 2018 04:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants