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.

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

Unread count is broken in stream-chat. #2309

Closed
Crysis21 opened this issue Sep 21, 2022 · 45 comments · Fixed by #2481
Closed

Unread count is broken in stream-chat. #2309

Crysis21 opened this issue Sep 21, 2022 · 45 comments · Fixed by #2481
Assignees
Labels
🐞 Bug An issue or PR related to a bug 🌐 SDK: StreamChat (LLC) Tasks related to the StreamChat LLC SDK 🔜 Upcoming This issue is fixed in “develop” and will be available in the next release

Comments

@Crysis21
Copy link

What did you do?

We found out the channels read count is broken. So the way stream works, a channel has a structure read info, which is held at channel level, for each user that is part of the channel. When we receive a message, we get the unread count from the channel to display it in the UI. This unread count, starts do display random values: 1,2,4,8,11,14 when the real numbers were 1,2,3,4,5,6. We read the unread count in 2 situations: when a message is received, or when the channel is updated. When a message is received, the count is wrong, but when the channel is updated, the count is correct. We can also see in the push notifications logs the unread count as a control number, so definetly the SDK has a bug again.

GetStream Environment

**GetStream Chat version:4.21.2
**iOS version:15.7
**Swift version:5.
**Xcode version:14
**Device:iPhone 11 Pro

Additional context

@polqf
Copy link
Contributor

polqf commented Sep 21, 2022

Hi @Crysis21,

First of all, could you please share a video so that we fully understand when and how those counters are changing?
Also, do you mean that these unread counts that act weirdly are per channel, or is it a global counter?

We are shipping this fix in the following release, which might be of your interest if the issue is with per-channel counters: #2288

@polqf
Copy link
Contributor

polqf commented Sep 26, 2022

Hi @Crysis21 , could you please confirm if the changes shared above fix the issue?

@polqf polqf added the 🔜 Upcoming This issue is fixed in “develop” and will be available in the next release label Sep 26, 2022
@Crysis21
Copy link
Author

hey @polqf , no the changes above don't fix the issue. The problem appears when you have 2 devices for the same account connected and they receive messages. Seems like a backend issue. Not happening when using only one device per account.

@polqf
Copy link
Contributor

polqf commented Sep 27, 2022

Ho @Crysis21, we are sorry to hear that.

Could you please share a video or any other info where we can see it happening? It would help us to reproduce it.

@nuno-vieira nuno-vieira added 🐞 Bug An issue or PR related to a bug and removed 🔜 Upcoming This issue is fixed in “develop” and will be available in the next release labels Sep 27, 2022
@nuno-vieira
Copy link
Member

Hi @Crysis21!

Can you please share a code snippet where are you reading the unread counts from the messages update?

Thank you,
Nuno

@nuno-vieira nuno-vieira added 🔎 Investigating This issue is currently being investigated (Not reproducible yet) and removed 🐞 Bug An issue or PR related to a bug labels Oct 6, 2022
@nuno-vieira
Copy link
Member

Closing this one for inactivity.

Feel free to re-open when you have more details.

Best,
Nuno

@Crysis21
Copy link
Author

Crysis21 commented Nov 2, 2022

hey @nuno-vieira and @polqf, the issue is there, but you guys didn't check.

Unread counts jump or display random values. This happens due to 2 getstream factors:

  1. ContentHandler for notifications, which tries to keep the channel in sync. Internally, when a new message is received, it increses a counter, without fetching the latest information from the backend.

  2. A while back they prompted us to upgrade push notification and change notifications mode from offline, to online mode. This means that push notifications are broadcasted all the time, regardless of the websocket connection status. Due to this change, NotificationService is executed for every message, while the application is also running. The result is that getsream is increasing the counter twice, one in NotificationService and once in the main application, which results in the app having random jumps in the notitication counter.

@Crysis21 Crysis21 changed the title Unread count is broken in stream-chat Unread count is broken in stream-chat. Nov 2, 2022
@Crysis21
Copy link
Author

Crysis21 commented Nov 2, 2022

Screenshot 2022-11-02 at 16 56 48

@nuno-vieira when I break the NotificationService in debugger, for every message that I receive, there are 2 events in the queue to be processed.

@nuno-vieira
Copy link
Member

Hi @Crysis21,

Without a good description of the issue, it is hard to understand what you mean. Only now it is clear that you are talking about the notification badge count. Either way, a video recording would be more helpful to understand the behaviour.

I just recorded one myself, with the app open, and with the notifications being displayed. It is noticeable that sometimes the badgeCount grows faster than the push notifications, but this is normal. The web socket is much faster than the push notifications. But in the end, the badge count will always be correct.

Video:

RPReplay_Final1667401729.mov

@Crysis21
Copy link
Author

Crysis21 commented Nov 3, 2022

Hey guys,

@nuno-vieira I am not talking about the badge count. I am talking that the ChatChannel.reads, at various points in time gets broken. We display the info from that, into the conversations list. If happens especially if you abuse it. Our app just prints the information straight from the ChatChannel reads information.

RPReplay_Final1667474299.MP4

@glennposadas
Copy link

glennposadas commented Nov 14, 2022

Hey guys! I wish I could show you the video recording of this issue on our app. We have been this issue having for a month now! I'm glad thought that I found this ticket. Thanks!

From our QA ticket (project list and project details have both the badge counts):

Test Loop:

----

WEB APP:

Send a chat message

----

iOS APP

Go to next screen:

If on project list → go to project details

If on project details → go to chat screen

If on chat screen <- go back to project details

If on project details ← go back to project list

Check badge count

@nuno-vieira nuno-vieira added 🐞 Bug An issue or PR related to a bug 🎨 SDK: StreamChatUI Tasks related to the StreamChatUI SDK and removed 🔎 Investigating This issue is currently being investigated (Not reproducible yet) labels Nov 14, 2022
@nuno-vieira
Copy link
Member

Hi @glennposadas,

Yes, this looks like a real issue. We are tracking this in our Support ZenDesk channel. But for visibility, we will re-open the issue here too.

We will let you know once we have more details about this.

Best,
Nuno

@nuno-vieira nuno-vieira reopened this Nov 14, 2022
@nuno-vieira nuno-vieira added 📆 Planned 🌐 SDK: StreamChat (LLC) Tasks related to the StreamChat LLC SDK and removed 🎨 SDK: StreamChatUI Tasks related to the StreamChatUI SDK labels Nov 14, 2022
@Crysis21
Copy link
Author

@nuno-vieira any chance we can get a fix for this? There are plenty of offline synchronization issues with the iOS SDK and we're seeing very little progress on them. It's been almost 2 months since I reported this one.

@nuno-vieira
Copy link
Member

Hi @Crysis21,

Unfortunately, I can't provide an ETA yet. We have limited capacity and we need to prioritise all the issues. Right now we have other important things to tackle. Also, you reported this 2 months ago, but you were 1month inactive where you didn't provide the full details of the issue.

@vjard33
Copy link

vjard33 commented Nov 17, 2022

Same issue here, we cannot use the badgeCount, the number is wrong.

@Crysis21
Copy link
Author

Hi @Crysis21,

Unfortunately, I can't provide an ETA yet. We have limited capacity and we need to prioritise all the issues. Right now we have other important things to tackle. Also, you reported this 2 months ago, but you were 1month inactive where you didn't provide the full details of the issue.

We provided enough details for the bugs. We also opened a ton of issues on Zendesk where we provided constant feedback and tried to help as much as possible.

@Crysis21
Copy link
Author

Crysis21 commented Dec 5, 2022

hey @nuno-vieira, any news on this?

@nuno-vieira
Copy link
Member

Hi @Crysis21,

We currently have the backend investigating as well, since there were some similar issues on Android. But right now we don't know the root cause of the issue yet.

Best,
Nuno

@nuno-vieira
Copy link
Member

@vjard33 Could you also share the exact steps to reproduce this on your side? On our DemoApp this is quite hard to reproduce, so maybe there is a better way to reproduce this.

Thanks!

@glennposadas
Copy link

Bump. Thanks all for the hard work. I hope we get the fix by January?

@vjard33
Copy link

vjard33 commented Dec 21, 2022

@nuno-vieira @polqf
Still have the problem on SDK 4.25.0
Pay attention to the badge count on ChannelViewController. there is +1 at the end on the third message (4 instead of 3).

RPReplay_Final1671630342.1.mp4

@nuno-vieira
Copy link
Member

Hi @vjard33,

4.25.0, does not mention it fixes this issue. We also didn't close it yet as well. The team is still investigating it.

Best,
Nuno

@glennposadas
Copy link

Bumped. Happy new year to y'all.

@Crysis21
Copy link
Author

Crysis21 commented Jan 9, 2023

hey guys, any update for an actual fix for this issue?

@polqf
Copy link
Contributor

polqf commented Jan 9, 2023

Hi @glennposadas , @Crysis21 , @vjard33
We believe to have fixed it in this PR: #2433

Would be amazing if you can give it a try

@nuno-vieira nuno-vieira added the 🔜 Upcoming This issue is fixed in “develop” and will be available in the next release label Jan 10, 2023
@nuno-vieira
Copy link
Member

This fix is now available in the 4.26.0 release.

Thank you for your patience.

Best,
Nuno

@Crysis21
Copy link
Author

Hey guys,

@polqf @nuno-vieira this is not working. I tried to see if your sample app works, but push notifications are not delivered in your sample. the device token is registered, but I don't get anything. Also, in configuration->Push Configuration does nothing (you have a break in code, so you might as well remove that misleading option.

The problem is again, your synchronization with the NotificationService extension. Your code doesn't cope with the fact that there are 2 sources changing the storage info. I have the perfect sample from my application.

If in the NotifiactionService stream-chat client that I initialize, I don't pass the appGroup (which means both the app and NSE will use the same storage information), the counts are working just fine in the app. This means you avoid any concurrency problems and the code works as expected.

If I use the same AppGroupIdentifier, which should enable us to sync the local storage when we receive push notifications, the unread counts are broken again. But this time, are broken in reverse. They no longer jump as before, they actually don;t count at all. Randomly you would see an update or two, but nothing more.

Why is push not working on your sample app? Did you guys even test your code in a real-life scenario, when users have a ton of push notifications? If so, why isn't your sample app fully working in this regard? Whenever we try to plug in all the components as in documentation, and have offline storage + push notifications sync, things are broken. It's been months.

Please watch the video below to check what I am talking about. In the first part of the video, I am using the same AppGroup identififer enabled and after the app restarts, I disable it. Please observe the behavior and tell me how the unreads counts are fixed. Please note that we render the reads info from the channel, we don't do any special magic.

The conclusion is, if we want to have unread counts fixed, we need to drop support for offline synchronization from push service and vice-versa.

Screenshot 2023-01-16 at 11 10 15

RPReplay_Final1673860086.MP4

@glennposadas @vjard33 is the issue fixed for you? Do you use the same app group identifier in notifications service extension?

Thank you.

@polqf
Copy link
Contributor

polqf commented Jan 16, 2023

Hi @Crysis21,

I think you are mixing up things here. Let me explain.

If you have been working on iOS for a while you would know that regarding push notifications there are important things to note. First one being not using the simulator, and second one being that in a testing environment, even using devices, there is a factor of unreliability when it comes to Apple and the delivery of notifications. This is completely unrelated to Stream, and it is wrong to say that those don't work in our Demo App.
We test pushes, as we test the rest of our code and demo apps, and those are working.

Leaving that important factor aside. Seems like there is something wrong in your app, as it seems like the channel controller is kept alive for a while after poping it (when it is still alive, it marks as read the messages as they come, and as you can in you video this is consistent in every time you do it, and only after a while it starts counting again)

As for the rest of the video, there is one particular issue that we haven't found for a lot of time, which is that at some point, when entering-leaving the channel many times, the count seems to not update properly. You are completely right on that one, and we'll try to get to it asap as we thought this was already fixed for a while.
The fix that we linked to this issue was not supposed to fix this particular issue, and therefore it makes no sense to expect it to be fixed by it.

Why is push not working on your sample app? Did you guys even test your code in a real-life scenario, when users have a ton of push notifications? If so, why isn't your sample app fully working in this regard? Whenever we try to plug in all the components as in documentation, and have offline storage + push notifications sync, things are broken. It's been months.

We thoroughly test what we do. And by that I am not saying that everything is perfect. We are constantly iterating and focusing on things based on priority. But I think making statements like "things are broken" is simply not true.

@Crysis21
Copy link
Author

@polqf allow me to disagree with you. the push notifications are being delievered correctly. I forgot to mention that we have the option of delivering push even when user is online (someone from support enabled it a while ago).

As for the channel being kept, I will double check, but is not the case as sometimes it happens to see this behavior on a cold start of the app. As soon as you exit the chat screen, we dismiss any ChatChannel objects.

The problem appears when the getStream SDK is configured to use the same app group as the app. This is the reason I attached you the line of code that makes the difference. If I go back to a previous version of getStream SDK, the counts start to jump. With 4.26, they don't update.

we're not using simulators at all for development, as we have apple sign-in. I will debug it more and come back at you.

@polqf
Copy link
Contributor

polqf commented Jan 16, 2023

