Hunt BOSH interleave requests issue #869
Conversation
michalwski
force-pushed
the
hunt-bosh-interleave-requests-issue
branch
2 times, most recently
from
ad81d64
to
773fe12
Compare
Nyco
changed the title
michalwski
force-pushed
the
hunt-bosh-interleave-requests-issue
branch
from
7a1a27f
to
1c1b2c1
Compare
michalwski
force-pushed
the
hunt-bosh-interleave-requests-issue
branch
from
1c1b2c1
to
1e8cf1d
Compare
| -export([test/1, sample/0, prop/1]). | ||
|
|
||
| -export([initial_state/1, command/1, precondition/2, postcondition/3, | ||
| next_state/3]). |
There was a problem hiding this comment.
According to Elvis:
Line 14 has a tab at column 0.
| -export([ct_config_giver/1]). | ||
|
|
||
| -record(state, {carol, | ||
| geralt, |
There was a problem hiding this comment.
According to Elvis:
Line 27 has a tab at column 0.
| ?FORALL(Cmds, commands(?MODULE, initial_state(Pid)), | ||
| ?TRAPEXIT( | ||
| begin | ||
| {History,State,Result} = run_commands(?MODULE, Cmds), |
There was a problem hiding this comment.
According to Elvis:
Missing space right "," on line 43
| escalus_client:stop(Client). | ||
|
|
||
| initial_state(Pid) -> | ||
| #state{carol = undefined, |
There was a problem hiding this comment.
According to Elvis:
Line 63 has a tab at column 0.
| ?FORALL(Cmds, commands(?MODULE, initial_state(Pid)), | ||
| ?TRAPEXIT( | ||
| begin | ||
| {History,State,Result} = run_commands(?MODULE, Cmds), |
There was a problem hiding this comment.
According to Elvis:
Missing space right "," on line 43
|
|
||
| initial_state(Pid) -> | ||
| #state{carol = undefined, | ||
| geralt = undefined, |
There was a problem hiding this comment.
According to Elvis:
Line 64 has a tab at column 0.
| maybe_stop_client(State#state.geralt), | ||
| ?WHENFAIL(ct:pal(error, "History: ~p~nState: ~p\nResult: ~p~n", | ||
| [History,State,Result]), | ||
| aggregate(command_names(Cmds), Result =:= ok)) |
There was a problem hiding this comment.
According to Elvis:
Line 48 has a tab at column 0.
| maybe_stop_client(State#state.carol), | ||
| maybe_stop_client(State#state.geralt), | ||
| ?WHENFAIL(ct:pal(error, "History: ~p~nState: ~p\nResult: ~p~n", | ||
| [History,State,Result]), |
There was a problem hiding this comment.
According to Elvis:
Line 47 has a tab at column 0.
| ?FORALL(Cmds, commands(?MODULE, initial_state(Pid)), | ||
| ?TRAPEXIT( | ||
| begin | ||
| {History,State,Result} = run_commands(?MODULE, Cmds), |
There was a problem hiding this comment.
According to Elvis:
Line 43 has a tab at column 0.
| prop(Config) -> | ||
| Pid = spawn_link(?MODULE, ct_config_giver, [Config]), | ||
| ?FORALL(Cmds, commands(?MODULE, initial_state(Pid)), | ||
| ?TRAPEXIT( |
There was a problem hiding this comment.
According to Elvis:
Line 41 has a tab at column 0.
| RMsgs = [exml_query:path(E, [{element, <<"body">>}, cdata]) | ||
| || E <- Stanzas], | ||
| SortedMsgs = lists:sort([Msg || {_, Msg} <- Msgs]), | ||
| ct:pal("waiting for: ~p~nreceived: ~p", [SortedMsgs, RMsgs]), |
There was a problem hiding this comment.
According to Elvis:
Remove the debug call to ct:pal/2 on line 171.
| {History,State,Result} = run_commands(?MODULE, Cmds), | ||
| maybe_stop_client(State#state.carol), | ||
| maybe_stop_client(State#state.geralt), | ||
| ?WHENFAIL(ct:pal(error, "History: ~p~nState: ~p\nResult: ~p~n", |
There was a problem hiding this comment.
According to Elvis:
Line 46 has a tab at column 0.
|
|
||
| prop(Config) -> | ||
| Pid = spawn_link(?MODULE, ct_config_giver, [Config]), | ||
| ?FORALL(Cmds, commands(?MODULE, initial_state(Pid)), |
There was a problem hiding this comment.
According to Elvis:
Line 40 has a tab at column 0.
| Pid = spawn_link(?MODULE, ct_config_giver, [Config]), | ||
| ?FORALL(Cmds, commands(?MODULE, initial_state(Pid)), | ||
| ?TRAPEXIT( | ||
| begin |
There was a problem hiding this comment.
According to Elvis:
Line 42 has a tab at column 0.
| ?WHENFAIL(ct:pal(error, "History: ~p~nState: ~p\nResult: ~p~n", | ||
| [History,State,Result]), | ||
| aggregate(command_names(Cmds), Result =:= ok)) | ||
| end)). |
There was a problem hiding this comment.
According to Elvis:
Line 49 has a tab at column 0.
| maybe_stop_client(State#state.carol), | ||
| maybe_stop_client(State#state.geralt), | ||
| ?WHENFAIL(ct:pal(error, "History: ~p~nState: ~p\nResult: ~p~n", | ||
| [History,State,Result]), |
There was a problem hiding this comment.
According to Elvis:
Missing space right "," on line 47
| maybe_stop_client(State#state.carol), | ||
| maybe_stop_client(State#state.geralt), | ||
| ?WHENFAIL(ct:pal(error, "History: ~p~nState: ~p\nResult: ~p~n", | ||
| [History,State,Result]), |
There was a problem hiding this comment.
According to Elvis:
Missing space right "," on line 47
| ?FORALL(Cmds, commands(?MODULE, initial_state(Pid)), | ||
| ?TRAPEXIT( | ||
| begin | ||
| {History,State,Result} = run_commands(?MODULE, Cmds), |
There was a problem hiding this comment.
According to Elvis:
Missing space right "," on line 43
| maybe_stop_client(State#state.carol), | ||
| maybe_stop_client(State#state.geralt), | ||
| ?WHENFAIL(ct:pal(error, "History: ~p~nState: ~p\nResult: ~p~n", | ||
| [History,State,Result]), |
There was a problem hiding this comment.
According to Elvis:
Missing space right "," on line 47
| maybe_stop_client(State#state.carol), | ||
| maybe_stop_client(State#state.geralt), | ||
| ?WHENFAIL(ct:pal(error, "History: ~p~nState: ~p\nResult: ~p~n", | ||
| [History,State,Result]), |
There was a problem hiding this comment.
According to Elvis:
Missing space right "," on line 47
| ?FORALL(Cmds, commands(?MODULE, initial_state(Pid)), | ||
| ?TRAPEXIT( | ||
| begin | ||
| {History,State,Result} = run_commands(?MODULE, Cmds), |
There was a problem hiding this comment.
According to Elvis:
Missing space right "," on line 43
michalwski
force-pushed
the
hunt-bosh-interleave-requests-issue
branch
2 times, most recently
from
0a12e4c
to
7219d3f
Compare
| RMsgs = [exml_query:path(E, [{element, <<"body">>}, cdata]) | ||
| || E <- Stanzas], | ||
| SortedMsgs = lists:sort([Msg || {_, Msg} <- Msgs]), | ||
| ct:pal("waiting for: ~p~nreceived: ~p", [SortedMsgs, RMsgs]), |
There was a problem hiding this comment.
According to Elvis:
Remove the debug call to ct:pal/2 on line 167.
michalwski
force-pushed
the
hunt-bosh-interleave-requests-issue
branch
from
7219d3f
to
0cbd01d
Compare
|
Should b7ca3b5 message reference PR 869 (this one)? Currently it references 868, which doesn't seem relevant. |
Yes, good catch! |
in some rare cases it is possible that stream:features arrive without the stream tag. In this case the xmlns:stream was not added which confused expath and probably other parsers
it is not possible to guarantee order of msgs received from BOSH is 100% tests
michalwski
force-pushed
the
hunt-bosh-interleave-requests-issue
branch
from
efdd38a
to
9a7ca19
Compare
The fix is only half line of code. Cached responses should be resent only if there is a handler, otherwise the RID is not incremented correctly when new request arrives and the session is terminated incorrectly. See PR #869 for more details about the bug.
michalwski
force-pushed
the
hunt-bosh-interleave-requests-issue
branch
from
9a7ca19
to
9609812
Compare
Proposed changes include:
ridprocessing when there was a response delivered to the hendler which send biggerridthen expectedxmlns:streamattr in<body>when stream features arrives in their own messageDetailed description of the deferred rids processing.
Assume expected
ridhas valueXThis bug happens when MongooseIM processes a request with
ridX+1beforeXand there is a pending response to the client. In this situation Mongoose puts the request into deferred list which is expected. Also it sends the pending response to the client using this deferred connection which results in adding theX+1ridto cached responses list (used to track retransmissions). Later when the deferredridX+1is process because the prev arrived, MongooseIM treats it as a retransmission (because theridX+1is on the cached responses list). Because of that the mainridis not updated correctly and expectsX+1whereas it should expect nowX+2. As a result the end the session is terminated upon receivingridX+3