Merge pull request #8214 from zmstone/0613-best-effort-json-for-hocon-type-check-errors

refactor: best-effort json for hocon type check errors

apps/emqx/rebar.config

@@ -29,7 +29,7 @@
     {esockd, {git, "https://github.com/emqx/esockd", {tag, "5.9.3"}}},
     {ekka, {git, "https://github.com/emqx/ekka", {tag, "0.12.9"}}},
     {gen_rpc, {git, "https://github.com/emqx/gen_rpc", {tag, "2.8.1"}}},
-    {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.28.1"}}},
+    {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.28.2"}}},
     {pbkdf2, {git, "https://github.com/emqx/erlang-pbkdf2.git", {tag, "2.0.4"}}},
     {recon, {git, "https://github.com/ferd/recon", {tag, "2.5.1"}}},
     {snabbkaffe, {git, "https://github.com/kafka4beam/snabbkaffe.git", {tag, "1.0.0"}}}

apps/emqx/src/emqx_hocon.erl

@@ -19,7 +19,9 @@
 
 -export([
     format_path/1,
-    check/2
+    check/2,
+    format_error/1,
+    format_error/2
 ]).
 
 %% @doc Format hocon config field path to dot-separated string in iolist format.
@@ -51,7 +53,36 @@ check(SchemaModule, HoconText) ->
             {error, Reason}
     end.
 
+%% @doc Check if the error error term is a hocon check error.
+%% Return {true, FirstError}, otherwise false.
+%% NOTE: Hocon tries to be comprehensive, so it returns all found errors
+-spec format_error(term()) -> {ok, binary()} | false.
+format_error(X) ->
+    format_error(X, #{}).
+
+format_error({_Schema, [#{kind := K} = First | Rest] = All}, Opts) when
+    K =:= validation_erorr orelse K =:= translation_error
+->
+    Update =
+        case maps:get(no_stracktrace, Opts) of
+            true ->
+                fun no_stracktrace/1;
+            false ->
+                fun(X) -> X end
+        end,
+    case Rest of
+        [] ->
+            {ok, emqx_logger_jsonfmt:best_effort_json(Update(First), [])};
+        _ ->
+            {ok, emqx_logger_jsonfmt:best_effort_json(lists:map(Update, All), [])}
+    end;
+format_error(_Other, _) ->
+    false.
+
 %% Ensure iolist()
 iol(B) when is_binary(B) -> B;
 iol(A) when is_atom(A) -> atom_to_binary(A, utf8);
 iol(L) when is_list(L) -> L.
+
+no_stracktrace(Map) ->
+    maps:without([stacktrace], Map).

apps/emqx/src/emqx_misc.erl

@@ -51,7 +51,8 @@
     gen_id/1,
     explain_posix/1,
     pmap/2,
-    pmap/3
+    pmap/3,
+    readable_error_msg/1
 ]).
 
 -export([
@@ -509,6 +510,20 @@ pad(L, 0) ->
 pad(L, Count) ->
     pad([$0 | L], Count - 1).
 
+readable_error_msg(Msg) when is_binary(Msg) -> Msg;
+readable_error_msg(Error) ->
+    case io_lib:printable_unicode_list(Error) of
+        true ->
+            unicode:characters_to_binary(Error, utf8);
+        false ->
+            case emqx_hocon:format_error(Error, #{no_stacktrace => true}) of
+                {ok, Msg} ->
+                    Msg;
+                false ->
+                    iolist_to_binary(io_lib:format("~0p", [Error]))
+            end
+    end.
+
 -ifdef(TEST).
 -include_lib("eunit/include/eunit.hrl").
 

apps/emqx/test/emqx_schema_tests.erl

@@ -99,7 +99,7 @@ bad_cipher_test() ->
     Sc = emqx_schema:server_ssl_opts_schema(#{}, false),
     Reason = {bad_ciphers, ["foo"]},
     ?assertThrow(
-        {_Sc, [{validation_error, #{reason := Reason}}]},
+        {_Sc, [#{kind := validation_error, reason := Reason}]},
         validate(Sc, #{
             <<"versions">> => [<<"tlsv1.2">>],
             <<"ciphers">> => [<<"foo">>]
@@ -129,7 +129,7 @@ ciperhs_schema_test() ->
     Sc = emqx_schema:ciphers_schema(undefined),
     WSc = #{roots => [{ciphers, Sc}]},
     ?assertThrow(
-        {_, [{validation_error, _}]},
+        {_, [#{kind := validation_error}]},
         hocon_tconf:check_plain(WSc, #{<<"ciphers">> => <<"foo,bar">>})
     ).
 
@@ -137,7 +137,7 @@ bad_tls_version_test() ->
     Sc = emqx_schema:server_ssl_opts_schema(#{}, false),
     Reason = {unsupported_ssl_versions, [foo]},
     ?assertThrow(
-        {_Sc, [{validation_error, #{reason := Reason}}]},
+        {_Sc, [#{kind := validation_error, reason := Reason}]},
         validate(Sc, #{<<"versions">> => [<<"foo">>]})
     ),
     ok.

apps/emqx_bridge/src/emqx_bridge_api.erl

@@ -699,21 +699,8 @@ filter_out_request_body(Conf) ->
     ],
     maps:without(ExtraConfs, Conf).
 
-error_msg(Code, Msg) when is_binary(Msg) ->
-    #{code => Code, message => Msg};
-error_msg(Code, {_, HoconErrors = [{Type, _} | _]}) when
-    Type == translation_error orelse Type == validation_error
-->
-    MessageFormat = [hocon_error(HoconError) || HoconError <- HoconErrors],
-    #{code => Code, message => bin(MessageFormat)};
 error_msg(Code, Msg) ->
-    #{code => Code, message => bin(io_lib:format("~p", [Msg]))}.
-
-hocon_error({Type, Message0}) when
-    Type == translation_error orelse Type == validation_error
-->
-    Message = maps:without([stacktrace], Message0),
-    emqx_logger_jsonfmt:best_effort_json(Message#{<<"type">> => Type}, []).
+    #{code => Code, message => emqx_misc:readable_error_msg(Msg)}.
 
 bin(S) when is_list(S) ->
     list_to_binary(S);

apps/emqx_connector/src/emqx_connector_api.erl

@@ -308,21 +308,8 @@ schema("/connectors/:id") ->
         end
     ).
 
-error_msg(Code, Msg) when is_binary(Msg) ->
-    #{code => Code, message => Msg};
-error_msg(Code, {_, HoconErrors = [{Type, _} | _]}) when
-    Type == translation_error orelse Type == validation_error
-->
-    MessageFormat = [hocon_error(HoconError) || HoconError <- HoconErrors],
-    #{code => Code, message => bin(MessageFormat)};
 error_msg(Code, Msg) ->
-    #{code => Code, message => bin(io_lib:format("~p", [Msg]))}.
-
-hocon_error({Type, Message0}) when
-    Type == translation_error orelse Type == validation_error
-->
-    Message = maps:without([stacktrace], Message0),
-    emqx_logger_jsonfmt:best_effort_json(Message#{<<"type">> => Type}, []).
+    #{code => Code, message => emqx_misc:readable_error_msg(Msg)}.
 
 format_resp(#{<<"type">> := ConnType, <<"name">> := ConnName} = RawConf) ->
     NumOfBridges = length(

apps/emqx_dashboard/src/emqx_dashboard_swagger.erl

@@ -185,12 +185,7 @@ translate_req(Request, #{module := Module, path := Path, method := Method}, Chec
         {ok, Request#{bindings => Bindings, query_string => QueryStr, body => NewBody}}
     catch
         throw:HoconError ->
-            Msg = serialize_hocon_error_msg(HoconError),
-            %Msg = [
-            %    io_lib:format("~ts : ~p", [Key -- "root.", Reason])
-            %    || {validation_error, #{path := Key, reason := Reason}} <- ValidErrors
-            % ],
-            % iolist_to_binary(string:join(Msg, ",")
+            Msg = hocon_error_msg(HoconError),
             {400, 'BAD_REQUEST', Msg}
     end.
 
@@ -826,25 +821,5 @@ to_ref(Mod, StructName, Acc, RefsAcc) ->
 schema_converter(Options) ->
     maps:get(schema_converter, Options, fun hocon_schema_to_spec/2).
 
-serialize_hocon_error_msg({_Schema, Errors}) ->
-    Msg =
-        case lists:map(fun hocon_error/1, Errors) of
-            [Error0] -> Error0;
-            Errors -> Errors
-        end,
-    iolist_to_binary(io_lib:format("~0p", [Msg]));
-serialize_hocon_error_msg(Error) ->
-    iolist_to_binary(io_lib:format("~0p", [Error])).
-
-hocon_error({Type, #{path := Path} = Error}) ->
-    Error1 = maps:without([path, stacktrace], Error),
-    Error1#{
-        path => sub_path(Path),
-        type => Type,
-        reason => remove_useless_field(maps:get(reason, Error, #{}))
-    }.
-
-sub_path(Path) -> string:trim(Path, leading, "root.").
-
-remove_useless_field(#{} = Field) -> maps:without([stacktrace], Field);
-remove_useless_field(Field) -> Field.
+hocon_error_msg(Reason) ->
+    emqx_misc:readable_error_msg(Reason).

apps/emqx_exhook/src/emqx_exhook_api.erl

@@ -478,26 +478,7 @@ call_cluster(Fun) ->
 %%--------------------------------------------------------------------
 %% Internal Funcs
 %%--------------------------------------------------------------------
-err_msg({_, HoconErrors = [{Type, _} | _]}) when
-    Type == translation_error orelse Type == validation_error
-->
-    MessageFormat = [hocon_error(HoconError) || HoconError <- HoconErrors],
-    list_to_binary(MessageFormat);
-err_msg(Msg) ->
-    list_to_binary(io_lib:format("~0p", [Msg])).
-
-hocon_error({Type, Message0}) when
-    Type == translation_error orelse Type == validation_error
-->
-    case maps:get(reason, Message0, undefined) of
-        undefined ->
-            Message = maps:without([stacktrace], Message0),
-            emqx_logger_jsonfmt:best_effort_json(Message#{<<"type">> => Type}, []);
-        Reason when is_binary(Reason) ->
-            Reason;
-        Reason ->
-            list_to_binary(io_lib:format("~0p", [Reason]))
-    end.
+err_msg(Msg) -> emqx_misc:readable_error_msg(Msg).
 
 get_raw_config() ->
     RawConfig = emqx:get_raw_config([exhook, servers], []),

apps/emqx_gateway/src/emqx_gateway_http.erl

@@ -493,12 +493,10 @@ reason2msg(
 ) ->
     fmtstr("Bad TLS configuration for ~p, reason: ~s", [Options, Reason]);
 reason2msg(
-    {#{roots := [{gateway, _}]}, ErrReports}
+    {#{roots := [{gateway, _}]}, [_ | _]} = Error
 ) ->
-    fmtstr(
-        "Invalid configurations, reason: ~s",
-        [validation_error_stringfy(ErrReports, [])]
-    );
+    Bin = emqx_misc:readable_error_msg(Error),
+    <<"Invalid configurations: ", Bin/binary>>;
 reason2msg(_) ->
     error.
 
@@ -512,25 +510,6 @@ codestr(501) -> 'NOT_IMPLEMENTED'.
 fmtstr(Fmt, Args) ->
     lists:flatten(io_lib:format(Fmt, Args)).
 
-validation_error_stringfy([], Reasons) ->
-    lists:join(", ", lists:reverse(Reasons));
-validation_error_stringfy(
-    [
-        {validation_error, #{
-            path := Path,
-            reason := unknown_fields,
-            unknown_fields := Fields
-        }}
-        | More
-    ],
-    Reasons
-) ->
-    ReasonStr = fmtstr("unknown fields ~p for ~s", [Fields, Path]),
-    validation_error_stringfy(More, [ReasonStr | Reasons]);
-validation_error_stringfy([Other | More], Reasons) ->
-    ReasonStr = <<(emqx_gateway_utils:stringfy(Other))/binary>>,
-    validation_error_stringfy(More, [ReasonStr | Reasons]).
-
 -spec with_authn(binary(), function()) -> any().
 with_authn(GwName0, Fun) ->
     with_gateway(GwName0, fun(GwName, _GwConf) ->
@@ -579,8 +558,9 @@ with_gateway(GwName0, Fun) ->
             return_http_error(400, "Invalid bind address");
         Class:Reason:Stk ->
             ?SLOG(error, #{
-                msg => "uncatched_error",
-                reason => {Class, Reason},
+                msg => "uncaught_exception",
+                exception => Class,
+                reason => Reason,
                 stacktrace => Stk
             }),
             reason2resp(Reason)

apps/emqx_modules/test/emqx_rewrite_SUITE.erl

@@ -219,10 +219,11 @@ t_update_re_failed(_Config) ->
         {badmatch,
             {error,
                 {_, [
-                    {validation_error, #{
+                    #{
+                        kind := validation_error,
                         reason := {Re, {"nothing to repeat", 0}},
                         value := Re
-                    }}
+                    }
                 ]}}},
         emqx_rewrite:update(Rules)
     ),

apps/emqx_prometheus/rebar.config

@@ -4,7 +4,7 @@
     {emqx, {path, "../emqx"}},
     %% FIXME: tag this as v3.1.3
     {prometheus, {git, "https://github.com/deadtrickster/prometheus.erl", {tag, "v4.8.1"}}},
-    {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.28.1"}}}
+    {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.28.2"}}}
 ]}.
 
 {edoc_opts, [{preprocess, true}]}.

apps/emqx_rule_engine/src/emqx_rule_engine_api.erl

@@ -334,26 +334,7 @@ replace_sql_clrf(#{<<"sql">> := SQL} = Params) ->
 %% Internal functions
 %%------------------------------------------------------------------------------
 
-err_msg({_, HoconErrors = [{Type, _} | _]}) when
-    Type == translation_error orelse Type == validation_error
-->
-    MessageFormat = [hocon_error(HoconError) || HoconError <- HoconErrors],
-    list_to_binary(MessageFormat);
-err_msg(Msg) ->
-    list_to_binary(io_lib:format("~0p", [Msg])).
-
-hocon_error({Type, Message0}) when
-    Type == translation_error orelse Type == validation_error
-->
-    case maps:get(reason, Message0, undefined) of
-        undefined ->
-            Message = maps:without([stacktrace], Message0),
-            emqx_logger_jsonfmt:best_effort_json(Message#{<<"type">> => Type}, []);
-        Reason when is_binary(Reason) ->
-            Reason;
-        Reason ->
-            list_to_binary(io_lib:format("~0p", [Reason]))
-    end.
+err_msg(Msg) -> emqx_misc:readable_error_msg(Msg).
 
 format_rule_resp(Rules) when is_list(Rules) ->
     [format_rule_resp(R) || R <- Rules];

mix.exs

@@ -65,7 +65,7 @@ defmodule EMQXUmbrella.MixProject do
       # in conflict by emqtt and hocon
       {:getopt, "1.0.2", override: true},
       {:snabbkaffe, github: "kafka4beam/snabbkaffe", tag: "1.0.0", override: true},
-      {:hocon, github: "emqx/hocon", tag: "0.28.1", override: true},
+      {:hocon, github: "emqx/hocon", tag: "0.28.2", override: true},
       {:emqx_http_lib, github: "emqx/emqx_http_lib", tag: "0.5.1", override: true},
       {:esasl, github: "emqx/esasl", tag: "0.2.0"},
       {:jose, github: "potatosalad/erlang-jose", tag: "1.11.2"},

rebar.config

@@ -66,7 +66,7 @@
     , {system_monitor, {git, "https://github.com/ieQu1/system_monitor", {tag, "3.0.3"}}}
     , {getopt, "1.0.2"}
     , {snabbkaffe, {git, "https://github.com/kafka4beam/snabbkaffe.git", {tag, "1.0.0"}}}
-    , {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.28.1"}}}
+    , {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.28.2"}}}
     , {emqx_http_lib, {git, "https://github.com/emqx/emqx_http_lib.git", {tag, "0.5.1"}}}
     , {esasl, {git, "https://github.com/emqx/esasl", {tag, "0.2.0"}}}
     , {jose, {git, "https://github.com/potatosalad/erlang-jose", {tag, "1.11.2"}}}