Merge pull request #12269 from HJianBo/return-detailed-error-message

chore: improve the error message if the parameter invalid
emqx · Jan 10, 2024 · 3706c70 · 3706c70
2 parents 7b211b5 + 6ab8e31
commit 3706c70
Show file tree

Hide file tree

Showing 4 changed files with 39 additions and 8 deletions.
diff --git a/apps/emqx_management/src/emqx_mgmt_api.erl b/apps/emqx_management/src/emqx_mgmt_api.erl
@@ -206,14 +206,19 @@ cluster_query(Tab, QString, QSchema, MsFun, FmtFun, Options) ->
         false ->
             {error, page_limit_invalid};
         Meta ->
-            {_CodCnt, NQString} = parse_qstring(QString, QSchema),
-            Nodes = emqx:running_nodes(),
-            ResultAcc = init_query_result(),
-            QueryState = init_query_state(Tab, NQString, MsFun, Meta, Options),
-            NResultAcc = do_cluster_query(
-                Nodes, QueryState, ResultAcc
-            ),
-            format_query_result(FmtFun, Meta, NResultAcc)
+            try
+                {_CodCnt, NQString} = parse_qstring(QString, QSchema),
+                Nodes = emqx:running_nodes(),
+                ResultAcc = init_query_result(),
+                QueryState = init_query_state(Tab, NQString, MsFun, Meta, Options),
+                NResultAcc = do_cluster_query(
+                    Nodes, QueryState, ResultAcc
+                ),
+                format_query_result(FmtFun, Meta, NResultAcc)
+            catch
+                throw:{bad_value_type, {Key, ExpectedType, AcutalValue}} ->
+                    {error, invalid_query_string_param, {Key, ExpectedType, AcutalValue}}
+            end
     end.
 
 %% @private

diff --git a/apps/emqx_management/src/emqx_mgmt_api_clients.erl b/apps/emqx_management/src/emqx_mgmt_api_clients.erl
@@ -699,6 +699,14 @@ list_clients(QString) ->
     case Result of
         {error, page_limit_invalid} ->
             {400, #{code => <<"INVALID_PARAMETER">>, message => <<"page_limit_invalid">>}};
+        {error, invalid_query_string_param, {Key, ExpectedType, AcutalValue}} ->
+            Message = list_to_binary(
+                io_lib:format(
+                    "the ~s parameter expected type is ~s, but the value is ~s",
+                    [Key, ExpectedType, emqx_utils_conv:str(AcutalValue)]
+                )
+            ),
+            {400, #{code => <<"INVALID_PARAMETER">>, message => Message}};
         {error, Node, {badrpc, R}} ->
             Message = list_to_binary(io_lib:format("bad rpc call ~p, Reason ~p", [Node, R])),
             {500, #{code => <<"NODE_DOWN">>, message => Message}};

diff --git a/apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl
@@ -167,6 +167,23 @@ t_clients(_) ->
     AfterKickoutResponse1 = emqx_mgmt_api_test_util:request_api(get, Client1Path),
     ?assertEqual({error, {"HTTP/1.1", 404, "Not Found"}}, AfterKickoutResponse1).
 
+t_clients_bad_value_type(_) ->
+    %% get /clients
+    AuthHeader = [emqx_common_test_http:default_auth_header()],
+    ClientsPath = emqx_mgmt_api_test_util:api_path(["clients"]),
+    QsString = cow_qs:qs([{<<"ip_address">>, <<"127.0.0.1:8080">>}]),
+    {ok, 400, Resp} = emqx_mgmt_api_test_util:request_api(
+        get, ClientsPath, QsString, AuthHeader, [], #{compatible_mode => true}
+    ),
+    ?assertMatch(
+        #{
+            <<"code">> := <<"INVALID_PARAMETER">>,
+            <<"message">> :=
+                <<"the ip_address parameter expected type is ip, but the value is 127.0.0.1:8080">>
+        },
+        emqx_utils_json:decode(Resp, [return_maps])
+    ).
+
 t_authz_cache(_) ->
     ClientId = <<"client_authz">>,
 

diff --git a/changes/ce/fix-12269.en.md b/changes/ce/fix-12269.en.md
@@ -0,0 +1 @@
+Returns 400 and more detailed error messages instead of 500 when query string validation fails in the `/clients` interface.