Merge pull request #7926 from DDDHuang/rule_api_errormsg

fix: better error message for rule engine

apps/emqx_bridge/src/emqx_bridge_api.erl

@@ -696,9 +696,20 @@ filter_out_request_body(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}, []).
+
 bin(S) when is_list(S) ->
     list_to_binary(S);
 bin(S) when is_atom(S) ->

apps/emqx_connector/src/emqx_connector_api.erl

@@ -310,9 +310,20 @@ schema("/connectors/:id") ->
 
 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}, []).
+
 format_resp(#{<<"type">> := ConnType, <<"name">> := ConnName} = RawConf) ->
     NumOfBridges = length(
         emqx_bridge:list_bridges_by_connector(

apps/emqx_connector/test/emqx_connector_api_SUITE.erl

@@ -608,7 +608,7 @@ t_ingress_mqtt_bridge_with_rules(_) ->
         post,
         uri(["rules"]),
         #{
-            <<"name">> => <<"A rule get messages from a source mqtt bridge">>,
+            <<"name">> => <<"A_rule_get_messages_from_a_source_mqtt_bridge">>,
             <<"enable">> => true,
             <<"outputs">> => [#{<<"function">> => "emqx_connector_api_SUITE:inspect"}],
             <<"sql">> => <<"SELECT * from \"$bridges/", BridgeIDIngress/binary, "\"">>
@@ -707,7 +707,7 @@ t_egress_mqtt_bridge_with_rules(_) ->
         post,
         uri(["rules"]),
         #{
-            <<"name">> => <<"A rule send messages to a sink mqtt bridge">>,
+            <<"name">> => <<"A_rule_send_messages_to_a_sink_mqtt_bridge">>,
             <<"enable">> => true,
             <<"outputs">> => [BridgeIDEgress],
             <<"sql">> => <<"SELECT * from \"t/1\"">>

apps/emqx_rule_engine/src/emqx_rule_engine_api.erl

@@ -334,9 +334,27 @@ 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.
+
 format_rule_resp(Rules) when is_list(Rules) ->
     [format_rule_resp(R) || R <- Rules];
 format_rule_resp(#{

apps/emqx_rule_engine/src/emqx_rule_engine_schema.erl

@@ -28,7 +28,7 @@
     desc/1
 ]).
 
--export([validate_sql/1]).
+-export([validate_sql/1, validate_rule_name/1]).
 
 namespace() -> rule_engine.
 
@@ -180,10 +180,25 @@ rule_name() ->
                 desc => ?DESC("rules_name"),
                 default => "",
                 required => true,
-                example => "foo"
+                example => "foo",
+                validator => fun ?MODULE:validate_rule_name/1
             }
         )}.
 
+validate_rule_name(Name) ->
+    RE = "^[A-Za-z0-9]+[A-Za-z0-9-_]*$",
+    try re:run(Name, RE) of
+        {match, _} ->
+            ok;
+        _Nomatch ->
+            Reason = list_to_binary(io_lib:format("Bad rule name ~p, expect ~p", [Name, RE])),
+            {error, Reason}
+    catch
+        _:_ ->
+            Reason = list_to_binary(io_lib:format("Bad rule name ~p, expect ~p", [Name, RE])),
+            {error, Reason}
+    end.
+
 outputs() ->
     [
         binary(),

apps/emqx_rule_engine/test/emqx_rule_engine_api_SUITE.erl

@@ -34,7 +34,8 @@ t_crud_rule_api(_Config) ->
         <<"enable">> => true,
         <<"id">> => RuleID,
         <<"outputs">> => [#{<<"function">> => <<"console">>}],
-        <<"sql">> => <<"SELECT * from \"t/1\"">>
+        <<"sql">> => <<"SELECT * from \"t/1\"">>,
+        <<"name">> => <<"test_rule">>
     },
     {201, Rule} = emqx_rule_engine_api:'/rules'(post, #{body => Params0}),
     %% if we post again with the same params, it return with 400 "rule id already exists"