Cleanup JsonRPC codec (#51)

Cleanup JSON-RPC codec * Move the error formatting out. * pre-process and post-process the message to handle null/undefined conversion. * Return decoding errors as proper JSON-RPC errors that could be sent right away to the peer. * Fix doc generation.
grisp · Nov 14, 2024 · f012db6 · f012db6
1 parent 0cf0109
commit f012db6
Show file tree

Hide file tree

Showing 4 changed files with 202 additions and 95 deletions.
diff --git a/rebar.config b/rebar.config
@@ -22,6 +22,8 @@
 
 {hex, [{doc, #{provider => ex_doc}}]}.
 
+{edoc_opts, [{preprocess, true}]}.
+
 {ex_doc, [
     {extras, [
           {"CHANGELOG.md", #{title => "Changelog"}},

diff --git a/src/grisp_connect_api.erl b/src/grisp_connect_api.erl
@@ -46,10 +46,24 @@ handle_msg(JSON) ->
 
 %--- Internal Funcitons --------------------------------------------------------
 
-handle_jsonrpc({batch, Batch}) ->
-    handle_rpc_messages(Batch, []);
-handle_jsonrpc({single, Rpc}) ->
-    handle_rpc_messages([Rpc], []).
+format_error({internal_error, parse_error, ID}) ->
+    {error, -32700, <<"Parse error">>, undefined, ID};
+format_error({internal_error, invalid_request, ID}) ->
+    {error, -32600, <<"Invalid request">>, undefined, ID};
+format_error({internal_error, method_not_found, ID}) ->
+    {error, -32601, <<"Method not found">>, undefined, ID};
+format_error({internal_error, invalid_params, ID}) ->
+    {error, -32602, <<"Invalid params">>, undefined, ID};
+format_error({internal_error, Reason, ID}) ->
+    {error, -32603, <<"Internal error">>, Reason, ID}.
+
+%FIXME: Batch are not supported yet. When receiving a batch of messages, as per
+%       the JSON-RPC standard, all the responses should goes in a single batch
+%       of responses.
+handle_jsonrpc(Messages) when is_list(Messages) ->
+    handle_rpc_messages(Messages, []);
+handle_jsonrpc(Message) ->
+    handle_rpc_messages([Message], []).
 
 handle_rpc_messages([], Replies) -> lists:reverse(Replies);
 handle_rpc_messages([{request, M, Params, ID} | Batch], Replies)
@@ -61,8 +75,8 @@ handle_rpc_messages([{result, _, _} = Res| Batch], Replies) ->
 handle_rpc_messages([{error, _Code, _Msg, _Data, _ID} = E | Batch], Replies) ->
     ?LOG_INFO("Received JsonRPC error: ~p",[E]),
     handle_rpc_messages(Batch, [handle_response(E)| Replies]);
-handle_rpc_messages([{internal_error, _, _} = E | Batch], Replies) ->
-    ?LOG_ERROR("JsonRPC: ~p",[E]),
+handle_rpc_messages([{decoding_error, _, _, _, _} = E | Batch], Replies) ->
+    ?LOG_ERROR("JsonRPC decoding error: ~p",[E]),
     handle_rpc_messages(Batch, Replies).
 
 handle_request(?method_get, #{type := <<"system_info">>} = _Params, ID) ->
@@ -80,16 +94,15 @@ handle_request(?method_post, #{type := <<"start_update">>} = Params, ID) ->
                 {error, -12, boot_system_not_validated, undefined, ID};
             {error, Reason} ->
                 ReasonBinary = iolist_to_binary(io_lib:format("~p", [Reason])),
-                grisp_connect_jsonrpc:format_error({internal_error, ReasonBinary, ID});
+                format_error({internal_error, ReasonBinary, ID});
             ok ->
                 {result, ok, ID}
         end,
         {send_response, grisp_connect_jsonrpc:encode(Reply)}
     catch
         throw:bad_key ->
             {send_response,
-             grisp_connect_jsonrpc:format_error(
-                {internal_error, invalid_params, ID})}
+             format_error({internal_error, invalid_params, ID})}
         end;
 handle_request(?method_post, #{type := <<"validate">>}, ID) ->
     Reply = case grisp_connect_updater:validate() of
@@ -99,7 +112,7 @@ handle_request(?method_post, #{type := <<"validate">>}, ID) ->
             {error, -13, validate_from_unbooted, PartitionIndex, ID};
         {error, Reason} ->
             ReasonBinary = iolist_to_binary(io_lib:format("~p", [Reason])),
-            grisp_connect_jsonrpc:format_error({internal_error, ReasonBinary, ID});
+            format_error({internal_error, ReasonBinary, ID});
         ok ->
             {result, ok, ID}
     end,
@@ -117,7 +130,7 @@ handle_request(?method_post, #{type := <<"cancel">>}, ID) ->
     {send_response,  grisp_connect_jsonrpc:encode(Reply)};
 handle_request(_T, _P, ID) ->
     Error = {internal_error, method_not_found, ID},
-    FormattedError = grisp_connect_jsonrpc:format_error(Error),
+    FormattedError = format_error(Error),
     {send_response, grisp_connect_jsonrpc:encode(FormattedError)}.
 
 handle_response(Response) ->

diff --git a/src/grisp_connect_jsonrpc.erl b/src/grisp_connect_jsonrpc.erl
@@ -3,10 +3,24 @@
 % API
 -export([decode/1]).
 -export([encode/1]).
--export([format_error/1]).
+
 
 %--- Types ---------------------------------------------------------------------
 
+-type json_rpc_message() ::
+    {request, Method :: binary(), Params :: map() | list(),
+     ReqRef :: binary() | integer()}
+  | {result, Result :: term(), ReqRef :: binary()}
+  | {notification, Method :: binary(), Params :: map() | list()}
+  | {error, Code :: integer(), Message :: undefined | binary(),
+     Data :: undefined | term(), ReqRef :: undefined | binary() | integer()}
+  | {decoding_error, Code :: integer(), Message :: undefined | binary(),
+     Data :: undefined | term(), ReqRef :: undefined | binary() | integer()}.
+
+
+
+%--- Macros --------------------------------------------------------------------
+
 -define(V, jsonrpc => <<"2.0">>).
 -define(is_valid(Message),
     (map_get(jsonrpc, Message) == <<"2.0">>)
@@ -17,103 +31,160 @@
 -define(is_params(Params),
     (is_map(Params) orelse is_list(Params))
 ).
+-define(is_id(ID),
+    (is_binary(ID) orelse is_integer(ID))
+).
+
 
-%--- API ----------------------------------------------------------------------
+%--- API -----------------------------------------------------------------------
 
-decode(Term) ->
-    case json_to_term(Term) of
+%% @doc Decode a JSONRpc text packet and decoded message or a list of decoded
+%% messages in the case of a batch.
+%%
+%% If it returns a list, the all the responses are supposed to be sent back in
+%% a batch too, as per the JSONRpc 2.0 specifications.
+%%
+%% If some decoding errors occure, a special error message with the tag
+%% `decoding_error' will be returned, this message can be encoded and sent back
+%% directly to the JSON-RPC peer.
+%%
+%% During JSON decoding, the `null' values are changed to `undefined' and when
+%% encoding, `undefined' values are changed back to `null'.
+%%
+%% The `method' will <b>always</b> be a binary, and `id' will always be either
+%% a binary or an integer.
+%%
+%% <p>The possible decoded messages are:
+%% <ul>
+%%     <li><b>`{request, Method :: binary(), Params :: map() | list(), ReqRef :: binary() | integer()}'</b></li>
+%%     <li><b>`{result, Result :: term(), ReqRef :: binary()}'</b></li>
+%%     <li><b>`{notification, Method :: binary(), Params :: map() | list()}'</b></li>
+%%     <li><b>`{error, Code :: integer(), Message :: undefined | binary(), Data :: undefined | term(), ReqRef :: undefined | binary() | integer()}'</b></li>
+%%     <li><b>`{decoding_error, Code :: integer(), Message :: undefined | binary(), Data :: undefined | term(), ReqRef :: undefined | binary() | integer()}'</b></li>
+%% </ul></p>
+-spec decode(Data :: iodata()) -> json_rpc_message() | [json_rpc_message()].
+decode(Data) ->
+    case json_to_term(iolist_to_binary(Data)) of
         [] ->
-            {single, {internal_error, invalid_request, null}};
+            [{decoding_error, -32600, <<"Invalid Request">>, undefined, undefined}];
         Messages when is_list(Messages) ->
-            {batch, [unpack(M) || M <- Messages]};
+            [unpack(M) || M <- Messages];
         Message when is_map(Message) ->
-            {single, unpack(Message)};
-        {error, _E} ->
-            {single, {internal_error, parse_error, null}}
+            unpack(Message);
+        {error, _Reason} ->
+            % Even though the data could have been a batch, we return a single
+            % error, as per JSON-RPC specifications
+            {decoding_error, -32700, <<"Parse error">>, undefined, undefined}
     end.
 
-encode([Message]) ->
-    encode(Message);
+%% @doc Encode a JSONRpc message or a list of JSONRpc messages to JSON text.
+%% For backward compatibility, the `method' can be an atom.
+-spec encode(Messages :: json_rpc_message() | [json_rpc_message()]) -> iodata().
 encode(Messages) when is_list(Messages) ->
     term_to_json([pack(M) || M <- Messages]);
 encode(Message) ->
     term_to_json(pack(Message)).
 
-format_error({internal_error, parse_error, ID}) ->
-    {error, -32700, <<"Parse error">>, undefined, ID};
-format_error({internal_error, invalid_request, ID}) ->
-    {error, -32600, <<"Invalid request">>, undefined, ID};
-format_error({internal_error, method_not_found, ID}) ->
-    {error, -32601, <<"Method not found">>, undefined, ID};
-format_error({internal_error, invalid_params, ID}) ->
-    {error, -32602, <<"Invalid params">>, undefined, ID};
-format_error({internal_error, Reason, ID}) ->
-    {error, -32603, <<"Internal error">>, Reason, ID}.
 
-%--- Internal -----------------------------------------------------------------
+%--- Internal ------------------------------------------------------------------
+
+as_bin(undefined) -> undefined;
+as_bin(Binary) when is_binary(Binary) -> Binary;
+as_bin(List) when is_list(List) -> list_to_binary(List).
+
+as_id(undefined) -> undefined;
+as_id(Integer) when is_integer(Integer) -> Integer;
+as_id(Binary) when is_binary(Binary) -> Binary;
+as_id(List) when is_list(List) -> list_to_binary(List).
 
 unpack(#{method := Method, params := Params, id := ID} = M)
-        when ?is_valid(M), ?is_method(Method), ?is_params(Params) ->
-    {request, Method, Params, ID};
+  when ?is_valid(M), ?is_method(Method), ?is_params(Params), ID =/= undefined ->
+    {request, as_bin(Method), Params, as_id(ID)};
 unpack(#{method := Method, id := ID} = M)
-        when ?is_valid(M), ?is_method(Method) ->
-    {request, Method, undefined, ID};
+  when ?is_valid(M), ?is_method(Method), ID =/= undefined ->
+    {request, as_bin(Method), undefined, as_id(ID)};
 unpack(#{method := Method, params := Params} = M)
-        when ?is_valid(M), ?is_method(Method), ?is_params(Params) ->
-    {notification, Method, Params};
+  when ?is_valid(M), ?is_method(Method), ?is_params(Params) ->
+    {notification, as_bin(Method), Params};
 unpack(#{method := Method} = M)
-        when ?is_valid(M), ?is_method(Method) ->
-    {notification, Method, undefined};
-unpack(#{method := Method, params := _Params, id := ID} = M)
-        when ?is_valid(M), ?is_method(Method) ->
-    {internal_error, invalid_params, ID};
+  when ?is_valid(M), ?is_method(Method) ->
+    {notification, as_bin(Method), undefined};
 unpack(#{result := Result, id := ID} = M)
-        when ?is_valid(M) ->
-    {result, Result, ID};
-unpack(#{error := #{code := Code,
-                    message := Message,
-                    data := Data},
-                    id := ID} = M)
-        when ?is_valid(M) ->
-    {error, Code, Message, Data, ID};
-unpack(#{error := #{code := Code,
-                    message := Message},
-                    id := ID} = M)
-        when ?is_valid(M) ->
-    {error, Code, Message, undefined, ID};
-unpack(M) ->
-    {internal_error, invalid_request, id(M)}.
-
-pack({request, Method, undefined, ID}) ->
+  when ?is_valid(M) ->
+    {result, Result, as_id(ID)};
+unpack(#{error := #{code := Code, message := Message, data := Data},
+         id := ID} = M)
+  when ?is_valid(M), is_integer(Code) ->
+    {error, Code, as_bin(Message), Data, as_id(ID)};
+unpack(#{error := #{code := Code, message := Message}, id := ID} = M)
+  when ?is_valid(M), is_integer(Code) ->
+    {error, Code, as_bin(Message), undefined, as_id(ID)};
+unpack(#{id := ID}) ->
+    {decoding_error, -32600, <<"Invalid request">>, undefined, as_id(ID)};
+unpack(_M) ->
+    {decoding_error, -32600, <<"Invalid request">>, undefined, undefined}.
+
+pack({request, Method, undefined, ID})
+  when is_binary(Method) orelse is_atom(Method), ?is_id(ID) ->
     #{?V, method => Method, id => ID};
-pack({request, Method, Params, ID}) ->
+pack({request, Method, Params, ID})
+  when is_binary(Method) orelse is_atom(Method),
+       Params =:= undefined orelse ?is_params(Params),
+       ?is_id(ID) ->
     #{?V, method => Method, params => Params, id => ID};
-pack({notification, Method, undefined}) ->
+pack({notification, Method, undefined})
+  when is_binary(Method) orelse is_atom(Method) ->
     #{?V, method => Method};
-pack({notification, Method, Params}) ->
+pack({notification, Method, Params})
+  when is_binary(Method), Params =:= undefined orelse ?is_params(Params) ->
     #{?V, method => Method, params => Params};
-pack({result, Result, ID}) ->
+pack({result, Result, ID})
+  when ?is_id(ID) ->
     #{?V, result => Result, id => ID};
-pack({error, Type, ID}) ->
-    pack(format_error({internal_error, Type, ID}));
-pack({error, Code, Message, undefined, undefined}) ->
+pack({ErrorTag, Code, Message, undefined, undefined})
+  when ErrorTag =:= error orelse ErrorTag =:= decoding_error, is_integer(Code),
+       Message =:= undefined orelse is_binary(Message) ->
     #{?V, error => #{code => Code, message => Message}, id => null};
-pack({error, Code, Message, undefined, ID}) ->
+pack({ErrorTag, Code, Message, undefined, ID})
+  when ErrorTag =:= error orelse ErrorTag =:= decoding_error, is_integer(Code),
+       Message =:= undefined orelse is_binary(Message), ?is_id(ID) ->
     #{?V, error => #{code => Code, message => Message}, id => ID};
-pack({error, Code, Message, Data, undefined}) ->
+pack({ErrorTag, Code, Message, Data, undefined})
+  when ErrorTag =:= error orelse ErrorTag =:= decoding_error, is_integer(Code),
+       Message =:= undefined orelse is_binary(Message) ->
     #{?V, error => #{code => Code, message => Message, data => Data, id => null}};
-pack({error, Code, Message, Data, ID}) ->
-    #{?V, error => #{code => Code, message => Message, data => Data}, id => ID}.
-
-id(Object) when is_map(Object) -> maps:get(id, Object, null);
-id(_Object) -> null.
-
+pack({ErrorTag, Code, Message, Data, ID})
+  when ErrorTag =:= error orelse ErrorTag =:= decoding_error, is_integer(Code),
+       Message =:= undefined orelse is_binary(Message), ?is_id(ID) ->
+    #{?V, error => #{code => Code, message => Message, data => Data}, id => ID};
+pack(Message) ->
+    erlang:error({badarg, Message}).
 
 json_to_term(Bin) ->
-    try jsx:decode(Bin, [{labels, attempt_atom}, return_maps])
+    try jsx:decode(Bin, [{labels, attempt_atom}, return_maps]) of
+        Json -> postprocess(Json)
     catch
         error:E -> {error, E}
     end.
 
-term_to_json(Map) ->
-    jsx:encode(Map).
+term_to_json(Term) ->
+    jsx:encode(preprocess(Term)).
+
+postprocess(null) -> undefined;
+postprocess(Integer) when is_integer(Integer) -> Integer;
+postprocess(Float) when is_float(Float) -> Float;
+postprocess(Binary) when is_binary(Binary) -> Binary;
+postprocess(List) when is_list(List) ->
+    [postprocess(E) || E <- List];
+postprocess(Map) when is_map(Map) ->
+    maps:map(fun(_K, V) -> postprocess(V) end, Map).
+
+preprocess(undefined) -> null;
+preprocess(Atom) when is_atom(Atom) -> Atom;
+preprocess(Integer) when is_integer(Integer) -> Integer;
+preprocess(Float) when is_float(Float) -> Float;
+preprocess(Binary) when is_binary(Binary) -> Binary;
+preprocess(List) when is_list(List) ->
+    [preprocess(E) || E <- List];
+preprocess(Map) when is_map(Map) ->
+    maps:map(fun(_K, V) -> preprocess(V) end, Map).