Refactor HttpClient to use IHttpClientFactory to prevent socket exhaustion#150
Merged
pontemonti merged 4 commits intousers/johanb/RealTimeThreatProtectionfrom Dec 31, 2025
Merged
Conversation
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Address feedback on API implementation for sending chat history
Refactor HttpClient to use IHttpClientFactory to prevent socket exhaustion
Dec 31, 2025
pontemonti
reviewed
Dec 31, 2025
| { | ||
| var userAgentConfiguration = toolOptions?.UserAgentConfiguration ?? Agent365SdkUserAgentConfiguration.Instance; | ||
| using var httpClient = RuntimeUtility.GetDefaultHttpClient(userAgentConfiguration: userAgentConfiguration); | ||
| var httpClient = this._httpClientFactory.CreateClient(); |
Contributor
There was a problem hiding this comment.
@copilot - Make sure that RuntimeUtility.GetDefaultHttpClient is still used - it will have to retrieve the client instance. We want the user agent header (and any other default behavior we add) to always be included, and the current implementation makes that difficult.
Contributor
Author
There was a problem hiding this comment.
Updated RuntimeUtility.GetDefaultHttpClient to accept IHttpClientFactory as an optional first parameter. The service now calls this method with the injected factory, centralizing the User-Agent header configuration and any other default behaviors.
Changes in commits:
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
pontemonti
approved these changes
Dec 31, 2025
8cc5052
into
users/johanb/RealTimeThreatProtection
1 check passed
pontemonti
added a commit
that referenced
this pull request
Jan 6, 2026
…148) * Initial implementation of API to send chat history to MCP platform. * Address PR feedback to clarify documentation. * Address PR feedback about documentation * Address PR feedback related to references to instance fields * Address PR feedback related to documentation * Address PR feedback about required vs optional properties * Update src/Tooling/Core/Services/McpToolServerConfigurationService.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/Tooling/Core/Utils/Utility.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Models/ChatHistoryMessageTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Use short-form Encoding.UTF8 instead of fully qualified name (#153) * Initial plan * Use Encoding.UTF8 instead of fully qualified name Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Document exception handling behavior in SendChatHistoryAsync XML comments (#152) * Initial plan * Update XML documentation for SendChatHistoryAsync methods to clarify exception handling Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Refactor HttpClient to use IHttpClientFactory to prevent socket exhaustion (#150) * Initial plan * Refactor HttpClient usage to use IHttpClientFactory Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Use RuntimeUtility.GetDefaultHttpClient with IHttpClientFactory Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Remove unnecessary using statement Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Add comprehensive test coverage for SendChatHistoryAsync parameter validation (#151) * Initial plan * Add comprehensive tests for SendChatHistoryAsync method - Added tests for missing Activity properties (conversation ID, message ID, user message) - All tests validate expected exception handling - Tests verify that InvalidOperationException is thrown with appropriate error messages - Tests cover both method overloads (with and without ToolOptions) - Existing null parameter validation tests remain unchanged Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Fix tests to properly mock distinct Activity property scenarios - SendChatHistoryAsync_MissingConversationId now properly mocks a null Conversation property - SendChatHistoryAsync_MissingMessageId now mocks null Id with valid Conversation - SendChatHistoryAsync_MissingUserMessage now mocks null Text with valid Conversation - Each test validates a distinct error path through the method - All 8 tests pass successfully Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Address code review feedback: improve code style - Remove excessive blank lines between methods - Use explicit cast syntax (Type)null! instead of null as Type! - Maintain consistent null handling across all test methods - All 8 tests continue to pass Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Update src/Tooling/Extensions/AgentFramework/Agent365AgentFrameworkSdkUserAgentConfiguration.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix missing IHttpClientFactory parameter in test constructor calls (#154) * Initial plan * Fix test failures by adding missing IHttpClientFactory parameter to McpToolServerConfigurationService constructor calls Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Remove exception throw for unsuccessful HTTP responses in SendChatHistoryAsync (#156) * Initial plan * Remove exception throw for failed HTTP requests in SendChatHistoryAsync Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Add happy path test coverage for SendChatHistoryAsync HTTP POST (#157) * Initial plan * Add comprehensive happy path tests for SendChatHistoryAsync method Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Fix flaky test behavior by using fixed timestamps instead of DateTimeOffset.UtcNow Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Update src/Tooling/Extensions/AzureAIFoundry/Agent365AzureAIFoundrySdkUserAgentConfiguration.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/Tooling/Extensions/SemanticKernel/Agent365SemanticKernelSdkUserAgentConfiguration.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Enforce IHttpClientFactory as required parameter in GetDefaultHttpClient (#162) * Initial plan * Make IHttpClientFactory required in GetDefaultHttpClient Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Add test coverage for non-success HTTP response scenarios in SendChatHistoryAsync (#161) * Initial plan * Add test coverage for non-success HTTP response scenarios (400, 401, 403, 404, 500) Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Refactor HTTP error status tests to use parameterized theory approach Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Update src/Tooling/Core/Models/ChatMessageRequest.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix HttpResponseMessage disposal in SendChatHistoryAsync (#163) * Initial plan * Wrap HttpResponseMessage in using statement for proper disposal Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Fix resource leak from undisposed HttpResponseMessage in test mocks (#164) * Initial plan * Fix HttpResponseMessage disposal issues by using lambda factory pattern Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Update src/Tests/Microsoft.Agents.A365.Tooling.Tests/Services/McpToolServerConfigurationServiceTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Fix HttpResponseMessage disposal race condition in async mock test (#165) * Initial plan * Fix HttpResponseMessage disposal race condition in test Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> * Redesign SendChatHistoryAsync to return a result object. * Add CancellationToken argument --------- Co-authored-by: Johan Broberg <johanb@microsoft.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
This file contains hidden or 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
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.
McpToolServerConfigurationServicewas creating newHttpClientinstances per request viaRuntimeUtility.GetDefaultHttpClient(), which prevents connection pooling and can cause socket exhaustion under load.Changes
IHttpClientFactoryparameter. When provided, uses the factory to create HttpClient instances; otherwise falls back to direct instantiation for backward compatibility.IHttpClientFactoryand pass it toRuntimeUtility.GetDefaultHttpClient()inSendChatHistoryAsyncandGetMCPServerFromToolingGatewayAsyncIHttpClientFactoryviaservices.AddHttpClient()IHttpClientFactoryin test setupExample
Before:
After:
This approach centralizes HttpClient configuration in
RuntimeUtility.GetDefaultHttpClientwhile leveragingIHttpClientFactoryfor connection pooling. User-Agent configuration and other default behaviors are consistently applied. HttpClient instances from the factory are not disposed—the factory manages their lifecycle and connection pooling.💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.