jason
jason copied to clipboard
michalmuskala
Some map keys not encodable
We recently had a situation where our map keys were mixed tuples eg. {:hi, 1}. Jason encoding was failing because it expected our map's keys to be of a type that implements the String.Chars protocol.
What is the rational for not recursively calling encode on the map's keys?
I think it would require a small change in this part of the code: https://github.com/michalmuskala/jason/blob/52748238fcb9bfb293fec6240474be79fdcdc29b/lib/encode.ex#L248
I'm happy to open a pr if you think this is a good idea!
What is the rational for not recursively calling encode on the map's keys?
How would that work? JSON keys can only be strings, you can't encode another structure there.
On a similar note. With Jason 1.1.2 When I do the following:
Jason.encode(%{{"some", "tuple"} => "val"})
I'd expect to receive {:error, Exception.t()} with Exception informing about {"some", "tuple"} not implementing the String.Chars protocol. Especially since the specification says {:error, Jason.EncodeError.t() | Exception.t()}. However, what I get is a raised exception. Is that a bug or is it intentional?
A thrown exception is pretty common in both erlang and elixir for invalid inputs. For valid inputs that just aren't able to be encoded I'd expect an EncodeError or so.
Right. You could argue though that trying to pass a key that does not implement String.Chars protocol is just as much an invalid input as trying to pass a value that does not implement Jason.Encoder protocol. The latter case results in
{:error, %Protocol.UndefinedError{}}
while the former throws an exception.
You could argue though that trying to pass a key that does not implement String.Chars protocol is just as much an invalid input as trying to pass a value that does not implement Jason.Encoder protocol.
Eh, I'm not sure I would. Something that does not implement the Jason.Encoder protocol is a potential valid input, it just needs a slight change (an implementation) to fix it. Passing something that can't be a string as a key is just outright invalid in all cases and cannot be as fixed as it shows an inherent issue in the structure of the passed in data to begin with. JSON is exceptionally strict in that keys must be strings. Where you can encode something in a value position in a variety of ways.
Something that does not implement the Jason.Encoder protocol is a potential valid input, it just needs a slight change (an implementation) to fix it.
In that case, something that does not implement String.Chars is a potential valid input too, it just need a slight change to fix it:
defimpl String.Chars, for: Tuple do
def to_string(tuple), do: tuple |> Tuple.to_list |> Enum.join(":")
end
So the main issue here is: whether Jason should raise error, or return error tuple, for such invalid value.
Jason.encode(%{{:hello, 1} => 1})
** (Protocol.UndefinedError) protocol String.Chars not implemented for {:hello, 1} of type Tuple...
Is there performance hit if check the protocol is implemented for value, or catch the protocol error, for valid input? If there is a way to not raise exception with minimum performance impact.. I think that could be reasonable "nice interface" of Jason.
For example, if exception is raised, and not handled properly, it may lead to nasty issues, such as exposing secret information to logging. Of course, that should be different topic, but as a user, when there are encode/1 andencode!, I expect encode/1 not to raise exception for known cases.
If there is an issue with performance, safe_decode or unsafe_decode could be introduced so there is always an escape hatch to the more performant decode function. Do you think that would work? The only problem is that this would cause breaking changes in the case of adding this functionality to the regular decode call.
The behavior of not encoding certain values I consider correct - JSON spec defines keys as strings and in Elixir the best concept of what a "string" is (with coercions), is the to_string function and the String.Chars protocol.
The main issue, as I see it, is backwards compatibility. Arguably Jason should be returning the error in the tuple here, rather than raising - after all it does the exact same thing, if the value is not encodable and key shouldn't be any different.
This could change potentially in the 2.0 release, though I currently have no plans for bigger changes like that. If you need to handle in now, I'd recommend a simple wrapper function intercepting the exception.
