Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add port and ip in the listener opts for websockets and bosh #3977

Merged
merged 3 commits into from
Mar 7, 2023

Conversation

NelsonVides
Copy link
Collaborator

@NelsonVides NelsonVides commented Mar 6, 2023

This adds missing elements to the listener_opts for websockets and bosh and enforces that in dialyzer.

@mongoose-im

This comment was marked as outdated.

@codecov
Copy link

codecov bot commented Mar 6, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.08 🎉

Comparison is base (7b91117) 83.55% compared to head (1ffe149) 83.63%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3977      +/-   ##
==========================================
+ Coverage   83.55%   83.63%   +0.08%     
==========================================
  Files         538      538              
  Lines       33974    33975       +1     
==========================================
+ Hits        28386    28415      +29     
+ Misses       5588     5560      -28     
Impacted Files Coverage Δ
src/c2s/mongoose_c2s.erl 87.69% <ø> (ø)
src/mod_websockets.erl 81.92% <ø> (ø)
src/ejabberd_cowboy.erl 76.19% <100.00%> (+0.38%) ⬆️
src/mod_bosh.erl 95.74% <100.00%> (ø)
src/mod_bosh_socket.erl 78.32% <100.00%> (ø)
src/mongoose_tcp_listener.erl 74.46% <0.00%> (-4.26%) ⬇️
src/domain/service_domain_db.erl 83.33% <0.00%> (-2.09%) ⬇️
src/mod_muc_log.erl 62.82% <0.00%> (ø)
src/pubsub/mod_pubsub.erl 73.66% <0.00%> (+0.18%) ⬆️
src/pubsub/mod_pubsub_db_rdbms.erl 95.60% <0.00%> (+0.25%) ⬆️
... and 9 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@NelsonVides NelsonVides force-pushed the c2s/listener_id_for_ws_and_bosh branch 3 times, most recently from 5138904 to 57f58f1 Compare March 6, 2023 14:20
@mongoose-im

This comment was marked as outdated.

@NelsonVides NelsonVides force-pushed the c2s/listener_id_for_ws_and_bosh branch from 57f58f1 to 5a6a6da Compare March 6, 2023 14:59
@mongoose-im

This comment was marked as outdated.

@NelsonVides NelsonVides marked this pull request as ready for review March 6, 2023 15:31
@NelsonVides NelsonVides force-pushed the c2s/listener_id_for_ws_and_bosh branch from 5a6a6da to 7f70812 Compare March 7, 2023 09:16
@mongoose-im
Copy link
Collaborator

mongoose-im commented Mar 7, 2023

small_tests_24 / small_tests / 7f70812
Reports root / small


small_tests_25 / small_tests / 7f70812
Reports root / small


ldap_mnesia_24 / ldap_mnesia / 7f70812
Reports root/ big
OK: 2225 / Failed: 0 / User-skipped: 825 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 7f70812
Reports root/ big
OK: 4177 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0


ldap_mnesia_25 / ldap_mnesia / 7f70812
Reports root/ big
OK: 2225 / Failed: 0 / User-skipped: 825 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 7f70812
Reports root/ big
OK: 4177 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0


dynamic_domains_mysql_redis_25 / mysql_redis / 7f70812
Reports root/ big
OK: 4151 / Failed: 0 / User-skipped: 114 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 7f70812
Reports root/ big
OK: 4557 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0


elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 7f70812
Reports root/ big
OK: 2727 / Failed: 0 / User-skipped: 662 / Auto-skipped: 0


internal_mnesia_25 / internal_mnesia / 7f70812
Reports root/ big
OK: 2367 / Failed: 0 / User-skipped: 683 / Auto-skipped: 0


pgsql_mnesia_25 / pgsql_mnesia / 7f70812
Reports root/ big
OK: 4557 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0


riak_mnesia_24 / riak_mnesia / 7f70812
Reports root/ big
OK: 2565 / Failed: 0 / User-skipped: 654 / Auto-skipped: 0


mysql_redis_25 / mysql_redis / 7f70812
Reports root/ big
OK: 4543 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0


