From a284ab9cf8f755b9d3acf30161aeff779bd46ddc Mon Sep 17 00:00:00 2001 From: DDDHuang <44492639+DDDHuang@users.noreply.github.com> Date: Thu, 12 May 2022 11:06:07 +0800 Subject: [PATCH 1/4] fix: better error message for rule engine --- apps/emqx_rule_engine/src/emqx_rule_engine_api.erl | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl index 7b843eb52..6f24ea50a 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl @@ -334,6 +334,11 @@ replace_sql_clrf(#{<<"sql">> := SQL} = Params) -> %% Internal functions %%------------------------------------------------------------------------------ +err_msg({_, [{validation_error, VMessage}]}) -> + Exp = maps:get(expected_data_type, VMessage), + Path = maps:get(path, VMessage), + ErrorArg = maps:get(got, VMessage), + list_to_binary(io_lib:format("Key ~p error, expect ~p , got ~p", [Path, Exp, ErrorArg])); err_msg(Msg) -> list_to_binary(io_lib:format("~0p", [Msg])). From 2de69c97ba341c074d713948aad7657ed849d299 Mon Sep 17 00:00:00 2001 From: DDDHuang <44492639+DDDHuang@users.noreply.github.com> Date: Thu, 12 May 2022 14:42:20 +0800 Subject: [PATCH 2/4] fix: ruleengine & connector & bridge api, better error message --- apps/emqx_bridge/src/emqx_bridge_api.erl | 11 +++++++++ .../emqx_connector/src/emqx_connector_api.erl | 11 +++++++++ .../src/emqx_rule_engine_api.erl | 23 +++++++++++++++---- .../src/emqx_rule_engine_schema.erl | 19 +++++++++++++-- 4 files changed, 57 insertions(+), 7 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index 76fae732a..33eda4f4d 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/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) -> diff --git a/apps/emqx_connector/src/emqx_connector_api.erl b/apps/emqx_connector/src/emqx_connector_api.erl index 8fb4f596d..5299257be 100644 --- a/apps/emqx_connector/src/emqx_connector_api.erl +++ b/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( diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl index 6f24ea50a..e4e83452c 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl +++ b/apps/emqx_rule_engine/src/emqx_rule_engine_api.erl @@ -334,14 +334,27 @@ replace_sql_clrf(#{<<"sql">> := SQL} = Params) -> %% Internal functions %%------------------------------------------------------------------------------ -err_msg({_, [{validation_error, VMessage}]}) -> - Exp = maps:get(expected_data_type, VMessage), - Path = maps:get(path, VMessage), - ErrorArg = maps:get(got, VMessage), - list_to_binary(io_lib:format("Key ~p error, expect ~p , got ~p", [Path, Exp, ErrorArg])); +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(#{ diff --git a/apps/emqx_rule_engine/src/emqx_rule_engine_schema.erl b/apps/emqx_rule_engine/src/emqx_rule_engine_schema.erl index fd4cb8e6e..eed257b27 100644 --- a/apps/emqx_rule_engine/src/emqx_rule_engine_schema.erl +++ b/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(), From 8516b457f3a85edbde32992ed8a458d538b5c167 Mon Sep 17 00:00:00 2001 From: DDDHuang <44492639+DDDHuang@users.noreply.github.com> Date: Thu, 12 May 2022 17:33:19 +0800 Subject: [PATCH 3/4] fix(SUTIE): connector api, bad rule name in SUITE --- apps/emqx_connector/test/emqx_connector_api_SUITE.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/emqx_connector/test/emqx_connector_api_SUITE.erl b/apps/emqx_connector/test/emqx_connector_api_SUITE.erl index 65a965d60..425266b70 100644 --- a/apps/emqx_connector/test/emqx_connector_api_SUITE.erl +++ b/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\"">> From 5210cd6e8d15867c1d5b174802b82029b2dfd8b3 Mon Sep 17 00:00:00 2001 From: DDDHuang <44492639+DDDHuang@users.noreply.github.com> Date: Thu, 12 May 2022 18:01:37 +0800 Subject: [PATCH 4/4] fix(test): rule engine api SUITE , bad rule name --- apps/emqx_rule_engine/test/emqx_rule_engine_api_SUITE.erl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/emqx_rule_engine/test/emqx_rule_engine_api_SUITE.erl b/apps/emqx_rule_engine/test/emqx_rule_engine_api_SUITE.erl index d562b0188..b0dbefd27 100644 --- a/apps/emqx_rule_engine/test/emqx_rule_engine_api_SUITE.erl +++ b/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"