Browse Source

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
firest 3 years atrás
parent
commit
af3519698c
1 changed files with 35 additions and 31 deletions
  1. 35 31
      apps/emqx_exhook/src/emqx_exhook_server.erl

+ 35 - 31
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) ->