nats-io/nats.net.v2

Deserializer is skipped for 0 length payloads

TheFourtyTwo opened this issue ยท 11 comments

Observed behavior

Create a deserializer that returns a object on empty payloads.
Send a message with an empty payload.
The result is null instead of the expected object.

The code directly returns the default value which is null for reference types.

Expected behavior

The deserializer should be called.
for some reference types, especially in F#, null is not the default value (for instance Map<'a,'b>).
A deserializer that returns Map.empty is provided, but the client is skipping the deserializer and returns null instead, which is not a valid value for Map<'a,'b>

Server and client version

Nats.net 2.1.2

Host environment

not specific

Steps to reproduce

Create a deserializer that returns a object on empty payloads.
Send a message with an empty payload.
The result is null instead of the expected object.

mtmk commented

This is done intentionally to be able to transfer null values and might break existing applications, however I did not consider F#. Could you give an example so we can try to find a solution?

For instance, an F# Map<string,string>, will be serialized as an empty payload (a lengthdelimited message with no fields).

If I call RequestAsync with a IDeserialize<Map<string,string>>, on an empty payload, it will return null.

But null is not a valid value for Map<string,string>, it should return Map.empty ,which is what the actual deserializer returns when called with an empty payload.

This is the same for lists, sets, and most F# types (records, unions)

mtmk commented

Thanks @thinkbeforecoding. I've virtually no experience in F# ๐Ÿ˜ข is it possible for you to give me a minimal project so I can try to figure out a solution?

Here is a sample:

#r "nuget: Nats.net"
open System
open NATS.Client.Core
open System.Buffers

let deserializer =
    { new INatsDeserialize<int list> with
        member _.Deserialize(buffer) =
            let array = Array.zeroCreate<byte>( int buffer.Length)
            buffer.CopyTo(array.AsSpan())
            let text = Text.Encoding.UTF8.GetString(array)
            text.Split(',',StringSplitOptions.RemoveEmptyEntries)
            |> Seq.map int
            |> Seq.toList
    }

// check that the deserializer successfully splits numbers into a list
let mutable msg1 = ReadOnlySequence("123,456,789"B)
deserializer.Deserialize(&msg1) = [ 123; 456; 789]
   
// check that the deserializer splits empty payloads into an empty list
let mutable msg2 = ReadOnlySequence<byte>() 
deserializer.Deserialize(&msg2) = [] 

(task {
    let client = NatsConnection()
    let sub = client.SubscribeAsync("test", serializer = deserializer, opts = NatsSubOpts(MaxMsgs = 2))
    let t = 
        task {
            let enumerator = sub.GetAsyncEnumerator()
            while! enumerator.MoveNextAsync() do
                if Object.ReferenceEquals(null, enumerator.Current.Data)  then
                    printfn "NULL"
                else
                    printfn "%A" enumerator.Current.Data
        }
    do! client.PublishAsync("test", "123,456,789")
    do! client.PublishAsync("test", "")

    do! t
}).Wait()

The deserializer converts the byte sequence to string, and split it on ',' the returns numbers as a list.
The empty payload returns an empty list, in F#, null is never a valid value for a list, and it is not possible to check that a list is null without a workaround (In the enumeration of the subscription, I have to use Object.ReferenceEqual(null,...) to check for null, otherwhise F# says The type 'int list' does not have 'null' as a proper value. This is really error prone since F# developers never expect a list to be null).

The two tests show that the serializer is behaving correctly, both with data, and empty data, with is a 0 length payload, return an empty list in this case.

But when going through the nats client, the empty list is published, and received as a null value. The deserializer is bypassed for empty payload.

The script can be put in a test.fsx file, and run using dotnet fsi test.fsx

I would expect the enumerator of the subscription to return an empty list (as does the deserializer), not a null value.

I don't think it would be costly to call the serializer for empty payloads. The only thing is if some existing deserializer are raising an exception because they don't expect this to happen. Maybe have a optional parameter in subscription opts, or have an extra interface that indicates that the deserializer need to handle empty payloads.

mtmk commented

I don't think it would be costly to call the serializer for empty payloads. The only thing is if some existing deserializer are raising an exception because they don't expect this to happen. Maybe have a optional parameter in subscription opts, or have an extra interface that indicates that the deserializer need to handle empty payloads.

potentially breaking an application is the main concern especially as we are starting to see more production deployments. Whatever we have now, we have to live with from now on. A new option seems to be the only solution to me.

mtmk commented

@TheFourtyTwo thanks for the example. I can reproduce it ๐Ÿ‘

It could even be a good idea to put the option at the connection level to avoid forgetting it in some consume options.

mtmk commented

It could even be a good idea to put the option at the connection level to avoid forgetting it in some consume options.

we can do. did you see the PR? #455