Handle IQ error responses in mod_ping

big_tests/tests/mod_ping_SUITE.erl

@@ -56,7 +56,8 @@ all_tests() ->
      active,
      active_keep_alive,
      server_ping_pong,
-     server_ping_pang
+     server_ping_pang,
+     service_unavailable_response
     ].
 
 suite() ->
@@ -184,6 +185,26 @@ wrong_ping(Config) ->
             escalus:assert(is_iq_error, [PingReq], PingResp)
         end).
 
+service_unavailable_response(Config) ->
+    escalus:fresh_story(Config, [{alice, 1}],
+        fun(Alice) ->
+            PingReq = wait_for_ping_req(Alice),
+            PingId = exml_query:attr(PingReq, <<"id">>),
+
+            ErrorStanzaBody = [#xmlel{name = <<"ping">>, attrs = [{<<"xmlns">>, ?NS_PING}]},
+                               #xmlel{name = <<"error">>, attrs = [{<<"type">>, <<"cancel">>}],
+                               children = [#xmlel{name = <<"service-unavailable">>,
+                                                  attrs = [{<<"xmlns">>, ?NS_STANZA_ERRORS}]}]}],
+            ErrorStanza = escalus_stanza:set_id(
+                            escalus_stanza:iq(domain(), <<"error">>, ErrorStanzaBody), PingId),
+            escalus_client:send(Alice, ErrorStanza),
+
+            ct:sleep(timer:seconds(1)),
+            TimeoutAction = ?config(timeout_action, Config),
+            check_connection(TimeoutAction, Alice),
+            escalus_client:kill_connection(Config, Alice)
+        end).
+
 active(ConfigIn) ->
     Domain = domain(),
     HostType = domain_helper:host_type(mim),

big_tests/tests/sm_SUITE.erl

@@ -157,13 +157,6 @@ init_per_group(stream_mgmt_disabled, Config) ->
     dynamic_modules:stop(host_type(), ?MOD_SM),
     rpc(mim(), mnesia, delete_table, [sm_session]),
     Config;
-init_per_group(ping_timeout = Group, Config) ->
-    dynamic_modules:ensure_modules(host_type(), required_modules(Config, group, Group)),
-    start_mod_ping(#{send_pings => true,
-                     ping_interval => ?PING_INTERVAL,
-                     ping_req_timeout => ?PING_REQUEST_TIMEOUT,
-                     timeout_action => kill}),
-    Config;
 init_per_group(Group, Config) ->
     dynamic_modules:ensure_modules(host_type(), required_modules(Config, group, Group)),
     Config.
@@ -206,9 +199,19 @@ required_modules(Config, Scope, Name) ->
                    stopped -> stopped;
                    ExtraOpts -> maps:merge(common_sm_opts(Config), ExtraOpts)
                end,
