Add JSONPath to the exception thrown by JsonDocument while parsing invalid JSON

[steveharter]

As an extension to https://github.com/dotnet/corefx/issues/37768, the JsonDocument class should support setting the Path property on JsonReaderException.

In addition, once this is complete, the serializer should re-throw JsonReaderException (as it does today) but now with the Path property pre-pended with the results from JsonReaderException to include the full Path of the serializer + JsonDocument.

See also PR dotnet/corefx#37938

[MarcoRossignoli]

@steveharter is this fixed?Should we close?

[ahsonkhan]

cc @bartonjs

[bartonjs]

Is the idea that the database building routine be in a try/catch where the catch modifies the exception object and then continues the same callstack/dispatch with throw;? Next most obvious alternative: That in the catch it throws a new Exception, wrapping the old one, and the outer one has the path set.

Either of those seems doable; either by making the StackRow value also keep track of the StartObject/StartArray (which would also eliminate the seek-backwards for every EndObject/EndArray at the expense of needing to grow the temporary buffer more often) or changing/overloading FindIndexOfFirstUnsetSizeOrLength to accept a "no later than" parameter. -- The StackRowStack knows how many elements already exist in the array (maybe off-by-one, but the data is there), and for an object property the name is available from the node after StartArray.

It's probably something easy enough for up-for-grabs.

I'd do it by renaming

        private static void Parse(
            ReadOnlySpan<byte> utf8JsonSpan,
            Utf8JsonReader reader,
            ref MetadataDb database,
            ref StackRowStack stack)
        {

to ParseCore, and making a new Parse with the same signature which just does

private static void Parse(
    ReadOnlySpan<byte> utf8JsonSpan,
    Utf8JsonReader reader,
    ref MetadataDb database,
    ref StackRowStack stack)
{
    try
    {
        ParseCore(utf8JsonSpan, reader, ref database, ref stack);
    }
    catch (JsonReaderException e)
    {
        // Using stack, database, and maybe Stack<string> parts, build the path
        e.Path = path;
    }
}

[gary-holland]

Hi,

Is JsonPath support for querying the JsonDocument/JsonElement structures still being looked at, as this is a major gap for us shifting from Newtonsoft to system.text.json.

What we need is an equivalent to:

var jsonPath = "$.my.path";
var json = JToken.Parse(jsonString);
var token = json.SelectToken(jsonPath);

Something like this would be ideal:

var jsonPath = "$.my.path";
var jsonDoc = JsonDocument.Parse(json);
var element = jsonDoc.SelectElement(jsonPath); //returns JsonElement
var elements = jsonDoc.SelectElements(jsonPath); //returns JsonElement.ArrayEnumerator

Thanks.

[ahsonkhan]

Is JsonPath support for querying the JsonDocument/JsonElement structures still being looked at

This issue is specifically about storing the JSON path information as part of the exception whenever the JsonDocument sees an invalid JSON while parsing, and not about full-fledged JsonPath support for querying the JSON data within the JsonDocument. Since this would be a separate feature, please create a separate issue for it. I would imagine the path support (at least for your use case), primarily be for convenience (requiring less code).

[gary-holland]

Thanks. @ahsonkhan. I've created a new issue dotnet/corefx#41537

[layomia]

Triage: action item is to remove this test and close this issue.

runtime/src/libraries/System.Text.Json/tests/Serialization/ExceptionTests.cs

Lines 366 to 380 in d4b06b1

	[Fact]
	[ActiveIssue("JsonElement needs to support Path")]
	public static void ExtensionPropertyRoundTripFails()
	{
	try
	{
	JsonSerializer.Deserialize<ClassWithExtensionProperty>(@"{""MyNestedClass"":{""UnknownProperty"":bad}}");
	Assert.True(false, "Expected JsonException was not thrown.");
	}
	catch (JsonException e)
	{
	// Until JsonElement supports populating Path ("UnknownProperty"), which will be prepended by the serializer ("MyNestedClass"), this will fail.
	Assert.Equal("$.MyNestedClass.UnknownProperty", e.Path);
	}
	}

[anreton]

It would be great if it was implemented.

For example, there is JsonConverter:

public class Options
{
	public Options(bool a, bool b, bool c)
	{
		this.A = a;
		this.B = b;
		this.C = c;
	}

	public bool A { get; }
	public bool B { get; }
	public bool C { get; }
}

public class OptionsJsonConverter : JsonConverter<Options>
{
	public override Options Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
	{
		using var jsonDocument = JsonDocument.ParseValue(ref reader);
		var rootElement = jsonDocument.RootElement;

		var a = rootElement.GetProperty("A").GetBoolean();
		var b = rootElement.GetProperty("B").GetBoolean();
		var c = rootElement.GetProperty("C").GetBoolean();

		return new Options(a, b, c);
	}

	public override void Write(Utf8JsonWriter writer, Options value, JsonSerializerOptions options)
	{
		writer.WriteStartObject();
		{
			writer.WriteBoolean(nameof(value.A), value.A);
			writer.WriteBoolean(nameof(value.B), value.B);
			writer.WriteBoolean(nameof(value.C), value.C);
		}
		writer.WriteEndObject();
	}
}

We are trying to deserialize:

const string json = @"""A"": 42";

var jsonOptions = new JsonSerializerOptions();
jsonOptions.Converters.Add(new OptionsJsonConverter());

var options = JsonSerializer.Deserialize<Options>(json, jsonOptions);

and get:

System.InvalidOperationException: 'The requested operation requires an element of type 'Object', but the target element has type 'String'.'

It would be great to get JsonException with the path and line number.