Просмотр исходного кода

fix: return human readable error message for most common cases

Stefan Strigler 2 лет назад
Родитель
Сommit
570bf165af

+ 27 - 8
apps/emqx_bridge/src/emqx_bridge_api.erl

@@ -420,6 +420,9 @@ schema("/bridges/:id/:operation") ->
             ],
             responses => #{
                 204 => <<"Operation success">>,
+                400 => error_schema(
+                    'BAD_REQUEST', "Problem with configuration of external service"
+                ),
                 404 => error_schema('NOT_FOUND', "Bridge not found or invalid operation"),
                 501 => error_schema('NOT_IMPLEMENTED', "Not Implemented"),
                 503 => error_schema('SERVICE_UNAVAILABLE', "Service unavailable")
@@ -440,7 +443,10 @@ schema("/nodes/:node/bridges/:id/:operation") ->
             ],
             responses => #{
                 204 => <<"Operation success">>,
-                400 => error_schema('BAD_REQUEST', "Forbidden operation, bridge not enabled"),
+                400 => error_schema(
+                    'BAD_REQUEST',
+                    "Problem with configuration of external service or bridge not enabled"
+                ),
                 404 => error_schema('NOT_FOUND', "Bridge not found or invalid operation"),
                 501 => error_schema('NOT_IMPLEMENTED', "Not Implemented"),
                 503 => error_schema('SERVICE_UNAVAILABLE', "Service unavailable")
@@ -555,8 +561,8 @@ schema("/bridges_probe") ->
             case emqx_bridge_resource:create_dry_run(ConnType, maps:remove(<<"type">>, Params)) of
                 ok ->
                     {204};
-                {error, Error} ->
-                    {400, error_msg('TEST_FAILED', Error)}
+                {error, Reason} when not is_tuple(Reason); element(1, Reason) =/= 'exit' ->
+                    {400, error_msg('TEST_FAILED', to_hr_reason(Reason))}
             end;
         BadRequest ->
             BadRequest
@@ -577,8 +583,8 @@ do_lookup_from_all_nodes(BridgeType, BridgeName, SuccCode, FormatFun) ->
             {SuccCode, FormatFun([R || {ok, R} <- Results])};
         {ok, [{error, not_found} | _]} ->
             ?BRIDGE_NOT_FOUND(BridgeType, BridgeName);
-        {error, ErrL} ->
-            {500, error_msg('INTERNAL_ERROR', ErrL)}
+        {error, Reason} ->
+            {500, error_msg('INTERNAL_ERROR', Reason)}
     end.
 
 lookup_from_local_node(BridgeType, BridgeName) ->
@@ -885,7 +891,7 @@ is_ok(ResL) ->
         )
     of
         [] -> {ok, [Res || {ok, Res} <- ResL]};
-        ErrL -> {error, ErrL}
+        ErrL -> hd(ErrL)
     end.
 
 filter_out_request_body(Conf) ->
@@ -934,8 +940,8 @@ call_operation(NodeOrAll, OperFunc, Args) ->
                         )
                     )
                 )};
-        {error, Reason} ->
-            {500, error_msg('INTERNAL_ERROR', Reason)}
+        {error, Reason} when not is_tuple(Reason); element(1, Reason) =/= 'exit' ->
+            {400, error_msg('BAD_REQUEST', to_hr_reason(Reason))}
     end.
 
 maybe_try_restart(all, start_bridges_to_all_nodes, Args) ->
@@ -969,6 +975,19 @@ supported_versions(start_bridge_to_node) -> [2];
 supported_versions(start_bridges_to_all_nodes) -> [2];
 supported_versions(_Call) -> [1, 2].
 
+to_hr_reason(nxdomain) ->
+    <<"Host not found">>;
+to_hr_reason(econnrefused) ->
+    <<"Connection refused">>;
+to_hr_reason({unauthorized_client, _}) ->
+    <<"Unauthorized client">>;
+to_hr_reason({not_authorized, _}) ->
+    <<"Not authorized">>;
+to_hr_reason({malformed_username_or_password, _}) ->
+    <<"Malformed username or password">>;
+to_hr_reason(Reason) ->
+    Reason.
+
 redact(Term) ->
     emqx_misc:redact(Term).
 

+ 76 - 3
apps/emqx_bridge/test/emqx_bridge_api_SUITE.erl

@@ -73,7 +73,7 @@ init_per_suite(Config) ->
     _ = application:stop(emqx_resource),
     _ = application:stop(emqx_connector),
     ok = emqx_mgmt_api_test_util:init_suite(
-        [emqx_rule_engine, emqx_bridge]
+        [emqx_rule_engine, emqx_bridge, emqx_authn]
     ),
     ok = emqx_common_test_helpers:load_config(
         emqx_rule_engine_schema,
@@ -83,7 +83,8 @@ init_per_suite(Config) ->
     Config.
 
 end_per_suite(_Config) ->
-    emqx_mgmt_api_test_util:end_suite([emqx_rule_engine, emqx_bridge]),
+    emqx_mgmt_api_test_util:end_suite([emqx_rule_engine, emqx_bridge, emqx_authn]),
+    mria:clear_table(emqx_authn_mnesia),
     ok.
 
 init_per_testcase(t_broken_bpapi_vsn, Config) ->
@@ -720,11 +721,83 @@ t_bridges_probe(Config) ->
     ?assertMatch(
         #{
             <<"code">> := <<"TEST_FAILED">>,
-            <<"message">> := <<"econnrefused">>
+            <<"message">> := <<"Connection refused">>
         },
         jsx:decode(ConnRefused)
     ),
 
+    {ok, 400, HostNotFound} = request(
+        post,
+        uri(["bridges_probe"]),
+        ?MQTT_BRIDGE(<<"nohost:2883">>)
+    ),
+    ?assertMatch(
+        #{
+            <<"code">> := <<"TEST_FAILED">>,
+            <<"message">> := <<"Host not found">>
+        },
+        jsx:decode(HostNotFound)
+    ),
+
+    AuthnConfig = #{
+        <<"mechanism">> => <<"password_based">>,
+        <<"backend">> => <<"built_in_database">>,
+        <<"user_id_type">> => <<"username">>
+    },
+    Chain = 'mqtt:global',
+    emqx:update_config(
+        [authentication],
+        {create_authenticator, Chain, AuthnConfig}
+    ),
+    User = #{user_id => <<"u">>, password => <<"p">>},
+    AuthenticatorID = <<"password_based:built_in_database">>,
+    {ok, _} = emqx_authentication:add_user(
+        Chain,
+        AuthenticatorID,
+        User
+    ),
+
+    {ok, 400, Unauthorized} = request(
+        post,
+        uri(["bridges_probe"]),
+        ?MQTT_BRIDGE(<<"127.0.0.1:1883">>)#{<<"proto_ver">> => <<"v4">>}
+    ),
+    ?assertMatch(
+        #{
+            <<"code">> := <<"TEST_FAILED">>,
+            <<"message">> := <<"Unauthorized client">>
+        },
+        jsx:decode(Unauthorized)
+    ),
+
+    {ok, 400, Malformed} = request(
+        post,
+        uri(["bridges_probe"]),
+        ?MQTT_BRIDGE(<<"127.0.0.1:1883">>)#{
+            <<"proto_ver">> => <<"v4">>, <<"password">> => <<"mySecret">>, <<"username">> => <<"u">>
+        }
+    ),
+    ?assertMatch(
+        #{
+            <<"code">> := <<"TEST_FAILED">>,
+            <<"message">> := <<"Malformed username or password">>
+        },
+        jsx:decode(Malformed)
+    ),
+
+    {ok, 400, NotAuthorized} = request(
+        post,
+        uri(["bridges_probe"]),
+        ?MQTT_BRIDGE(<<"127.0.0.1:1883">>)
+    ),
+    ?assertMatch(
+        #{
+            <<"code">> := <<"TEST_FAILED">>,
+            <<"message">> := <<"Not authorized">>
+        },
+        jsx:decode(NotAuthorized)
+    ),
+
     {ok, 400, BadReq} = request(
         post,
         uri(["bridges_probe"]),

+ 1 - 0
changes/ce/fix-10066.en.md

@@ -0,0 +1 @@
+Return human readable error message for `/briges_probe` and `[/node/:node]/bridges/:id/:operation` API calls and set HTTP status code to `400` instead of `500`.

+ 1 - 0
changes/ce/fix-10066.zh.md

@@ -0,0 +1 @@
+为 `/briges_probe` 和 `[/node/:node]/bridges/:id/:operation` 的 API 调用返回人类可读的错误信息,并将 HTTP 状态代码设置为 `400` 而不是 `500`。