Mark the FunctionCall/ResultContent.Exception properties as [JsonIgnore] (#5492)

Given implementation details of the JSON source generator today, even with the converter applied to these properties, code is still being generated for Exception, leading to unsuppressable trimmer warnings.
This commit is contained in:
Stephen Toub 2024-10-09 16:13:05 -04:00 коммит произвёл GitHub
Родитель a51631ed08
Коммит 397010983e
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
7 изменённых файлов: 59 добавлений и 152 удалений

Просмотреть файл

@ -49,11 +49,11 @@ public sealed class FunctionCallContent : AIContent
/// Gets or sets any exception that occurred while mapping the original function call data to this class.
/// </summary>
/// <remarks>
/// When an instance of <see cref="FunctionCallContent"/> is serialized using <see cref="JsonSerializer"/>, any exception
/// stored in this property will be serialized as a string. When deserialized, the string will be converted back to an instance
/// of the base <see cref="Exception"/> type. As such, consumers shouldn't rely on the exact type of the exception stored in this property.
/// This property is for information purposes only. The <see cref="Exception"/> is not serialized as part of serializing
/// instances of this class with <see cref="JsonSerializer"/>; as such, upon deserialization, this property will be <see langword="null"/>.
/// Consumers should not rely on <see langword="null"/> indicating success.
/// </remarks>
[JsonConverter(typeof(FunctionCallExceptionConverter))]
[JsonIgnore]
public Exception? Exception { get; set; }
/// <summary>Gets a string representing this instance to display in the debugger.</summary>

Просмотреть файл

@ -1,96 +0,0 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
using System;
using System.ComponentModel;
#if NET
using System.Runtime.ExceptionServices;
#endif
using System.Text.Json;
using System.Text.Json.Serialization;
using Microsoft.Shared.Diagnostics;
namespace Microsoft.Extensions.AI;
/// <summary>Serializes an exception as a string and deserializes it back as a base <see cref="Exception"/> containing that contents as a message.</summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public sealed class FunctionCallExceptionConverter : JsonConverter<Exception>
{
private const string ClassNamePropertyName = "className";
private const string MessagePropertyName = "message";
private const string InnerExceptionPropertyName = "innerException";
private const string StackTracePropertyName = "stackTraceString";
/// <inheritdoc />
public override void Write(Utf8JsonWriter writer, Exception value, JsonSerializerOptions options)
{
_ = Throw.IfNull(writer);
_ = Throw.IfNull(value);
// Schema and property order taken from Exception.GetObjectData() implementation.
writer.WriteStartObject();
writer.WriteString(ClassNamePropertyName, value.GetType().ToString());
writer.WriteString(MessagePropertyName, value.Message);
writer.WritePropertyName(InnerExceptionPropertyName);
if (value.InnerException is Exception innerEx)
{
Write(writer, innerEx, options);
}
else
{
writer.WriteNullValue();
}
writer.WriteString(StackTracePropertyName, value.StackTrace);
writer.WriteEndObject();
}
/// <inheritdoc />
public override Exception? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
if (reader.TokenType != JsonTokenType.StartObject)
{
throw new JsonException();
}
using var doc = JsonDocument.ParseValue(ref reader);
return ParseExceptionCore(doc.RootElement);
static Exception ParseExceptionCore(JsonElement element)
{
string? message = null;
string? stackTrace = null;
Exception? innerEx = null;
foreach (JsonProperty property in element.EnumerateObject())
{
switch (property.Name)
{
case MessagePropertyName:
message = property.Value.GetString();
break;
case StackTracePropertyName:
stackTrace = property.Value.GetString();
break;
case InnerExceptionPropertyName when property.Value.ValueKind is not JsonValueKind.Null:
innerEx = ParseExceptionCore(property.Value);
break;
}
}
#pragma warning disable CA2201 // Do not raise reserved exception types
Exception result = new(message, innerEx);
#pragma warning restore CA2201
#if NET
if (stackTrace != null)
{
ExceptionDispatchInfo.SetRemoteStackTrace(result, stackTrace);
}
#endif
return result;
}
}
}

Просмотреть файл