mssql_mnesia_25 / odbc_mssql_mnesia / 7f70812
Reports root/ big
OK: 4554 / Failed: 0 / User-skipped: 100 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 7f70812
Reports root/ big
OK: 4173 / Failed: 1 / User-skipped: 91 / Auto-skipped: 0

service_domain_db_SUITE:db:db_deleted_from_one_node_while_service_disabled_on_another
{error,
  {{badmatch,{error,not_found}},
   [{service_domain_db_SUITE,
      db_deleted_from_one_node_while_service_disabled_on_another,1,
      [{file,
         "/home/circleci/project/big_tests/tests/service_domain_db_SUITE.erl"},
       {line,588}]},
    {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1782}]},
    {test_server,run_test_case_eval1,6,
      [{file,"test_server.erl"},{line,1291}]},
    {test_server,run_test_case_eval,9,
      [{file,"test_server.erl"},{line,1223}]}]}}

Report log


dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 7f70812
Reports root/ big
OK: 4174 / Failed: 0 / User-skipped: 91 / Auto-skipped: 0

BoshSocket = #bosh_socket{sid = Sid, pid = self(), peer = Peer, peercert = PeerCert},
C2SOpts = #{access => all,
shaper => none,
xml_socket => true,
max_stanza_size => 0,
hibernate_after => 0,
c2s_state_timeout => 5000,
backwards_compatible_session => true},
backwards_compatible_session => true,
port => Port, ip_tuple => IPTuple, proto => tcp},
Copy link
Member

@chrzaszcz chrzaszcz Mar 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This port should be the listener's one. I ran the test, see:

when=2023-03-07T10:55:39.308479+00:00 level=debug what=bosh_socket_init pid=<0.5400.0> at=mod_bosh_socket:init/1:208 sid=5b6140fbac13ae46568a8634e4ffe33ce0469041 peer={{127,0,0,1},49421} c2s_pid=<0.5401.0>
(...)
when=2023-03-07T10:55:39.354863+00:00 level=debug what=bosh_socket_init pid=<0.5436.0> at=mod_bosh_socket:init/1:208 sid=e738430a2769beb809207398d02a8bad38ad7721 peer={{127,0,0,1},49423} c2s_pid=<0.5437.0>

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Riiiiiiight, I was understanding bosh wrong!
Should be fixed now 🙂

@mongoose-im
Copy link
Collaborator

mongoose-im commented Mar 7, 2023

small_tests_24 / small_tests / 1ffe149
Reports root / small


small_tests_25 / small_tests / 1ffe149
Reports root / small


ldap_mnesia_24 / ldap_mnesia / 1ffe149
Reports root/ big
OK: 2243 / Failed: 1 / User-skipped: 825 / Auto-skipped: 0

