Document exception handling behavior in SendChatHistoryAsync XML comments#152
Merged
pontemonti merged 2 commits intousers/johanb/RealTimeThreatProtectionfrom Dec 31, 2025
Conversation
…exception handling 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
Document exception handling behavior in SendChatHistoryAsync XML comments
Dec 31, 2025
pontemonti
approved these changes
Dec 31, 2025
pontemonti
merged commit b172cc0
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.
Addresses review feedback to clarify exception handling and null property behavior in
IMcpToolServerConfigurationService.SendChatHistoryAsyncmethods.Changes
Exception documentation: Added
<exception>tags documenting thatArgumentNullExceptionis thrown for null parameters andInvalidOperationExceptionis thrown when required turn context properties (Conversation.Id,Activity.Id,Activity.Text) are nullHTTP failure behavior: Added
<remarks>section clarifying that HTTP exceptions (HttpRequestException,TaskCanceledException) are caught and logged but not rethrown—the method completes successfully even when HTTP requests failThis makes the API contract explicit for consumers who need to understand when exceptions will be thrown versus silently handled.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.