Merge pull request #13913 from thalesmg/20241001-m-fix-thrown-msg-v2-api

fix(action/source api): return thrown messages

apps/emqx_bridge/src/emqx_bridge_v2.erl

@@ -1174,10 +1174,14 @@ post_config_update([ConfRootKey, BridgeType, BridgeName], _Req, NewConf, OldConf
         ok ->
             ok;
         {error, timeout} ->
-            throw(<<
-                "Timed out trying to remove action or source.  Please try again and,"
-                " if the error persists, try disabling the connector before retrying."
-            >>);
+            ErrorContext = #{
+                error => uninstall_timeout,
+                reason => <<
+                    "Timed out trying to remove action or source.  Please try again and,"
+                    " if the error persists, try disabling the connector before retrying."
+                >>
+            },
+            throw(ErrorContext);
         {error, not_found} ->
             %% Should not happen, unless config is inconsistent.
             throw(<<"Referenced connector not found">>)

apps/emqx_bridge/src/emqx_bridge_v2_api.erl

@@ -292,7 +292,8 @@ schema("/actions/:id") ->
             responses => #{
                 200 => actions_get_response_body_schema(),
                 404 => error_schema('NOT_FOUND', "Bridge not found"),
-                400 => error_schema('BAD_REQUEST', "Update bridge failed")
+                400 => error_schema('BAD_REQUEST', "Update bridge failed"),
+                503 => error_schema('SERVICE_UNAVAILABLE', "Service unavailable")
             }
         },
         delete => #{
@@ -503,7 +504,8 @@ schema("/sources/:id") ->
             responses => #{
                 200 => sources_get_response_body_schema(),
                 404 => error_schema('NOT_FOUND', "Source not found"),
-                400 => error_schema('BAD_REQUEST', "Update source failed")
+                400 => error_schema('BAD_REQUEST', "Update source failed"),
+                503 => error_schema('SERVICE_UNAVAILABLE', "Service unavailable")
             }
         },
         delete => #{
@@ -1446,6 +1448,10 @@ do_create_or_update_bridge(ConfRootKey, BridgeType, BridgeName, Conf, HttpStatus
             PreOrPostConfigUpdate =:= post_config_update
         ->
             ?BAD_REQUEST(emqx_utils_api:to_json(redact(Reason)));
+        {error, Reason} when is_binary(Reason) ->
+            ?BAD_REQUEST(Reason);
+        {error, #{error := uninstall_timeout} = Reason} ->
+            ?SERVICE_UNAVAILABLE(emqx_utils_api:to_json(redact(Reason)));
         {error, Reason} when is_map(Reason) ->
             ?BAD_REQUEST(emqx_utils_api:to_json(redact(Reason)))
     end.

apps/emqx_bridge/test/emqx_bridge_v2_api_SUITE.erl

@@ -636,6 +636,24 @@ create_action_api(Name, Type, Params) ->
     ]),
     emqx_mgmt_api_test_util:simplify_result(Res).
 
+update_action_api(Name, Type, Params) ->
+    Res = emqx_bridge_v2_testlib:update_bridge_api([
+        {bridge_kind, action},
+        {action_type, Type},
+        {action_name, Name},
+        {action_config, Params}
+    ]),
+    emqx_mgmt_api_test_util:simplify_result(Res).
+
+update_source_api(Name, Type, Params) ->
+    Res = emqx_bridge_v2_testlib:update_bridge_api([
+        {bridge_kind, source},
+        {source_type, Type},
+        {source_name, Name},
+        {source_config, Params}
+    ]),
+    emqx_mgmt_api_test_util:simplify_result(Res).
+
 list_sources_api() ->
     Res = emqx_bridge_v2_testlib:list_sources_http_api(),
     emqx_mgmt_api_test_util:simplify_result(Res).
@@ -1908,3 +1926,83 @@ t_kind_dependencies(Config) when is_list(Config) ->
         []
     ),
     ok.
+
+%% Verifies that we return thrown messages as is to the API.
+t_thrown_messages(matrix) ->
+    [
+        [single, actions],
+        [single, sources]
+    ];
+t_thrown_messages(Config) when is_list(Config) ->
+    meck:expect(?CONNECTOR_IMPL, on_remove_channel, fun(_ConnResId, ConnState, _ActionResid) ->
+        timer:sleep(20_000),
+        {ok, ConnState}
+    end),
+    ?check_trace(
+        begin
+            [_SingleOrCluster, Kind | _] = group_path(Config),
+            ConnectorType = ?SOURCE_CONNECTOR_TYPE,
+            ConnectorName = <<"c">>,
+            {ok, {{_, 201, _}, _, _}} =
+                emqx_bridge_v2_testlib:create_connector_api([
+                    {connector_config, source_connector_create_config(#{})},
+                    {connector_name, ConnectorName},
+                    {connector_type, ConnectorType}
+                ]),
+            do_t_thrown_messages(Kind, Config, ConnectorName),
+            meck:expect(?CONNECTOR_IMPL, on_remove_channel, 3, {ok, connector_state}),
+            ok
+        end,
+        []
+    ),
+    ok.
+
+do_t_thrown_messages(actions, _Config, ConnectorName) ->
+    Name = <<"a1">>,
+    %% MQTT
+    Type = ?SOURCE_TYPE,
+    CreateConfig = mqtt_action_create_config(#{
+        <<"connector">> => ConnectorName
+    }),
+    {201, _} = create_action_api(
+        Name,
+        Type,
+        CreateConfig
+    ),
+    UpdateConfig = maps:remove(<<"type">>, CreateConfig),
+    ?assertMatch(
+        {503, #{
+            <<"message">> :=
+                #{<<"reason">> := <<"Timed out trying to remove", _/binary>>}
+        }},
+        update_action_api(
+            Name,
+            Type,
+            UpdateConfig
+        )
+    ),
+    ok;
+do_t_thrown_messages(sources, _Config, ConnectorName) ->
+    Name = <<"s1">>,
+    Type = ?SOURCE_TYPE,
+    CreateConfig = source_create_config(#{
+        <<"connector">> => ConnectorName
+    }),
+    {201, _} = create_source_api(
+        Name,
+        Type,
+        CreateConfig
+    ),
+    UpdateConfig = maps:remove(<<"type">>, CreateConfig),
+    ?assertMatch(
+        {503, #{
+            <<"message">> :=
+                #{<<"reason">> := <<"Timed out trying to remove", _/binary>>}
+        }},
+        update_source_api(
+            Name,
+            Type,
+            UpdateConfig
+        )
+    ),
+    ok.

changes/ce/fix-13913.en.md

@@ -0,0 +1 @@
+Fixed an issue with the actions and source HTTP APIs in which a 500 status code would be return if a time out occurred while trying to update or delete a resource.