Keep LineNumber, BytePositionInLine and Path when calling JsonSerializer.Deserialize<TValue>(ref reader)

[ivarne]

Description

A common pattern when implementing JsonConverter<MyType> is to read a few properties and forward the Utf8JsonReader to JsonSerializer.Deserialize<TValue>(ref reader) to read some common type that don't need special handling. If a JsonException is thrown in the forwarded call, LineNumber, BytePositionInLine and Path is wrong with regards to the original json that was tried to deserialize.

I'd like to keep the references LineNumber, BytePositionInLine and Path relative to the full json.

Reproduction Steps

using System.Text.Json;
using System.Text.Json.Serialization;

var json = @"{
    ""name"": ""test"",
    ""props"": [
        ""prop1"",
        ""prop2"",
        [""this"", ""is"", ""a"", ""error""]
    ]
}";
try
{
    var comp = JsonSerializer.Deserialize<MyComponent>(json, new JsonSerializerOptions(JsonSerializerDefaults.Web));
}
catch (JsonException e)
{
    Console.WriteLine(e.Message);
    Console.WriteLine($"Path: {e.Path}");
    Console.WriteLine($"LineNumber: {e.LineNumber}");
    Console.WriteLine($"BytePositionInLine: {e.BytePositionInLine}");
}

[JsonConverter(typeof(MyComponentConverter))]
public class MyComponent
{
    public string? Name { get; set; }
    public List<string>? Props { get; set; }
}

public class MyComponentConverter : JsonConverter<MyComponent>
{
    public override MyComponent? Read(ref Utf8JsonReader reader, Type type, JsonSerializerOptions options)
    {
        var component = new MyComponent();
        while (reader.Read() && reader.TokenType != JsonTokenType.EndObject)
        {
            var propName = reader.GetString();
            reader.Read();
            switch (propName)
            {
                case "name":
                    component.Name = reader.GetString();
                    break;
                case "props":
                    // use default behaviour for this list
                    component.Props = JsonSerializer.Deserialize<List<string>>(ref reader, options);
                    break;
                default:
                    reader.Skip();
                    break;
            }
        }

        return component;
    }

    public override void Write(Utf8JsonWriter writer, MyComponent value, JsonSerializerOptions options) => throw new NotImplementedException();
}

Expected behavior

I expect the JsonException to tell me where the parser was, when the error occurred. Just like it does when I don't use the custom converter.

Message: The JSON value could not be converted to System.String. Path: $.props[2] | LineNumber: 5 | BytePositionInLine: 9.
Path: $.props[2]
LineNumber: 5
BytePositionInLine: 9

Actual behavior

The actual result resets the e.Path, e.LineNumber and e.BytePositionInLine when the Utf8JsonReader is passed to JsonSerializer.Deserialize

Message: The JSON value could not be converted to System.String. Path: $[2] | LineNumber: 3 | BytePositionInLine: 9.
Path: $[2]
LineNumber: 3
BytePositionInLine: 9

Regression?

I don't know, but assume this has always been this way.

Known Workarounds

I can read everything inside the custom converter from the Utf8JsonReader without forwarding the simple components to the builtin tools.

Configuration

dotnet 6 and 7

Other information

I have a draft solution that does not forward path information in main...ivarne:runtime:custom-recursive-json-parsing. Not sure if it is correct. I don't understand all the concepts here.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

A common pattern when implementing JsonConverter<MyType> is to read a few properties and forward the Utf8JsonReader to JsonSerializer.Deserialize<TValue>(ref reader) to read some common type that don't need special handling. If a JsonException is thrown in the forwarded call, LineNumber, BytePositionInLine and Path is wrong with regards to the original json that was tried to deserialize.

I'd like to keep the references LineNumber, BytePositionInLine and Path relative to the full json.

Reproduction Steps

using System.Text.Json;
using System.Text.Json.Serialization;

var json = @"{
    ""name"": ""test"",
    ""props"": [
        ""prop1"",
        ""prop2"",
        [""this"", ""is"", ""a"", ""error""]
    ]
}";
try
{
    var comp = JsonSerializer.Deserialize<MyComponent>(json, new JsonSerializerOptions(JsonSerializerDefaults.Web));
}
catch (JsonException e)
{
    Console.WriteLine(e.Message);
    Console.WriteLine($"Path: {e.Path}");
    Console.WriteLine($"LineNumber: {e.LineNumber}");
    Console.WriteLine($"BytePositionInLine: {e.BytePositionInLine}");
}

[JsonConverter(typeof(MyComponentConverter))]
public class MyComponent
{
    public string? Name { get; set; }
    public List<string>? Props { get; set; }
}

public class MyComponentConverter : JsonConverter<MyComponent>
{
    public override MyComponent? Read(ref Utf8JsonReader reader, Type type, JsonSerializerOptions options)
    {
        var component = new MyComponent();
        while (reader.Read() && reader.TokenType != JsonTokenType.EndObject)
        {
            var propName = reader.GetString();
            reader.Read();
            switch (propName)
            {
                case "name":
                    component.Name = reader.GetString();
                    break;
                case "props":
                    // use default behaviour for this list
                    component.Props = JsonSerializer.Deserialize<List<string>>(ref reader, options);
                    break;
                default:
                    reader.Skip();
                    break;
            }
        }

        return component;
    }

    public override void Write(Utf8JsonWriter writer, MyComponent value, JsonSerializerOptions options) => throw new NotImplementedException();
}

Expected behavior

I expect the JsonException to tell me where the parser was, when the error occurred. Just like it does when I don't use the custom converter.

Message: The JSON value could not be converted to System.String. Path: $.props[2] | LineNumber: 5 | BytePositionInLine: 9.
Path: $.props[2]
LineNumber: 5
BytePositionInLine: 9

Actual behavior

The actual result resets the e.Path, e.LineNumber and e.BytePositionInLine when the Utf8JsonReader is passed to JsonSerializer.Deserialize

Message: The JSON value could not be converted to System.String. Path: $[2] | LineNumber: 3 | BytePositionInLine: 9.
Path: $[2]
LineNumber: 3
BytePositionInLine: 9

Regression?

I don't know, but assume this has always been this way.

Known Workarounds

I can read everything inside the custom converter from the Utf8JsonReader without forwarding the simple components to the builtin tools.

Configuration

dotnet 6 and 7

Other information

I have a draft solution that does not forward path information in main...ivarne:runtime:custom-recursive-json-parsing. Not sure if it is correct. I don't understand all the concepts here.

Author:	ivarne
Assignees:	-
Labels:	area-System.Text.Json, untriaged
Milestone:	-

[eiriktsarpalis]

Duplicate of #67403. This is a known issue with custom converters, since their current model is not capable of passing serialization state to nested values.

See #63795 for potential future improvements that might help work around the issue.

[ivarne]

Thank you so much for triaging the issue. It's a duplicate of #67403. I'm sorry for not finding that one.

I've looked at #63795, and don't really understand how the proposed solution will be required to fix this issue, nor that it will even help with this issue. At least when looking at the proposed solution, I think this is an orthogonal issue.

This issue is about forwarding some of the internal state of the Utf8JsonReader when taking "a copy of it" for a scoped reader in private static Utf8JsonReader GetReaderScopedToNextValue(ref Utf8JsonReader reader, scoped ref ReadStack state). Until #28482 is fixed, the state is not even accessible to users, so any manual form of forwarding the info, would be impossible, and you'd also need to somehow set the private filelds of Utf8JsonReader in order for the JsonException to be populated with the correct path and position.

[eiriktsarpalis]

The error messages constructed in JsonSerializer exception reports are constructed based on state in the internal ReadStack type, and not Utf8JsonReader per se:

runtime/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ReadStack.cs

Line 259 in 33cb335

public string JsonPath()

Until we expose the serialization state type (which is distinct from the JSON reader state in Utf8JsonReader) and accompanying converter methods that are capable of accepting that state, then it should not be possible to report on serialization state correctly in case of exceptions.

[ivarne]

Yes, I saw that after my previous response, trying to update my suggestion to forward the Path info as well. I share your assesment that it would require bigger changes. I was only looking at LineNumber and BytePositionInLine. Those are properties on the Utf8JsonReader, and can easily be forwarded as I do in main...ivarne:runtime:custom-recursive-json-parsing.