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

Improve error handling in mnesia API #3896

Merged
merged 2 commits into from
Dec 15, 2022

Conversation

jacekwegr
Copy link
Contributor

@jacekwegr jacekwegr commented Dec 8, 2022

This PR changes some of the returned values from the "mnesia" module's functions to be more consistent with the whole GraphQL API and adds a new scalar GraphQL type "NodeName" to check whether the provided string is a valid Erlang node name.

@mongoose-im

This comment was marked as outdated.

@codecov
Copy link

codecov bot commented Dec 8, 2022

Codecov Report

Base: 83.21% // Head: 83.21% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (f6ab093) compared to base (146cab3).
Patch coverage: 85.00% of modified lines in pull request are covered.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3896   +/-   ##
=======================================
  Coverage   83.21%   83.21%           
=======================================
  Files         535      535           
  Lines       34269    34275    +6     
=======================================
+ Hits        28517    28522    +5     
- Misses       5752     5753    +1     
Impacted Files Coverage Δ
src/ejabberd_admin.erl 65.62% <ø> (ø)
src/ejabberd_commands.erl 28.37% <ø> (ø)
src/mnesia_api.erl 77.34% <76.19%> (+2.72%) ⬆️
...l/admin/mongoose_graphql_mnesia_admin_mutation.erl 91.66% <88.88%> (ø)
src/ejabberd_ctl.erl 56.17% <100.00%> (+0.27%) ⬆️
...phql/admin/mongoose_graphql_mnesia_admin_query.erl 100.00% <100.00%> (ø)
src/graphql/mongoose_graphql_scalar.erl 89.18% <100.00%> (+0.95%) ⬆️
src/async_pools/mongoose_aggregator_worker.erl 63.33% <0.00%> (-5.01%) ⬇️
src/smart_markers/mod_smart_markers_rdbms.erl 91.66% <0.00%> (-3.34%) ⬇️
src/mam/mod_mam_muc_rdbms_arch_async.erl 82.85% <0.00%> (-2.86%) ⬇️
... and 6 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.

@jacekwegr jacekwegr force-pushed the improve-error-handling-in-mnesia-api branch from 268f017 to af3b39c Compare December 9, 2022 09:29
@mongoose-im

This comment was marked as outdated.

@jacekwegr jacekwegr force-pushed the improve-error-handling-in-mnesia-api branch from af3b39c to 7adaca7 Compare December 9, 2022 10:15
@mongoose-im

This comment was marked as outdated.

@jacekwegr jacekwegr force-pushed the improve-error-handling-in-mnesia-api branch from 7adaca7 to 429d930 Compare December 9, 2022 11:43
@mongoose-im

This comment was marked as outdated.

@jacekwegr jacekwegr marked this pull request as ready for review December 12, 2022 09:44
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.

It all looks good, I just think that maybe the NodeName could be automatically converted to an atom? Nodes are atoms in Erlang and we need them as atoms anyway.

@mongoose-im

This comment was marked as outdated.

@jacekwegr jacekwegr force-pushed the improve-error-handling-in-mnesia-api branch from e6b93f3 to d44f1df Compare December 13, 2022 14:34
@mongoose-im

This comment was marked as outdated.

@jacekwegr jacekwegr force-pushed the improve-error-handling-in-mnesia-api branch from d44f1df to 1fcfb88 Compare December 14, 2022 08:35
@mongoose-im

This comment was marked as outdated.

@jacekwegr jacekwegr force-pushed the improve-error-handling-in-mnesia-api branch from 1fcfb88 to fa72d70 Compare December 14, 2022 12:18
@mongoose-im
Copy link
Collaborator

mongoose-im commented Dec 14, 2022

small_tests_24 / small_tests / fa72d70
Reports root / small


small_tests_25 / small_tests / fa72d70
Reports root / small


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / fa72d70
Reports root/ big
OK: 4172 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / fa72d70
Reports root/ big
OK: 2218 / Failed: 0 / User-skipped: 823 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / fa72d70
Reports root/ big
OK: 4172 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0


dynamic_domains_mysql_redis_25 / mysql_redis / fa72d70
Reports root/ big
OK: 4146 / Failed: 0 / User-skipped: 114 / Auto-skipped: 0


ldap_mnesia_25 / ldap_mnesia / fa72d70
Reports root/ big
OK: 2218 / Failed: 0 / User-skipped: 823 / Auto-skipped: 0


internal_mnesia_25 / internal_mnesia / fa72d70
Reports root/ big
OK: 2360 / Failed: 0 / User-skipped: 681 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / fa72d70
Reports root/ big
OK: 4546 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0


elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / fa72d70
Reports root/ big
OK: 2718 / Failed: 0 / User-skipped: 662 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / fa72d70
Reports root/ big
OK: 4180 / Failed: 1 / User-skipped: 88 / Auto-skipped: 0

disco_and_caps_SUITE:disco_with_caps:user_can_query_friend_resources
{error,{{assertion_failed,assert_many,false,[is_roster_set],[],[]},
    [{escalus_new_assert,assert_true,2,
               [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_new_assert.erl"},
                {line,84}]},
     {escalus_story,'-make_all_clients_friends/1-fun-0-',2,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
             {line,112}]},
     {escalus_utils,'-each_with_index/3-fun-0-',3,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
             {line,87}]},
     {lists,foldl_1,3,[{file,"lists.erl"},{line,1355}]},
     {escalus_utils,'-each_with_index/3-fun-0-',3,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
             {line,87}]},
     {lists,foldl,3,[{file,"lists.erl"},{line,1350}]},
     {escalus_utils,distinct_pairs,2,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_utils.erl"},
             {line,60}]},
     {escalus_story,make_all_clients_friends,1,
            [{file,"/home/circleci/project/big_tests/_build/default/lib/escalus/src/escalus_story.erl"},
             {line,106}]}]}}

Report log


riak_mnesia_24 / riak_mnesia / fa72d70
Reports root/ big
OK: 2556 / Failed: 0 / User-skipped: 654 / Auto-skipped: 0


pgsql_mnesia_25 / pgsql_mnesia / fa72d70
Reports root/ big
OK: 4546 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0


mysql_redis_25 / mysql_redis / fa72d70
Reports root/ big
OK: 4532 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0


mssql_mnesia_25 / odbc_mssql_mnesia / fa72d70
Reports root/ big
OK: 4546 / Failed: 0 / User-skipped: 97 / 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 👍

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.

It looks good. I just added a few minor comments, maybe we could address them before the merge?

src/mnesia_api.erl Outdated Show resolved Hide resolved
src/mnesia_api.erl Outdated Show resolved Hide resolved
src/mnesia_api.erl Outdated Show resolved Hide resolved
@jacekwegr jacekwegr force-pushed the improve-error-handling-in-mnesia-api branch from fa72d70 to f6ab093 Compare December 15, 2022 11:13
@mongoose-im
Copy link
Collaborator

mongoose-im commented Dec 15, 2022

small_tests_24 / small_tests / f6ab093
Reports root / small


small_tests_25 / small_tests / f6ab093
Reports root / small


dynamic_domains_pgsql_mnesia_24 / pgsql_mnesia / f6ab093
Reports root/ big
OK: 4172 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0


ldap_mnesia_24 / ldap_mnesia / f6ab093
Reports root/ big
OK: 2218 / Failed: 0 / User-skipped: 823 / Auto-skipped: 0


dynamic_domains_pgsql_mnesia_25 / pgsql_mnesia / f6ab093
Reports root/ big
OK: 4172 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0


ldap_mnesia_25 / ldap_mnesia / f6ab093
Reports root/ big
OK: 2218 / Failed: 0 / User-skipped: 823 / Auto-skipped: 0


dynamic_domains_mysql_redis_25 / mysql_redis / f6ab093
Reports root/ big
OK: 4146 / Failed: 0 / User-skipped: 114 / Auto-skipped: 0


internal_mnesia_25 / internal_mnesia / f6ab093
Reports root/ big
OK: 2360 / Failed: 0 / User-skipped: 681 / Auto-skipped: 0


pgsql_mnesia_24 / pgsql_mnesia / f6ab093
Reports root/ big
OK: 4546 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0


dynamic_domains_mssql_mnesia_25 / odbc_mssql_mnesia / f6ab093
Reports root/ big
OK: 4172 / Failed: 0 / User-skipped: 88 / Auto-skipped: 0


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

