jet / FsCodec

F# Event-Union Contract Encoding with versioning tolerant converters supporting System.Text.Json and Newtonsoft.Json
https://github.com/jet/dotnet-templates
Apache License 2.0
83 stars 19 forks source link

RejectNullConverter PoC attempt #112

Open bartelink opened 7 months ago

bartelink commented 7 months ago

In the spirit of https://github.com/jet/FsCodec/pull/87 this attempts to guard against the read path producing null instances of FSharpList<'T> and FSharpSet<'T>.

However, while the docs suggest that this general approach should work, I've run aground with a NullReferenceException that seems similar to https://github.com/dotnet/runtime/issues/86483 when inspected in the debugger (elementTypeInfo is null)

System.NullReferenceException
Object reference not set to an instance of an object.
   at System.Text.Json.Serialization.JsonCollectionConverter`2.OnTryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, TCollection& value)
   at System.Text.Json.Serialization.JsonConverter`1.TryRead(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options, ReadStack& state, T& value, Boolean& isPopulatedValue)
   at System.Text.Json.Serialization.JsonResumableConverter`1.Read(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options)
   at FsCodec.SystemTextJson.RejectNullConverter`1.Read(Utf8JsonReader& reader, Type typeToConvert, JsonSerializerOptions options)
bartelink commented 7 months ago

@eiriktsarpalis sorrynotsorry for the atting but I'm out of ideas on how to achieve this via other routes

edited: While the NRE will be a blocker for the overall thing to work (and I'd be interested to see if it is quick fix for someone that knows the code), further research shows me that the converter is not called for missing fields so this general approach is probably a dead end for now)

NOTE It builds fine with the V8 SDK - the tools in this repo are still on V6 as this approach is the first thing that triggers in a need for V7+ features (JsonSerializerOptions.Default) are relevant for

eiriktsarpalis commented 7 months ago

Seems related to https://github.com/dotnet/runtime/issues/50205. It's a quirk in some of the built-in converters that cannot be composed outside of their created context.