The problem appears when the getStream SDK is configured to use the same app group as the app. This is the reason I attached you the line of code that makes the difference. If I go back to a previous version of getStream SDK, the counts start to jump. With 4.26, they don't update.

The "jumping" issue is the one that was being tracked here, and it is supposed to be fixed now. We'll get deeper into the one you just reported. If you happen to find any way to consistently reproduce it, please let us know.

@Crysis21
Copy link
Author

@polqf @nuno-vieira yes, the consistent way of reproducing it is activating the line in NotificationServiceExtension.

        var config = ChatClientConfig(apiKey: .init(AppConstants.Messaging.getStreamApiKey))

       //  this causes concurrency issues and breaks counts
       //  config.applicationGroupIdentifier = AppConstants.Defaults.appGroupIdentifier

Also, we have a Desktop Client which uses the JS SDK. You can see how our implementation responds to all events from getStream, when the other client reads the messages. You can also see that our app is not stuck with the channel reading events as you suggested, otherwise the unread counter should not incremend on Desktop app. This is a clear side by side comparison of your iOS sdk and JS SDK.

Screen.Recording.2023-01-17.at.09.05.31.mov

Let me know if need anymore help or you would like to check how our apps are configured in dashboard, maybe that makes a difference to your testing setup.

@Crysis21
Copy link
Author

@polqf Let me know if you need more context for this issue.

@polqf
Copy link
Contributor

polqf commented Feb 1, 2023

@Crysis21 we are looking into it once again. We'll keep you posted with updates

@polqf
Copy link
Contributor

polqf commented Feb 13, 2023

#2481 fixes the issue you were seeing.

Just a heads up that there is an issue we've seen in an edge case where you get a lot of messages at the same time, in which the SDK seems to not process all of them, and the unread count might be less than the actual value. This is something we have only seen in pretty intense testing cases, but worth a mention. We'll be working on this soon, as well, but we believe your issue should have been fixed by the PR linked above.

If you have some time to give it a try, please confirm if the issue is fixed.

@Crysis21
Copy link
Author

@polqf thank you, I will test it in the next 2 days.

Regarding the edge case you mentioned: if there is a high volume of messages, it appears to be some form of memory leak from the content handler. The Notification Service Extension reaches the 24mb memory limit and is terminated by the system. Maybe this is what makes it skip counts.

We noticed this because we have end 2 end encryption and when the extension is killed, the message is pushed to notifications centre encrypted, which is a bad UX for us.

@polqf
Copy link
Contributor

polqf commented Feb 14, 2023

@Crysis21 this is not the issue we identified, but good to know that. Could you expand on that so we can open an internal ticket? You are basically saying that when trying to decrypt the message from the extension, it gets terminated so the extension just "pushes" the original encrypted message to the user, right?

As per the issue I was referring to, it only happens when in foreground and receiving a huge load of messages, which are then handled by the host app.

@glennposadas
Copy link

@glennposadas @vjard33 is the issue fixed for you? Do you use the same app group identifier in notifications service extension?

Unfortunately, my ticket at work got reopened, apparently, this issue came up once again. I'm using 4.26.0. But while typing this, I noticed that this ticket fix is under the 4.27.0. Perhaps, I should just update the library once more.

@glennposadas
Copy link

Up. Still happening in 4.27.1.

@polqf
Copy link
Contributor

polqf commented Mar 29, 2023

Hi @glennposadas , please expand on what's happening, as it seems it might be a different issue. We tested the scenarios that were described in this issue, and none of them were reproducible after the fixes.

Recordings, snippets or logs would be appreciated.

@glennposadas
Copy link

I am not sure if I can share a demo here. How can I send it to you?

@glennposadas
Copy link

glennposadas commented Mar 30, 2023

@polqf I just got the permission to share this.

720.mov

@glennposadas
Copy link

Probably fixed.


-        if let count = event.unreadCount?.messages {
-          proj.unreadChatsCount = NSNumber(value: count)
-        }
+        // iOS339, be aware of using the `unreadCount` object of the sdk.
+        // For example, `event.unreadCount?.messages` will give a different number.
+        // It's probably the total unread chat counts of all channels that the user belongs to.
+        //
+        // Here, this is the most correct number. We are accessing the unreadCount of the channel.
+        proj.unreadChatsCount = NSNumber(value: event.channel.unreadCount.messages)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐞 Bug An issue or PR related to a bug 🌐 SDK: StreamChat (LLC) Tasks related to the StreamChat LLC SDK 🔜 Upcoming This issue is fixed in “develop” and will be available in the next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants