From 01da4c6a90391745d8f15488c5c39dfa4a8c398f Mon Sep 17 00:00:00 2001 From: markwallace-microsoft <127216156+markwallace-microsoft@users.noreply.github.com> Date: Fri, 5 Jul 2024 23:20:10 +0100 Subject: [PATCH 1/2] Include request data when operation is cancelled --- .../RestApiOperationRunner.cs | 8 ++++ .../OpenApi/RestApiOperationRunnerTests.cs | 40 ++++++++++++++++ .../Plugins/OpenApi/RepairServiceTests.cs | 48 +++++++++++++++++-- .../Functions/KernelFunction.cs | 8 +++- 4 files changed, 100 insertions(+), 4 deletions(-) diff --git a/dotnet/src/Functions/Functions.OpenApi/RestApiOperationRunner.cs b/dotnet/src/Functions/Functions.OpenApi/RestApiOperationRunner.cs index b7bc593c76b2..99ff2f276d15 100644 --- a/dotnet/src/Functions/Functions.OpenApi/RestApiOperationRunner.cs +++ b/dotnet/src/Functions/Functions.OpenApi/RestApiOperationRunner.cs @@ -221,6 +221,14 @@ private async Task SendAsync( throw; } + catch (OperationCanceledException ex) + { + ex.Data.Add(HttpRequestMethod, requestMessage.Method.Method); + ex.Data.Add(UrlFull, requestMessage.RequestUri?.ToString()); + ex.Data.Add(HttpRequestBody, payload); + + throw; + } catch (KernelException ex) { ex.Data.Add(HttpRequestMethod, requestMessage.Method.Method); diff --git a/dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationRunnerTests.cs b/dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationRunnerTests.cs index b836ec18ed80..72284c96548f 100644 --- a/dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationRunnerTests.cs +++ b/dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationRunnerTests.cs @@ -1206,6 +1206,39 @@ public async Task ItShouldSetHttpRequestMessageOptionsAsync() Assert.Equal(options.KernelArguments, kernelFunctionContext.Arguments); } + [Fact] + public async Task ItShouldIncludeRequestDataWhenOperationCanceledExceptionIsThrownAsync() + { + // Arrange + this._httpMessageHandlerStub.ExceptionToThrow = new OperationCanceledException(); + //this._httpMessageHandlerStub.ResponseToReturn.Content = new StringContent("fake-content", Encoding.UTF8, "fake/type"); + + var operation = new RestApiOperation( + "fake-id", + new Uri("https://fake-random-test-host"), + "fake-path", + HttpMethod.Post, + "fake-description", + [], + payload: null + ); + + var arguments = new KernelArguments + { + { "payload", JsonSerializer.Serialize(new { value = "fake-value" }) }, + { "content-type", "application/json" } + }; + + var sut = new RestApiOperationRunner(this._httpClient, this._authenticationHandlerMock.Object); + + // Act & Assert + var canceledException = await Assert.ThrowsAsync(() => sut.RunAsync(operation, arguments)); + Assert.Equal("The operation was canceled.", canceledException.Message); + Assert.Equal("POST", canceledException.Data["http.request.method"]); + Assert.Equal("https://fake-random-test-host/fake-path", canceledException.Data["url.full"]); + Assert.Equal("{\"value\":\"fake-value\"}", canceledException.Data["http.request.body"]); + } + public class SchemaTestData : IEnumerable { public IEnumerator GetEnumerator() @@ -1302,6 +1335,8 @@ private sealed class HttpMessageHandlerStub : DelegatingHandler public HttpResponseMessage ResponseToReturn { get; set; } + public Exception ExceptionToThrow { get; set; } + public HttpMessageHandlerStub() { this.ResponseToReturn = new HttpResponseMessage(System.Net.HttpStatusCode.OK) @@ -1312,6 +1347,11 @@ public HttpMessageHandlerStub() protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { + if (this.ExceptionToThrow is not null) + { + throw this.ExceptionToThrow; + } + this.RequestMessage = request; this.RequestContent = request.Content is null ? null : await request.Content.ReadAsByteArrayAsync(cancellationToken); diff --git a/dotnet/src/IntegrationTests/Plugins/OpenApi/RepairServiceTests.cs b/dotnet/src/IntegrationTests/Plugins/OpenApi/RepairServiceTests.cs index f6bcb3c01be8..313fe8cf1785 100644 --- a/dotnet/src/IntegrationTests/Plugins/OpenApi/RepairServiceTests.cs +++ b/dotnet/src/IntegrationTests/Plugins/OpenApi/RepairServiceTests.cs @@ -1,4 +1,5 @@ // Copyright (c) Microsoft. All rights reserved. +using System; using System.Net.Http; using System.Text.Json; using System.Text.Json.Serialization; @@ -17,7 +18,7 @@ public async Task ValidateInvokingRepairServicePluginAsync() { // Arrange var kernel = new Kernel(); - using var stream = System.IO.File.OpenRead("Plugins/repair-service.json"); + using var stream = System.IO.File.OpenRead("Plugins/OpenApi/repair-service.json"); using HttpClient httpClient = new(); var plugin = await kernel.ImportPluginFromOpenApiAsync( @@ -73,7 +74,7 @@ public async Task HttpOperationExceptionIncludeRequestInfoAsync() { // Arrange var kernel = new Kernel(); - using var stream = System.IO.File.OpenRead("Plugins/repair-service.json"); + using var stream = System.IO.File.OpenRead("Plugins/OpenApi/repair-service.json"); using HttpClient httpClient = new(); var plugin = await kernel.ImportPluginFromOpenApiAsync( @@ -107,12 +108,53 @@ public async Task HttpOperationExceptionIncludeRequestInfoAsync() } } + [Fact(Skip = "This test is for manual verification.")] + public async Task KernelFunctionCanceledExceptionIncludeRequestInfoAsync() + { + // Arrange + var kernel = new Kernel(); + using var stream = System.IO.File.OpenRead("Plugins/OpenApi/repair-service.json"); + using HttpClient httpClient = new(); + + var plugin = await kernel.ImportPluginFromOpenApiAsync( + "RepairService", + stream, + new OpenApiFunctionExecutionParameters(httpClient) { IgnoreNonCompliantErrors = true, EnableDynamicPayload = false }); + + var arguments = new KernelArguments + { + ["payload"] = """{ "title": "Engine oil change", "description": "Need to drain the old engine oil and replace it with fresh oil.", "assignedTo": "", "date": "", "image": "" }""" + }; + + var id = 99999; + + // Update Repair + arguments = new KernelArguments + { + ["payload"] = $"{{ \"id\": {id}, \"assignedTo\": \"Karin Blair\", \"date\": \"2024-04-16\", \"image\": \"https://www.howmuchisit.org/wp-content/uploads/2011/01/oil-change.jpg\" }}" + }; + + try + { + httpClient.Timeout = TimeSpan.FromMilliseconds(10); // Force a timeout + + await plugin["updateRepair"].InvokeAsync(kernel, arguments); + Assert.Fail("Expected KernelFunctionCanceledException"); + } + catch (KernelFunctionCanceledException ex) + { + Assert.Equal("The invocation of function 'updateRepair' was canceled.", ex.Message); + Assert.Equal("Patch", ex.Data["http.request.method"]); + Assert.Equal("https://piercerepairsapi.azurewebsites.net/repairs", ex.Data["url.full"]); + } + } + [Fact(Skip = "This test is for manual verification.")] public async Task UseDelegatingHandlerAsync() { // Arrange var kernel = new Kernel(); - using var stream = System.IO.File.OpenRead("Plugins/repair-service.json"); + using var stream = System.IO.File.OpenRead("Plugins/OpenApi/repair-service.json"); using var httpHandler = new HttpClientHandler(); using var customHandler = new CustomHandler(httpHandler); diff --git a/dotnet/src/SemanticKernel.Abstractions/Functions/KernelFunction.cs b/dotnet/src/SemanticKernel.Abstractions/Functions/KernelFunction.cs index 9e50f653f5f8..5ff1e073bdf6 100644 --- a/dotnet/src/SemanticKernel.Abstractions/Functions/KernelFunction.cs +++ b/dotnet/src/SemanticKernel.Abstractions/Functions/KernelFunction.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. using System; +using System.Collections; using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; @@ -432,7 +433,12 @@ private static void HandleException( // visible to a consumer if that's needed. if (ex is OperationCanceledException cancelEx) { - throw new KernelFunctionCanceledException(kernel, kernelFunction, arguments, result, cancelEx); + var kernelEx = new KernelFunctionCanceledException(kernel, kernelFunction, arguments, result, cancelEx); + foreach (DictionaryEntry entry in cancelEx.Data) + { + kernelEx.Data.Add(entry.Key, entry.Value); + } + throw kernelEx; } } } From 1a8727506240a3677edb61dbac3a49ebfcf443a3 Mon Sep 17 00:00:00 2001 From: markwallace-microsoft <127216156+markwallace-microsoft@users.noreply.github.com> Date: Mon, 8 Jul 2024 16:22:55 +0100 Subject: [PATCH 2/2] Address code review feedback --- .../Plugins/OpenApi/RepairServiceTests.cs | 5 +++-- .../Functions/KernelFunction.cs | 8 +------- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/dotnet/src/IntegrationTests/Plugins/OpenApi/RepairServiceTests.cs b/dotnet/src/IntegrationTests/Plugins/OpenApi/RepairServiceTests.cs index 313fe8cf1785..ac63ac9bcf54 100644 --- a/dotnet/src/IntegrationTests/Plugins/OpenApi/RepairServiceTests.cs +++ b/dotnet/src/IntegrationTests/Plugins/OpenApi/RepairServiceTests.cs @@ -144,8 +144,9 @@ public async Task KernelFunctionCanceledExceptionIncludeRequestInfoAsync() catch (KernelFunctionCanceledException ex) { Assert.Equal("The invocation of function 'updateRepair' was canceled.", ex.Message); - Assert.Equal("Patch", ex.Data["http.request.method"]); - Assert.Equal("https://piercerepairsapi.azurewebsites.net/repairs", ex.Data["url.full"]); + Assert.NotNull(ex.InnerException); + Assert.Equal("Patch", ex.InnerException.Data["http.request.method"]); + Assert.Equal("https://piercerepairsapi.azurewebsites.net/repairs", ex.InnerException.Data["url.full"]); } } diff --git a/dotnet/src/SemanticKernel.Abstractions/Functions/KernelFunction.cs b/dotnet/src/SemanticKernel.Abstractions/Functions/KernelFunction.cs index 5ff1e073bdf6..9e50f653f5f8 100644 --- a/dotnet/src/SemanticKernel.Abstractions/Functions/KernelFunction.cs +++ b/dotnet/src/SemanticKernel.Abstractions/Functions/KernelFunction.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. using System; -using System.Collections; using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; @@ -433,12 +432,7 @@ private static void HandleException( // visible to a consumer if that's needed. if (ex is OperationCanceledException cancelEx) { - var kernelEx = new KernelFunctionCanceledException(kernel, kernelFunction, arguments, result, cancelEx); - foreach (DictionaryEntry entry in cancelEx.Data) - { - kernelEx.Data.Add(entry.Key, entry.Value); - } - throw kernelEx; + throw new KernelFunctionCanceledException(kernel, kernelFunction, arguments, result, cancelEx); } } }