fix(mgmt_api): Friendly HTTP Status Code for Listeners.

2021-09-23 09:16:33 +08:00 · 2021-09-23 09:16:33 +08:00 · e737f18548
parent 54b3dd8718
commit e737f18548
3 changed files with 61 additions and 25 deletions
--- a/apps/emqx/src/emqx_listeners.erl
+++ b/apps/emqx/src/emqx_listeners.erl
@ -46,6 +46,7 @@
 -export([post_config_update/4]).

 -define(CONF_KEY_PATH, [listeners]).
+-define(TYPES_STRING, ["tcp","ssl","ws","wss","quic"]).

 %% @doc List configured listeners.
 -spec(list() -> [{ListenerId :: atom(), ListenerConf :: map()}]).
@ -349,11 +350,10 @@ listener_id(Type, ListenerName) ->
    list_to_atom(lists:append([str(Type), ":", str(ListenerName)])).

 parse_listener_id(Id) ->
-    try
    [Type, Name] = string:split(str(Id), ":", leading),
-        {list_to_existing_atom(Type), list_to_atom(Name)}
-    catch
-        _ : _ -> error({invalid_listener_id, Id})
+    case lists:member(Type, ?TYPES_STRING) of
+        true -> {list_to_existing_atom(Type), list_to_atom(Name)};
+        false -> {error, {invalid_listener_id, Id}}
    end.

 zone(Opts) ->
--- a/apps/emqx_management/src/emqx_mgmt.erl
+++ b/apps/emqx_management/src/emqx_mgmt.erl
@ -508,12 +508,16 @@ update_listener(Id, Config) ->
    [update_listener(Node, Id, Config) || Node <- ekka_mnesia:running_nodes()].

 update_listener(Node, Id, Config) when Node =:= node() ->
-    {Type, Name} = emqx_listeners:parse_listener_id(Id),
+    case emqx_listeners:parse_listener_id(Id) of
+        {error, {invalid_listener_id, Id}} ->
+            {error, {invalid_listener_id, Id}};
+        {Type, Name} ->
            case emqx:update_config([listeners, Type, Name], Config, #{}) of
                {ok, #{raw_config := RawConf}} ->
                    RawConf#{node => Node, id => Id, running => true};
                {error, Reason} ->
-            error(Reason)
+                    {error, Reason}
+            end
    end;
 update_listener(Node, Id, Config) ->
    rpc_call(Node, update_listener, [Node, Id, Config]).
--- a/apps/emqx_management/src/emqx_mgmt_api_listeners.erl
+++ b/apps/emqx_management/src/emqx_mgmt_api_listeners.erl
@ -35,6 +35,11 @@
 -define(NODE_LISTENER_NOT_FOUND, <<"Node name or listener id not found">>).
 -define(NODE_NOT_FOUND_OR_DOWN, <<"Node not found or Down">>).
 -define(LISTENER_NOT_FOUND, <<"Listener id not found">>).
+-define(ADDR_PORT_INUSE, <<"Addr port in use">>).
+-define(CONFIG_SCHEMA_ERROR, <<"Config schema error">>).
+-define(INVALID_LISTENER_PROTOCOL, <<"Invalid listener type">>).
+-define(UPDATE_CONFIG_FAILED, <<"Update configuration failed">>).
+-define(OPERATION_FAILED, <<"Operation failed">>).

 api_spec() ->
    {
@ -49,11 +54,11 @@ api_spec() ->
        []
    }.

-define(TYPES, [tcp, ssl, ws, wss, quic]).
+-define(TYPES_ATOM, [tcp, ssl, ws, wss, quic]).
 req_schema() ->
    Schema = [emqx_mgmt_api_configs:gen_schema(
        emqx:get_raw_config([listeners, T, default], #{}))
-     || T <- ?TYPES],
+     || T <- ?TYPES_ATOM],
    #{oneOf => Schema}.

 resp_schema() ->
@ -91,8 +96,12 @@ api_list_update_listeners_by_id() ->
            parameters => [param_path_id()],
            requestBody => emqx_mgmt_util:schema(req_schema(), <<"Listener Config">>),
            responses => #{
+                <<"400">> =>
+                    emqx_mgmt_util:error_schema(?UPDATE_CONFIG_FAILED, ['BAD_LISTENER_ID', 'BAD_CONFIG_SCHEMA']),
                <<"404">> =>
                    emqx_mgmt_util:error_schema(?LISTENER_NOT_FOUND, ['BAD_LISTENER_ID']),
+                <<"500">> =>
+                    emqx_mgmt_util:error_schema(?OPERATION_FAILED, ['INTERNAL_ERROR']),
                <<"200">> =>
                    emqx_mgmt_util:array_schema(resp_schema(), <<"Create or update listener successfully">>)}},
        delete => #{
@ -115,7 +124,7 @@ api_list_listeners_on_node() ->
                <<"404">> =>
                    emqx_mgmt_util:error_schema(?NODE_NOT_FOUND_OR_DOWN, ['RESOURCE_NOT_FOUND']),
                <<"500">> =>
-                    emqx_mgmt_util:error_schema(<<"Operation Failed">>, ['INTERNAL_ERROR']),
+                    emqx_mgmt_util:error_schema(?OPERATION_FAILED, ['INTERNAL_ERROR']),
                <<"200">> =>
                    emqx_mgmt_util:schema(resp_schema(), <<"List listeners successfully">>)}}},
    {"/nodes/:node/listeners", Metadata, list_listeners_on_node}.
@ -136,9 +145,13 @@ api_get_update_listener_by_id_on_node() ->
            parameters => [param_path_node(), param_path_id()],
            requestBody => emqx_mgmt_util:schema(req_schema(), <<"Listener Config">>),
            responses => #{
+                <<"400">> =>
+                    emqx_mgmt_util:error_schema(?UPDATE_CONFIG_FAILED, ['BAD_LISTENER_ID', 'BAD_CONFIG_SCHEMA']),
                <<"404">> =>
                    emqx_mgmt_util:error_schema(?NODE_LISTENER_NOT_FOUND,
                        ['BAD_NODE_NAME', 'BAD_LISTENER_ID']),
+                <<"500">> =>
+                    emqx_mgmt_util:error_schema(?OPERATION_FAILED, ['INTERNAL_ERROR']),
                <<"200">> =>
                    emqx_mgmt_util:schema(resp_schema(), <<"Get listener successfully">>)}},
        delete => #{
@ -160,7 +173,7 @@ api_manage_listeners() ->
                param_path_id(),
                param_path_operation()],
            responses => #{
-                <<"500">> => emqx_mgmt_util:error_schema(<<"Operation Failed">>, ['INTERNAL_ERROR']),
+                <<"500">> => emqx_mgmt_util:error_schema(?OPERATION_FAILED, ['INTERNAL_ERROR']),
                <<"200">> => emqx_mgmt_util:schema(<<"Operation success">>)}}},
    {"/listeners/:id/operation/:operation", Metadata, manage_listeners}.

@ -173,7 +186,7 @@ api_manage_listeners_on_node() ->
                param_path_id(),
                param_path_operation()],
            responses => #{
-                <<"500">> => emqx_mgmt_util:error_schema(<<"Operation Failed">>, ['INTERNAL_ERROR']),
+                <<"500">> => emqx_mgmt_util:error_schema(?OPERATION_FAILED, ['INTERNAL_ERROR']),
                <<"200">> => emqx_mgmt_util:schema(<<"Operation success">>)}}},
    {"/nodes/:node/listeners/:id/operation/:operation", Metadata, manage_listeners}.

@ -221,10 +234,23 @@ crud_listeners_by_id(get, #{bindings := #{id := Id}}) ->
            {200, format(Listeners)}
    end;
 crud_listeners_by_id(put, #{bindings := #{id := Id}, body := Conf}) ->
-    return_listeners(emqx_mgmt:update_listener(Id, Conf));
+    Results = format(emqx_mgmt:update_listener(Id, Conf)),
+    case lists:filter(fun filter_errors/1, Results) of
+        [{error, {invalid_listener_id, Id}} | _] ->
+            {400, #{code => 'BAD_REQUEST', message => ?INVALID_LISTENER_PROTOCOL}};
+        [{error, {emqx_machine_schema, _}} | _] ->
+            {400, #{code => 'BAD_REQUEST', message => ?CONFIG_SCHEMA_ERROR}};
+        [{error, {eaddrinuse, _}} | _] ->
+            {400, #{code => 'BAD_REQUEST', message => ?ADDR_PORT_INUSE}};
+        [{error, Reason} | _] ->
+            {500, #{code => 'UNKNOWN_ERROR', message => err_msg(Reason)}};
+        [] ->
+            {200, Results}
+    end;
+
 crud_listeners_by_id(delete, #{bindings := #{id := Id}}) ->
    Results = emqx_mgmt:remove_listener(Id),
-    case lists:filter(fun({error, _}) -> true; (_) -> false end, Results) of
+    case lists:filter(fun filter_errors/1, Results) of
        [] -> {200};
        Errors -> {500, #{code => 'UNKNOW_ERROR', message => err_msg(Errors)}}
    end.
@ -250,6 +276,14 @@ crud_listener_by_id_on_node(get, #{bindings := #{id := Id, node := Node}}) ->
    end;
 crud_listener_by_id_on_node(put, #{bindings := #{id := Id, node := Node}, body := Conf}) ->
    case emqx_mgmt:update_listener(atom(Node), Id, Conf) of
+        {error, nodedown} ->
+            {404, #{code => 'RESOURCE_NOT_FOUND', message => ?NODE_NOT_FOUND_OR_DOWN}};
+        {error, {invalid_listener_id, _}} ->
+            {400, #{code => 'BAD_REQUEST', message => ?INVALID_LISTENER_PROTOCOL}};
+        {error, {emqx_machine_schema, _}} ->
+            {400, #{code => 'BAD_REQUEST', message => ?CONFIG_SCHEMA_ERROR}};
+        {error, {eaddrinuse, _}} ->
+            {400, #{code => 'BAD_REQUEST', message => ?ADDR_PORT_INUSE}};
        {error, Reason} ->
            {500, #{code => 'UNKNOW_ERROR', message => err_msg(Reason)}};
        Listener ->
@ -301,13 +335,6 @@ do_manage_listeners2(<<"restart">>, Param) ->
            {500, #{code => 'UNKNOW_ERROR', message => err_msg(Reason)}}
    end.

-return_listeners(Listeners) ->
-    Results = format(Listeners),
-    case lists:filter(fun({error, _}) -> true; (_) -> false end, Results) of
-        [] -> {200, Results};
-        Errors -> {500, #{code => 'UNKNOW_ERROR', message => manage_listeners_err(Errors)}}
-    end.
-
 manage_listeners_err(Errors) ->
    list_to_binary(lists:foldl(fun({Node, Err}, Str) ->
            err_msg_str(#{node => Node, error => Err}) ++ "; " ++ Str
@ -332,6 +359,11 @@ trans_running(Conf) ->
            Running
    end.

+filter_errors({error, _}) ->
+    true;
+filter_errors(_) ->
+    false.
+
 jsonable_resp(bind, Port) when is_integer(Port) ->
    {bind, Port};
 jsonable_resp(bind, {Addr, Port}) when is_tuple(Addr); is_integer(Port)->