-
Notifications
You must be signed in to change notification settings - Fork 986
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
fix: handle network change for filter subs #21863
Conversation
Jenkins BuildsClick to see older builds (44)
|
c328767
to
67184f6
Compare
8be9a47
to
aa54f33
Compare
Closing this as testing is complete and issue has been addressed. |
aa54f33
to
08c1b84
Compare
This is a fix added to handle network change in mobile between wifi and cellular where filter stops working after switch from wifi to cellular. I have tested with network change and did not notice any issues, infact it was fixed. @churik @pavloburykh i would like e2e tests to run in order to get sanity before i merge the underlying status-go PR. Would also be great if we can test network switch scenarios. @ilmotta , if we are planning to do an intermediate |
89% of end-end tests have passed
Failed tests (4)Click to expandClass TestCommunityOneDeviceMerged:
Class TestWalletMultipleDevice:
Class TestDeepLinksOneDevice:
Expected to fail tests (2)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Passed tests (50)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePR:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestFallbackMultipleDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestWalletOneDevice:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestWalletMultipleDevice:
Class TestDeepLinksOneDevice:
|
Thanks for suggesting the change coming soon from status-im/status-go#6232. By next week we hope to define if we are going to release 2.32.1 and what we want to include. I did a quick check, and git conflicts are getting in the way, although they are probably solvable. go-waku versions have drifted from status-go release and develop branches. This other merge commit status-im/status-go@e7cc535 also needs to be brought in onto the status-go release branch before the others, otherwise more conflicts arise. |
@chaitanyaprem @ilmotta @churik Testing Summary ReportDevices and Builds:
Scope of Testing:
User Scenarios:
### Tested Situations:
General Observations:Messages were received promptly with minimal delays in most cases. Issues Identified:Issue 1: Prolonged Delay in Group Chat Message Reception (3-4 minutes)
Expected Result: Actual Result: Issue 2: Community Message History Loss (50% Missing)
Expected Result: Actual Result: Additional Reproduction Scenario: While on mobile data, select Mobile Data + Wi-Fi in settings. |
@chaitanyaprem Could you please confirm whether these are actual bugs or expected behavior? Issue 1 doesn’t entirely look like a bug, but since group chats don’t exhibit similar problems in other scenarios and everything works fine under the same conditions for 1-1 chats and communities, I decided to report it as a separate issue. Issue 2 seems similar to this bug, and I’m unsure if it can be resolved by this PR or if a specific solution is required. |
Thanks for the detailed tests @Horupa-Olena Issue-2 definitely doesn't seem to be related to this PR and probably an issue with store nodes or history query when there is a network change. issue-1 seems interesting. Delay of 3-4 minutes means that periodic store query seems to be getting delayed by that much time which should not happen. Odd that this is not happening for communities or 1-1 chats but only group chats.
Lastly could you share the content-topic and envelopeHash/messageHash of a message that was received and 1 that was not received. it would help debug quicker. |
Thank you for your response!
Could you please advise where I can find this information? |
ah, i am wondering if this means periodic store query is disabled. Maybe someone from mobile team can confirm what this means. If periodic store query is disabled, it can explain the delay.
ah, so some of the messages sent to the same group chat were not delivered while some were? That was kind of my question. But anyways if i know the messageId / envelopeHash and content-topic of a message which was successfully delivered vs a message that got delayed or not delivered would help debug better.
|
that is interesting, will take a look at the logs to see if i find anything unique in the first mobile connection switch. |
One thing to note here is community history is only fetched for past 24hours max at any point in time. If messages older than 24hours need to be fetched user has to manually invoke the fetch. Not sure how to do that in mobile though. |
Thank you for the note. I checked the community, and indeed, the history is only for the past 24 hours. |
@chaitanyaprem Thank you for the instructions. I’ll try to reproduce the issue again now and capture these logs. I’ll get back to you as soon as I’m done. |
@chaitanyaprem I opened the logs from iOS (the sender) and tried to find the messageHash/envelopeHash/messageContent, but I couldn’t locate them. Log (reciever): The only thing I found in the Status logs was information about sent messages. Perhaps this might give you some insights; it starts with this line:
These are all the messages after the delay. Please try searching the logs for messageHash/envelopeHash/messageContent as well. |
As it is difficult to find messageID while sending from mobile and @Horupa-Olena confirms that if sender is used from nightly build the message delay or loss is not observed in same scenario i.e switching from wifi to 4G. Looks like some other issue wrt sending is occuring. On looking at One of them could be due to store query failures which happen intermittently and has no realted to changes in underlying PR. on top of this, as per tests the overall situation has significantly improved when switching between Wi-Fi and mobile networks we can raise the issue noticed separately and go ahead for now. Thanks a lot for the tests @Horupa-Olena ! |
Closing as testing is done and underlying PR is merged. |
fixes filter not working when network switches from wifi to cellular.
Corresponding status go PR status-im/status-go#6232
Summary
Platforms
Functional
Steps to test
I would suggest testing network change (wifi to cellular, cellular to wifi etc) scenarios and reception of messages prior and after network change. Following are scenarios that should get validated
status: ready
Risk
Described potential risks and worst case scenarios.
Tick one: