chore(gw): improve http error messages

apps/emqx_gateway/src/emqx_gateway_api.erl

@@ -83,7 +83,7 @@ gateway(post, Request) ->
                     {ok, NGwConf} ->
                         {201, NGwConf};
                     {error, Reason} ->
-                        return_http_error(500, Reason)
+                        emqx_gateway_http:reason2resp(Reason)
                 end
         end
     catch

apps/emqx_gateway/src/emqx_gateway_api_clients.erl

@@ -745,7 +745,8 @@ common_client_props() ->
                        "due to exceeding the length">>})}
     , {awaiting_rel_cnt,
        mk(integer(),
-          #{ desc => <<"Number of awaiting PUBREC packet">>})}
+          %% FIXME: PUBREC ??
+          #{ desc => <<"Number of awaiting acknowledge packet">>})}
     , {awaiting_rel_max,
        mk(integer(),
           #{ desc => <<"Maximum allowed number of awaiting PUBREC "
@@ -755,25 +756,25 @@ common_client_props() ->
           #{ desc => <<"Number of bytes received by EMQ X Broker">>})}
     , {recv_cnt,
        mk(integer(),
-          #{ desc => <<"Number of TCP packets received">>})}
+          #{ desc => <<"Number of socket packets received">>})}
     , {recv_pkt,
        mk(integer(),
-          #{ desc => <<"Number of MQTT packets received">>})}
+          #{ desc => <<"Number of protocol packets received">>})}
     , {recv_msg,
        mk(integer(),
-          #{ desc => <<"Number of PUBLISH packets received">>})}
+          #{ desc => <<"Number of message packets received">>})}
     , {send_oct,
        mk(integer(),
           #{ desc => <<"Number of bytes sent">>})}
     , {send_cnt,
        mk(integer(),
-          #{ desc => <<"Number of TCP packets sent">>})}
+          #{ desc => <<"Number of socket packets sent">>})}
     , {send_pkt,
        mk(integer(),
-          #{ desc => <<"Number of MQTT packets sent">>})}
+          #{ desc => <<"Number of protocol packets sent">>})}
     , {send_msg,
        mk(integer(),
-          #{ desc => <<"Number of PUBLISH packets sent">>})}
+          #{ desc => <<"Number of message packets sent">>})}
     , {mailbox_len,
        mk(integer(),
           #{ desc => <<"Process mailbox size">>})}

apps/emqx_gateway/src/emqx_gateway_conf.erl

@@ -248,7 +248,8 @@ update(Req) ->
     res(emqx_conf:update([gateway], Req, #{override_to => cluster})).
 
 res({ok, Result}) -> {ok, Result};
-res({error, {pre_config_update,emqx_gateway_conf,Reason}}) -> {error, Reason};
+res({error, {pre_config_update,?MODULE,Reason}}) -> {error, Reason};
+res({error, {post_config_update,?MODULE,Reason}}) -> {error, Reason};
 res({error, Reason}) -> {error, Reason}.
 
 bin({LType, LName}) ->
@@ -314,12 +315,12 @@ pre_config_update(_, {load_gateway, GwName, Conf}, RawConf) ->
             NConf = tune_gw_certs(fun convert_certs/2, GwName, Conf),
             {ok, emqx_map_lib:deep_merge(RawConf, #{GwName => NConf})};
         _ ->
-            {error, already_exist}
+            badres_gateway(already_exist, GwName)
     end;
 pre_config_update(_, {update_gateway, GwName, Conf}, RawConf) ->
     case maps:get(GwName, RawConf, undefined) of
         undefined ->
-            {error, not_found};
+            badres_gateway(not_found, GwName);
         _ ->
             NConf = maps:without([<<"listeners">>, ?AUTHN_BIN], Conf),
             {ok, emqx_map_lib:deep_merge(RawConf, #{GwName => NConf})}
@@ -341,13 +342,13 @@ pre_config_update(_, {add_listener, GwName, {LType, LName}, Conf}, RawConf) ->
                    RawConf,
                    #{GwName => #{<<"listeners">> => NListener}})};
         _ ->
-            {error, already_exist}
+            badres_listener(already_exist, GwName, LType, LName)
     end;
 pre_config_update(_, {update_listener, GwName, {LType, LName}, Conf}, RawConf) ->
     case emqx_map_lib:deep_get(
            [GwName, <<"listeners">>, LType, LName], RawConf, undefined) of
         undefined ->
-            {error, not_found};
+            badres_listener(not_found, GwName, LType, LName);
         OldConf ->
             NConf = convert_certs(certs_dir(GwName), Conf, OldConf),
             NListener = #{LType => #{LName => NConf}},
@@ -374,14 +375,14 @@ pre_config_update(_, {add_authn, GwName, Conf}, RawConf) ->
                    RawConf,
                    #{GwName => #{?AUTHN_BIN => Conf}})};
         _ ->
-            {error, already_exist}
+            badres_authn(already_exist, GwName)
     end;
 pre_config_update(_, {add_authn, GwName, {LType, LName}, Conf}, RawConf) ->
     case emqx_map_lib:deep_get(
            [GwName, <<"listeners">>, LType, LName],
            RawConf, undefined) of
         undefined ->
-            {error, not_found};
+            badres_listener(not_found, GwName, LType, LName);
         Listener ->
             case maps:get(?AUTHN_BIN, Listener, undefined) of
                 undefined ->
@@ -391,14 +392,14 @@ pre_config_update(_, {add_authn, GwName, {LType, LName}, Conf}, RawConf) ->
                                    #{LType => #{LName => NListener}}}},
                     {ok, emqx_map_lib:deep_merge(RawConf, NGateway)};
                 _ ->
-                    {error, already_exist}
+                    badres_listener_authn(already_exist, GwName, LType, LName)
             end
     end;
 pre_config_update(_, {update_authn, GwName, Conf}, RawConf) ->
     case emqx_map_lib:deep_get(
            [GwName, ?AUTHN_BIN], RawConf, undefined) of
         undefined ->
-            {error, not_found};
+            badres_authn(not_found, GwName);
         _ ->
             {ok, emqx_map_lib:deep_merge(
                    RawConf,
@@ -409,11 +410,11 @@ pre_config_update(_, {update_authn, GwName, {LType, LName}, Conf}, RawConf) ->
            [GwName, <<"listeners">>, LType, LName],
            RawConf, undefined) of
         undefined ->
-            {error, not_found};
+            badres_listener(not_found, GwName, LType, LName);
         Listener ->
             case maps:get(?AUTHN_BIN, Listener, undefined) of
                 undefined ->
-                    {error, not_found};
+                    badres_listener_authn(not_found, GwName, LType, LName);
                 Auth ->
                     NListener = maps:put(
                                   ?AUTHN_BIN,
@@ -437,6 +438,38 @@ pre_config_update(_, UnknownReq, _RawConf) ->
     logger:error("Unknown configuration update request: ~0p", [UnknownReq]),
     {error, badreq}.
 
+badres_gateway(not_found, GwName) ->
+    {error, {badres, #{resource => gateway, gateway => GwName,
+                       reason => not_found}}};
+badres_gateway(already_exist, GwName) ->
+    {error, {badres, #{resource => gateway, gateway => GwName,
+                       reason => already_exist}}}.
+
+badres_listener(not_found, GwName, LType, LName) ->
+    {error, {badres, #{resource => listener, gateway => GwName,
+                       listener => {GwName, LType, LName},
+                       reason => not_found}}};
+badres_listener(already_exist, GwName, LType, LName) ->
+    {error, {badres, #{resource => listener, gateway => GwName,
+                       listener => {GwName, LType, LName},
+                       reason => already_exist}}}.
+
+badres_authn(not_found, GwName) ->
+    {error, {badres, #{resource => authn, gateway => GwName,
+                       reason => not_found}}};
+badres_authn(already_exist, GwName) ->
+    {error, {badres, #{resource => authn, gateway => GwName,
+                       reason => already_exist}}}.
+
+badres_listener_authn(not_found, GwName, LType, LName) ->
+    {error, {badres, #{resource => listener_authn, gateway => GwName,
+                       listener => {GwName, LType, LName},
+                       reason => not_found}}};
+badres_listener_authn(already_exist, GwName, LType, LName) ->
+    {error, {badres, #{resource => listener_authn, gateway => GwName,
+                       listener => {GwName, LType, LName},
+                       reason => already_exist}}}.
+
 -spec post_config_update(list(atom()),
                          emqx_config:update_request(),
                          emqx_config:config(),

apps/emqx_gateway/src/emqx_gateway_http.erl

@@ -23,6 +23,8 @@
 
 -define(AUTHN, ?EMQX_AUTHENTICATION_CONFIG_ROOT_NAME_ATOM).
 
+-import(emqx_gateway_utils, [listener_id/3]).
+
 %% Mgmt APIs - gateway
 -export([ gateways/1
         ]).
@@ -59,10 +61,7 @@
         , with_authn/2
         , with_listener_authn/3
         , checks/2
-        , schema_bad_request/0
+        , reason2resp/1
-        , schema_not_found/0
-        , schema_internal_error/0
-        , schema_no_content/0
         ]).
 
 -type gateway_summary() ::
@@ -131,7 +130,7 @@ current_connections_count(GwName) ->
 get_listeners_status(GwName, Config) ->
     Listeners = emqx_gateway_utils:normalize_config(Config),
     lists:map(fun({Type, LisName, ListenOn, _, _}) ->
-        Name0 = emqx_gateway_utils:listener_id(GwName, Type, LisName),
+        Name0 = listener_id(GwName, Type, LisName),
         Name = {Name0, ListenOn},
         LisO = #{id => Name0, type => Type, name => LisName},
         case catch esockd:listener(Name) of
@@ -223,12 +222,7 @@ remove_authn(GwName, ListenerId) ->
 
 confexp(ok) -> ok;
 confexp({ok, Res}) -> {ok, Res};
-confexp({error, badarg}) ->
+confexp({error, Reason}) -> error(Reason).
-    error({update_conf_error, badarg});
-confexp({error, not_found}) ->
-    error({update_conf_error, not_found});
-confexp({error, already_exist}) ->
-    error({update_conf_error, already_exist}).
 
 %%--------------------------------------------------------------------
 %% Mgmt APIs - clients
@@ -322,6 +316,59 @@ with_channel(GwName, ClientId, Fun) ->
 %% Utils
 %%--------------------------------------------------------------------
 
+-spec reason2resp({atom(), map()} | any()) -> binary() | any().
+reason2resp({badconf, #{key := Key, value := Value, reason := Reason}}) ->
+    fmt400err("Bad config value '~s' for '~s', reason: ~s",
+              [Value, Key, Reason]);
+reason2resp({badres, #{resource := gateway,
+                       gateway := GwName,
+                       reason := not_found}}) ->
+    fmt400err("The ~s gateway is unloaded", [GwName]);
+
+reason2resp({badres, #{resource := gateway,
+                       gateway := GwName,
+                       reason := already_exist}}) ->
+    fmt400err("The ~s gateway has loaded", [GwName]);
+
+reason2resp({badres, #{resource := listener,
+                       listener := {GwName, LType, LName},
+                       reason := not_found}}) ->
+    fmt400err("Listener ~s not found",
+              [listener_id(GwName, LType, LName)]);
+
+reason2resp({badres, #{resource := listener,
+                       listener := {GwName, LType, LName},
+                       reason := already_exist}}) ->
+    fmt400err("The listener ~s of ~s already exist",
+              [listener_id(GwName, LType, LName), GwName]);
+
+reason2resp({badres, #{resource := authn,
+                       gateway := GwName,
+                       reason := not_found}}) ->
+    fmt400err("The authentication not found on ~s", [GwName]);
+
+reason2resp({badres, #{resource := authn,
+                       gateway := GwName,
+                       reason := already_exist}}) ->
+    fmt400err("The authentication already exist on ~s", [GwName]);
+
+reason2resp({badres, #{resource := listener_authn,
+                       listener := {GwName, LType, LName},
+                       reason := not_found}}) ->
+    fmt400err("The authentication not found on ~s",
+              [listener_id(GwName, LType, LName)]);
+
+reason2resp({badres, #{resource := listener_authn,
+                       listener := {GwName, LType, LName},
+                       reason := already_exist}}) ->
+    fmt400err("The authentication already exist on ~s",
+              [listener_id(GwName, LType, LName)]);
+
+reason2resp(R) -> return_http_error(500, R).
+
+fmt400err(Fmt, Args) ->
+    return_http_error(400, io_lib:format(Fmt, Args)).
+
 -spec return_http_error(integer(), any()) -> {integer(), binary()}.
 return_http_error(Code, Msg) ->
     {Code, emqx_json:encode(
@@ -378,19 +425,12 @@ with_gateway(GwName0, Fun) ->
             Path = lists:concat(
                      lists:join(".", lists:map(fun to_list/1, Path0))),
             return_http_error(404, "Resource not found. path: " ++ Path);
-        %% Exceptions from: confexp/1
-        error : {update_conf_error, badarg} ->
-            return_http_error(400, "Bad arguments");
-        error : {update_conf_error, not_found} ->
-            return_http_error(404, "Resource not found");
-        error : {update_conf_error, already_exist} ->
-            return_http_error(400, "Resource already exist");
         Class : Reason : Stk ->
             ?SLOG(error, #{ msg => "uncatched_error"
                           , reason => {Class, Reason}
                           , stacktrace => Stk
                           }),
-            return_http_error(500, {Class, Reason, Stk})
+            reason2resp(Reason)
     end.
 
 -spec checks(list(), map()) -> ok.
@@ -408,20 +448,6 @@ to_list(A) when is_atom(A) ->
 to_list(B) when is_binary(B) ->
     binary_to_list(B).
 
-%%--------------------------------------------------------------------
-%% common schemas
-
-schema_bad_request() ->
-    emqx_mgmt_util:error_schema(
-      <<"Some Params missed">>, ['PARAMETER_MISSED']).
-schema_internal_error() ->
-    emqx_mgmt_util:error_schema(
-      <<"Ineternal Server Error">>, ['INTERNAL_SERVER_ERROR']).
-schema_not_found() ->
-    emqx_mgmt_util:error_schema(<<"Resource Not Found">>).
-schema_no_content() ->
-    #{description => <<"No Content">>}.
-
 %%--------------------------------------------------------------------
 %% Internal funcs
 

apps/emqx_gateway/src/emqx_gateway_insta_sup.erl

@@ -112,7 +112,7 @@ init([Gateway, Ctx, _GwDscrptr]) ->
         true ->
             case cb_gateway_load(State) of
                 {error, Reason} ->
-                    {stop, {load_gateway_failure, Reason}};
+                    {stop, Reason};
                 {ok, NState} ->
                     {ok, NState}
             end
@@ -360,7 +360,7 @@ cb_gateway_unload(State = #state{name = GwName,
                           , reason => {Class, Reason}
                           , stacktrace => Stk
                           }),
-            {error, {Class, Reason, Stk}}
+            {error, Reason}
     after
         _ = do_deinit_authn(State#state.authns)
     end.
@@ -381,7 +381,7 @@ cb_gateway_load(State = #state{name = GwName,
         case CbMod:on_gateway_load(Gateway, NCtx) of
             {error, Reason} ->
                 do_deinit_authn(AuthnNames),
-                throw({callback_return_error, Reason});
+                {error, Reason};
             {ok, ChildPidOrSpecs, GwState} ->
                 ChildPids = start_child_process(ChildPidOrSpecs),
                 {ok, State#state{
@@ -403,7 +403,7 @@ cb_gateway_load(State = #state{name = GwName,
                           , reason => {Class, Reason1}
                           , stacktrace => Stk
                           }),
-            {error, {Class, Reason1, Stk}}
+            {error, Reason1}
     end.
 
 cb_gateway_update(Config,
@@ -412,7 +412,7 @@ cb_gateway_update(Config,
     try
         #{cbkmod := CbMod} = emqx_gateway_registry:lookup(GwName),
         case CbMod:on_gateway_update(Config, detailed_gateway_info(State), GwState) of
-            {error, Reason} -> throw({callback_return_error, Reason});
+            {error, Reason} -> {error, Reason};
             {ok, ChildPidOrSpecs, NGwState} ->
                 %% XXX: Hot-upgrade ???
                 ChildPids = start_child_process(ChildPidOrSpecs),
@@ -430,7 +430,7 @@ cb_gateway_update(Config,
                           , reason => {Class, Reason1}
                           , stacktrace => Stk
                           }),
-            {error, {Class, Reason1, Stk}}
+            {error, Reason1}
     end.
 
 start_child_process([]) -> [];

apps/emqx_gateway/src/emqx_gateway_utils.erl

@@ -90,6 +90,7 @@ childspec(Id, Type, Mod, Args) ->
     -> {ok, pid()}
      | {error, supervisor:startchild_err()}.
 supervisor_ret({ok, Pid, _Info}) -> {ok, Pid};
+supervisor_ret({error, {Reason, _Child}}) -> {error, Reason};
 supervisor_ret(Ret) -> Ret.
 
 -spec find_sup_child(Sup :: pid() | atom(), ChildId :: supervisor:child_id())

apps/emqx_gateway/src/exproto/emqx_exproto_impl.erl

@@ -75,7 +75,13 @@ stop_grpc_server(GwName) ->
 start_grpc_client_channel(_GwName, undefined) ->
     undefined;
 start_grpc_client_channel(GwName, Options = #{address := Address}) ->
-    {Host, Port} = emqx_gateway_utils:parse_address(Address),
+    {Host, Port} = try emqx_gateway_utils:parse_address(Address)
+                   catch error : badarg ->
+                         throw({badconf, #{key => address,
+                                           value => Address,
+                                           reason => illegal_grpc_address
+                                          }})
+                   end,
     case maps:to_list(maps:get(ssl, Options, #{})) of
         [] ->
             SvrAddr = compose_http_uri(http, Host, Port),

apps/emqx_gateway/src/lwm2m/emqx_lwm2m_impl.erl

@@ -50,14 +50,20 @@ unreg() ->
 on_gateway_load(_Gateway = #{ name := GwName,
                               config := Config
                             }, Ctx) ->
-    %% Xml registry
+    XmlDir = maps:get(xml_dir, Config),
-    {ok, RegPid} = emqx_lwm2m_xml_object_db:start_link(maps:get(xml_dir, Config)),
+    case emqx_lwm2m_xml_object_db:start_link(XmlDir) of
-
+        {ok, RegPid} ->
             Listeners = emqx_gateway_utils:normalize_config(Config),
             ListenerPids = lists:map(fun(Lis) ->
                                              start_listener(GwName, Ctx, Lis)
                                      end, Listeners),
-    {ok, ListenerPids, _GwState = #{ctx => Ctx, registry => RegPid}}.
+            {ok, ListenerPids, _GwState = #{ctx => Ctx, registry => RegPid}};
+        {error, Reason} ->
+            throw({badconf, #{ key => xml_dir
+                             , value => XmlDir
+                             , reason => Reason
+                             }})
+    end.
 
 on_gateway_update(Config, Gateway, GwState = #{ctx := Ctx}) ->
     GwName = maps:get(name, Gateway),

apps/emqx_gateway/src/lwm2m/emqx_lwm2m_xml_object_db.erl

@@ -47,6 +47,11 @@
 %% API Function Definitions
 %% ------------------------------------------------------------------
 
+-spec start_link(string())
+    -> {ok, pid()}
+     | ignore
+     | {error, no_xml_files_found}
+     | {error, term()}.
 start_link(XmlDir) ->
     gen_server:start_link({local, ?MODULE}, ?MODULE, [XmlDir], []).
 
@@ -85,8 +90,11 @@ stop() ->
 init([XmlDir]) ->
     _ = ets:new(?LWM2M_OBJECT_DEF_TAB, [set, named_table, protected]),
     _ = ets:new(?LWM2M_OBJECT_NAME_TO_ID_TAB, [set, named_table, protected]),
-    load(XmlDir),
+    case load(XmlDir) of
-    {ok, #state{}}.
+        ok ->
+            {ok, #state{}};
+        {error, Reason} -> {stop, Reason}
+    end.
 
 handle_call(_Request, _From, State) ->
     {reply, ignored, State}.
@@ -116,7 +124,7 @@ load(BaseDir) ->
                     Wild
             end,
     case filelib:wildcard(Wild2) of
-        [] -> error(no_xml_files_found, BaseDir);
+        [] -> {error, no_xml_files_found};
         AllXmlFiles -> load_loop(AllXmlFiles)
     end.
 

apps/emqx_gateway/test/emqx_gateway_conf_SUITE.erl

@@ -245,8 +245,9 @@ t_load_unload_gateway(_) ->
                          ?CONF_STOMP_AUTHN_1,
                          ?CONF_STOMP_LISTENER_1),
     {ok, _} = emqx_gateway_conf:load_gateway(stomp, StompConf1),
-    {error, already_exist} =
+    ?assertMatch(
-        emqx_gateway_conf:load_gateway(stomp, StompConf1),
+       {error, {badres, #{reason := already_exist}}},
+       emqx_gateway_conf:load_gateway(stomp, StompConf1)),
     assert_confs(StompConf1, emqx:get_raw_config([gateway, stomp])),
 
     {ok, _} = emqx_gateway_conf:update_gateway(stomp, StompConf2),
@@ -255,8 +256,9 @@ t_load_unload_gateway(_) ->
     ok = emqx_gateway_conf:unload_gateway(stomp),
     ok = emqx_gateway_conf:unload_gateway(stomp),
 
-    {error, not_found} =
+    ?assertMatch(
-        emqx_gateway_conf:update_gateway(stomp, StompConf2),
+       {error, {badres, #{reason := not_found}}},
+       emqx_gateway_conf:update_gateway(stomp, StompConf2)),
 
     ?assertException(error, {config_not_found, [gateway, stomp]},
                      emqx:get_raw_config([gateway, stomp])),
@@ -280,8 +282,9 @@ t_load_remove_authn(_) ->
 
     ok = emqx_gateway_conf:remove_authn(<<"stomp">>),
 
-    {error, not_found} =
+    ?assertMatch(
-        emqx_gateway_conf:update_authn(<<"stomp">>, ?CONF_STOMP_AUTHN_2),
+       {error, {badres, #{reason := not_found}}},
+       emqx_gateway_conf:update_authn(<<"stomp">>, ?CONF_STOMP_AUTHN_2)),
 
     ?assertException(
        error, {config_not_found, [gateway, stomp, authentication]},
@@ -312,9 +315,10 @@ t_load_remove_listeners(_) ->
     ok = emqx_gateway_conf:remove_listener(
            <<"stomp">>, {<<"tcp">>, <<"default">>}),
 
-    {error, not_found} =
+    ?assertMatch(
+       {error, {badres, #{reason := not_found}}},
        emqx_gateway_conf:update_listener(
-          <<"stomp">>, {<<"tcp">>, <<"default">>}, ?CONF_STOMP_LISTENER_2),
+         <<"stomp">>, {<<"tcp">>, <<"default">>}, ?CONF_STOMP_LISTENER_2)),
 
     ?assertException(
        error, {config_not_found, [gateway, stomp, listeners, tcp, default]},
@@ -352,9 +356,10 @@ t_load_remove_listener_authn(_) ->
     ok = emqx_gateway_conf:remove_authn(
            <<"stomp">>, {<<"tcp">>, <<"default">>}),
 
-    {error, not_found} =
+    ?assertMatch(
+       {error, {badres, #{reason := not_found}}},
        emqx_gateway_conf:update_authn(
-          <<"stomp">>, {<<"tcp">>, <<"default">>}, ?CONF_STOMP_AUTHN_2),
+         <<"stomp">>, {<<"tcp">>, <<"default">>}, ?CONF_STOMP_AUTHN_2)),
 
     Path = [gateway, stomp, listeners, tcp, default, authentication],
     ?assertException(
@@ -426,9 +431,12 @@ t_add_listener_with_certs_content(_) ->
     ok = emqx_gateway_conf:remove_listener(
            <<"stomp">>, {<<"ssl">>, <<"default">>}),
     assert_ssl_confs_files_deleted(SslConf),
-    {error, not_found} =
+
+    ?assertMatch(
+       {error, {badres, #{reason := not_found}}},
        emqx_gateway_conf:update_listener(
-          <<"stomp">>, {<<"ssl">>, <<"default">>}, ?CONF_STOMP_LISTENER_SSL_2),
+         <<"stomp">>, {<<"ssl">>, <<"default">>}, ?CONF_STOMP_LISTENER_SSL_2)),
+
     ?assertException(
        error, {config_not_found, [gateway, stomp, listeners, ssl, default]},
        emqx:get_raw_config([gateway, stomp, listeners, ssl, default])