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

Fix panic syncing community token gated #18024

Merged
merged 1 commit into from
Nov 30, 2023
Merged

Fix panic syncing community token gated #18024

merged 1 commit into from
Nov 30, 2023

Conversation

cammellos
Copy link
Contributor

status: ready

@cammellos cammellos self-assigned this Nov 29, 2023
@status-im-auto
Copy link
Member

status-im-auto commented Nov 29, 2023 •
edited
Loading

Jenkins Builds

❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ c2c9fdb #1 2023-11-29 14:07:33 ~7 min android-e2e 🤖apk 📲
✔️ c2c9fdb #1 2023-11-29 14:08:09 ~8 min ios 📱ipa 📲
✔️ c2c9fdb #1 2023-11-29 14:08:39 ~8 min android 🤖apk 📲
✔️ c2c9fdb #1 2023-11-29 14:11:58 ~11 min tests 📄log
✔️ 210cf64 #2 2023-11-30 13:57:46 ~7 min ios 📱ipa 📲
✔️ 210cf64 #2 2023-11-30 13:57:50 ~7 min android-e2e 🤖apk 📲
✔️ 210cf64 #2 2023-11-30 13:59:04 ~8 min android 🤖apk 📲
✔️ 210cf64 #2 2023-11-30 14:02:24 ~12 min tests 📄log

@pavloburykh
Copy link
Contributor

@cammellos thanks for the PR! Still facing crash after sync on IOS while on Android it is okay.

ISSUE 1 IOS app crashes after sync with Desktop

IOS 16.7.2 iPhone X

logs (16).zip

telegram-cloud-document-2-5422407811906681993.mp4

@churik
Copy link
Member

churik commented Nov 29, 2023

For me, it was crashed just after logging in (in community list) - IOS only:
logs (68).zip

@cammellos
Copy link
Contributor Author

@pavloburykh are you syncing a token gated community? if you sync without a token gated community, does the issue still occours?

@cammellos
Copy link
Contributor Author

For me, it was crashed just after logging in (in community list) - IOS only: logs (68).zip

on android nothing? (I need the crash logs, but not sure how to get them from a ios real device, maybe @qfrank knows?)

@pavloburykh
Copy link
Contributor

@pavloburykh are you syncing a token gated community? if you sync without a token gated community, does the issue still occours?

I have just tried to sync with newly created user who doesn't have any data yet and IOS didn't crash.

But then I have tried to sync once again with a user who has token gated communities and it didn't crash too...

So this crash is NOT reproducible every time and therefore it is hard to say if there is any relation to the existing data of synced user.

@qfrank
Copy link
Contributor

qfrank commented Nov 30, 2023

For me, it was crashed just after logging in (in community list) - IOS only: logs (68).zip

on android nothing? (I need the crash logs, but not sure how to get them from a ios real device, maybe @qfrank knows?)

to get them from an ios real device, the approach is the same as this guideline @cammellos

@cammellos
Copy link
Contributor Author

@churik @pavloburykh could you try to replicate the issue following the instructions in the doc #18024 (comment) please and send over the logs?
Thank you (and thanks @qfrank for reminding me yet again :) )

@qfrank
Copy link
Contributor

qfrank commented Nov 30, 2023 •
edited
Loading

@churik @pavloburykh could you try to replicate the issue following the instructions in the doc #18024 (comment) please and send over the logs? Thank you (and thanks @qfrank for reminding me yet again :) )

they won't get useful logs .. but after following the guideline, they can take a screenshot for the located stacktrace like this, that's pretty useful and acurate.

TBO, the guideline is suitable for developers rather than QA. @cammellos 🙂

@qfrank
Copy link
Contributor

qfrank commented Nov 30, 2023 •
edited
Loading

For me, it was crashed just after logging in (in community list) - IOS only: logs (68).zip

curious is it reproducable between 2 ios devices? @churik @pavloburykh

@churik
Copy link
Member

churik commented Nov 30, 2023 •
edited
Loading

I can't reproduce it today with the same steps, so perhaps it is the same #16560

@pavloburykh
Copy link
Contributor

@cammellos I think we are ready to merge the PR. The main issue with Android crash and contacts restoring is fixed, thank you!

The rest of the issues from this #18021 will be logged separately.

@pavloburykh
Copy link
Contributor

@cammellos before merge, could you please rebase go and mobile branches once again and I will run e2e tests.

@cammellos cammellos force-pushed the bug/fix-panic-sync-community branch from c2c9fdb to 210cf64 Compare November 30, 2023 13:49
@status-im-auto
Copy link
Member

88% of end-end tests have passed

Total executed tests: 48
Failed tests: 2
Expected to fail tests: 4
Passed tests: 42

IDs of failed tests: 703133,703086 

IDs of expected to fail tests: 702732,703503,702731,702808

Failed tests (2)

Click to expand
  • Rerun failed tests

    • Class TestCommunityOneDeviceMerged:

    1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133

    Device 1: Find `Button` by `accessibility id`: `show-profiles`
    Device 1: Tap on found: Button 

    critical/chats/test_public_chat_browsing.py:273: in test_restore_multiaccount_with_waku_backup_remove_switch
    self.errors.verify_no_errors()
base_test_case.py:191: in verify_no_errors
    pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
 Incorrect contacts number restored: 0 instead of 2
E    member_open was not restored from waku-backup!!
E    admin_closed was not restored from waku-backup!!
E    member_closed was not restored from waku-backup!!



    Device sessions

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_mark_all_messages_as_read, id: 703086

    Device 1: Tap on found: Button
    Device 1: Click until Text by accessibility id: community-description-text will be presented 

    critical/chats/test_public_chat_browsing.py:769: in test_community_mark_all_messages_as_read
    self.errors.verify_no_errors()
base_test_case.py:191: in verify_no_errors
    pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
 Unread messages badge is shown in community element while there are no unread messages
E    New messages badge is shown in community channel element while there are no unread messages



    Device sessions

    Expected to fail tests (4)

    Click to expand

    Class TestCommunityOneDeviceMerged:

    1. test_community_discovery, id: 703503 

    Test is not run, e2e blocker

    [[reason: [NOTRUN] Curated communities not loading, https://github.com//issues/17852]]

    Class TestGroupChatMultipleDeviceMergedNewUI:

    1. test_group_chat_pin_messages, id: 702732 

    Test is not run, e2e blocker

    [[reason: [NOTRUN] Pin feature is in development]]

    2. test_group_chat_offline_pn, id: 702808

    Device 3: Looking for a message by text: message from old member
    Device 3: Looking for a message by text: message from new member 

    critical/chats/test_group_chat.py:323: in test_group_chat_offline_pn
    self.errors.verify_no_errors()
base_test_case.py:191: in verify_no_errors
    pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
 Messages PN was not fetched from offline

    [[Data delivery issue]]

    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_pin_messages, id: 702731 

    Test is not run, e2e blocker

    [[reason: [NOTRUN] Pin feature is in development]]

    Passed tests (42)

    Click to expand

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_several_images_send_reply, id: 703194
    Device sessions

    2. test_community_one_image_send_reply, id: 702859
    Device sessions

    3. test_community_emoji_send_copy_paste_reply, id: 702840
    Device sessions

    4. test_community_contact_block_unblock_offline, id: 702894
    Device sessions

    5. test_community_message_delete, id: 702839
    Device sessions

    6. test_community_message_send_check_timestamps_sender_username, id: 702838
    Device sessions

    7. test_community_links_with_previews_github_youtube_twitter_gif_send_enable, id: 702844
    Device sessions

    8. test_community_message_edit, id: 702843
    Device sessions

    9. test_community_unread_messages_badge, id: 702841
    Device sessions

    Class TestCommunityMultipleDeviceMergedTwo:

    1. test_community_markdown_support, id: 702809
    Device sessions

    2. test_community_hashtag_links_to_community_channels, id: 702948
    Device sessions

    3. test_community_mentions_push_notification, id: 702786
    Device sessions

    4. test_community_leave, id: 702845
    Device sessions

    5. test_community_join_when_node_owner_offline, id: 703629
    Device sessions

    Class TestDeepLinksOneDevice:

    1. test_links_open_universal_links_from_chat, id: 704613
    Device sessions

    2. test_links_open_universal_links_from_other_apps, id: 704614
    Device sessions

    3. test_links_deep_links, id: 702775
    Device sessions

    Class TestGroupChatMultipleDeviceMergedNewUI:

    1. test_group_chat_mute_chat, id: 703495
    Device sessions

    2. test_group_chat_send_image_save_and_share, id: 703297
    Device sessions

    3. test_group_chat_reactions, id: 703202
    Device sessions

    4. test_group_chat_join_send_text_messages_push, id: 702807
    Device sessions

    Class TestActivityCenterContactRequestMultipleDevicePR:

    1. test_add_contact_field_validation, id: 702777
    Device sessions

    2. test_activity_center_contact_request_accept_swipe_mark_all_as_read, id: 702851
    Device sessions

    3. test_activity_center_contact_request_decline, id: 702850
    Device sessions

    Class TestActivityMultipleDevicePRTwo:

    1. test_activity_center_mentions, id: 702957
    Device sessions

    2. test_activity_center_admin_notification_accept_swipe, id: 702958
    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:

    1. test_1_1_chat_delete_via_long_press_relogin, id: 702784
    Device sessions

    2. test_1_1_chat_is_shown_message_sent_delivered_from_offline, id: 702783
    Device sessions

    3. test_1_1_chat_mute_chat, id: 703496
    Device sessions

    Class TestActivityMultipleDevicePR:

    1. test_navigation_jump_to, id: 702936
    Device sessions

    2. test_activity_center_reply_read_unread_delete_filter_swipe, id: 702947
    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_emoji_send_reply_and_open_link, id: 702782
    Device sessions

    2. test_1_1_chat_text_message_delete_push_disappear, id: 702733
    Device sessions

    3. test_1_1_chat_push_emoji, id: 702813
    Device sessions

    4. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
    Device sessions

    5. test_1_1_chat_edit_message, id: 702855
    Device sessions

    6. test_1_1_chat_send_image_save_and_share, id: 703391
    Device sessions

    7. test_1_1_chat_message_reaction, id: 702730
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    2. test_community_undo_delete_message, id: 702869
    Device sessions

    3. test_community_navigate_to_channel_when_relaunch, id: 702846
    Device sessions

    4. test_community_mute_community_and_channel, id: 703382
    Device sessions

    @churik
    Copy link
    Member

    churik commented Nov 30, 2023

    @cammellos can you please merge PR and update status-go on our side?
    Thanks!

    @cammellos cammellos merged commit 210cf64 into develop Nov 30, 2023
    6 of 7 checks passed
    @cammellos cammellos deleted the bug/fix-panic-sync-community branch November 30, 2023 15:35
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers
    No reviews
    Assignees

    @cammellos cammellos

    Labels
    None yet
    Projects
    Pipeline for QA
    Archived in project
    Legacy QA Automation Project
    Archived in project
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.
    5 participants
    @cammellos @status-im-auto @pavloburykh @churik @qfrank