Add Happy Eyeballs Support #1187
Conversation
- Create HappyEyeballsCallback to provide and track state around a callback for SocketsHttpHandler. - Create HappyHttpClient to provide easy access to a shared HTTP client service
I'm almost certain this will break *a lot*, especially as I'm coalescing all the `new HttpClient()`s down into the service. Eh, that's why it's WIP.
|
Potential future work:
|
- Don't expose SharedSocketsHandler, as it can cause problems if not treated with a lot of care. - Expose a SharedHappyEyeballsCallback for systems that want to synchronize state. - Make HappyEyeballsCallback Disposable. - Just clear the array, not much stateful stuff in there. - Switch PluginRepository to use its own Handler, with the shared callback.
- New AsyncUtils for helper methods related to async/Task operation. - Add FirstSuccessfulTask to help race a list of Tasks. - Swap HappyEyeballsCallback over to using FirstSuccessfulTask. - Fix a concurrency bug on the Address Family cache.
Testing StrategyTesting this particular change is a bit annoying as With that out of the way, testing this seems easiest with
Some more advanced simulation is possible as well. For example, traffic to Cloudflare's IPv6 network can be completely blocked (see their IP list) using |
If IPv6 support isn't detected, skip the entire race process and simply use the IPv4 connection.
- Rename "ipv4WaitMillis" to "ipv6GracePeriod" - Allow a fast IPv6 failure to immediately start an IPv4 attempt - If IPv6 fails to connect in the grace period, continue with the race. - Change AsyncUtils#FirstSuccessfulTask to take an ICollection instead
| var tryConnectIPv6 = this.AttemptConnection(AddressFamily.InterNetworkV6, context, linkedToken.Token); | ||
| var timedV6Attempt = Task.WhenAny(tryConnectIPv6, Task.Delay(this.ipv6GracePeriod, linkedToken.Token)); | ||
|
|
||
| if (await timedV6Attempt == tryConnectIPv6 && tryConnectIPv6.IsCompletedSuccessfully) | ||
| { | ||
| stream = tryConnectIPv6.GetAwaiter().GetResult(); | ||
| } | ||
| else | ||
| { | ||
| var race = AsyncUtils.FirstSuccessfulTask(new List<Task<NetworkStream>> | ||
| { | ||
| tryConnectIPv6, | ||
| this.AttemptConnection(AddressFamily.InterNetwork, context, linkedToken.Token), | ||
| }); | ||
|
|
||
| // If our connections all fail, this will explode with an exception. | ||
| stream = race.GetAwaiter().GetResult(); | ||
| } |
There was a problem hiding this comment.
Feeling messy again, but unsure if I want to start doing some really fun things to FirstSuccessfulTask to make it FirstSuccessfulTaskWithDelayBetweenTasksAndFastFailureToTextTask.
:/
This pull request aims to solve a number of issues related to bad IPv6 compatibility, especially an inability to load Kamori when one's ISP hiccups. This is done by implementing a variant of the Happy Eyeballs algorithm, as described in RFC 8305. The specific implementation we're using here was derived from a pull request to the
jellyfinproject, with some tweaks that hopefully would help our use case.This pull request provides a new class meant for public consumption:
HappyEyeballsCallback. This class provides aConnectCallbackmethod that can be assigned to aSocketsHttpHandlerwhen creating anHttpClient.HappyEyeballsCallbackimplements the following behavior:forcedAddressFamilywill be used and all evaluation will be skipped.AddressFamilyfor the DNS name being used. If it exists, the cached value is used and future steps will be skipped.When a connection is successfully established, the resulting
AddressFamilyis cached for that DNS hostname and all future connections for the life of the application will use thatAddressFamily.The
HappyEyeballsCallbackclass provides state in the form of the DNS/AddressFamily cache, which can (but does not have to be) shared between multipleHttpClients if desired. Plugin developers should normally create and maintain their own callback with their own state.This pull request also adds a new service, the
HappyHttpClient, which is intended to be shared between all Dalamud subsystems that can use a commonHttpClientand obsoletesUtil.HttpClient. Further, all uses ofUtil.HttpClientand any new non-customizedHttpClients are replaced with references to theHttpClientprovided by this service.This pull request, if merged, fixes #1186.