From 6ca055c1d4aa0920d08fc72c7d87f0a4deb72dd9 Mon Sep 17 00:00:00 2001
From: King Star <mcxin.y@gmail.com>
Date: Sat, 10 Oct 2026 23:10:52 +0800
Subject: [PATCH] fix(functions): preserve JSON converter errors

Avoid falling back to raw arguments when JSON deserialization fails, so custom converter validation details reach the caller. Quote plain string arguments only when they are not valid JSON.

Fixes #14575.
---
 .../Functions/KernelFunctionFromMethod.cs     |  35 +++++-
 .../KernelFunctionFromMethodTests1.cs         | 111 ++++++++++++++++++
 2 files changed, 142 insertions(+), 4 deletions(-)

diff --git a/dotnet/src/SemanticKernel.Core/Functions/KernelFunctionFromMethod.cs b/dotnet/src/SemanticKernel.Core/Functions/KernelFunctionFromMethod.cs
index 9385ae02bcaf..1c1a1cc11fd8 100644
--- a/dotnet/src/SemanticKernel.Core/Functions/KernelFunctionFromMethod.cs
+++ b/dotnet/src/SemanticKernel.Core/Functions/KernelFunctionFromMethod.cs
@@ -787,6 +787,37 @@ private static (Func<KernelFunction, Kernel, KernelArguments, CancellationToken,
     [RequiresDynamicCode("Uses reflection to deserialize given value if no source generated metadata provided via JSOs, making it incompatible with AOT scenarios.")]
     private static bool TryToDeserializeValue(object value, Type targetType, JsonSerializerOptions? jsonSerializerOptions, out object? deserializedValue)
     {
+        if (value is string stringValue)
+        {
+            JsonDocumentOptions documentOptions = new()
+            {
+                AllowTrailingCommas = jsonSerializerOptions?.AllowTrailingCommas ?? false,
+                CommentHandling = jsonSerializerOptions?.ReadCommentHandling is JsonCommentHandling.Skip or JsonCommentHandling.Allow
+                    ? JsonCommentHandling.Skip
+                    : JsonCommentHandling.Disallow,
+                MaxDepth = jsonSerializerOptions?.MaxDepth ?? 0,
+            };
+
+            JsonDocument document;
+            try
+            {
+                document = JsonDocument.Parse(stringValue, documentOptions);
+            }
+            catch (JsonException)
+            {
+                // Some connectors pass JSON string arguments without their quotes.
+                document = JsonDocument.Parse(JsonSerializer.Serialize(stringValue), documentOptions);
+            }
+
+            using (document)
+            {
+                // Let JsonException from a custom converter propagate so the caller sees its validation message.
+                deserializedValue = document.RootElement.Deserialize(targetType, jsonSerializerOptions);
+            }
+
+            return true;
+        }
+
         try
         {
             deserializedValue = value switch
@@ -809,10 +840,6 @@ private static bool TryToDeserializeValue(object value, Type targetType, JsonSer
         {
             // There is no compatible JsonConverter for targetType or its serializable members.
         }
-        catch (JsonException)
-        {
-            // The JSON is invalid.
-        }
 
         deserializedValue = null;
         return false;
diff --git a/dotnet/src/SemanticKernel.UnitTests/Functions/KernelFunctionFromMethodTests1.cs b/dotnet/src/SemanticKernel.UnitTests/Functions/KernelFunctionFromMethodTests1.cs
index 2c154029617a..60c438bd76d4 100644
--- a/dotnet/src/SemanticKernel.UnitTests/Functions/KernelFunctionFromMethodTests1.cs
+++ b/dotnet/src/SemanticKernel.UnitTests/Functions/KernelFunctionFromMethodTests1.cs
@@ -1420,6 +1420,75 @@ public async Task ItCanDeserializeJsonStringAsync()
         Assert.Equal(28, actualArgValue.Id);
     }
 
+    [Theory]
+    [InlineData(false)]
+    [InlineData(true)]
+    public async Task ItPreservesJsonConverterErrorsAsync(bool useJsonElement)
+    {
+        // Arrange
+        object value = useJsonElement ? JsonElement.Parse("500") : "500";
+        var function = KernelFunctionFactory.CreateFromMethod((JsonConverterTestQuantity quantity) => quantity.Value);
+
+        // Act
+        JsonException exception = await Assert.ThrowsAsync<JsonException>(
+            () => function.InvokeAsync(this._kernel, new() { ["quantity"] = value }));
+
+        // Assert
+        Assert.Contains("A quantity is between 1 and 100.", exception.Message, StringComparison.Ordinal);
+    }
+
+    [Fact]
+    public async Task ItDeserializesPlainStringArgumentsForJsonConverterTypesAsync()
+    {
+        // Arrange
+        const string expected = "FR7630006000011234567890189";
+        JsonConverterTestIban? actual = null;
+        var function = KernelFunctionFactory.CreateFromMethod((JsonConverterTestIban iban) => { actual = iban; });
+
+        // Act
+        await function.InvokeAsync(this._kernel, new() { ["iban"] = expected });
+
+        // Assert
+        Assert.Equal(expected, actual?.Value);
+    }
+
+    [Fact]
+    public async Task ItPreservesJsonConverterErrorsForPlainStringArgumentsAsync()
+    {
+        // Arrange
+        var function = KernelFunctionFactory.CreateFromMethod((JsonConverterTestIban iban) => iban.Value);
+
+        // Act
+        JsonException exception = await Assert.ThrowsAsync<JsonException>(
+            () => function.InvokeAsync(this._kernel, new() { ["iban"] = "FR76" }));
+
+        // Assert
+        Assert.Contains("An IBAN has 15 to 34 characters.", exception.Message, StringComparison.Ordinal);
+    }
+
+    [Fact]
+    public async Task ItUsesJsonSerializerOptionsWhenParsingPlainStringArgumentsAsync()
+    {
+        // Arrange
+        static int GetId(CustomTypeForJsonTests value) => value.Id;
+
+        var options = new JsonSerializerOptions
+        {
+            AllowTrailingCommas = true,
+            ReadCommentHandling = JsonCommentHandling.Skip,
+        };
+        var function = KernelFunctionFromMethod.Create(
+            ((Func<CustomTypeForJsonTests, int>)GetId).Method,
+            jsonSerializerOptions: options,
+            functionName: "GetId");
+
+        // Act
+        FunctionResult result = await function.InvokeAsync(this._kernel, new() { ["value"] = "/*comment*/{\"id\":28,}" });
+
+        // Assert
+        Assert.Equal(28, result.GetValue<int>());
+    }
+
     [Fact]
     public async Task ItCanDeserializeThirdPartyJsonPrimitivesAsync()
     {
@@ -1590,4 +1659,46 @@ private sealed class ThirdPartyJsonPrimitive(string jsonToReturn)
     {
         public override string ToString() => jsonToReturn;
     }
+
+#pragma warning disable CA1812 // Instantiated by System.Text.Json.
+    [JsonConverter(typeof(JsonConverterTestQuantityConverter))]
+    private sealed record JsonConverterTestQuantity(int Value);
+
+    private sealed class JsonConverterTestQuantityConverter : JsonConverter<JsonConverterTestQuantity>
+    {
+        public override JsonConverterTestQuantity Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+        {
+            int value = reader.GetInt32();
+            if (value is < 1 or > 100)
+            {
+                throw new JsonException("A quantity is between 1 and 100.");
+            }
+
+            return new JsonConverterTestQuantity(value);
+        }
+
+        public override void Write(Utf8JsonWriter writer, JsonConverterTestQuantity value, JsonSerializerOptions options) =>
+            writer.WriteNumberValue(value.Value);
+    }
+
+    [JsonConverter(typeof(JsonConverterTestIbanConverter))]
+    private sealed record JsonConverterTestIban(string Value);
+
+    private sealed class JsonConverterTestIbanConverter : JsonConverter<JsonConverterTestIban>
+    {
+        public override JsonConverterTestIban Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+        {
+            string value = reader.GetString()!;
+            if (value.Length is < 15 or > 34)
+            {
+                throw new JsonException("An IBAN has 15 to 34 characters.");
+            }
+
+            return new JsonConverterTestIban(value);
+        }
+
+        public override void Write(Utf8JsonWriter writer, JsonConverterTestIban value, JsonSerializerOptions options) =>
+            writer.WriteStringValue(value.Value);
+    }
+#pragma warning restore CA1812
 }