From 5195f03b640ed626bf60ab5142dfbee587d89701 Mon Sep 17 00:00:00 2001
From: Mark Wallace <127216156+markwallace-microsoft@users.noreply.github.com>
Date: Thu, 30 May 2024 19:43:59 +0100
Subject: [PATCH] .Net: Include request metadata in KernelException if response
cannot be serialized (#6407)
### Motivation and Context
We include additional metadata in `HttpOperationException` i.e. request
uri, payload, etc. When the response serialization fails we currently
throw a `KernelException` which doesn't include the HTTP request
metadata. This PR changes which exception is thrown to
`HttpOperationException` so we have one type of exception for clients to
catch and this includes all of the additional metadata . This is a
behaviour change but it enables clients to catch a single exception and
to get access to the request uri, payload, etc.
### Description
### Contribution Checklist
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone :smile:
---------
Co-authored-by: Dmytro Struk <13853051+dmytrostruk@users.noreply.github.com>
---
.../RestApiOperationRunner.cs | 29 +++++++++++++++++++
.../OpenApi/RestApiOperationRunnerTests.cs | 6 +++-
.../Plugins/RepairServiceTests.cs | 4 +--
.../Http/HttpOperationException.cs | 7 +++++
.../KernelException.cs | 4 +++
5 files changed, 47 insertions(+), 3 deletions(-)
diff --git a/dotnet/src/Functions/Functions.OpenApi/RestApiOperationRunner.cs b/dotnet/src/Functions/Functions.OpenApi/RestApiOperationRunner.cs
index 2a8a40e232cf..6f541b9dc55d 100644
--- a/dotnet/src/Functions/Functions.OpenApi/RestApiOperationRunner.cs
+++ b/dotnet/src/Functions/Functions.OpenApi/RestApiOperationRunner.cs
@@ -24,6 +24,21 @@ internal sealed class RestApiOperationRunner
private const string DefaultResponseKey = "default";
+ ///
+ /// HTTP request method.
+ ///
+ private const string HttpRequestMethod = "http.request.method";
+
+ ///
+ /// The HTTP request payload body.
+ ///
+ private const string HttpRequestBody = "http.request.body";
+
+ ///
+ /// Absolute URL describing a network resource according to RFC3986.
+ ///
+ private const string UrlFull = "url.full";
+
///
/// List of payload builders/factories.
///
@@ -186,9 +201,23 @@ private async Task SendAsync(
}
catch (HttpOperationException ex)
{
+#pragma warning disable CS0618 // Type or member is obsolete
ex.RequestMethod = requestMessage.Method.Method;
ex.RequestUri = requestMessage.RequestUri;
ex.RequestPayload = payload;
+#pragma warning restore CS0618 // Type or member is obsolete
+
+ 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);
+ ex.Data.Add(UrlFull, requestMessage.RequestUri?.ToString());
+ ex.Data.Add(HttpRequestBody, payload);
throw;
}
diff --git a/dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationRunnerTests.cs b/dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationRunnerTests.cs
index cb9e9b977749..c48f551c36f4 100644
--- a/dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationRunnerTests.cs
+++ b/dotnet/src/Functions/Functions.UnitTests/OpenApi/RestApiOperationRunnerTests.cs
@@ -1048,7 +1048,11 @@ public async Task ItShouldThrowExceptionForUnsupportedContentTypeAsync()
var sut = new RestApiOperationRunner(this._httpClient, this._authenticationHandlerMock.Object);
// Act & Assert
- await Assert.ThrowsAsync(() => sut.RunAsync(operation, arguments));
+ var kernelException = await Assert.ThrowsAsync(() => sut.RunAsync(operation, arguments));
+ Assert.Equal("The content type `fake/type` is not supported.", kernelException.Message);
+ Assert.Equal("POST", kernelException.Data["http.request.method"]);
+ Assert.Equal("https://fake-random-test-host/fake-path", kernelException.Data["url.full"]);
+ Assert.Equal("{\"value\":\"fake-value\"}", kernelException.Data["http.request.body"]);
}
[Fact]
diff --git a/dotnet/src/IntegrationTests/Plugins/RepairServiceTests.cs b/dotnet/src/IntegrationTests/Plugins/RepairServiceTests.cs
index eb625bd19559..f5da4448ef02 100644
--- a/dotnet/src/IntegrationTests/Plugins/RepairServiceTests.cs
+++ b/dotnet/src/IntegrationTests/Plugins/RepairServiceTests.cs
@@ -101,8 +101,8 @@ public async Task HttpOperationExceptionIncludeRequestInfoAsync()
catch (HttpOperationException ex)
{
Assert.Equal("Response status code does not indicate success: 404 (Not Found).", ex.Message);
- Assert.Equal("Patch", ex.RequestMethod);
- Assert.Equal("https://piercerepairsapi.azurewebsites.net/repairs", ex.RequestUri!.ToString());
+ Assert.Equal("Patch", ex.Data["http.request.method"]);
+ Assert.Equal("https://piercerepairsapi.azurewebsites.net/repairs", ex.Data["url.full"]);
}
}
diff --git a/dotnet/src/SemanticKernel.Abstractions/Http/HttpOperationException.cs b/dotnet/src/SemanticKernel.Abstractions/Http/HttpOperationException.cs
index 25a182244c7f..e0ac98141f8e 100644
--- a/dotnet/src/SemanticKernel.Abstractions/Http/HttpOperationException.cs
+++ b/dotnet/src/SemanticKernel.Abstractions/Http/HttpOperationException.cs
@@ -8,6 +8,10 @@ namespace Microsoft.SemanticKernel;
///
/// Represents an exception specific to HTTP operations.
///
+///
+/// Instances of this class optionally contain telemetry information in the Exception.Data property using keys that are consistent with the OpenTelemetry standard.
+/// See https://opentelemetry.io/ for more information.
+///
public class HttpOperationException : Exception
{
///
@@ -65,6 +69,7 @@ public HttpOperationException(HttpStatusCode? statusCode, string? responseConten
///
/// This information is only available in limited circumstances e.g. when using Open API plugins.
///
+ [Obsolete("This property is obsolete and will be removed in a future version. Use the Exception.Data['Name'] instead.")]
public string? RequestMethod { get; set; }
///
@@ -73,6 +78,7 @@ public HttpOperationException(HttpStatusCode? statusCode, string? responseConten
///
/// This information is only available in limited circumstances e.g. when using Open API plugins.
///
+ [Obsolete("This property is obsolete and will be removed in a future version. Use the Exception.Data['Url'] instead.")]
public Uri? RequestUri { get; set; }
///
@@ -81,5 +87,6 @@ public HttpOperationException(HttpStatusCode? statusCode, string? responseConten
///
/// This information is only available in limited circumstances e.g. when using Open API plugins.
///
+ [Obsolete("This property is obsolete and will be removed in a future version. Use the Exception.Data['Data'] instead.")]
public object? RequestPayload { get; set; }
}
diff --git a/dotnet/src/SemanticKernel.Abstractions/KernelException.cs b/dotnet/src/SemanticKernel.Abstractions/KernelException.cs
index ea62aa07ae81..482414a94160 100644
--- a/dotnet/src/SemanticKernel.Abstractions/KernelException.cs
+++ b/dotnet/src/SemanticKernel.Abstractions/KernelException.cs
@@ -7,6 +7,10 @@ namespace Microsoft.SemanticKernel;
///
/// Represents the base exception from which all Semantic Kernel exceptions derive.
///
+///
+/// Instances of this class optionally contain telemetry information in the Exception.Data property using keys that are consistent with the OpenTelemetry standard.
+/// See https://opentelemetry.io/ for more information.
+///
public class KernelException : Exception
{
///