Incorrect lifetime of `HttpMessageHandler`

[OronDF343]

Issue description Configuring the Vonage client using the recommended methods leads to creation of a new HttpMessageHandler for every request or for every scope (depending on the API used). This behavior can lead to exhaustion of available ports when making a large amount of requests.

See the official HttpClient guidelines for more info. The recommendation is either to use IHttpClientFactory (named or typed clients) or to use long-lived HttpClient instances with PooledConnectionLifetime set on the handler.

The following are the code paths that I found that have this problem:

Problem path 1 Most API requests will be sent from here: https://github.com/Vonage/vonage-dotnet-sdk/blob/7f6c7a00c585a89c262dff50639c9052bfcf21c1/Vonage/Request/ApiRequest.cs#L255-L257 The Configuration.Client property will always return a new HttpClient, created like this (slightly different when using throttling): https://github.com/Vonage/vonage-dotnet-sdk/blob/7f6c7a00c585a89c262dff50639c9052bfcf21c1/Vonage/Configuration.cs#L151-L154 The ClientHandler property is null by default, and therefore will be null when configuring from appsettings.json (Configuration.Instance is used in this case). Therefore, every API call will create a new default handler.

Workaround 1 Set Configuration.Instance.ClientHandler during startup. This also allows configuring proxy settings (works for me).

Problem path 2 Some of the clients inside VonageClient are initialized like this: https://github.com/Vonage/vonage-dotnet-sdk/blob/7f6c7a00c585a89c262dff50639c9052bfcf21c1/Vonage/VonageClient.cs#L191-L193 Which calls: https://github.com/Vonage/vonage-dotnet-sdk/blob/7f6c7a00c585a89c262dff50639c9052bfcf21c1/Vonage/VonageClient.cs#L143-L145 This means that every time a VonageClient is constructed, new handlers are created. This will happen on every scope if registered as such using AddVonageClientScoped.

Also note that the creation of the handlers here is hardcoded, which makes it impossible to override or to configure proxy settings.

Workaround 2 Use a singleton VonageClient (Not ideal. No workaround exists for proxy settings)

[Tr00d]

Hi @OronDF343,

Thanks for raising that point. Indeed, HttpClients are created at two different places at the moment. I'll have a look into it soon enough, and keep you posted.

[Tr00d]

Hi @OronDF343,

It turns out to be a bit trickier than I initially thought.

1. Even if the PoolConnectionLifetime is the recommended solution, it is weirdly not available in netstandard2.0.
2. The HttpClientFactory would not cover 100% of use cases, as not everybody uses the DI resolution to create a client.

So, I noticed a package exist to port the SocketsHttpHandler to netstandard2.0. I've been able to try it locally and it seems to work as expected. I'll use this handler in the codebase, and offer the possibility to override default values for both PoolConnectionLifetime and PooledConnectionIdleTimeout from the configuration file. I'll initialize Configuration.Instance.ClientHandler with a socket handler, which will be reused when creating clients.

[Tr00d]

Hi @OronDF343,

I have good news to share. Any HttpClient now use a single instance of SocketsHttpHandler, with a default connection lifetime of 10 minutes and a default idle timeout of 1 minute.

These values can be overriden through configuration, as explained in the readme. You can still provide your own HttpMessageHandler by assigning the value to Configuration.ClientHandler.

In case you're wondering, the implementation works: https://github.com/Vonage/vonage-dotnet-sdk/blob/7f9f55674582c8a3f0d5bd70ac06295827e5cd2d/Vonage.Test/ConfigurationTest.cs#L211-L242

This will get released later today in v7.1.0

[OronDF343]

Thanks

Implementation looks good to me, however if you ever plan to use multi-targeting with this library then please consider conditionally using the built-in SocketsHttpHandler on platforms where it exists, since the backported library is not up to date with the latest .NET code.

[Tr00d]

At the moment, there's no short-time plan to use multi-targeting. Obviously, the gap between netstandard2.0 and netstandard2.1 is going to grow over time but this is something we may address later on.