-
Notifications
You must be signed in to change notification settings - Fork 984
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
Push notifications are not delivered to IOS app #21120
Comments
UPDATE: sometimes PNs go through. @mariia-skrypnyk and @churik were able to receive some PNs. |
I have investigated the issue: What looks like, in at least one instance:
Looking at kibana's logs, I can see that no filter node has pushed that message to the right client: This is the missing message. The peer id of the sender is "16U*hhQfss" You can see a couple of minutes later, a successful push: But now looking at the client log, it looks like they never registered for that particular topic, so i'll start from scratch investigating. |
Looking at this message now:
The message was received by a push notification server: |
The response, was sent:
Notice the time: 10:46:10.404 And the content topic: /0x25a24866 If we go back to the client:
That's when we register the filter.
But then 2 seconds later it's actually tried to register
So the timeline is:
Message would be received between 3 & 4, but filter is not pushed as the client hasn't yet register the filter. You can see similar examples from the logs, where we query store nodes just after scheduling register a filter, but we receive messages too early. status-im/status-go#5440 introduced the ticker. Before the issue was probably already present (registering a filter was probably asynchronous, although I think on v1 it was sync), but most likely was never picked up as the window was smaller. Potential solutions:
Either ways, this looks like a fairly critical message reliability issue, lowering the batch ticker (it's currently I believe 5 seconds, which is quite high) is probably the quickest to lower the possibility of that happening. cc @churik @fryorcraken @chaitanyaprem @richard-ramos @ilmotta and thank you @pavloburykh for all the logs |
As a way to not block release, I'd say that we can reduce the ticker length to a length which fast enough to make this issue potentially not happen (maybe something like 500ms or less?). Then we can try work out a proper solution. @chaitanyaprem and I had talked before about this possible window on which messages would not be received due to the ticker and doing the storenode request could be a good solution indeed, taking into account that there might be a delay for the message to arrive to the storenode. Nwaku storenodes will only accept messages with a timestamp +-20s its current time, so based on this, when we register a filter and the ticker is triggered, we asynchronously wait 20s, and then send a store request. What do you think, @cammellos and @chaitanyaprem? |
Improved in status-im/status-go@4a18c85 as a temp measure; the issue requires more investigation / actions, so postponed to 2.31 |
I am wondering if doing a store query after 20secs would help, because current missing messages periodic store query anyways happens every 20seconds. Which means anyways worst-case these messages would be fetched in next store query isn't it (unless i am missing something)? |
Issue is still valid for me. @status-im/mobile could you please also re-check if you are receiving push notifications on IOS? |
still valid. No pushes on IOS. Devices:
|
@pavloburykh checked on 19/09 nightly on iOS - no PN . iPhone 13, iOS 17 |
I still have notifications working on release 2.30 and in PRs (enabled from profile - and from onboarding) IPhone 14 Pro, IOS 17.6.1 |
referring issue being tracked for go-waku waku-org/go-waku#1211 but looks like the fix has not been propagated correctly it into the This is why @churik is not facing any issues as she is testing on 2.30 whereas others might be testing on develop branch. |
pushed a fix in #21297 @pavloburykh @churik can you please get this revalidated and confirm if the issue still happens. |
i havent gotten any push notification in 2.30 release. tested it just now in dms nothing arrives. iphone xs @chaitanyaprem @churik edit: its worse actually. after 5 hours decided to open the mobile app and then had to wait for about 3-4 minutes before any new messages appeared inside the app including the DM i sent from another account. unfortunately this is how its been for the past months: to just load any new messages i need to wait for multiple minutes upon opening the app every time. there doesnt seem to be any prioritization either i can wait just as long for any dm to appear as i can for any random channel in a community having unread messages. i hope there are issues addressing these loading times bc i just have status inu status and like 3 sparse dm chats |
@ilmotta |
Could be, but I don't see how at the moment. If we could reproduce this issue reliably it would be pretty easy to create a new build to login in the old way and check if it's related to the issue you mentioned. But basically we just moved one step from the messenger to happen after login, but the time to setup is 100% the same. But I think what we did in #20173 was a necessary step. We can't mess up the login and onboarding step. Based on past investigation, a big slice of the cost to set-up filters during login is related to the cost of decrypting communities. Assuming many don't want to use communities and chats and might just want to use the wallet or something else, login should happen as fast as possible. We have a few ways to improve the login:
I remember that we still had room left to optimize the way we process community chats. This is the problematic function https://github.com/status-im/status-go/blob/77ef8f1fb7e6ee08b6f09e4e19cf7b4b5f062a16/protocol/messenger.go#L1769
This is a terrible experience @86doteth...
I would say so. The performance problem to initialize the messenger is well known to us, we are just now seeing that it can get worse quickly depending on the type/amount of data the user has. |
I'll try to investigate the timing and report it as a separate performance issue. Basically, the more communities you have, the later you'll start to receive the messages. And yes, it was not a question of the necessity of changing the login; rather, this problematic function is now handled after login, which gives the impression of a significant delay (to my understanding) Thank you for reply, @ilmotta |
@
@churik I have logged a separate issue here #21358 |
we're closing this one as the problem with performance moved here and so far we're receiving IOS push notifications in develop and in release. Will open a new one if it will reproduce again |
with release i guess you mean 2.31? bc i havent gotten any push notifs on 2.30 |
@86doteth I'm not sure if I asked you (can't see from the comments here), but there are some things to consider:
Push notifications were working for me on release 2.30. |
|
@86doteth please, try to re-enable it in Profile, re-login and see if it helps. |
@churik |
reopened it with no-steps, as we're unable to reproduce it. |
@churik
|
Reproducing on IOS 17.5.1, iPhone 15
Preconditions: User A - message sender (any device), User B - PN receiver, IOS device
Steps:
Actual result: PN is not received by User B
Andr_PN_sender.zip
IOS_PN_receiver.zip
PNs_IOS.mp4
Additional Information
The text was updated successfully, but these errors were encountered: