From 3162fe7a27931d6d5094786d48aea846f0e3d69a Mon Sep 17 00:00:00 2001 From: Thales Macedo Garitezi Date: Thu, 1 Aug 2024 15:23:14 -0300 Subject: [PATCH] feat: prettify some error explanations --- .../src/emqx_bridge_mqtt_connector.erl | 62 +++++++++++++------ .../emqx_bridge_mqtt_v2_subscriber_SUITE.erl | 31 ++++++++-- 2 files changed, 69 insertions(+), 24 deletions(-) diff --git a/apps/emqx_bridge_mqtt/src/emqx_bridge_mqtt_connector.erl b/apps/emqx_bridge_mqtt/src/emqx_bridge_mqtt_connector.erl index eb8b4e1ad..02cf6595f 100644 --- a/apps/emqx_bridge_mqtt/src/emqx_bridge_mqtt_connector.erl +++ b/apps/emqx_bridge_mqtt/src/emqx_bridge_mqtt_connector.erl @@ -98,7 +98,7 @@ on_start(ResourceId, #{server := Server} = Conf) -> server => Server }}; {error, Reason} -> - {error, Reason} + {error, emqx_maybe:define(explain_error(Reason), Reason)} end. on_add_channel( @@ -356,7 +356,12 @@ on_get_status(_ResourceId, State) -> Workers = [{Pool, Worker} || {Pool, PN} <- Pools, {_Name, Worker} <- ecpool:workers(PN)], try emqx_utils:pmap(fun get_status/1, Workers, ?HEALTH_CHECK_TIMEOUT) of Statuses -> - combine_status(Statuses) + case combine_status(Statuses) of + {Status, Msg} -> + {Status, State, Msg}; + Status -> + Status + end catch exit:timeout -> ?status_connecting @@ -375,7 +380,21 @@ combine_status(Statuses) -> %% Natural order of statuses: [connected, connecting, disconnected] %% * `disconnected` wins over any other status %% * `connecting` wins over `connected` - case lists:reverse(lists:usort(Statuses)) of + ToStatus = fun + ({S, _Reason}) -> S; + (S) when is_atom(S) -> S + end, + CompareFn = fun(S1A, S2A) -> + S1 = ToStatus(S1A), + S2 = ToStatus(S2A), + S1 > S2 + end, + case lists:usort(CompareFn, Statuses) of + [{Status, Reason} | _] -> + case explain_error(Reason) of + undefined -> Status; + Msg -> {Status, Msg} + end; [Status | _] -> Status; [] -> @@ -525,13 +544,7 @@ log_connect_error_reason(Level, {tcp_closed, _} = Reason, Name) -> msg => "ingress_client_connect_failed", reason => Reason, name => Name, - explain => - << - "Your MQTT connection attempt was unsuccessful. " - "It might be at its maximum capacity for handling new connections. " - "To diagnose the issue further, you can check the server logs for " - "any specific messages related to the unavailability or connection limits." - >> + explain => explain_error(Reason) }); log_connect_error_reason(Level, econnrefused = Reason, Name) -> ?tp(emqx_bridge_mqtt_connector_econnrefused_error, #{}), @@ -539,15 +552,7 @@ log_connect_error_reason(Level, econnrefused = Reason, Name) -> msg => "ingress_client_connect_failed", reason => Reason, name => Name, - explain => - << - "This error indicates that your connection attempt to the MQTT server was rejected. " - "In simpler terms, the server you tried to connect to refused your request. " - "There can be multiple reasons for this. " - "For example, the MQTT server you're trying to connect to might be down or not " - "running at all or you might have provided the wrong address " - "or port number for the server." - >> + explain => explain_error(Reason) }); log_connect_error_reason(Level, Reason, Name) -> ?SLOG(Level, #{ @@ -556,6 +561,25 @@ log_connect_error_reason(Level, Reason, Name) -> name => Name }). +explain_error(econnrefused) -> + << + "This error indicates that your connection attempt to the MQTT server was rejected. " + "In simpler terms, the server you tried to connect to refused your request. " + "There can be multiple reasons for this. " + "For example, the MQTT server you're trying to connect to might be down or not " + "running at all or you might have provided the wrong address " + "or port number for the server." + >>; +explain_error({tcp_closed, _}) -> + << + "Your MQTT connection attempt was unsuccessful. " + "It might be at its maximum capacity for handling new connections. " + "To diagnose the issue further, you can check the server logs for " + "any specific messages related to the unavailability or connection limits." + >>; +explain_error(_Reason) -> + undefined. + handle_disconnect(_Reason) -> ok. diff --git a/apps/emqx_bridge_mqtt/test/emqx_bridge_mqtt_v2_subscriber_SUITE.erl b/apps/emqx_bridge_mqtt/test/emqx_bridge_mqtt_v2_subscriber_SUITE.erl index 9030d2ac7..e0598fa1e 100644 --- a/apps/emqx_bridge_mqtt/test/emqx_bridge_mqtt_v2_subscriber_SUITE.erl +++ b/apps/emqx_bridge_mqtt/test/emqx_bridge_mqtt_v2_subscriber_SUITE.erl @@ -131,6 +131,9 @@ hookpoint(Config) -> BridgeId = bridge_id(Config), emqx_bridge_resource:bridge_hookpoint(BridgeId). +simplify_result(Res) -> + emqx_bridge_v2_testlib:simplify_result(Res). + %%------------------------------------------------------------------------------ %% Testcases %%------------------------------------------------------------------------------ @@ -247,21 +250,39 @@ t_receive_via_rule(Config) -> ), ok. -t_connect_with_more_clients_than_the_broker_accepts(Config0) -> +t_connect_with_more_clients_than_the_broker_accepts(Config) -> + Name = ?config(connector_name, Config), OrgConf = emqx_mgmt_listeners_conf:get_raw(tcp, default), on_exit(fun() -> emqx_mgmt_listeners_conf:update(tcp, default, OrgConf) end), NewConf = OrgConf#{<<"max_connections">> => 3}, {ok, _} = emqx_mgmt_listeners_conf:update(tcp, default, NewConf), - ConnectorConfig0 = ?config(connector_config, Config0), - ConnectorConfig = ConnectorConfig0#{<<"pool_size">> := 100}, - Config = emqx_utils:merge_opts(Config0, [{connector_config, ConnectorConfig}]), ?check_trace( #{timetrap => 10_000}, begin - {ok, _} = emqx_bridge_v2_testlib:create_bridge_api(Config), + ?assertMatch( + {201, #{ + <<"status">> := <<"disconnected">>, + <<"status_reason">> := + <<"Your MQTT connection attempt was unsuccessful", _/binary>> + }}, + simplify_result( + emqx_bridge_v2_testlib:create_connector_api( + Config, + #{<<"pool_size">> => 100} + ) + ) + ), ?block_until(#{?snk_kind := emqx_bridge_mqtt_connector_tcp_closed}), + ?assertMatch( + {200, #{ + <<"status">> := <<"disconnected">>, + <<"status_reason">> := + <<"Your MQTT connection attempt was unsuccessful", _/binary>> + }}, + simplify_result(emqx_bridge_v2_testlib:get_connector_api(mqtt, Name)) + ), ok end, fun(Trace) ->