+    PingConfig = case Name of
+                    ping_timeout ->
+                        #{send_pings => true,
+                          ping_interval => ?PING_INTERVAL,
+                          ping_req_timeout => ?PING_REQUEST_TIMEOUT,
+                          timeout_action => kill};
+                    _ ->
+                        #{}
+                 end,
     Backend = mongoose_helper:mnesia_or_rdbms_backend(),
     [{mod_stream_management, config_parser_helper:mod_config(mod_stream_management, SMConfig)},
-     {mod_offline, config_parser_helper:mod_config(mod_offline, #{backend => Backend})}].
+     {mod_offline, config_parser_helper:mod_config(mod_offline, #{backend => Backend})},
+     {mod_ping, config_parser_helper:mod_config(mod_ping, PingConfig)}].
 
 required_sm_opts(group, parallel) ->
     #{ack_freq => never};
@@ -256,10 +259,6 @@ register_some_smid_h(Config) ->
     TestSmids = lists:map(fun register_smid/1, lists:seq(1, 3)),
     [{smid_test, TestSmids} | Config].
 
-start_mod_ping(Opts) ->
-    HostType = domain_helper:host_type(mim),
-    dynamic_modules:start(HostType, mod_ping, config_parser_helper:mod_config(mod_ping, Opts)).
-
 %%--------------------------------------------------------------------
 %% Tests
 %%--------------------------------------------------------------------

src/mod_ping.erl

@@ -138,25 +138,35 @@ filter_local_packet({_, _, _, Stanza} = Acc, _Params, _Extra) ->
 -spec user_send_iq(mongoose_acc:t(), mongoose_c2s_hooks:params(), gen_hook:extra()) ->
     mongoose_c2s_hooks:result().
 user_send_iq(Acc, #{c2s_data := StateData}, #{host_type := HostType}) ->
-    case {mongoose_acc:stanza_type(Acc),
-          mongoose_c2s:get_mod_state(StateData, ?MODULE)} of
-        {<<"result">>, {ok, #ping_handler{id = PingId, time = T0}}} ->
-            IqResponse = mongoose_acc:element(Acc),
-            IqId = exml_query:attr(IqResponse, <<"id">>),
-            case {IqId, PingId} of
-                {Id, Id} ->
-                    Jid = mongoose_c2s:get_jid(StateData),
-                    TDelta = erlang:monotonic_time(millisecond) - T0,
-                    mongoose_hooks:user_ping_response(HostType, #{}, Jid, IqResponse, TDelta),
-                    Action = {{timeout, ping_timeout}, cancel},
-                    {stop, mongoose_c2s_acc:to_acc(Acc, actions, Action)};
-                _ ->
-                    {ok, Acc}
-            end;
+    StanzaType = mongoose_acc:stanza_type(Acc),
+    ModState = mongoose_c2s:get_mod_state(StateData, ?MODULE),
+    handle_stanza(StanzaType, ModState, Acc, StateData, HostType).
+
+handle_stanza(Type, {ok, PingHandler}, Acc, StateData, HostType) when Type == <<"result">>;
+                                                                      Type == <<"error">> ->
+    handle_ping_response(Type, PingHandler, Acc, StateData, HostType);
+handle_stanza(_, _, Acc, _, _) ->
+    {ok, Acc}.
+
+handle_ping_response(Type, #ping_handler{id = PingId, time = T0}, Acc, StateData, HostType) ->
+    IqResponse = mongoose_acc:element(Acc),
+    IqId = exml_query:attr(IqResponse, <<"id">>),
+    case IqId of
+        PingId ->
+            Jid = mongoose_c2s:get_jid(StateData),
+            TDelta = erlang:monotonic_time(millisecond) - T0,
+            mongoose_hooks:user_ping_response(HostType, #{}, Jid, IqResponse, TDelta),
+            Action = determine_action(Type),
+            {stop, mongoose_c2s_acc:to_acc(Acc, actions, Action)};
         _ ->
             {ok, Acc}
     end.
 
+determine_action(<<"result">>) ->
+    {{timeout, ping_timeout}, cancel};
+determine_action(<<"error">>) ->
+    [{{timeout, ping_timeout}, cancel}, {{timeout, ping_error}, 0, fun ping_c2s_handler/2}].
+
 -spec user_send_packet(mongoose_acc:t(), mongoose_c2s_hooks:params(), gen_hook:extra()) ->
     mongoose_c2s_hooks:result().
 user_send_packet(Acc, _Params, #{host_type := HostType}) ->
@@ -189,8 +199,15 @@ ping_c2s_handler(ping_timeout, StateData) ->
     Jid = mongoose_c2s:get_jid(StateData),
     HostType = mongoose_c2s:get_host_type(StateData),
     mongoose_hooks:user_ping_response(HostType, #{}, Jid, timeout, 0),
-    case gen_mod:get_module_opt(HostType, ?MODULE, timeout_action) of
-        kill -> mongoose_c2s_acc:new(#{stop => {shutdown, ping_timeout}});
+    handle_ping_action(HostType, ping_timeout);
+ping_c2s_handler(ping_error, StateData) ->
+    HostType = mongoose_c2s:get_host_type(StateData),
+    handle_ping_action(HostType, ping_error).
+
+handle_ping_action(HostType, Reason) ->
+    TimeoutAction = gen_mod:get_module_opt(HostType, ?MODULE, timeout_action),
+    case TimeoutAction of
+        kill -> mongoose_c2s_acc:new(#{stop => {shutdown, Reason}});
         _ -> mongoose_c2s_acc:new()
     end.
 
@@ -217,11 +234,12 @@ ping_get(Id) ->
            children = [#xmlel{name = <<"ping">>, attrs = [{<<"xmlns">>, ?NS_PING}]}]}.
 
 -spec is_ping_error(exml:element()) -> boolean().
-is_ping_error(#xmlel{name = <<"iq">>,
-                      attrs = [{<<"type">>, <<"error">>}, _, _, _],
-                      children = [#xmlel{name = <<"ping">>,
-                                         attrs = [{<<"xmlns">>, ?NS_PING}]},
-                                  #xmlel{name = <<"error">>}]}) ->
-    true;
-is_ping_error(_) ->
-    false.
+is_ping_error(Stanza) ->
+    case exml_query:attr(Stanza, <<"type">>) of
+        <<"error">> ->
+            undefined =/= exml_query:subelement_with_name_and_ns(Stanza, <<"ping">>, ?NS_PING)
+            andalso
+            undefined =/= exml_query:subelement(Stanza, <<"error">>);
+        _ ->
+            false
+    end.