From 90ee450a84ce4efd327464267384d8ebe5d05465 Mon Sep 17 00:00:00 2001 From: DDDHuang <44492639+DDDHuang@users.noreply.github.com> Date: Wed, 16 Feb 2022 16:08:45 +0800 Subject: [PATCH 1/6] feat: support http error code & error code api --- apps/emqx/include/http_api.hrl | 74 +++++++++++ .../src/emqx_dashboard_error_code.erl | 56 ++++++++ .../src/emqx_dashboard_error_code_api.erl | 91 +++++++++++++ .../test/emqx_dashboard_error_code_SUITE.erl | 121 ++++++++++++++++++ .../src/emqx_mgmt_api_banned.erl | 2 +- .../src/emqx_topic_metrics_api.erl | 30 +++-- apps/emqx_prometheus/rebar.config | 3 +- mix.exs | 2 +- rebar.config | 2 +- 9 files changed, 365 insertions(+), 16 deletions(-) create mode 100644 apps/emqx/include/http_api.hrl create mode 100644 apps/emqx_dashboard/src/emqx_dashboard_error_code.erl create mode 100644 apps/emqx_dashboard/src/emqx_dashboard_error_code_api.erl create mode 100644 apps/emqx_dashboard/test/emqx_dashboard_error_code_SUITE.erl diff --git a/apps/emqx/include/http_api.hrl b/apps/emqx/include/http_api.hrl new file mode 100644 index 000000000..4e7053565 --- /dev/null +++ b/apps/emqx/include/http_api.hrl @@ -0,0 +1,74 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2017-2022 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- + +%% Bad Request +-define(BAD_REQUEST, 'BAD_REQUEST'). + +-define(ALREADY_EXISTED, 'ALREADY_EXISTED'). +-define(BAD_CONFIG_SCHEMA, 'BAD_CONFIG_SCHEMA'). +-define(BAD_LISTENER_ID, 'BAD_LISTENER_ID'). +-define(BAD_NODE_NAME, 'BAD_NODE_NAME'). +-define(BAD_RPC, 'BAD_RPC'). +-define(BAD_TOPIC, 'BAD_TOPIC'). +-define(EXCEED_LIMIT, 'EXCEED_LIMIT'). +-define(INVALID_PARAMETER, 'INVALID_PARAMETER'). +-define(CONFLICT, 'CONFLICT'). +-define(NO_DEFAULT_VALUE, 'NO_DEFAULT_VALUE'). +-define(MESSAGE_ID_SCHEMA_ERROR, 'MESSAGE_ID_SCHEMA_ERROR'). + +%% Resource Not Found +-define(NOT_FOUND, 'NOT_FOUND'). +-define(CLIENTID_NOT_FOUND, 'CLIENTID_NOT_FOUND'). +-define(CLIENT_NOT_FOUND, 'CLIENT_NOT_FOUND'). +-define(MESSAGE_ID_NOT_FOUND, 'MESSAGE_ID_NOT_FOUND'). +-define(RESOURCE_NOT_FOUND, 'RESOURCE_NOT_FOUND'). +-define(TOPIC_NOT_FOUND, 'TOPIC_NOT_FOUND'). +-define(USER_NOT_FOUND, 'USER_NOT_FOUND'). + +%% Internal error +-define(INTERNAL_ERROR, 'INTERNAL_ERROR'). +-define(SOURCE_ERROR, 'SOURCE_ERROR'). +-define(UPDATE_FAILED, 'UPDATE_FAILED'). +-define(REST_FAILED, 'REST_FAILED'). +-define(CLIENT_NOT_RESPONSE, 'CLIENT_NOT_RESPONSE'). + +%% All codes +-define(ERROR_CODES, + [ {'BAD_REQUEST', <<"Request parameters are not legal">>} + , {'ALREADY_EXISTED', <<"Resource already existed">>} + , {'BAD_CONFIG_SCHEMA', <<"Configuration data is not legal">>} + , {'BAD_LISTENER_ID', <<"Bad listener ID">>} + , {'BAD_NODE_NAME', <<"Bad Node Name">>} + , {'BAD_RPC', <<"RPC Failed. Check the cluster status and the requested node status">>} + , {'BAD_TOPIC', <<"Topic syntax error, Topic needs to comply with the MQTT protocol standard">>} + , {'EXCEED_LIMIT', <<"Create resources that exceed the maximum limit or minimum limit">>} + , {'INVALID_PARAMETER', <<"Request parameters is not legal and exceeds the boundary value">>} + , {'CONFLICT', <<"Conflicting request resources">>} + , {'NO_DEFAULT_VALUE', <<"Request parameters do not use default values">>} + , {'MESSAGE_ID_SCHEMA_ERROR', <<"Message ID parsing error">>} + , {'MESSAGE_ID_NOT_FOUND', <<"Message ID does not exist">>} + , {'NOT_FOUND', <<"Resource was not found or does not exist">>} + , {'CLIENTID_NOT_FOUND', <<"Client ID was not found or does not exist">>} + , {'CLIENT_NOT_FOUND', <<"Client was not found or does not exist(usually not a MQTT client)">>} + , {'RESOURCE_NOT_FOUND', <<"Resource not found">>} + , {'TOPIC_NOT_FOUND', <<"Topic not found">>} + , {'USER_NOT_FOUND', <<"User not found">>} + , {'INTERNAL_ERROR', <<"Server inter error">>} + , {'SOURCE_ERROR', <<"Source error">>} + , {'UPDATE_FAILED', <<"Update failed">>} + , {'REST_FAILED', <<"Reset source or config failed">>} + , {'CLIENT_NOT_RESPONSE', <<"Client not responding">>} + ]). diff --git a/apps/emqx_dashboard/src/emqx_dashboard_error_code.erl b/apps/emqx_dashboard/src/emqx_dashboard_error_code.erl new file mode 100644 index 000000000..4f2b88c23 --- /dev/null +++ b/apps/emqx_dashboard/src/emqx_dashboard_error_code.erl @@ -0,0 +1,56 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2020-2022 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- + +-module(emqx_dashboard_error_code). + +-include_lib("emqx/include/http_api.hrl"). + +-export([ all/0 + , list/0 + , look_up/1 + , description/1 + , format/1 + ]). + +all() -> + [Name || {Name, _Description} <- ?ERROR_CODES]. + +list() -> + [format(Code) || Code <- ?ERROR_CODES]. + +look_up(Code) -> + look_up(Code, ?ERROR_CODES). +look_up(_Code, []) -> + {error, not_found}; +look_up(Code, [{Code, Description} | _List]) -> + {ok, format({Code, Description})}; +look_up(Code, [_ | List]) -> + look_up(Code, List). + +description(Code) -> + description(Code, ?ERROR_CODES). +description(_Code, []) -> + {error, not_found}; +description(Code, [{Code, Description} | _List]) -> + {ok, Description}; +description(Code, [_ | List]) -> + description(Code, List). + +format({Code, Description}) -> + #{ + code => Code, + description => Description + }. diff --git a/apps/emqx_dashboard/src/emqx_dashboard_error_code_api.erl b/apps/emqx_dashboard/src/emqx_dashboard_error_code_api.erl new file mode 100644 index 000000000..3ba260603 --- /dev/null +++ b/apps/emqx_dashboard/src/emqx_dashboard_error_code_api.erl @@ -0,0 +1,91 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2020-2022 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- +-module(emqx_dashboard_error_code_api). + +-behaviour(minirest_api). + +-include_lib("emqx/include/http_api.hrl"). +-include("emqx_dashboard.hrl"). +-include_lib("typerefl/include/types.hrl"). + +-export([ api_spec/0 + , fields/1 + , paths/0 + , schema/1 + , namespace/0 + ]). + +-export([ error_codes/2 + , error_code/2 + ]). + +namespace() -> "dashboard". + +api_spec() -> + emqx_dashboard_swagger:spec(?MODULE, #{check_schema => true, translate_body => true}). + +paths() -> + [ "/error_codes" + , "/error_codes/:code" + ]. + +schema("/error_codes") -> + #{ + 'operationId' => error_codes, + get => #{ + security => [], + description => <<"API Error Codes">>, + responses => #{ + 200 => hoconsc:array(hoconsc:ref(?MODULE, error_code)) + } + } + }; +schema("/error_codes/:code") -> + #{ + 'operationId' => error_code, + get => #{ + security => [], + description => <<"API Error Codes">>, + parameters => [ + {code, hoconsc:mk(hoconsc:enum(emqx_dashboard_error_code:all()), #{ + desc => <<"API Error Codes">>, + in => path, + example => hd(emqx_dashboard_error_code:all())})} + ], + responses => #{ + 200 => hoconsc:ref(?MODULE, error_code) + } + } + }. + +fields(error_code) -> + [ + {code, hoconsc:mk(string(), #{desc => <<"Code Name">>})}, + {description, hoconsc:mk(string(), #{desc => <<"Description">>})} + ]. + + +error_codes(_, _) -> + {200, emqx_dashboard_error_code:list()}. + +error_code(_, #{bindings := #{code := Name}}) -> + case emqx_dashboard_error_code:look_up(Name) of + {ok, Code} -> + {200, Code}; + {error, not_found} -> + Message = list_to_binary(io_lib:format("Code name ~p not found", [Name])), + {404, ?NOT_FOUND, Message} + end. diff --git a/apps/emqx_dashboard/test/emqx_dashboard_error_code_SUITE.erl b/apps/emqx_dashboard/test/emqx_dashboard_error_code_SUITE.erl new file mode 100644 index 000000000..bed79b349 --- /dev/null +++ b/apps/emqx_dashboard/test/emqx_dashboard_error_code_SUITE.erl @@ -0,0 +1,121 @@ +%%-------------------------------------------------------------------- +%% Copyright (c) 2020-2022 EMQ Technologies Co., Ltd. All Rights Reserved. +%% +%% Licensed under the Apache License, Version 2.0 (the "License"); +%% you may not use this file except in compliance with the License. +%% You may obtain a copy of the License at +%% +%% http://www.apache.org/licenses/LICENSE-2.0 +%% +%% Unless required by applicable law or agreed to in writing, software +%% distributed under the License is distributed on an "AS IS" BASIS, +%% WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +%% See the License for the specific language governing permissions and +%% limitations under the License. +%%-------------------------------------------------------------------- + +-module(emqx_dashboard_error_code_SUITE). + +-compile(nowarn_export_all). +-compile(export_all). + +-include_lib("emqx/include/http_api.hrl"). + +-include_lib("eunit/include/eunit.hrl"). + +-define(SERVER, "http://127.0.0.1:18083/api/v5"). + +all() -> + emqx_common_test_helpers:all(?MODULE). + +init_per_suite(Config) -> + mria:start(), + application:load(emqx_dashboard), + emqx_common_test_helpers:start_apps([emqx_conf, emqx_dashboard], fun set_special_configs/1), + Config. + +set_special_configs(emqx_dashboard) -> + Config = #{ + default_username => <<"admin">>, + default_password => <<"public">>, + listeners => [#{ + protocol => http, + port => 18083 + }] + }, + emqx_config:put([emqx_dashboard], Config), + ok; +set_special_configs(_) -> + ok. + +end_per_suite(Config) -> + end_suite(), + Config. + +end_suite() -> + application:unload(emqx_management), + emqx_common_test_helpers:stop_apps([emqx_dashboard]). + +t_all_code(_) -> + HrlDef = ?ERROR_CODES, + All = emqx_dashboard_error_code:all(), + ?assertEqual(length(HrlDef), length(All)), + ok. + +t_list_code(_) -> + List = emqx_dashboard_error_code:list(), + [?assert(exist(CodeName, List)) || CodeName <- emqx_dashboard_error_code:all()], + ok. + +t_look_up_code(_) -> + Fun = + fun(CodeName) -> + {ok, _} = emqx_dashboard_error_code:look_up(CodeName) + end, + lists:foreach(Fun, emqx_dashboard_error_code:all()), + {error, not_found} = emqx_dashboard_error_code:look_up('_____NOT_EXIST_NAME'), + ok. + +t_description_code(_) -> + {error, not_found} = emqx_dashboard_error_code:description('_____NOT_EXIST_NAME'), + {ok, <<"Request parameters are not legal">>} = + emqx_dashboard_error_code:description('BAD_REQUEST'), + ok. + +t_format_code(_) -> + #{code := 'TEST_SUITE_CODE', description := <<"for test suite">>} = + emqx_dashboard_error_code:format({'TEST_SUITE_CODE', <<"for test suite">>}), + ok. + +t_api_codes(_) -> + Url = ?SERVER ++ "/error_codes", + {ok, List} = request(Url), + [?assert(exist(atom_to_binary(CodeName, utf8), List)) || CodeName <- emqx_dashboard_error_code:all()], + ok. + +t_api_code(_) -> + Url = ?SERVER ++ "/error_codes/BAD_REQUEST", + {ok, #{<<"code">> := <<"BAD_REQUEST">>, + <<"description">> := <<"Request parameters are not legal">>}} = request(Url), + ok. + +exist(_CodeName, []) -> + false; +exist(CodeName, [#{code := CodeName, description := _} | _List]) -> + true; +exist(CodeName, [#{<<"code">> := CodeName, <<"description">> := _} | _List]) -> + true; +exist(CodeName, [_ | List]) -> + exist(CodeName, List). + +request(Url) -> + Request = {Url, []}, + case httpc:request(get, Request, [], []) of + {error, Reason} -> + {error, Reason}; + {ok, {{"HTTP/1.1", Code, _}, _, Return} } + when Code >= 200 andalso Code =< 299 -> + {ok, emqx_json:decode(Return, [return_maps])}; + {ok, {Reason, _, _}} -> + {error, Reason} + end. diff --git a/apps/emqx_management/src/emqx_mgmt_api_banned.erl b/apps/emqx_management/src/emqx_mgmt_api_banned.erl index bbacbcdfe..2ecd894ee 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_banned.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_banned.erl @@ -104,7 +104,7 @@ fields(ban) -> {who, hoconsc:mk(binary(), #{ desc => <<"Client info as banned type">>, nullable => false, - example => <<"Badass坏"/utf8>>})}, + example => <<"Banned name"/utf8>>})}, {by, hoconsc:mk(binary(), #{ desc => <<"Commander">>, nullable => true, diff --git a/apps/emqx_modules/src/emqx_topic_metrics_api.erl b/apps/emqx_modules/src/emqx_topic_metrics_api.erl index edd0add46..889268a4a 100644 --- a/apps/emqx_modules/src/emqx_topic_metrics_api.erl +++ b/apps/emqx_modules/src/emqx_topic_metrics_api.erl @@ -41,10 +41,9 @@ , fields/1 ]). --define(ERROR_TOPIC, 'ERROR_TOPIC'). -define(EXCEED_LIMIT, 'EXCEED_LIMIT'). -define(BAD_TOPIC, 'BAD_TOPIC'). --define(BAD_RPC, 'BAD_RPC'). +-define(TOPIC_NOT_FOUND, 'TOPIC_NOT_FOUND'). -define(BAD_REQUEST, 'BAD_REQUEST'). api_spec() -> @@ -62,7 +61,8 @@ schema("/mqtt/topic_metrics") -> #{ description => <<"List topic metrics">> , tags => ?API_TAG_MQTT , responses => - #{200 => mk(array(hoconsc:ref(topic_metrics)), #{ desc => <<"List all topic metrics">>})} + #{200 => + mk(array(hoconsc:ref(topic_metrics)), #{ desc => <<"List all topic metrics">>})} } , put => #{ description => <<"Reset topic metrics by topic name. Or reset all Topic Metrics">> @@ -72,7 +72,9 @@ schema("/mqtt/topic_metrics") -> reset_examples()) , responses => #{ 204 => <<"Reset topic metrics successfully">> - , 404 => emqx_dashboard_swagger:error_codes([?ERROR_TOPIC], <<"Topic not found">>) + , 404 => + emqx_dashboard_swagger:error_codes( + [?TOPIC_NOT_FOUND], <<"Topic not found">>) } } , post => @@ -81,8 +83,10 @@ schema("/mqtt/topic_metrics") -> , 'requestBody' => [topic(body)] , responses => #{ 204 => <<"Create topic metrics success">> - , 409 => emqx_dashboard_swagger:error_codes([?EXCEED_LIMIT], <<"Topic metrics exceeded max limit 512">>) - , 400 => emqx_dashboard_swagger:error_codes([?BAD_REQUEST, ?BAD_TOPIC], <<"Topic metrics already existed or bad topic">>) + , 409 => emqx_dashboard_swagger:error_codes([?EXCEED_LIMIT], + <<"Topic metrics exceeded max limit 512">>) + , 400 => emqx_dashboard_swagger:error_codes([?BAD_REQUEST, ?BAD_TOPIC], + <<"Topic metrics already existed or bad topic">>) } } }; @@ -94,7 +98,8 @@ schema("/mqtt/topic_metrics/:topic") -> , parameters => [topic(path)] , responses => #{ 200 => mk(ref(topic_metrics), #{ desc => <<"Topic metrics">> }) - , 404 => emqx_dashboard_swagger:error_codes([?ERROR_TOPIC], <<"Topic not found">>) + , 404 => emqx_dashboard_swagger:error_codes([?TOPIC_NOT_FOUND], + <<"Topic not found">>) } } , delete => @@ -103,7 +108,8 @@ schema("/mqtt/topic_metrics/:topic") -> , parameters => [topic(path)] , responses => #{ 204 => <<"Removed topic metrics successfully">>, - 404 => emqx_dashboard_swagger:error_codes([?ERROR_TOPIC], <<"Topic not found">>) + 404 => emqx_dashboard_swagger:error_codes([?TOPIC_NOT_FOUND], + <<"Topic not found">>) } } }. @@ -111,7 +117,9 @@ schema("/mqtt/topic_metrics/:topic") -> fields(reset) -> [ {topic , mk( binary() - , #{ desc => <<"Topic Name. If this parameter is not present, all created topic metrics will be reset">> + , #{ desc => + <<"Topic Name. If this parameter is not present," + " all created topic metrics will be reset">> , example => <<"testtopic/1">> , nullable => true})} , {action @@ -399,10 +407,10 @@ reason2httpresp(already_existed) -> {400, #{code => ?BAD_TOPIC, message => Msg}}; reason2httpresp(topic_not_found) -> Msg = <<"Topic not found">>, - {404, #{code => ?ERROR_TOPIC, message => Msg}}; + {404, #{code => ?TOPIC_NOT_FOUND, message => Msg}}; reason2httpresp(not_found) -> Msg = <<"Topic not found">>, - {404, #{code => ?ERROR_TOPIC, message => Msg}}. + {404, #{code => ?TOPIC_NOT_FOUND, message => Msg}}. get_cluster_response(Args) -> case erlang:apply(?MODULE, cluster_accumulation_metrics, Args) of diff --git a/apps/emqx_prometheus/rebar.config b/apps/emqx_prometheus/rebar.config index b0f6432d2..c8b94178a 100644 --- a/apps/emqx_prometheus/rebar.config +++ b/apps/emqx_prometheus/rebar.config @@ -4,8 +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.24.0"}}}, - {minirest, {git, "https://github.com/emqx/minirest", {tag, "1.2.11"}}} + {hocon, {git, "https://github.com/emqx/hocon.git", {tag, "0.24.0"}}} ]}. {edoc_opts, [{preprocess, true}]}. diff --git a/mix.exs b/mix.exs index 367d3d737..5614aa654 100644 --- a/mix.exs +++ b/mix.exs @@ -57,7 +57,7 @@ defmodule EMQXUmbrella.MixProject do {:mria, github: "emqx/mria", tag: "0.2.0", override: true}, {:ekka, github: "emqx/ekka", tag: "0.12.1", override: true}, {:gen_rpc, github: "emqx/gen_rpc", tag: "2.8.0", override: true}, - {:minirest, github: "emqx/minirest", tag: "1.2.11", override: true}, + {:minirest, github: "emqx/minirest", tag: "1.2.12", override: true}, {:ecpool, github: "emqx/ecpool", tag: "0.5.2"}, {:replayq, "0.3.3", override: true}, {:pbkdf2, github: "emqx/erlang-pbkdf2", tag: "2.0.4", override: true}, diff --git a/rebar.config b/rebar.config index 37f59df9a..66f667bb5 100644 --- a/rebar.config +++ b/rebar.config @@ -56,7 +56,7 @@ , {mria, {git, "https://github.com/emqx/mria", {tag, "0.2.0"}}} , {ekka, {git, "https://github.com/emqx/ekka", {tag, "0.12.1"}}} , {gen_rpc, {git, "https://github.com/emqx/gen_rpc", {tag, "2.8.0"}}} - , {minirest, {git, "https://github.com/emqx/minirest", {tag, "1.2.11"}}} + , {minirest, {git, "https://github.com/emqx/minirest", {tag, "1.2.12"}}} , {ecpool, {git, "https://github.com/emqx/ecpool", {tag, "0.5.2"}}} , {replayq, "0.3.3"} , {pbkdf2, {git, "https://github.com/emqx/erlang-pbkdf2.git", {tag, "2.0.4"}}} From 98a11f3c15b1392006b4760cf697322673c4aa79 Mon Sep 17 00:00:00 2001 From: DDDHuang <44492639+DDDHuang@users.noreply.github.com> Date: Fri, 18 Feb 2022 10:43:32 +0800 Subject: [PATCH 2/6] fix(api): emqx_connector_api error code format --- apps/emqx/include/http_api.hrl | 2 + .../emqx_connector/src/emqx_connector_api.erl | 42 +++++++++++-------- .../test/emqx_connector_api_SUITE.erl | 4 +- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/apps/emqx/include/http_api.hrl b/apps/emqx/include/http_api.hrl index 4e7053565..ac09df989 100644 --- a/apps/emqx/include/http_api.hrl +++ b/apps/emqx/include/http_api.hrl @@ -27,6 +27,7 @@ -define(INVALID_PARAMETER, 'INVALID_PARAMETER'). -define(CONFLICT, 'CONFLICT'). -define(NO_DEFAULT_VALUE, 'NO_DEFAULT_VALUE'). +-define(DEPENDENCY_EXISTS, 'DEPENDENCY_EXISTS'). -define(MESSAGE_ID_SCHEMA_ERROR, 'MESSAGE_ID_SCHEMA_ERROR'). %% Resource Not Found @@ -58,6 +59,7 @@ , {'INVALID_PARAMETER', <<"Request parameters is not legal and exceeds the boundary value">>} , {'CONFLICT', <<"Conflicting request resources">>} , {'NO_DEFAULT_VALUE', <<"Request parameters do not use default values">>} + , {'DEPENDENCY_EXISTS', <<"Resource is dependent by another resource">>} , {'MESSAGE_ID_SCHEMA_ERROR', <<"Message ID parsing error">>} , {'MESSAGE_ID_NOT_FOUND', <<"Message ID does not exist">>} , {'NOT_FOUND', <<"Resource was not found or does not exist">>} diff --git a/apps/emqx_connector/src/emqx_connector_api.erl b/apps/emqx_connector/src/emqx_connector_api.erl index c13a1b2cd..031492d86 100644 --- a/apps/emqx_connector/src/emqx_connector_api.erl +++ b/apps/emqx_connector/src/emqx_connector_api.erl @@ -50,10 +50,10 @@ api_spec() -> paths() -> ["/connectors_test", "/connectors", "/connectors/:id"]. -error_schema(Code, Message) -> - [ {code, mk(string(), #{example => Code})} - , {message, mk(string(), #{example => Message})} - ]. +error_schema(Codes, Message) when is_list(Message) -> + error_schema(Codes, list_to_binary(Message)); +error_schema(Codes, Message) when is_binary(Message) -> + emqx_dashboard_swagger:error_codes(Codes, Message). put_request_body_schema() -> emqx_dashboard_swagger:schema_with_examples( @@ -134,8 +134,8 @@ schema("/connectors_test") -> summary => <<"Test creating connector">>, requestBody => post_request_body_schema(), responses => #{ - 200 => <<"Test connector OK">>, - 400 => error_schema('TEST_FAILED', "connector test failed") + 204 => <<"Test connector OK">>, + 400 => error_schema(['TEST_FAILED'], "connector test failed") } } }; @@ -160,7 +160,7 @@ schema("/connectors") -> requestBody => post_request_body_schema(), responses => #{ 201 => get_response_body_schema(), - 400 => error_schema('ALREADY_EXISTS', "connector already exists") + 400 => error_schema(['ALREADY_EXISTS'], "connector already exists") } } }; @@ -175,7 +175,7 @@ schema("/connectors/:id") -> parameters => param_path_id(), responses => #{ 200 => get_response_body_schema(), - 404 => error_schema('NOT_FOUND', "Connector not found") + 404 => error_schema(['NOT_FOUND'], "Connector not found") } }, put => #{ @@ -186,8 +186,7 @@ schema("/connectors/:id") -> requestBody => put_request_body_schema(), responses => #{ 200 => get_response_body_schema(), - 400 => error_schema('UPDATE_FAIL', "Update failed"), - 404 => error_schema('NOT_FOUND', "Connector not found") + 404 => error_schema(['NOT_FOUND'], "Connector not found") }}, delete => #{ tags => [<<"connectors">>], @@ -196,15 +195,17 @@ schema("/connectors/:id") -> parameters => param_path_id(), responses => #{ 204 => <<"Delete connector successfully">>, - 400 => error_schema('DELETE_FAIL', "Delete failed") + 403 => error_schema(['DEPENDENCY_EXISTS'], "Cannot remove dependent connector"), + 404 => error_schema(['NOT_FOUND'], "Delete failed, not found") }} }. '/connectors_test'(post, #{body := #{<<"type">> := ConnType} = Params}) -> case emqx_connector:create_dry_run(ConnType, maps:remove(<<"type">>, Params)) of - ok -> {200}; + ok -> + {204}; {error, Error} -> - {400, error_msg('BAD_ARG', Error)} + {400, error_msg(['TEST_FAILED'], Error)} end. '/connectors'(get, _Request) -> @@ -221,14 +222,16 @@ schema("/connectors/:id") -> {ok, #{raw_config := RawConf}} -> Id = emqx_connector:connector_id(ConnType, ConnName), {201, format_resp(Id, RawConf)}; - {error, Error} -> {400, error_msg('BAD_ARG', Error)} + {error, Error} -> + {400, error_msg('ALREADY_EXISTS', Error)} end end. '/connectors/:id'(get, #{bindings := #{id := Id}}) -> ?TRY_PARSE_ID(Id, case emqx_connector:lookup(ConnType, ConnName) of - {ok, Conf} -> {200, format_resp(Id, Conf)}; + {ok, Conf} -> + {200, format_resp(Id, Conf)}; {error, not_found} -> {404, error_msg('NOT_FOUND', <<"connector not found">>)} end); @@ -241,7 +244,8 @@ schema("/connectors/:id") -> case emqx_connector:update(ConnType, ConnName, Params) of {ok, #{raw_config := RawConf}} -> {200, format_resp(Id, RawConf)}; - {error, Error} -> {400, error_msg('BAD_ARG', Error)} + {error, Error} -> + {500, error_msg('INTERNAL_ERROR', Error)} end; {error, not_found} -> {404, error_msg('NOT_FOUND', <<"connector not found">>)} @@ -252,12 +256,14 @@ schema("/connectors/:id") -> case emqx_connector:lookup(ConnType, ConnName) of {ok, _} -> case emqx_connector:delete(ConnType, ConnName) of - {ok, _} -> {204}; + {ok, _} -> + {204}; {error, {post_config_update, _, {dependency_bridges_exist, BridgeID}}} -> {403, error_msg('DEPENDENCY_EXISTS', <<"Cannot remove the connector as it's in use by a bridge: ", BridgeID/binary>>)}; - {error, Error} -> {400, error_msg('BAD_ARG', Error)} + {error, Error} -> + {500, error_msg('INTERNAL_ERROR', Error)} end; {error, not_found} -> {404, error_msg('NOT_FOUND', <<"connector not found">>)} diff --git a/apps/emqx_connector/test/emqx_connector_api_SUITE.erl b/apps/emqx_connector/test/emqx_connector_api_SUITE.erl index 9e5e44273..5684611be 100644 --- a/apps/emqx_connector/test/emqx_connector_api_SUITE.erl +++ b/apps/emqx_connector/test/emqx_connector_api_SUITE.erl @@ -377,7 +377,7 @@ t_mqtt_conn_update(_) -> %% then we try to update 'server' of the connector, to an unavailable IP address %% the update should fail because of 'unreachable' or 'connrefused' - {ok, 400, _ErrorMsg} = request(put, uri(["connectors", ConnctorID]), + {ok, 500, _ErrorMsg} = request(put, uri(["connectors", ConnctorID]), ?MQTT_CONNECTOR2(<<"127.0.0.1:2603">>)), %% we fix the 'server' parameter to a normal one, it should work {ok, 200, _} = request(put, uri(["connectors", ConnctorID]), @@ -468,7 +468,7 @@ t_mqtt_conn_update3(_) -> t_mqtt_conn_testing(_) -> %% APIs for testing the connectivity %% then we add a mqtt connector, using POST - {ok, 200, <<>>} = request(post, uri(["connectors_test"]), + {ok, 204, <<>>} = request(post, uri(["connectors_test"]), ?MQTT_CONNECTOR2(<<"127.0.0.1:1883">>)#{ <<"type">> => ?CONNECTR_TYPE, <<"name">> => ?BRIDGE_NAME_EGRESS From a5d8f2ce3b77b31f45ed58f94b43cc1ac323e002 Mon Sep 17 00:00:00 2001 From: DDDHuang <44492639+DDDHuang@users.noreply.github.com> Date: Fri, 18 Feb 2022 10:57:04 +0800 Subject: [PATCH 3/6] fix(api): banned api error code format --- apps/emqx/src/emqx_banned.erl | 4 +++- .../emqx_management/src/emqx_mgmt_api_banned.erl | 16 +++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/apps/emqx/src/emqx_banned.erl b/apps/emqx/src/emqx_banned.erl index ddb81fee4..fce9b29cd 100644 --- a/apps/emqx/src/emqx_banned.erl +++ b/apps/emqx/src/emqx_banned.erl @@ -127,7 +127,9 @@ parse(Params) -> until = Until }; false -> - {error, "already_expired"} + ErrorReason = + io_lib:format("Cannot create expired banned, ~p to ~p", [At, Until]), + {error, ErrorReason} end end. pares_who(#{as := As, who := Who}) -> diff --git a/apps/emqx_management/src/emqx_mgmt_api_banned.erl b/apps/emqx_management/src/emqx_mgmt_api_banned.erl index 2ecd894ee..8633ccfc2 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_banned.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_banned.erl @@ -67,8 +67,9 @@ schema("/banned") -> 'requestBody' => hoconsc:mk(hoconsc:ref(ban)), responses => #{ 200 => [{data, hoconsc:mk(hoconsc:array(hoconsc:ref(ban)), #{})}], - 400 => emqx_dashboard_swagger:error_codes(['ALREADY_EXISTED'], - <<"Banned already existed">>) + 400 => emqx_dashboard_swagger:error_codes( + ['ALREADY_EXISTED', 'BAD_REQUEST'], + <<"Banned already existed, or bad args">>) } } }; @@ -89,8 +90,9 @@ schema("/banned/:as/:who") -> ], responses => #{ 204 => <<"Delete banned success">>, - 404 => emqx_dashboard_swagger:error_codes(['RESOURCE_NOT_FOUND'], - <<"Banned not found">>) + 404 => emqx_dashboard_swagger:error_codes( + ['NOT_FOUND'], + <<"Banned not found. May be the banned time has been exceeded">>) } } }. @@ -134,12 +136,12 @@ banned(get, #{query_string := Params}) -> banned(post, #{body := Body}) -> case emqx_banned:parse(Body) of {error, Reason} -> - {400, #{code => 'PARAMS_ERROR', message => list_to_binary(Reason)}}; + {400, 'BAD_REQUEST', list_to_binary(Reason)}; Ban -> case emqx_banned:create(Ban) of {ok, Banned} -> {200, format(Banned)}; {error, {already_exist, Old}} -> - {400, #{code => 'ALREADY_EXISTED', message => format(Old)}} + {400, 'ALREADY_EXISTED', format(Old)} end end. @@ -148,7 +150,7 @@ delete_banned(delete, #{bindings := Params}) -> [] -> #{as := As0, who := Who0} = Params, Message = list_to_binary(io_lib:format("~p: ~s not found", [As0, Who0])), - {404, #{code => 'RESOURCE_NOT_FOUND', message => Message}}; + {404, 'NOT_FOUND', Message}; _ -> ok = emqx_banned:delete(Params), {204} From 240bac0235c1d365523ac50751d672fdcd9f76ca Mon Sep 17 00:00:00 2001 From: DDDHuang <44492639+DDDHuang@users.noreply.github.com> Date: Mon, 21 Feb 2022 10:37:52 +0800 Subject: [PATCH 4/6] fix(api): bridge api bad spec --- apps/emqx_bridge/src/emqx_bridge_api.erl | 37 +++++++++++++++--------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/apps/emqx_bridge/src/emqx_bridge_api.erl b/apps/emqx_bridge/src/emqx_bridge_api.erl index d78cbf5f2..330c607c3 100644 --- a/apps/emqx_bridge/src/emqx_bridge_api.erl +++ b/apps/emqx_bridge/src/emqx_bridge_api.erl @@ -23,11 +23,17 @@ -import(hoconsc, [mk/2, array/1, enum/1]). %% Swagger specs from hocon schema --export([api_spec/0, paths/0, schema/1, namespace/0]). +-export([ api_spec/0 + , paths/0 + , schema/1 + , namespace/0 + ]). %% API callbacks --export(['/bridges'/2, '/bridges/:id'/2, - '/bridges/:id/operation/:operation'/2]). +-export([ '/bridges'/2 + , '/bridges/:id'/2 + , '/bridges/:id/operation/:operation'/2 + ]). -export([ lookup_from_local_node/2 ]). @@ -70,10 +76,12 @@ api_spec() -> paths() -> ["/bridges", "/bridges/:id", "/bridges/:id/operation/:operation"]. -error_schema(Code, Message) -> - [ {code, mk(string(), #{example => Code})} - , {message, mk(string(), #{example => Message})} - ]. +error_schema(Code, Message) when is_atom(Code) -> + error_schema([Code], Message); +error_schema(Codes, Message) when is_list(Message) -> + error_schema(Codes, list_to_binary(Message)); +error_schema(Codes, Message) when is_list(Codes) andalso is_binary(Message) -> + emqx_dashboard_swagger:error_codes(Codes, Message). get_response_body_schema() -> emqx_dashboard_swagger:schema_with_examples(emqx_bridge_schema:get_response(), @@ -214,7 +222,7 @@ schema("/bridges") -> bridge_info_examples(post)), responses => #{ 201 => get_response_body_schema(), - 400 => error_schema('BAD_ARG', "Create bridge failed") + 400 => error_schema('BAD_REQUEST', "Create bridge failed") } } }; @@ -242,7 +250,8 @@ schema("/bridges/:id") -> bridge_info_examples(put)), responses => #{ 200 => get_response_body_schema(), - 400 => error_schema('BAD_ARG', "Update bridge failed") + 404 => error_schema('NOT_FOUND', "Bridge not found"), + 400 => error_schema('BAD_REQUEST', "Update bridge failed") } }, delete => #{ @@ -299,8 +308,10 @@ schema("/bridges/:id/operation/:operation") -> case emqx_bridge:lookup(BridgeType, BridgeName) of {ok, _} -> case ensure_bridge_created(BridgeType, BridgeName, Conf) of - ok -> lookup_from_all_nodes(BridgeType, BridgeName, 200); - {error, Error} -> {400, Error} + ok -> + lookup_from_all_nodes(BridgeType, BridgeName, 200); + {error, Error} -> + {400, Error} end; {error, not_found} -> {404, error_msg('NOT_FOUND',<<"bridge not found">>)} @@ -335,7 +346,7 @@ lookup_from_local_node(BridgeType, BridgeName) -> '/bridges/:id/operation/:operation'(post, #{bindings := #{id := Id, operation := Op}}) -> ?TRY_PARSE_ID(Id, case operation_to_conf_req(Op) of - invalid -> {404, error_msg('BAD_ARG', <<"invalid operation">>)}; + invalid -> {400, error_msg('BAD_REQUEST', <<"invalid operation">>)}; UpReq -> case emqx_conf:update(emqx_bridge:config_key_path() ++ [BridgeType, BridgeName], {UpReq, BridgeType, BridgeName}, #{override_to => cluster}) of @@ -357,7 +368,7 @@ ensure_bridge_created(BridgeType, BridgeName, Conf) -> Conf, #{override_to => cluster}) of {ok, _} -> ok; {error, Reason} -> - {error, error_msg('BAD_ARG', Reason)} + {error, error_msg('BAD_REQUEST', Reason)} end. zip_bridges([BridgesFirstNode | _] = BridgesAllNodes) -> From ea860f1ca6d96c723004dfb343bea399da720d54 Mon Sep 17 00:00:00 2001 From: DDDHuang <44492639+DDDHuang@users.noreply.github.com> Date: Mon, 21 Feb 2022 10:42:33 +0800 Subject: [PATCH 5/6] fix: emqx_cluster_rpc start link --- apps/emqx_conf/src/emqx_cluster_rpc.erl | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/apps/emqx_conf/src/emqx_cluster_rpc.erl b/apps/emqx_conf/src/emqx_cluster_rpc.erl index 2c8b14393..1947d400d 100644 --- a/apps/emqx_conf/src/emqx_cluster_rpc.erl +++ b/apps/emqx_conf/src/emqx_cluster_rpc.erl @@ -75,7 +75,14 @@ start_link() -> start_link(node(), ?MODULE, get_retry_ms()). start_link(Node, Name, RetryMs) -> - gen_server:start_link({local, Name}, ?MODULE, [Node, RetryMs], []). + case gen_server:start_link({local, Name}, ?MODULE, [Node, RetryMs], []) of + {ok, Pid} -> + {ok, Pid}; + {error, {already_started, Pid}} -> + {ok, Pid}; + {error, Reason} -> + {error, Reason} + end. %% @doc return {ok, TnxId, MFARes} the first MFA result when all MFA run ok. %% return {error, MFARes} when the first MFA result is no ok or {ok, term()}. From ee4ced82c56888e897da4513996baa3bec1ef222 Mon Sep 17 00:00:00 2001 From: DDDHuang <44492639+DDDHuang@users.noreply.github.com> Date: Wed, 23 Feb 2022 13:55:34 +0800 Subject: [PATCH 6/6] fix: remove jmeter SUITE --- .github/workflows/run_api_tests.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/run_api_tests.yaml b/.github/workflows/run_api_tests.yaml index 719692ff3..5b30b3d7a 100644 --- a/.github/workflows/run_api_tests.yaml +++ b/.github/workflows/run_api_tests.yaml @@ -74,7 +74,6 @@ jobs: - api_publish - api_user - api_login - - api_banned - api_alarms - api_nodes - api_topic_metrics