Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrect paths in errors and exceptions #182

Open
NWalker1208 opened this issue Sep 30, 2024 · 0 comments
Open

Incorrect paths in errors and exceptions #182

NWalker1208 opened this issue Sep 30, 2024 · 0 comments

Comments

@NWalker1208
Copy link

I've found myself needing to use the JsonSerializerSettings.Error property to capture certain types of exceptions that can occur during deserialization. That setting contains an error handler delegate which is passed an ErrorEventArgs object. I am relying on the args.ErrorContext.Path property to contain the full path to the value which caused a given exception. This works fine for most types of objects, but I found that the JsonSubtypes library seems to break the paths of any errors that occur within the types of objects it is configured to convert. I've provided a handful of examples below that show how errors behave normally with Newtonsoft.Json versus how they behave when using JsonSubtypes. See this documentation for how errors are meant to be handled in Newtonsoft.Json.

Source/destination types

internal class RootObject {
  public NestedObject? Property1 { get; set; }
  public BaseClass? Property2 { get; set; }
  public SubClassB? Property3 { get; set; }
}

internal class NestedObject {
  public bool Value { get; set; }
}

internal abstract class BaseClass { }

internal class SubClassA : BaseClass {
  public float Value { get; set; }
}

internal class SubClassB : BaseClass {
  public int Value { get; set; }
}

Source/destination JSON

Example 1: No errors.

{
  "Property1": {
    "Value": true
  },
  "Property2": {
    "$kind": "a",
    "Value": 3.14
  },
  "Property3": {
    "Value": 42
  }
}

Example 2: Error that doesn't involve JsonSubtypes.

{
  "Property1": {
    "Value": "not a bool"
  }
}

Example 3: Error from an object that uses a discriminator.

{
  "Property2": {
    "$kind": "a",
    "Value": "not a float"
  }
}

Example 4: Error from an object that doesn't use a discriminator.

{
  "Property3": {
    "Value": "not an int"
  }
}

Expected behavior

Example 1

No errors received by JsonSerializerSettings.Error and no exception thrown by serializer.Deserialize.

Example 2

JsonSerializerSettings.Error receives a single error event for which args.CurrentObject == args.ErrorContext.OriginalObject, and that event has a path of "Property1.Value". The exception thrown by serializer.Deserialize has the same path.

Example 3

JsonSerializerSettings.Error receives a single error event for which args.CurrentObject == args.ErrorContext.OriginalObject, and that event has a path of "Property2.Value". The exception thrown by serializer.Deserialize has the same path.

Example 4

JsonSerializerSettings.Error receives a single error event for which args.CurrentObject == args.ErrorContext.OriginalObject, and that event has a path of "Property3.Value". The exception thrown by serializer.Deserialize has the same path.

Actual behavior

Examples 1 and 2

Both of these behave as expected. I've just included them as baseline examples.

Example 3

JsonSerializerSettings.Error receives two error events for which args.CurrentObject == args.ErrorContext.OriginalObject. The first has a path of "Value", while the second has a path of "Property2". The exception thrown by serializer.Deserialize just has the path "Value".

Example 4

JsonSerializerSettings.Error receives two error events for which args.CurrentObject == args.ErrorContext.OriginalObject. The first has a path of "Value", while the second has a path of "Property3". The exception thrown by serializer.Deserialize just has the path "Value".

Steps to reproduce

Full code example
using JsonSubTypes;
using Newtonsoft.Json;

namespace JsonSubtypesBug;

internal class Program {
  static void Main() {
    // Expected result: No errors/exceptions
    // Behaves as expected.
    Console.WriteLine("=== TEST 1 ===");
    PrintDeserializationErrors("""
      {
        "Property1": {
          "Value": true
        },
        "Property2": {
          "$kind": "a",
          "Value": 3.14
        },
        "Property3": {
          "Value": 42
        }
      }
      """);

    // Expected result: Error at "Property1.Value", exception from the same path.
    // Behaves as expected.
    Console.WriteLine("\n=== TEST 2 ===");
    PrintDeserializationErrors("""
      {
        "Property1": {
          "Value": "not a bool"
        }
      }
      """);

    // Expected result: Error at "Property2.Value", exception from the same path.
    // Actual result: Error at "Value" and at "Property2", exception from "Value".
    Console.WriteLine("\n=== TEST 3 ===");
    PrintDeserializationErrors("""
      {
        "Property2": {
          "$kind": "a",
          "Value": "not a float"
        }
      }
      """);

    // Expected result: Error at "Property3.Value", exception from the same path.
    // Actual result: Error at "Value" and at "Property3", exception from "Value".
    Console.WriteLine("\n=== TEST 4 ===");
    PrintDeserializationErrors("""
      {
        "Property3": {
          "Value": "not an int"
        }
      }
      """);
  }

  static void PrintDeserializationErrors(string json) {
    JsonSerializerSettings settings = new() {
      Converters = {
        JsonSubtypesConverterBuilder
          .Of<BaseClass>("$kind")
          .RegisterSubtype<SubClassA>("a")
          .RegisterSubtype<SubClassB>("b")
          .Build()
      },
      Error = HandleError
    };
    var serializer = JsonSerializer.CreateDefault(settings);

    try {
      using var jsonReader = new JsonTextReader(new StringReader(json));
      serializer.Deserialize<RootObject>(jsonReader);
      Console.WriteLine("No exception was thrown.");
    } catch (JsonReaderException ex) {
      Console.WriteLine($"Caught exception that originated from '{ex.Path}': {ex.Message}");
    }

    void HandleError(object? sender, Newtonsoft.Json.Serialization.ErrorEventArgs args) {
      if (args.CurrentObject == args.ErrorContext.OriginalObject) {
        Console.WriteLine($"Encountered error at '{args.ErrorContext.Path}': {args.ErrorContext.Error.Message}");
      }
    }
  }
}

// Types to deserialize

internal class RootObject {
  public NestedObject? Property1 { get; set; }
  public BaseClass? Property2 { get; set; }
  public SubClassB? Property3 { get; set; }
}

internal class NestedObject {
  public bool Value { get; set; }
}

internal abstract class BaseClass { }

internal class SubClassA : BaseClass {
  public float Value { get; set; }
}

internal class SubClassB : BaseClass {
  public int Value { get; set; }
}

Possible Causes

I'm not super familiar with the internals of this library, but I did poke around a little bit while diagnosing this issue and came across a couple details that might be of help.

First of all, it seems that the JsonReader object is responsible for maintaining the current path being read. When deserializing a polymorphic type, JsonSubtypes creates a new JsonReader specifically for the JSON object being read. However, the way it does this doesn't initialize the reader with the current path. Instead, the reader starts out with an empty path. See this method in the source code.

I used my debugger to force the reader to instead be created using new JTokenReader(jToken, reader.Path), but this led to a new issue. It seems that to avoid getting stuck in an infinite recursion loop, JsonSubtypes relies on the reader path being empty as a means of indicating that it has already begun reading a given object. See this line of source code.

Using my debugger to forcefully break out of that loop (even when the path isn't empty), I was able to get error paths to appear as they should. The error still appears to occur twice (i.e., there are two events for which args.CurrentObject == args.ErrorContext.OriginalObject), but the path the error holds is at least correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant