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.

Sign up for GitHub

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

Jump to bottom

feat_: retry dns discovery on failure #21380

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

feat_: retry dns discovery on failure #21380

wants to merge 1 commit into from

Conversation

richard-ramos
Copy link
Member

@richard-ramos richard-ramos commented Oct 3, 2024 •
edited by pavloburykh
Loading

Might fix #21371

Requires:

This PR adds a retry mechanism for dns discovery URLs that fail to be retrieved by periodically attempting to resolve the URL with a backoff period that will increase up to 1m.

This PR is very simple to test: we should disconnect the internet and login. Once logged in, reconnect to the internet. The retry mechanism will attempt to retrieve the values from the DNS Discovery URL, and peer discovery should happen again.

@status-im-auto
Copy link
Member

status-im-auto commented Oct 3, 2024 •
edited
Loading

Jenkins Builds

❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 690ba69 #1 2024-10-03 16:12:56 ~5 min tests 📄log
✔️ 690ba69 #1 2024-10-03 16:16:39 ~8 min android-e2e 🤖apk 📲
✔️ 690ba69 #1 2024-10-03 16:17:58 ~10 min android 🤖apk 📲
✔️ 690ba69 #1 2024-10-03 16:22:13 ~14 min ios 📱ipa 📲

@pavloburykh pavloburykh self-assigned this Oct 4, 2024
@pavloburykh
Copy link
Contributor

@richard-ramos thanks for the PR. Could you please ping us once corresponding go PR is reviewed and approved? After that we will proceed with testing on mobile side.

@richard-ramos
Copy link
Member Author

Reviewed and approved!

@status-im-auto
Copy link
Member

98% of end-end tests have passed

Total executed tests: 51
Failed tests: 1
Expected to fail tests: 0
Passed tests: 50

IDs of failed tests: 703503

Failed tests (1)

Click to expand
  • Rerun failed tests

    • Class TestCommunityOneDeviceMerged:

    1. test_community_discovery, id: 703503

    # STEP: Check Discover Communities content
    Device 1: Find `CommunitiesTab` by `accessibility id`: `communities-stack-tab` 

    critical/chats/test_public_chat_browsing.py:261: in test_community_discovery
    self.home.communities_tab.click()
../views/base_element.py:90: in click
    element = self.find_element()
../views/base_element.py:79: in find_element
    raise NoSuchElementException(
 Device 1: CommunitiesTab by accessibility id: `communities-stack-tab` is not found on the screen; For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception



    Device sessions

    Passed tests (50)

    Click to expand

    Class TestActivityCenterContactRequestMultipleDevicePR:

    1. test_activity_center_contact_request_accept_swipe_mark_all_as_read, id: 702851
    Device sessions

    2. test_activity_center_contact_request_decline, id: 702850
    Device sessions

    3. test_add_contact_field_validation, id: 702777
    Device sessions

    Class TestDeepLinksOneDevice:

    1. test_links_open_universal_links_from_chat, id: 704613
    Device sessions

    2. test_links_deep_links_profile, id: 702775
    Device sessions

    3. test_deep_links_communities, id: 739307
    Device sessions

    Class TestGroupChatMultipleDeviceMergedNewUI:

    1. test_group_chat_reactions, id: 703202
    Device sessions

    2. test_group_chat_join_send_text_messages_push, id: 702807
    Device sessions

    3. test_group_chat_offline_pn, id: 702808
    Device sessions

    4. test_group_chat_pin_messages, id: 702732
    Device sessions

    5. test_group_chat_send_image_save_and_share, id: 703297
    Device sessions

    6. test_group_chat_mute_chat, id: 703495
    Device sessions

    Class TestCommunityMultipleDeviceMergedTwo:

    1. test_community_leave, id: 702845
    Device sessions

    2. test_community_mentions_push_notification, id: 702786
    Device sessions

    3. test_community_markdown_support, id: 702809
    Device sessions

    4. test_community_hashtag_links_to_community_channels, id: 702948
    Device sessions

    5. test_community_join_when_node_owner_offline, id: 703629
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    2. test_community_navigate_to_channel_when_relaunch, id: 702846
    Device sessions

    3. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133
    Device sessions

    4. test_community_undo_delete_message, id: 702869
    Device sessions

    5. test_community_mute_community_and_channel, id: 703382
    Device sessions

    Class TestWalletMultipleDevice:

    1. test_wallet_send_asset_from_drawer, id: 727230
    2. test_wallet_send_eth, id: 727229

    Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:

    1. test_1_1_chat_mute_chat, id: 703496
    Device sessions

    2. test_1_1_chat_is_shown_message_sent_delivered_from_offline, id: 702783
    Device sessions

    3. test_1_1_chat_delete_via_long_press_relogin, id: 702784
    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_edit_message, id: 702855
    Device sessions

    2. test_1_1_chat_message_reaction, id: 702730
    Device sessions

    3. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
    Device sessions

    4. test_1_1_chat_pin_messages, id: 702731
    Device sessions

    5. test_1_1_chat_text_message_delete_push_disappear, id: 702733
    Device sessions

    6. test_1_1_chat_push_emoji, id: 702813
    Device sessions

    7. test_1_1_chat_emoji_send_reply_and_open_link, id: 702782
    Device sessions

    8. test_1_1_chat_send_image_save_and_share, id: 703391
    Device sessions

    Class TestActivityMultipleDevicePR:

    1. test_activity_center_reply_read_unread_delete_filter_swipe, id: 702947
    Device sessions

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_emoji_send_copy_paste_reply, id: 702840
    Device sessions

    2. test_community_contact_block_unblock_offline, id: 702894
    Device sessions

    3. test_community_mark_all_messages_as_read, id: 703086
    Device sessions

    4. test_community_links_with_previews_github_youtube_twitter_gif_send_enable, id: 702844
    Device sessions

    5. test_community_unread_messages_badge, id: 702841
    Device sessions

    6. test_community_message_delete, id: 702839
    Device sessions

    7. test_community_message_send_check_timestamps_sender_username, id: 702838
    Device sessions

    8. test_community_edit_delete_message_when_offline, id: 704615
    Device sessions

    9. test_community_one_image_send_reply, id: 702859
    Device sessions

    10. test_community_message_edit, id: 702843
    Device sessions

    11. test_community_several_images_send_reply, id: 703194
    Device sessions

    Class TestWalletOneDevice:

    1. test_wallet_add_remove_regular_account, id: 727231
    Device sessions

    Class TestActivityMultipleDevicePRTwo:

    1. test_activity_center_admin_notification_accept_swipe, id: 702958
    Device sessions

    2. test_activity_center_mentions, id: 702957
    Device sessions

    @pavloburykh
    Copy link
    Contributor

    Hey @richard-ramos! Thanks for the PR.

    This PR is very simple to test: we should disconnect the internet and login. Once logged in, reconnect to the internet. The retry mechanism will attempt to retrieve the values from the DNS Discovery URL, and peer discovery should happen again.

    I have tried to test this flow and peers appear after reconnecting to the internet. Although I am facing another problem in this case which is described below. This problem is not PR related as I am reproducing it in develop. I wonder if you don't mind to check it in scope of this PR. Otherwise I can log it separately (as it might be related to the issues of mobile online/offline handling)

    ISSUE 1 Messages are not sent after coming back online

    Preconditions: User A and B are mutual contacts

    Steps:

    1. Logout by User A and B
    2. Disconnect form internet by both users
    3. Login by both users
    4. User A sends stack of messages to User B
    5. Re connect to internet by both users
    6. See if messages are sent and delivered

    Actual result: messages are stuck greyed out for sender (User A). New messages also not being sent. The messages start being sending only after relogin

    andr_User_B_receiver_logs.zip
    sender_User_A_ios_logs.zip

    messages_stuck_offline_handling.mp4

    @richard-ramos
    Copy link
    Member Author

    Thank you, @pavloburykh!

    If possible please open a separate issue for the problem you ran into, as this seems to not be related to this PR :(

    @pavloburykh
    Copy link
    Contributor

    pavloburykh commented Oct 4, 2024 •
    edited
    Loading

    Thank you, @pavloburykh!

    If possible please open a separate issue for the problem you ran into, as this seems to not be related to this PR :(

    sounds good @richard-ramos, thanks. Just to make sure: do I understand correctly that this PR should potentially fix #21371 ?

    Though I do not know how to verify it as there are no reproduction steps, it happened randomly.

    In terms of steps to test which you have described in PR description I do not see any difference compared to develop, i.e. peers appear for me after reconnection both in this PR and develop. So I am little bit unsure how exactly/what exactly I should verify while testing, what difference should be in comparison to develop from user perspective?

    @richard-ramos
    Copy link
    Member Author

    This PR should help for the cases on which DNS fails to resolve the bootnodes.
    This error seems to happen rarely tho, but has been reported a couple of times, with the fix being in most cases just restarting the client.

    @Horupa-Olena
    Copy link

    @richard-ramos Thanks for your PR! I don't see any issues on my side, and the PR can be merged.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers
    No reviews
    Assignees

    @pavloburykh pavloburykh

    @Horupa-Olena Horupa-Olena

    Labels
    request-manual-qa Tested - OK
    Projects
    Pipeline for QA
    Status: MERGE
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.

    Sometimes sender's contact request remains in pending state despite receiver has accepted it
    5 participants
    @richard-ramos @status-im-auto @pavloburykh @Horupa-Olena @churik