search

element-hq / element-ios

A glossy Matrix collaboration client for iOS
https://element.io
GNU Affero General Public License v3.0
1.73k stars 499 forks source link

Missing push notifications #1696

Open manuroe opened 6 years ago

manuroe commented 6 years ago

There are several reports of people complaining Riot sometimes misses to notify them like in the Riot iOS room.

I reproduced a possible similar bug at https://github.com/matrix-org/riot-ios-rageshakes/issues/748 where the notification happened with a 3 min delay. Looking at system logs, they show that the app tried to start at the time of the notification but it fired a 30s watchdog because it was infinitely looping at https://github.com/vector-im/riot-ios/blob/898353e47d0b70486885e322a2bd3620176e2383/Riot/AppDelegate.m#L315-L320

Note: this piece of code was for fixing spontaneous logout (https://github.com/vector-im/riot-ios/pull/1653)

manuroe commented 6 years ago

A way to reproduce missing notifications:

-> The device will never show a notification for the message.

Looking at OS logs, Riot fired the watchdog described above.

manuroe commented 6 years ago

After testing, the bug described in the previous comment is fixed by https://github.com/vector-im/riot-ios/commit/9f2fe587e38465778e4250706688d10aa04385fa.

csett86 commented 6 years ago

Further push notification crash logs:

Push-notification-crash-logs.zip

At least for the last one, 18.12. 13:32 I know I missed a push notification for a message sent 13:31 to me. The other logs look very similar anyway.

manuroe commented 6 years ago

Thanks @csett86 for your logs that confirm the fix I made is the right one.

manuroe commented 6 years ago

Dave now receives a burst of notifications when he unlocks his phone in the morning.

manuroe commented 6 years ago

The solution seems to move the storage of MXAccount credentials from NSUserDefaults to a simple file with the appropriate read access right like NSFileProtectionCompleteUntilFirstUserAuthentication.

The reason of this move is explained at https://forums.developer.apple.com/thread/15685#45849: "IMO the best way around this is to avoid NSUserDefaults for stuff that you rely on in code paths that can execute in the background."

SB-68846 commented 6 years ago

Is there any updates on this issue? Most of our employees on iOS are experiencing it to varying degree. Android works fine.

Happy to help with additional info if needed.

cwmke commented 6 years ago

I have noticed that I don't get push notifications at all on iphone X but they work for the most part on an older ipad.