rustler icon indicating copy to clipboard operation
rustler copied to clipboard

Serde Behavior

Open benhaney opened this issue 1 year ago • 3 comments

This is a summary and continuation of https://github.com/benhaney/Jsonrs/pull/26, by @filmor's request.

It was recently suggested to me that Jsonrs should switch to using Rustler's new Serde integration as its backend, which I'm very excited to do. But before I can, I need to make sure that Rustler's use of Serde can match what Jsonrs expects, so I don't break backwards compatibility. This issue represents the problems I've seen so far.

In approximate order of importance:

  1. Rustler explicitly serializes the atoms :ok and :error into the strings "Ok" and "Err", inconsistent with the serialization of all other atoms. This is by far the biggest issue and should also be the easiest to fix.
  2. I handle encoding Elixir's Decimal struct at the Rust layer, so I will either need that same support in Rustler (sorry!) or a way to pass some custom struct encoding functions to the deserializer. I think the former would be a lot easier if it isn't too objectionable (and I'm happy to contribute the implementation I already have), but I recognize that it would be a bit weird for a general purpose NIF library like Rustler to specially handle struct encoding of specific Elixir packages, even if they're very popular.
  3. I noticed some tests related to error tuples failed when testing Jsonrs using Rustler's Serde integration. An example:
     code:  Jsonrs.decode("invalid") === {:error, "expected value at line 1 column 1"}
     left:  {:error, "SerializationError(\"expected value at line 1 column 1\")"}
     right: {:error, "expected value at line 1 column 1"}
    
    This isn't necessarily something that Rustler should deal with, it just wasn't immediately obvious to me why some of these changed and I need to make a note to look into why it happened at all and what I might need to do in Jsonrs to hammer the errors back into the format I was seeing before. I'm not terribly against changing error strings, but it would technically be a breaking change for Jsonrs so I'd like to avoid it where reasonable. If anyone's immediately aware of what changed here, it would save me the investigation.

I think that's all I've found so far. Let me know your thoughts and what I can do to help with any of this!

benhaney avatar Jul 07 '24 10:07 benhaney

On 1.: What this tries to achieve is to make {ok, Bla} and {error, Blubb} map to and from Rust Result types. I'll see whether there is a better way.

On 2.: I'd like to solve this via explicit registration of struct conversions.

filmor avatar Jul 07 '24 14:07 filmor

@filmor When you have a chance, could you take a look at #639 and let me know if it seems like a good approach to fixing the first issue? It's mostly a cleanup of a change I made to my own serde_rustler fork, and I was originally just focusing on correcting the transcoding behavior, but I think this version should preserve the ok/error<->Result conversion for explicit encode/decode just fine.

Once we have that sorted out, I'd love to hear if you have any thoughts on how the interface for the second issue (registering struct conversions) should look

benhaney avatar Jul 19 '24 18:07 benhaney

On the struct serde: I think registering using inventory, like what we do for resources and nifs already will do for the global "instance", but I'll need to give this a bit more thought.

As a first step, Deserializer and Serializer need to be extended to keep and use a map struct name -> dyn StructDe/encoder. This can be a simple dispatch mechanism specific to structs, I think.

filmor avatar Jul 20 '24 13:07 filmor