From c3e6f3c3b213c9623fdc1aa2e348e98047f264ed Mon Sep 17 00:00:00 2001 From: Gilbert Date: Thu, 28 Feb 2019 09:54:28 +0800 Subject: [PATCH] Improve try catch syntax (#2263) * Replace case catch.. with try ... catch Prior to this change, case catch ... would cost a lot of performance because it would retrieve the whole stacktrace. However, try...catch will not retrieve the whole stacktrace. So try...catch syntax is better. --- include/logger.hrl | 1 - src/emqx_access_control.erl | 12 +++++++----- src/emqx_acl_internal.erl | 12 +++++++----- src/emqx_bridge.erl | 28 ++++++++++++++++------------ src/emqx_keepalive.erl | 15 ++++++++------- src/emqx_mountpoint.erl | 11 +++++++---- src/emqx_protocol.erl | 13 ++++++------- src/emqx_vm.erl | 34 +++++++++++++++++++++------------- 8 files changed, 72 insertions(+), 54 deletions(-) diff --git a/include/logger.hrl b/include/logger.hrl index a2ee404d9..f87668cc7 100644 --- a/include/logger.hrl +++ b/include/logger.hrl @@ -39,4 +39,3 @@ begin (logger:log(Level,#{},#{report_cb => fun(_) -> {(Format), (Args)} end})) end). - diff --git a/src/emqx_access_control.erl b/src/emqx_access_control.erl index 8b38889e7..2af522ef0 100644 --- a/src/emqx_access_control.erl +++ b/src/emqx_access_control.erl @@ -17,6 +17,7 @@ -behaviour(gen_server). -include("emqx.hrl"). +-include("logger.hrl"). -export([start_link/0]). -export([authenticate/2]). @@ -68,7 +69,7 @@ authenticate(Credentials, _Password, []) -> end; authenticate(Credentials, Password, [{Mod, State, _Seq} | Mods]) -> - case catch Mod:check(Credentials, Password, State) of + try Mod:check(Credentials, Password, State) of ok -> ok; {ok, IsSuper} when is_boolean(IsSuper) -> {ok, #{is_superuser => IsSuper}}; @@ -79,9 +80,11 @@ authenticate(Credentials, Password, [{Mod, State, _Seq} | Mods]) -> ignore -> authenticate(Credentials, Password, Mods); {error, Reason} -> - {error, Reason}; - {'EXIT', Error} -> - {error, Error} + {error, Reason} + catch + error:Reason:StackTrace -> + ?LOG(error, "Authenticate failed. StackTrace: ~p", [StackTrace]), + {error, Reason} end. %% @doc Check ACL @@ -206,4 +209,3 @@ code_change(_OldVsn, State, _Extra) -> reply(Reply, State) -> {reply, Reply, State}. - diff --git a/src/emqx_acl_internal.erl b/src/emqx_acl_internal.erl index 6d5b9885e..57cc12b73 100644 --- a/src/emqx_acl_internal.erl +++ b/src/emqx_acl_internal.erl @@ -17,6 +17,7 @@ -behaviour(emqx_acl_mod). -include("emqx.hrl"). +-include("logger.hrl"). -export([all_rules/0]). @@ -63,7 +64,7 @@ load_rules_from_file(AclFile) -> emqx_logger:error("[ACL_INTERNAL] Failed to read ~s: ~p", [AclFile, Reason]), {error, Reason} end. - + filter(_PubSub, {allow, all}) -> true; filter(_PubSub, {deny, all}) -> @@ -105,17 +106,18 @@ match(Credentials, Topic, [Rule|Rules]) -> -spec(reload_acl(state()) -> ok | {error, term()}). reload_acl(#{acl_file := AclFile}) -> - case catch load_rules_from_file(AclFile) of + try load_rules_from_file(AclFile) of ok -> emqx_logger:info("Reload acl_file ~s successfully", [AclFile]), ok; {error, Error} -> - {error, Error}; - {'EXIT', Error} -> {error, Error} + catch + error:Reason:StackTrace -> + ?LOG(error, "Reload acl failed. StackTrace: ~p", [StackTrace]), + {error, Reason} end. -spec(description() -> string()). description() -> "Internal ACL with etc/acl.conf". - diff --git a/src/emqx_bridge.erl b/src/emqx_bridge.erl index acfe844f1..1ee5612e6 100644 --- a/src/emqx_bridge.erl +++ b/src/emqx_bridge.erl @@ -60,19 +60,21 @@ show_forwards(Name) -> -spec(add_forward(atom(), binary()) -> ok | {error, already_exists | validate_fail}). add_forward(Name, Topic) -> - case catch emqx_topic:validate({filter, Topic}) of + try emqx_topic:validate({filter, Topic}) of true -> - gen_server:call(name(Name), {add_forward, Topic}); - {'EXIT', _Reason} -> + gen_server:call(name(Name), {add_forward, Topic}) + catch + _Error:_Reason -> {error, validate_fail} end. -spec(del_forward(atom(), binary()) -> ok | {error, validate_fail}). del_forward(Name, Topic) -> - case catch emqx_topic:validate({filter, Topic}) of + try emqx_topic:validate({filter, Topic}) of true -> - gen_server:call(name(Name), {del_forward, Topic}); - _ -> + gen_server:call(name(Name), {del_forward, Topic}) + catch + _Error:_Reason -> {error, validate_fail} end. @@ -82,19 +84,21 @@ show_subscriptions(Name) -> -spec(add_subscription(atom(), binary(), integer()) -> ok | {error, already_exists | validate_fail}). add_subscription(Name, Topic, QoS) -> - case catch emqx_topic:validate({filter, Topic}) of + try emqx_topic:validate({filter, Topic}) of true -> - gen_server:call(name(Name), {add_subscription, Topic, QoS}); - {'EXIT', _Reason} -> + gen_server:call(name(Name), {add_subscription, Topic, QoS}) + catch + _Error:_Reason -> {error, validate_fail} end. -spec(del_subscription(atom(), binary()) -> ok | {error, validate_fail}). del_subscription(Name, Topic) -> - case catch emqx_topic:validate({filter, Topic}) of + try emqx_topic:validate({filter, Topic}) of true -> - gen_server:call(name(Name), {del_subscription, Topic}); - _ -> + gen_server:call(name(Name), {del_subscription, Topic}) + catch + error:_Reason -> {error, validate_fail} end. diff --git a/src/emqx_keepalive.erl b/src/emqx_keepalive.erl index bd1b9b9ef..4cc71764a 100644 --- a/src/emqx_keepalive.erl +++ b/src/emqx_keepalive.erl @@ -27,21 +27,22 @@ start(_, 0, _) -> {ok, #keepalive{}}; start(StatFun, TimeoutSec, TimeoutMsg) -> - case catch StatFun() of + try StatFun() of {ok, StatVal} -> {ok, #keepalive{statfun = StatFun, statval = StatVal, tsec = TimeoutSec, tmsg = TimeoutMsg, tref = timer(TimeoutSec, TimeoutMsg)}}; {error, Error} -> - {error, Error}; - {'EXIT', Reason} -> + {error, Error} + catch + _Error:Reason -> {error, Reason} end. %% @doc Check keepalive, called when timeout... -spec(check(keepalive()) -> {ok, keepalive()} | {error, term()}). check(KeepAlive = #keepalive{statfun = StatFun, statval = LastVal, repeat = Repeat}) -> - case catch StatFun() of + try StatFun() of {ok, NewVal} -> if NewVal =/= LastVal -> {ok, resume(KeepAlive#keepalive{statval = NewVal, repeat = 0})}; @@ -51,8 +52,9 @@ check(KeepAlive = #keepalive{statfun = StatFun, statval = LastVal, repeat = Repe {error, timeout} end; {error, Error} -> - {error, Error}; - {'EXIT', Reason} -> + {error, Error} + catch + _Error:Reason -> {error, Reason} end. @@ -69,4 +71,3 @@ cancel(_) -> timer(Secs, Msg) -> erlang:send_after(timer:seconds(Secs), self(), Msg). - diff --git a/src/emqx_mountpoint.erl b/src/emqx_mountpoint.erl index 544b986c1..8b85ebb94 100644 --- a/src/emqx_mountpoint.erl +++ b/src/emqx_mountpoint.erl @@ -15,6 +15,7 @@ -module(emqx_mountpoint). -include("emqx.hrl"). +-include("logger.hrl"). -export([mount/2, unmount/2]). -export([replvar/2]). @@ -33,9 +34,12 @@ mount(MountPoint, TopicFilters) when is_list(TopicFilters) -> unmount(undefined, Msg) -> Msg; unmount(MountPoint, Msg = #message{topic = Topic}) -> - case catch split_binary(Topic, byte_size(MountPoint)) of - {MountPoint, Topic1} -> Msg#message{topic = Topic1}; - _Other -> Msg + try split_binary(Topic, byte_size(MountPoint)) of + {MountPoint, Topic1} -> Msg#message{topic = Topic1} + catch + _Error:Reason -> + ?LOG(error, "Unmount error : ~p", [Reason]), + Msg end. replvar(undefined, _Vars) -> @@ -49,4 +53,3 @@ feed_var({<<"%u">>, undefined}, MountPoint) -> MountPoint; feed_var({<<"%u">>, Username}, MountPoint) -> emqx_topic:feed_var(<<"%u">>, Username, MountPoint). - diff --git a/src/emqx_protocol.erl b/src/emqx_protocol.erl index c6260168c..db1fbc57c 100644 --- a/src/emqx_protocol.erl +++ b/src/emqx_protocol.erl @@ -245,22 +245,22 @@ received(Packet = ?PACKET(Type), PState) -> process_packet(Packet1, inc_stats(recv, Type, PState2)) end catch - error : protocol_error -> + error:protocol_error -> deliver({disconnect, ?RC_PROTOCOL_ERROR}, PState1), {error, protocol_error, PState}; - error : subscription_identifier_invalid -> + error:subscription_identifier_invalid -> deliver({disconnect, ?RC_SUBSCRIPTION_IDENTIFIERS_NOT_SUPPORTED}, PState1), {error, subscription_identifier_invalid, PState1}; - error : topic_alias_invalid -> + error:topic_alias_invalid -> deliver({disconnect, ?RC_TOPIC_ALIAS_INVALID}, PState1), {error, topic_alias_invalid, PState1}; - error : topic_filters_invalid -> + error:topic_filters_invalid -> deliver({disconnect, ?RC_TOPIC_FILTER_INVALID}, PState1), {error, topic_filters_invalid, PState1}; - error : topic_name_invalid -> + error:topic_name_invalid -> deliver({disconnect, ?RC_TOPIC_FILTER_INVALID}, PState1), {error, topic_filters_invalid, PState1}; - error : Reason -> + error:Reason -> deliver({disconnect, ?RC_MALFORMED_PACKET}, PState1), {error, Reason, PState1} end. @@ -974,4 +974,3 @@ reason_codes_compat(unsuback, _ReasonCodes, _ProtoVer) -> undefined; reason_codes_compat(PktType, ReasonCodes, _ProtoVer) -> [emqx_reason_codes:compat(PktType, RC) || RC <- ReasonCodes]. - diff --git a/src/emqx_vm.erl b/src/emqx_vm.erl index 85019d8c5..0e12067c0 100644 --- a/src/emqx_vm.erl +++ b/src/emqx_vm.erl @@ -351,30 +351,38 @@ port_info(PortTerm, memory_used) -> port_info(PortTerm, specific) -> Port = transform_port(PortTerm), Props = case erlang:port_info(Port, name) of - {_, Type} when Type =:= "udp_inet"; + {_, Type} when Type =:= "udp_inet"; Type =:= "tcp_inet"; Type =:= "sctp_inet" -> - case catch inet:getstat(Port) of + try inet:getstat(Port) of {ok, Stats} -> [{statistics, Stats}]; - _ -> [] - end ++ - case catch inet:peername(Port) of - {ok, Peer} -> [{peername, Peer}]; {error, _} -> [] + catch + _Error:_Reason -> [] end ++ - case catch inet:sockname(Port) of + try inet:peername(Port) of + {ok, Peer} -> [{peername, Peer}]; + _ -> [] + catch + _Error:_Reason -> [] + end ++ + try inet:sockname(Port) of {ok, Local} -> [{sockname, Local}]; {error, _} -> [] + catch + _Error:_Reason -> [] end ++ - case catch inet:getopts(Port, ?SOCKET_OPTS ) of + try inet:getopts(Port, ?SOCKET_OPTS ) of {ok, Opts} -> [{options, Opts}]; {error, _} -> [] + catch + _Error:_Reason -> [] end; - {_, "efile"} -> - []; - _ -> - [] - end, + {_, "efile"} -> + []; + _ -> + [] + end, {specific, Props}; port_info(PortTerm, Keys) when is_list(Keys) -> Port = transform_port(PortTerm),