@ -20,10 +20,31 @@ public sealed class FunctionResultContent : AIContent
/// </summary>
/// <param name="callId">The function call ID for which this is the result.</param>
/// <param name="name">The function name that produced the result.</param>
/// <param name="result">The function call result.</param>
/// <param name="exception">Any exception that occurred when invoking the function.</param>
/// <param name="result">
/// This may be <see langword="null"/> if the function returned <see langword="null"/>, if the function was void-returning
/// and thus had no result, or if the function call failed. Typically, however, in order to provide meaningfully representative
/// information to an AI service, a human-readable representation of those conditions should be supplied.
/// </param>
[JsonConstructor]
public FunctionResultContent(string callId, string name, object? result = null, Exception? exception = null)
public FunctionResultContent(string callId, string name, object? result)
{
CallId = Throw.IfNull(callId);
Name = Throw.IfNull(name);
Result = result;
}
/// <summary>
/// Initializes a new instance of the <see cref="FunctionResultContent"/> class.
/// </summary>
/// <param name="callId">The function call ID for which this is the result.</param>
/// <param name="name">The function name that produced the result.</param>
/// <param name="result">
/// This may be <see langword="null"/> if the function returned <see langword="null"/>, if the function was void-returning
/// and thus had no result, or if the function call failed. Typically, however, in order to provide meaningfully representative
/// information to an AI service, a human-readable representation of those conditions should be supplied.
/// </param>
/// <param name="exception">Any exception that occurred when invoking the function.</param>
public FunctionResultContent(string callId, string name, object? result, Exception? exception)
{
CallId = Throw.IfNull(callId);
Name = Throw.IfNull(name);
@ -35,9 +56,13 @@ public sealed class FunctionResultContent : AIContent
/// Initializes a new instance of the <see cref="FunctionResultContent"/> class.
/// </summary>
/// <param name="functionCall">The function call for which this is the result.</param>
/// <param name="result">The function call result.</param>
/// <param name="result">
/// This may be <see langword="null"/> if the function returned <see langword="null"/>, if the function was void-returning
/// and thus had no result, or if the function call failed. Typically, however, in order to provide meaningfully representative
/// information to an AI service, a human-readable representation of those conditions should be supplied.
/// </param>
/// <param name="exception">Any exception that occurred when invoking the function.</param>
public FunctionResultContent(FunctionCallContent functionCall, object? result = null, Exception? exception = null)
public FunctionResultContent(FunctionCallContent functionCall, object? result, Exception? exception = null)
: this(Throw.IfNull(functionCall).CallId, functionCall.Name, result, exception)
{
}
@ -59,17 +84,22 @@ public sealed class FunctionResultContent : AIContent
/// <summary>
/// Gets or sets the result of the function call, or a generic error message if the function call failed.
/// </summary>
/// <remarks>
/// This may be <see langword="null"/> if the function returned <see langword="null"/>, if the function was void-returning
/// and thus had no result, or if the function call failed. Typically, however, in order to provide meaningfully representative
/// information to an AI service, a human-readable representation of those conditions should be supplied.
/// </remarks>
public object? Result { get; set; }
/// <summary>
/// Gets or sets an exception that occurred if the function call failed.
/// </summary>
/// <remarks>
/// When an instance of <see cref="FunctionResultContent"/> is serialized using <see cref="JsonSerializer"/>, any exception
/// stored in this property will be serialized as a string. When deserialized, the string will be converted back to an instance
/// of the base <see cref="Exception"/> type. As such, consumers shouldn't rely on the exact type of the exception stored in this property.
/// This property is for information purposes only. The <see cref="Exception"/> is not serialized as part of serializing
/// instances of this class with <see cref="JsonSerializer"/>; as such, upon deserialization, this property will be <see langword="null"/>.
/// Consumers should not rely on <see langword="null"/> indicating success.
/// </remarks>
[JsonConverter(typeof(FunctionCallExceptionConverter))]
[JsonIgnore]
public Exception? Exception { get; set; }
/// <summary>Gets a string representing this instance to display in the debugger.</summary>

Просмотреть файл

@ -3,6 +3,7 @@
using System;
using System.Collections.Generic;
using System.Diagnostics.CodeAnalysis;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Text.Json.Serialization.Metadata;
@ -16,6 +17,8 @@ internal static partial class JsonDefaults
public static JsonSerializerOptions Options { get; } = CreateDefaultOptions();
/// <summary>Creates the default <see cref="JsonSerializerOptions"/> to use for serialization-related operations.</summary>
[UnconditionalSuppressMessage("AotAnalysis", "IL3050", Justification = "DefaultJsonTypeInfoResolver is only used when reflection-based serialization is enabled")]
[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026", Justification = "DefaultJsonTypeInfoResolver is only used when reflection-based serialization is enabled")]
private static JsonSerializerOptions CreateDefaultOptions()
{
// If reflection-based serialization is enabled by default, use it, as it's the most permissive in terms of what it can serialize,
@ -28,9 +31,7 @@ internal static partial class JsonDefaults
var options = new JsonSerializerOptions(JsonSerializerDefaults.Web)
{
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
#pragma warning disable IL3050, IL2026 // only used when reflection-based serialization is enabled
TypeInfoResolver = new DefaultJsonTypeInfoResolver(),
#pragma warning restore IL3050, IL2026
};
options.MakeReadOnly();

Просмотреть файл

@ -18,11 +18,6 @@
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
</PropertyGroup>
<PropertyGroup>
<!-- TEMPORARY UNTIL JSON SOURCE GENERATOR ISSUE CAN BE SORTED OUT -->
<NoWarn>$(NoWarn);IL2026</NoWarn>
</PropertyGroup>
<PropertyGroup>
<InjectSharedCollectionExtensions>true</InjectSharedCollectionExtensions>
<InjectSharedEmptyCollections>true</InjectSharedEmptyCollections>

Просмотреть файл

@ -5,9 +5,6 @@ using System;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Linq;
#if NET
using System.Runtime.ExceptionServices;
#endif
using System.Text.Json;
using System.Text.Json.Nodes;
using System.Threading;
@ -89,41 +86,19 @@ public class FunctionCallContentTests
{
// Arrange
var ex = new InvalidOperationException("hello", new NullReferenceException("bye"));
#if NET
ExceptionDispatchInfo.SetRemoteStackTrace(ex, "stack trace");
#endif
var sut = new FunctionCallContent("callId1", "functionName") { Exception = ex };
var sut = new FunctionCallContent("callId1", "functionName", new Dictionary<string, object?> { ["key"] = "value" }) { Exception = ex };
// Act
var json = JsonSerializer.SerializeToNode(sut, TestJsonSerializerContext.Default.Options);
var deserializedSut = JsonSerializer.Deserialize<FunctionCallContent>(json, TestJsonSerializerContext.Default.Options);
// Assert
JsonObject jsonEx = Assert.IsType<JsonObject>(json!["exception"]);
Assert.Equal(4, jsonEx.Count);
Assert.Equal("System.InvalidOperationException", (string?)jsonEx["className"]);
Assert.Equal("hello", (string?)jsonEx["message"]);
#if NET
Assert.StartsWith("stack trace", (string?)jsonEx["stackTraceString"]);
#endif
JsonObject jsonExInner = Assert.IsType<JsonObject>(jsonEx["innerException"]);
Assert.Equal(4, jsonExInner.Count);
Assert.Equal("System.NullReferenceException", (string?)jsonExInner["className"]);
Assert.Equal("bye", (string?)jsonExInner["message"]);
Assert.Null(jsonExInner["innerException"]);
Assert.Null(jsonExInner["stackTraceString"]);
Assert.NotNull(deserializedSut);
Assert.IsType<Exception>(deserializedSut.Exception);
Assert.Equal("hello", deserializedSut.Exception.Message);
#if NET
Assert.StartsWith("stack trace", deserializedSut.Exception.StackTrace);
#endif
Assert.IsType<Exception>(deserializedSut.Exception.InnerException);
Assert.Equal("bye", deserializedSut.Exception.InnerException.Message);
Assert.Null(deserializedSut.Exception.InnerException.StackTrace);
Assert.Null(deserializedSut.Exception.InnerException.InnerException);
Assert.Equal("callId1", deserializedSut.CallId);
Assert.Equal("functionName", deserializedSut.Name);
Assert.NotNull(deserializedSut.Arguments);
Assert.Single(deserializedSut.Arguments);
Assert.Null(deserializedSut.Exception);
}
[Fact]

Просмотреть файл

@ -12,7 +12,7 @@ public class FunctionResultContentTests
[Fact]
public void Constructor_PropsDefault()
{
FunctionResultContent c = new("callId1", "functionName");
FunctionResultContent c = new("callId1", "functionName", null);
Assert.Equal("callId1", c.CallId);
Assert.Equal("functionName", c.Name);
Assert.Null(c.RawRepresentation);
@ -54,7 +54,7 @@ public class FunctionResultContentTests
[Fact]
public void Constructor_PropsRoundtrip()
{
FunctionResultContent c = new("callId1", "functionName");
FunctionResultContent c = new("callId1", "functionName", null);
Assert.Null(c.RawRepresentation);
object raw = new();
@ -106,7 +106,7 @@ public class FunctionResultContentTests
public void ItShouldBeSerializableAndDeserializableWithException()
{
// Arrange
var sut = new FunctionResultContent("callId1", "functionName") { Exception = new InvalidOperationException("hello") };
var sut = new FunctionResultContent("callId1", "functionName", null, new InvalidOperationException("hello"));
// Act
var json = JsonSerializer.Serialize(sut, TestJsonSerializerContext.Default.Options);
@ -114,7 +114,9 @@ public class FunctionResultContentTests
// Assert
Assert.NotNull(deserializedSut);
Assert.IsType<Exception>(deserializedSut.Exception);
Assert.Contains("hello", deserializedSut.Exception.Message);
Assert.Equal(sut.Name, deserializedSut.Name);
Assert.Equal(sut.CallId, deserializedSut.CallId);
Assert.Equal(sut.Result, deserializedSut.Result?.ToString());
Assert.Null(deserializedSut.Exception);
}
}