Conversation
lib/elixir/test/elixir/json_test.exs
Outdated
| assert ["{\"a\":", _, ",\"b\":", _, ",\"d\":", _, 125] = | ||
| json = JSON.encode_to_iodata(%WithOnly{a: :a, b: "b", c: make_ref(), d: [?d]}) |
There was a problem hiding this comment.
@michalmuskala you can see I chose to encode the longest possible part of the field as a binary, which includes the preceding field (either { or ,), the quotes, and the trailing :.
Co-authored-by: Eksperimental <eksperimental@autistici.org>
Co-authored-by: Eksperimental <eksperimental@autistici.org>
Co-authored-by: Wojtek Mach <wojtekmach@users.noreply.github.com>
| :elixir_json.decode(binary, acc, Map.new(decoders)) | ||
| catch | ||
| :error, :unexpected_end -> | ||
| {:error, {:unexpected_end, byte_size(binary)}} |
There was a problem hiding this comment.
Have you considered a JSON.Error or JSON.DecodeError exception with these, matching Jason.DecodeError? In Req we gracefully handle json errors like this:
iex> Req.get(plug: & &1 |> Plug.Conn.put_resp_content_type("application/json") |> Plug.Conn.send_resp(200, "{"))
{:error, %Jason.DecodeError{position: 1, token: nil, data: "{"}}(which I'm still somewhat skeptical of cause it's not like users can do much with these errors anyway but this was a somewhat common request to have this graceful handling)
so it'd be nice to have an exception struct I could return instead. Not a big deal though, I can invent one for Req.
There was a problem hiding this comment.
We can raise a specific error on JSON.decode!, and I will make it so, but we don't use the {:error, Exception.t()} struct style anywhere in Elixir, so I am not sure we should do it here.
lib/elixir/lib/json.ex
Outdated
|
|
||
| @moduledoc since: "1.18.0" | ||
|
|
||
| @type decode_error :: |
There was a problem hiding this comment.
Should we call this decode_error_reason? We could also document the reasons here maybe.
lib/elixir/lib/json.ex
Outdated
| "[123,\"string\",{\"key\":\"value\"}]" | ||
|
|
||
| """ | ||
| def encode!(term, encoder \\ &encode_value/2) do |
Co-authored-by: Andrea Leopardi <an.leopardi@gmail.com> Co-authored-by: Adrian Salamon <adr.salamon@gmail.com>
lib/elixir/lib/json.ex
Outdated
| @doc """ | ||
| This is the default function used to recursively encode each value. | ||
| """ | ||
| def encode_value(value, encoder) when is_atom(value) do |
There was a problem hiding this comment.
It is the same as the protocol, inlined for performance. But I am not a fan of its name. Suggestions? What about encode_callback? or default_encode? or default_encode_callback?
There was a problem hiding this comment.
Given it isn't technically an @callback, even if it is a generic term _callback might be confusing since its specific meaning in OTP.
default_encode captures the intent well I think.
|
💚 💙 💜 💛 ❤️ |
| "[123,\"string\",{\"key\":\"value\"}]" | ||
|
|
||
| """ | ||
| @spec encode!(a, (a -> iodata())) :: binary() when a: var |
There was a problem hiding this comment.
This spec seems wrong, the callback takes two arguments, not one. Same on line 356
|
Thank you for implementing this! |
8 participants
Note to code reviewers: no need to review the
elixir_json.erlmodule, that was ported from Erlang for backwards compatibility reasons. It will be removed in future Elixir versions.encode_valuecc @michalmuskala