Skip to content

Commit

Permalink
Change mod_ping to handle ping timeout with sm
Browse files Browse the repository at this point in the history
  • Loading branch information
jacekwegr committed Oct 19, 2023
1 parent f989a92 commit 969ddc0
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 2 deletions.
54 changes: 53 additions & 1 deletion big_tests/tests/sm_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@
-define(LONG_TIMEOUT, 3600).
-define(SHORT_TIMEOUT, 1).
-define(SMALL_SM_BUFFER, 3).
-define(PING_REQUEST_TIMEOUT, 1).
-define(PING_INTERVAL, 3).

%%--------------------------------------------------------------------
%% Suite configuration
Expand All @@ -57,7 +59,8 @@ groups() ->
{parallel_manual_ack_freq_1, [parallel], parallel_manual_ack_freq_1_cases()},
{manual_ack_freq_2, [], manual_ack_freq_2_cases()},
{stale_h, [], stale_h_cases()},
{parallel_unacknowledged_message_hook, [parallel], parallel_unacknowledged_message_hook_cases()}
{parallel_unacknowledged_message_hook, [parallel], parallel_unacknowledged_message_hook_cases()},
{ping_timeout, [], ping_timeout_cases()}
].

parallel_cases() ->
Expand Down Expand Up @@ -124,6 +127,9 @@ parallel_unacknowledged_message_hook_cases() ->
unacknowledged_message_hook_resume,
unacknowledged_message_hook_filter].

ping_timeout_cases() ->
[ping_timeout].

%%--------------------------------------------------------------------
%% Init & teardown
%%--------------------------------------------------------------------
Expand Down Expand Up @@ -151,6 +157,13 @@ 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.
Expand Down Expand Up @@ -210,6 +223,9 @@ required_sm_opts(group, parallel_unacknowledged_message_hook) ->
#{ack_freq => 1};
required_sm_opts(group, manual_ack_freq_long_session_timeout) ->
#{ack_freq => 1, buffer_max => 1000};
required_sm_opts(group, ping_timeout) ->
#{ack_freq => 1,
resume_timeout => ?SHORT_TIMEOUT};
required_sm_opts(testcase, resume_expired_session_returns_correct_h) ->
#{ack_freq => 1,
resume_timeout => ?SHORT_TIMEOUT,
Expand Down Expand Up @@ -240,6 +256,10 @@ 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
%%--------------------------------------------------------------------
Expand Down Expand Up @@ -591,6 +611,38 @@ resend_unacked_after_resume_timeout(Config) ->
escalus_connection:stop(Bob),
escalus_connection:stop(NewAlice).

ping_timeout(Config) ->
% ct:sleep(timer:seconds(10)),

logger_ct_backend:start(),
logger_ct_backend:capture(error),

%% connect Alice and simulate ping timeout by dropping connection
Alice = connect_fresh(Config, alice, sr_presence),

escalus_client:wait_for_stanza(Alice),
ct:sleep(?PING_REQUEST_TIMEOUT + ?PING_INTERVAL + timer:seconds(1)),

%% attempt to resume the session after the connection drop
NewAlice = sm_helper:kill_and_connect_with_resume_session_without_waiting_for_result(Alice),

%% after resume_timeout, we expect the session to be closed
escalus_connection:get_stanza(NewAlice, failed_resumption),

%% bind a new session and expect unacknowledged messages to be resent
escalus_session:session(escalus_session:bind(NewAlice)),
send_initial_presence(NewAlice),

% check if the error occurred
FilterFun = fun(_, Msg) ->
re:run(Msg, "parse_iq_id_failed", [global]) /= nomatch
end,
?assertEqual([], logger_ct_backend:recv(FilterFun)),
logger_ct_backend:stop_capture(),

%% stop the connection
escalus_connection:stop(NewAlice).

resume_expired_session_returns_correct_h(Config) ->
%% connect bob and alice
Bob = connect_fresh(Config, bob, sr_presence),
Expand Down
27 changes: 26 additions & 1 deletion src/mod_ping.erl
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
-export([user_send_packet/3,
user_send_iq/3,
user_ping_response/3,
filter_local_packet/3,
iq_ping/5]).

%% Record that will be stored in the c2s state when the server pings the client,
Expand All @@ -40,7 +41,8 @@
%%====================================================================

hooks(HostType) ->
[{user_ping_response, HostType, fun ?MODULE:user_ping_response/3, #{}, 100}
[{user_ping_response, HostType, fun ?MODULE:user_ping_response/3, #{}, 100},

Check warning on line 44 in src/mod_ping.erl

View check run for this annotation

Codecov / codecov/patch

src/mod_ping.erl#L44

Added line #L44 was not covered by tests
{filter_local_packet, HostType, fun ?MODULE:filter_local_packet/3, #{}, 100}
| c2s_hooks(HostType)].

-spec c2s_hooks(mongooseim:host_type()) -> gen_hook:hook_list(mongoose_c2s_hooks:fn()).
Expand Down Expand Up @@ -120,6 +122,19 @@ iq_ping(Acc, _From, _To, #iq{sub_el = SubEl} = IQ, _) ->
%% Hook callbacks
%%====================================================================

-spec filter_local_packet(Acc, Params, Extra) -> {ok, Acc} | {stop, drop} when
Acc :: mongoose_hooks:filter_packet_acc(),
Params :: map(),
Extra :: gen_hook:extra().
filter_local_packet({_, _, _, Stanza} = Acc, _Params, _Extra) ->
case is_ping_error(Stanza) of

Check warning on line 130 in src/mod_ping.erl

View check run for this annotation

Codecov / codecov/patch

src/mod_ping.erl#L130

Added line #L130 was not covered by tests
true ->
?LOG_DEBUG(#{what => ping_error_received, acc => Acc}),
{stop, drop};

Check warning on line 133 in src/mod_ping.erl

View check run for this annotation

Codecov / codecov/patch

src/mod_ping.erl#L132-L133

Added lines #L132 - L133 were not covered by tests
false ->
{ok, Acc}

Check warning on line 135 in src/mod_ping.erl

View check run for this annotation

Codecov / codecov/patch

src/mod_ping.erl#L135

Added line #L135 was not covered by tests
end.

-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}) ->
Expand Down Expand Up @@ -200,3 +215,13 @@ ping_get(Id) ->
#xmlel{name = <<"iq">>,
attrs = [{<<"type">>, <<"get">>}, {<<"id">>, 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;

Check warning on line 225 in src/mod_ping.erl

View check run for this annotation

Codecov / codecov/patch

src/mod_ping.erl#L225

Added line #L225 was not covered by tests
is_ping_error(_) ->
false.

Check warning on line 227 in src/mod_ping.erl

View check run for this annotation

Codecov / codecov/patch

src/mod_ping.erl#L227

Added line #L227 was not covered by tests

0 comments on commit 969ddc0

Please sign in to comment.