feat: Use SocketsHttpHandler to configure HttpClient in .NET 6+
#2931
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2931 +/- ##
==========================================
+ Coverage 81.42% 81.44% +0.01%
==========================================
Files 465 465
Lines 29563 29603 +40
Branches 3278 3285 +7
==========================================
+ Hits 24073 24110 +37
- Misses 4697 4698 +1
- Partials 793 795 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
|
nrcventura
reviewed
Dec 20, 2024
| /// <typeparam name="TReturn"></typeparam> | ||
| /// <param name="task"></param> | ||
| /// <returns></returns> | ||
| public static TReturn RunSync<TReturn>(Func<Task<TReturn>> task) |
There was a problem hiding this comment.
Marty and I talked about removing this helper method.
nrcventura
reviewed
Dec 20, 2024
| @@ -65,7 +102,9 @@ public override IHttpResponse Send(IHttpRequest request) | |||
| req.Content.Headers.Add(contentHeader.Key, contentHeader.Value); | |||
| } | |||
|
|
|||
| var response = _httpClientWrapper.SendAsync(req).GetAwaiter().GetResult(); | |||
| Log.Finest($"Request({request.RequestGuid}: Sending"); | |||
| var response = AsyncHelper.RunSync(() => _httpClientWrapper.SendAsync(req)); | |||
There was a problem hiding this comment.
We may not need this helper method, because ConfigureAwait(false) seems to help resolve the same problem in a simpler manner.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NRHttpClientto dynamically create an instance ofSocketsHttpHandlerwhen the app is running in .NET 6 or later, and configuredPooledConnectionTimeoutandPooledConnectionIdleTimeoutto help the agent respond better to DNS changes. The connection timeout value is hard-coded to 5 minutes, which overrides the default of "never" and is probably sufficient for our needs. I didn't see a compelling reason to add a configuration setting for this value. Resolves Update HttpClient usage to better support DNS changes on .NET/.NET Core #1845.Additional refactoring:
AsyncHelper) to safely execute async methods synchronously. Used inNRHttpClient.Send()as an attempt to avoid deadlocks in the (eventual) call toHttpClient.SendAsync(). Taken from https://stackoverflow.com/a/56928748/2078975 which seems to be modeled after https://learn.microsoft.com/en-us/archive/msdn-magazine/2015/july/async-programming-brownfield-async-development#the-thread-pool-hack.... harvest finishedmessage even if no events were sent. This will help in troubleshooting recently reported deadlocks when sending data.IHttpClientFactory.CreateClient()toGetOrCreateClient()to better reflect what the method does.IHttpClientand the classes that implement itConnectionHandler.SendDataOverWire<T>()to make exception handling a bit cleaner and more clear.throwthe exception again anyway.usings inNRHttpClientfor objects that areIDisposable(HttpRequestMessage,ByteArrayContent)