From af3519698c588ba4fbb552feff5e7db03ea334a6 Mon Sep 17 00:00:00 2001 From: firest Date: Mon, 18 Apr 2022 11:54:41 +0800 Subject: [PATCH] fix(exhook): use error code to replace exception in the URL parse we should return 400 and why to the API caller, not crash and return 500 --- apps/emqx_exhook/src/emqx_exhook_server.erl | 66 +++++++++++---------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/apps/emqx_exhook/src/emqx_exhook_server.erl b/apps/emqx_exhook/src/emqx_exhook_server.erl index 002e9e5b0..36f5f403a 100644 --- a/apps/emqx_exhook/src/emqx_exhook_server.erl +++ b/apps/emqx_exhook/src/emqx_exhook_server.erl @@ -91,35 +91,39 @@ load(_Name, #{enable := false}) -> disable; load(Name, #{request_timeout := Timeout, failed_action := FailedAction} = Opts) -> ReqOpts = #{timeout => Timeout, failed_action => FailedAction}, - {SvrAddr, ClientOpts} = channel_opts(Opts), - case - emqx_exhook_sup:start_grpc_client_channel( - Name, - SvrAddr, - ClientOpts - ) - of - {ok, _ChannPoolPid} -> - case do_init(Name, ReqOpts) of - {ok, HookSpecs} -> - %% Register metrics - Prefix = lists:flatten(io_lib:format("exhook.~ts.", [Name])), - ensure_metrics(Prefix, HookSpecs), - %% Ensure hooks - ensure_hooks(HookSpecs), - {ok, #{ - name => Name, - options => ReqOpts, - channel => _ChannPoolPid, - hookspec => HookSpecs, - prefix => Prefix - }}; - {error, Reason} -> - emqx_exhook_sup:stop_grpc_client_channel(Name), - {load_error, Reason} + case channel_opts(Opts) of + {ok, {SvrAddr, ClientOpts}} -> + case + emqx_exhook_sup:start_grpc_client_channel( + Name, + SvrAddr, + ClientOpts + ) + of + {ok, _ChannPoolPid} -> + case do_init(Name, ReqOpts) of + {ok, HookSpecs} -> + %% Register metrics + Prefix = lists:flatten(io_lib:format("exhook.~ts.", [Name])), + ensure_metrics(Prefix, HookSpecs), + %% Ensure hooks + ensure_hooks(HookSpecs), + {ok, #{ + name => Name, + options => ReqOpts, + channel => _ChannPoolPid, + hookspec => HookSpecs, + prefix => Prefix + }}; + {error, Reason} -> + emqx_exhook_sup:stop_grpc_client_channel(Name), + {load_error, Reason} + end; + {error, _} = E -> + E end; - {error, _} = E -> - E + Error -> + Error end. %% @private @@ -130,7 +134,7 @@ channel_opts(Opts = #{url := URL}) -> ), case uri_string:parse(URL) of #{scheme := <<"http">>, host := Host, port := Port} -> - {format_http_uri("http", Host, Port), ClientOpts}; + {ok, {format_http_uri("http", Host, Port), ClientOpts}}; #{scheme := <<"https">>, host := Host, port := Port} -> SslOpts = case maps:get(ssl, Opts, undefined) of @@ -154,9 +158,9 @@ channel_opts(Opts = #{url := URL}) -> transport_opts => SslOpts } }, - {format_http_uri("https", Host, Port), NClientOpts}; + {ok, {format_http_uri("https", Host, Port), NClientOpts}}; Error -> - error({bad_server_url, URL, Error}) + {error, {bad_server_url, URL, Error}} end. format_http_uri(Scheme, Host, Port) ->