pubsub_SUITE:dag+basic:subscribe_unsubscribe_test
{error,{{badmatch,false},
    [{pubsub_tools,check_response,2,
             [{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
            {line,491}]},
     {pubsub_tools,receive_response,3,
             [{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
            {line,481}]},
     {pubsub_tools,receive_and_check_response,4,
             [{file,"/home/circleci/project/big_tests/tests/pubsub_tools.erl"},
            {line,471}]},
     {escalus_story,story,4,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
             {line,72}]},
     {test_server,ts_tc,3,[{file,"test_server.erl"},{line,1783}]},
     {test_server,run_test_case_eval1,6,
            [{file,"test_server.erl"},{line,1292}]},
     {test_server,run_test_case_eval,9,
            [{file,"test_server.erl"},{line,1224}]}]}}

Report log


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / 1ffe149
Reports root/ big
OK: 4177 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0


ldap_mnesia_25 / ldap_mnesia / 1ffe149
Reports root/ big
OK: 2225 / Failed: 0 / User-skipped: 825 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / 1ffe149
Reports root/ big
OK: 4174 / Failed: 0 / User-skipped: 91 / Auto-skipped: 0


dynamic_domains_mysql_redis_25 / mysql_redis / 1ffe149
Reports root/ big
OK: 4151 / Failed: 0 / User-skipped: 114 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / 1ffe149
Reports root/ big
OK: 4177 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / 1ffe149
Reports root/ big
OK: 4557 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0


internal_mnesia_25 / internal_mnesia / 1ffe149
Reports root/ big
OK: 2367 / Failed: 0 / User-skipped: 683 / Auto-skipped: 0


elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / 1ffe149
Reports root/ big
OK: 2727 / Failed: 0 / User-skipped: 662 / Auto-skipped: 0


pgsql_mnesia_25 / pgsql_mnesia / 1ffe149
Reports root/ big
OK: 4557 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0


mssql_mnesia_25 / odbc_mssql_mnesia / 1ffe149
Reports root/ big
OK: 4553 / Failed: 1 / User-skipped: 100 / Auto-skipped: 0

muc_SUITE:register:user_submits_registration_form
{error,
  {{assertion_failed,assert,is_iq_result,
     [{xmlel,<<"iq">>,
        [{<<"type">>,<<"set">>},
         {<<"id">>,<<"cfa4c6a377c30bedd66e737cf38c2f7d">>},
         {<<"to">>,<<"muc.localhost">>}],
        [{xmlel,<<"query">>,
           [{<<"xmlns">>,<<"jabber:iq:register">>}],
           [{xmlel,<<"x">>,
            [{<<"xmlns">>,<<"jabber:x:data">>},
             {<<"type">>,<<"submit">>}],
            [{xmlel,<<"field">>,
               [{<<"type">>,<<"hidden">>},
                {<<"var">>,<<"FORM_TYPE">>}],
               [{xmlel,<<"value">>,[],
                  [{xmlcdata,<<"jabber:iq:register">>}]}]},
             {xmlel,<<"field">>,
               [{<<"type">>,<<"text-single">>},
                {<<"var">>,<<"nick">>}],
               [{xmlel,<<"value">>,[],
                  [{xmlcdata,
                     <<"thirdwitchroom-9aed839284">>}]}]}]}]}]}],
     {xmlel,<<"iq">>,
       [{<<"from">>,<<"muc.localhost">>},
        {<<"to">>,
         <<"alice_user_submits_registration_form_2545@localhost/res1">>},
        {<<"type">>,<<"error">>},
        {<<"id">>,<<"cfa4c6a377c30bedd66e737cf38c2f7d">>}],
       [{xmlel,<<"query">>,
          [{<<"xmlns">>,<<"jabber:iq:register">>}],
          [{xmlel,<<"x">>,
             [{<<"xmlns">>,<<"jabber:x:data">>},
            {<<"type">>,<<"submit">>}],
             [{xmlel,<<"field">>,
              [{<<"type">>,<<"hidden">>},
               {<<"var">>,<<"FORM_TYPE">>}],
              [{xmlel,<<"value">>,[],
         ...

Report log


riak_mnesia_24 / riak_mnesia / 1ffe149
Reports root/ big
OK: 2565 / Failed: 0 / User-skipped: 654 / Auto-skipped: 0


mysql_redis_25 / mysql_redis / 1ffe149
Reports root/ big
OK: 4543 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0


mssql_mnesia_25 / odbc_mssql_mnesia / 1ffe149
Reports root/ big
OK: 4554 / Failed: 0 / User-skipped: 100 / Auto-skipped: 0

Copy link
Member

@chrzaszcz chrzaszcz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍 thanks for fixing the issue.

@@ -319,27 +319,27 @@ get_session_socket(Sid) ->
end.


-spec maybe_start_session(req(), exml:element()) ->
-spec maybe_start_session(req(), exml:element(), map()) ->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not type-spec them all as mongoose_listener:options()? This would give better information about the contents.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it doesn't have all of the options that belong to the mongoose_listener:options here and then dialyzer would fail :|

transport := TransportOpts0, protocol := ProtocolOpts0} = Opts,
Handlers = [ Handler#{ip_tuple => IPTuple, port => Port, proto => tcp} || Handler <- Handlers0 ],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not the prettiest (listener opts mixed up with handler opts), but I guess it's the simplest option, so it's ok.

@chrzaszcz chrzaszcz merged commit 4d6a413 into master Mar 7, 2023
@chrzaszcz chrzaszcz deleted the c2s/listener_id_for_ws_and_bosh branch March 7, 2023 14:24
@jacekwegr jacekwegr added this to the 6.1.0 milestone Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants