refactor(config): improve the return value of emqx:update_config/2,3

apps/emqx/src/emqx_config.erl

@@ -82,7 +82,8 @@
     end).
 
 -export_type([update_request/0, raw_config/0, config/0,
-              update_opts/0, update_cmd/0, update_args/0]).
+              update_opts/0, update_cmd/0, update_args/0,
+              update_error/0, update_result/0]).
 
 -type update_request() :: term().
 -type update_cmd() :: {update, update_request()} | remove.
@@ -91,6 +92,13 @@
         rawconf_with_defaults => boolean()
     }.
 -type update_args() :: {update_cmd(), Opts :: update_opts()}.
+-type update_stage() :: pre_config_update | post_config_update.
+-type update_error() :: {update_stage(), module(), term()} | {save_configs, term()} | term().
+-type update_result() :: #{
+    config := emqx_config:config(),
+    raw_config := emqx_config:raw_config(),
+    post_config_update => #{module() => any()}
+}.
 
 %% raw_config() is the config that is NOT parsed and tranlated by hocon schema
 -type raw_config() :: #{binary() => term()} | undefined.

apps/emqx/src/emqx_config_handler.erl

@@ -46,10 +46,10 @@
                     ]).
 
 -callback pre_config_update(emqx_config:update_request(), emqx_config:raw_config()) ->
-    emqx_config:update_request().
+    {ok, emqx_config:update_request()} | {error, term()}.
 
 -callback post_config_update(emqx_config:update_request(), emqx_config:config(),
-    emqx_config:config()) -> any().
+    emqx_config:config()) -> ok | {ok, Result::any()} | {error, Reason::term()}.
 
 -type state() :: #{
     handlers := handlers(),
@@ -60,7 +60,7 @@ start_link() ->
     gen_server:start_link({local, ?MODULE}, ?MODULE, {}, []).
 
 -spec update_config(module(), emqx_config:config_key_path(), emqx_config:update_args()) ->
-    {ok, emqx_config:config(), emqx_config:raw_config()} | {error, term()}.
+    {ok, emqx_config:update_result()} | {error, emqx_config:update_error()}.
 update_config(SchemaModule, ConfKeyPath, UpdateArgs) ->
     gen_server:call(?MODULE, {change_config, SchemaModule, ConfKeyPath, UpdateArgs}).
 
@@ -79,24 +79,23 @@ handle_call({add_child, ConfKeyPath, HandlerName}, _From,
     {reply, ok, State#{handlers =>
         emqx_map_lib:deep_put(ConfKeyPath, Handlers, #{?MOD => HandlerName})}};
 
-handle_call({change_config, SchemaModule, ConfKeyPath, {_Cmd, Opts} = UpdateArgs}, _From,
+handle_call({change_config, SchemaModule, ConfKeyPath, UpdateArgs}, _From,
             #{handlers := Handlers} = State) ->
     OldConf = emqx_config:get([]),
     OldRawConf = emqx_config:get_raw([]),
-    Result = try
-        {NewRawConf, OverrideConf} = process_upadate_request(ConfKeyPath, OldRawConf,
-            Handlers, UpdateArgs),
-        {AppEnvs, CheckedConf} = emqx_config:check_config(SchemaModule, NewRawConf),
-        _ = do_post_config_update(ConfKeyPath, Handlers, OldConf, CheckedConf, UpdateArgs),
-        case emqx_config:save_configs(AppEnvs, CheckedConf, NewRawConf, OverrideConf) of
-            ok -> {ok, emqx_config:get([]), return_rawconf(Opts)};
-            Err -> Err
+    Reply = try
+        case process_update_request(ConfKeyPath, OldRawConf, Handlers, UpdateArgs) of
+            {ok, NewRawConf, OverrideConf} ->
+                check_and_save_configs(SchemaModule, ConfKeyPath, Handlers, NewRawConf, OldConf,
+                    OverrideConf, UpdateArgs);
+            {error, Result} ->
+                {error, Result}
         end
     catch Error:Reason:ST ->
         ?LOG(error, "change_config failed: ~p", [{Error, Reason, ST}]),
         {error, Reason}
     end,
-    {reply, Result, State};
+    {reply, Reply, State};
 
 handle_call(_Request, _From, State) ->
     Reply = ok,
@@ -114,32 +113,56 @@ terminate(_Reason, _State) ->
 code_change(_OldVsn, State, _Extra) ->
     {ok, State}.
 
-process_upadate_request(ConfKeyPath, OldRawConf, _Handlers, {remove, _Opts}) ->
+process_update_request(ConfKeyPath, OldRawConf, _Handlers, {remove, _Opts}) ->
     BinKeyPath = bin_path(ConfKeyPath),
     NewRawConf = emqx_map_lib:deep_remove(BinKeyPath, OldRawConf),
     OverrideConf = emqx_map_lib:deep_remove(BinKeyPath, emqx_config:read_override_conf()),
-    {NewRawConf, OverrideConf};
-process_upadate_request(ConfKeyPath, OldRawConf, Handlers, {{update, UpdateReq}, _Opts}) ->
-    NewRawConf = do_update_config(ConfKeyPath, Handlers, OldRawConf, UpdateReq),
-    OverrideConf = update_override_config(NewRawConf),
-    {NewRawConf, OverrideConf}.
+    {ok, NewRawConf, OverrideConf};
+process_update_request(ConfKeyPath, OldRawConf, Handlers, {{update, UpdateReq}, _Opts}) ->
+    case do_update_config(ConfKeyPath, Handlers, OldRawConf, UpdateReq) of
+        {ok, NewRawConf} ->
+            OverrideConf = update_override_config(NewRawConf),
+            {ok, NewRawConf, OverrideConf};
+        Error -> Error
+    end.
 
 do_update_config([], Handlers, OldRawConf, UpdateReq) ->
     call_pre_config_update(Handlers, OldRawConf, UpdateReq);
 do_update_config([ConfKey | ConfKeyPath], Handlers, OldRawConf, UpdateReq) ->
     SubOldRawConf = get_sub_config(bin(ConfKey), OldRawConf),
     SubHandlers = maps:get(ConfKey, Handlers, #{}),
-    NewUpdateReq = do_update_config(ConfKeyPath, SubHandlers, SubOldRawConf, UpdateReq),
-    call_pre_config_update(Handlers, OldRawConf, #{bin(ConfKey) => NewUpdateReq}).
+    case do_update_config(ConfKeyPath, SubHandlers, SubOldRawConf, UpdateReq) of
+        {ok, NewUpdateReq} ->
+            call_pre_config_update(Handlers, OldRawConf, #{bin(ConfKey) => NewUpdateReq});
+        Error ->
+            Error
+    end.
+
+check_and_save_configs(SchemaModule, ConfKeyPath, Handlers, NewRawConf, OldConf, OverrideConf,
+        UpdateArgs) ->
+    {AppEnvs, CheckedConf} = emqx_config:check_config(SchemaModule, NewRawConf),
+    case do_post_config_update(ConfKeyPath, Handlers, OldConf, CheckedConf, UpdateArgs, #{}) of
+        {ok, Result0} ->
+            case save_configs(AppEnvs, CheckedConf, NewRawConf, OverrideConf, UpdateArgs) of
+                {ok, Result1} ->
+                    {ok, Result1#{post_config_update => Result0}};
+                Error -> Error
+            end;
+        Error -> Error
+    end.
 
-do_post_config_update([], Handlers, OldConf, NewConf, UpdateArgs) ->
-    call_post_config_update(Handlers, OldConf, NewConf, up_req(UpdateArgs));
-do_post_config_update([ConfKey | ConfKeyPath], Handlers, OldConf, NewConf, UpdateArgs) ->
+do_post_config_update([], Handlers, OldConf, NewConf, UpdateArgs, Result) ->
+    call_post_config_update(Handlers, OldConf, NewConf, up_req(UpdateArgs), Result);
+do_post_config_update([ConfKey | ConfKeyPath], Handlers, OldConf, NewConf, UpdateArgs, Result) ->
     SubOldConf = get_sub_config(ConfKey, OldConf),
     SubNewConf = get_sub_config(ConfKey, NewConf),
     SubHandlers = maps:get(ConfKey, Handlers, #{}),
-    _ = do_post_config_update(ConfKeyPath, SubHandlers, SubOldConf, SubNewConf, UpdateArgs),
-    call_post_config_update(Handlers, OldConf, NewConf, up_req(UpdateArgs)).
+    case do_post_config_update(ConfKeyPath, SubHandlers, SubOldConf, SubNewConf, UpdateArgs,
+            Result) of
+        {ok, Result1} ->
+            call_post_config_update(Handlers, OldConf, NewConf, up_req(UpdateArgs), Result1);
+        Error -> Error
+    end.
 
 get_sub_config(ConfKey, Conf) when is_map(Conf) ->
     maps:get(ConfKey, Conf, undefined);
@@ -149,15 +172,30 @@ get_sub_config(_, _Conf) -> %% the Conf is a primitive
 call_pre_config_update(Handlers, OldRawConf, UpdateReq) ->
     HandlerName = maps:get(?MOD, Handlers, undefined),
     case erlang:function_exported(HandlerName, pre_config_update, 2) of
-        true -> HandlerName:pre_config_update(UpdateReq, OldRawConf);
+        true ->
+            case HandlerName:pre_config_update(UpdateReq, OldRawConf) of
+                {ok, NewUpdateReq} -> {ok, NewUpdateReq};
+                {error, Reason} -> {error, {pre_config_update, HandlerName, Reason}}
+            end;
         false -> merge_to_old_config(UpdateReq, OldRawConf)
     end.
 
-call_post_config_update(Handlers, OldConf, NewConf, UpdateReq) ->
+call_post_config_update(Handlers, OldConf, NewConf, UpdateReq, Result) ->
     HandlerName = maps:get(?MOD, Handlers, undefined),
     case erlang:function_exported(HandlerName, post_config_update, 3) of
-        true -> HandlerName:post_config_update(UpdateReq, NewConf, OldConf);
-        false -> ok
+        true ->
+            case HandlerName:post_config_update(UpdateReq, NewConf, OldConf) of
+                ok -> {ok, Result};
+                {ok, Result1} -> {ok, Result#{HandlerName => Result1}};
+                {error, Reason} -> {error, {post_config_update, HandlerName, Reason}}
+            end;
+        false -> {ok, Result}
+    end.
+
+save_configs(AppEnvs, CheckedConf, NewRawConf, OverrideConf, {_Cmd, Opts}) ->
+    case emqx_config:save_configs(AppEnvs, CheckedConf, NewRawConf, OverrideConf) of
+        ok -> {ok, #{config => emqx_config:get([]), raw_config => return_rawconf(Opts)}};
+        {error, Reason} -> {error, {save_configs, Reason}}
     end.
 
 %% The default callback of config handlers
@@ -166,9 +204,9 @@ call_post_config_update(Handlers, OldConf, NewConf, UpdateReq) ->
 %%   2. either the old or the new config is not of map type
 %% the behaviour is merging the new the config to the old config if they are maps.
 merge_to_old_config(UpdateReq, RawConf) when is_map(UpdateReq), is_map(RawConf) ->
-    maps:merge(RawConf, UpdateReq);
+    {ok, maps:merge(RawConf, UpdateReq)};
 merge_to_old_config(UpdateReq, _RawConf) ->
-    UpdateReq.
+    {ok, UpdateReq}.
 
 update_override_config(RawConf) ->
     OldConf = emqx_config:read_override_conf(),