graphql_server_SUITE:admin_http:clustering_http_tests:remove_dead_from_cluster_http
{error,
  {{badrpc,
     {'EXIT',
       {{try_clause,
          {badrpc,{'EXIT',{aborted,{no_exists,node,storage_type}}}}},
        [{mongoose_cluster,table_type,2,
           [{file,"/home/circleci/project/src/mongoose_cluster.erl"},
          {line,158}]},
         {mongoose_cluster,'-unsafe_join/2-lc$^0/1-0-',2,
           [{file,"/home/circleci/project/src/mongoose_cluster.erl"},
          {line,119}]},
         {mongoose_cluster,unsafe_join,2,
           [{file,"/home/circleci/project/src/mongoose_cluster.erl"},
          {line,120}]},
         {mongoose_cluster,with_app_stopped,2,
           [{file,"/home/circleci/project/src/mongoose_cluster.erl"},
          {line,221}]},
         {global,trans,4,[{file,"global.erl"},{line,463}]},
         {mongoose_cluster,join,1,[]}]}}},
   [{distributed_helper,rpc,
      [#{node => mongooseim3@localhost,timeout => 60000},
       mongoose_cluster,join,
       [mongooseim@localhost]],
      [{file,
         "/home/circleci/project/big_tests/tests/distributed_helper.erl"},
       {line,121}]},
    {graphql_server_SUITE,remove_dead_from_cluster_http,1,
      [{file,
         "/home/circleci/project/big_tests/tests/graphql_server_SUITE.erl"},
       {line,247}]},
    {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


elasticsearch_and_cassandra_25 / elasticsearch_and_cassandra_mnesia / f6ab093
Reports root/ big
OK: 2718 / Failed: 0 / User-skipped: 662 / Auto-skipped: 0


pgsql_mnesia_25 / pgsql_mnesia / f6ab093
Reports root/ big
OK: 4546 / Failed: 0 / User-skipped: 97 / Auto-skipped: 0


mysql_redis_25 / mysql_redis / f6ab093
Reports root/ big
OK: 4532 / Failed: 0 / User-skipped: 111 / Auto-skipped: 0


mssql_mnesia_25 / odbc_mssql_mnesia / f6ab093
Reports root/ big
OK: 4557 / Failed: 1 / User-skipped: 97 / Auto-skipped: 0

pep_SUITE:pep_tests:unsubscribe_after_presence_unsubscription
{error,
  {{badmatch,
     [{xmlel,<<"message">>,
        [{<<"from">>,
        <<"alice_unsubscribe_after_presence_unsubscription_2677@localhost">>},
         {<<"to">>,
        <<"bob_unsubscribe_after_presence_unsubscription_2677@localhost/res1">>},
         {<<"type">>,<<"headline">>}],
        [{xmlel,<<"event">>,
           [{<<"xmlns">>,
           <<"http://jabber.org/protocol/pubsub#event">>}],
           [{xmlel,<<"items">>,
            [{<<"node">>,<<"hgoWNDHgdwiHBFt+Jb07aA==">>}],
            [{xmlel,<<"item">>,
               [{<<"id">>,<<"salmon">>}],
               [{xmlel,<<"entry">>,
                  [{<<"xmlns">>,
                  <<"http://www.w3.org/2005/Atom">>}],
                  []}]}]}]},
         {xmlel,<<"headers">>,
           [{<<"xmlns">>,<<"http://jabber.org/protocol/shim">>}],
           []}]}]},
   [{pep_SUITE,'-unsubscribe_after_presence_unsubscription/1-fun-0-',2,
      [{file,"/home/circleci/project/big_tests/tests/pep_SUITE.erl"},
       {line,384}]},
    {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,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


riak_mnesia_24 / riak_mnesia / f6ab093
Reports root/ big
OK: 2556 / Failed: 0 / User-skipped: 654 / Auto-skipped: 0

@chrzaszcz chrzaszcz merged commit 16e1546 into master Dec 15, 2022
@chrzaszcz chrzaszcz deleted the improve-error-handling-in-mnesia-api branch December 15, 2022 12:58
@chrzaszcz chrzaszcz added this to the 6.0.0 milestone Dec 15, 2022
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.

3 participants