Messagekit: Random crash when calling `scrollToBottom` with "Invalid update: invalid number of items in section 0"

[hyouuu]

According to SO, there is a bug with UICollectionView's performBatchUpdate that could cause crashes in certain cases, and adding a collectionView.numberOfItems(inSection) fixes it: https://stackoverflow.com/a/46751421/1032900

In our case, one user faced frequent crashes when we call scrollToBottom in viewWillAppear for a child of MessagesViewController, and by adding numberOfItems(inSection: 0) before this line: https://github.com/MessageKit/MessageKit/blob/69c5414c224a98891e14984e83228740e4339219/Sources/Views/MessagesCollectionView.swift#L92 did fix it.

Should we consider adding numberOfItems(inSection: 0)? What you guys think?

• What version of MessageKit are you using? 1.0-beta.1
• What version of iOS are you running on? 11.1 - 11.4
• What version of Swift are you running on? 4.1
• What device(s) are you testing on? Are these simulators? iPhone 8 Plus & iPhone X, not simulators.
• Is the issue you're experiencing reproducable in the example app? Not so far

[SD10]

@hyouuu I'm not sure how to solve this one to be honest. There's a lot of bugs in UIKit surrounding scrolling to bottom for UICollectionView. In my app, we don't even use the MessageKit scroll to bottom method for this reason. If we start changing things we're just trading one bug for another bug. I'm open to a different approach, but I wouldn't make changes unless we can demonstrate both an issue and a working solution.

The reason we're using performBatchUpdates is because UICollectionView doesn't seem to scroll to bottom until it reaches a specific content size. Wrapping it in performBatchUpdates seems to work around this

[hyouuu]

Great to know it's a confirmed bug - wonder what do you use for your own app?
What's the initial reason not to use UICollectionView's open func scrollToItem(at indexPath: IndexPath, at scrollPosition: UICollectionViewScrollPosition, animated: Bool)?

[hyouuu]

@SD10 @zhongwuzw thoughts on the above?

[SD10]

@hyouuu The reason we're using performBatchUpdates is because UICollectionView doesn't seem to scroll to bottom until it reaches a specific content size. Wrapping it in performBatchUpdates seems to work around this

[hyouuu]

@SD10 I do understand that part as you already stated it in your previous comment. My question was why not using scrollToItem method from UICollectionView. Did an experiment and by changing the function to the following it works perfectly for me (smoother too):

public func scrollToBottom(animated: Bool = false) {
        let lastSection = numberOfSections - 1
        let lastItem = numberOfItems(inSection: lastSection) - 1
        // Using centeredVertically so that footer is shown too
        scrollToItem(at: IndexPath(item: lastItem, section: lastSection), at: .centeredVertically, animated: animated)
}

[SD10]

@hyouuu "... UICollectionView doesn't seem to scroll to bottom until it reaches a specific content size"

Have you tried when the UICollectionView only has the minimum number of messages required to scroll?

[SD10]

Here is the same issue we are experiencing @hyouuu
https://github.com/jessesquires/JSQMessagesViewController/issues/256

[hyouuu]

Mmm interesting - let me do more tests. If it's only not working for a specific scenario, should we scope that and only performBatchUpdates for that case? Since the majority cases don't need performBatchUpdates, and it sounds costly (no proof though :p).

[hyouuu]

Did many more tests on different sizes and so far it's been working fine - haven't tried to manipulate by pixels though. Will keep scrollToItem in my app for a while to test more

[SD10]

Idk to be honest. It would be nice to have a consistent solution. I don't like the animation caused by performBatchUpdates but right now it's the best we have.

FWIW, it's usually small content sizes where scrolling doesn't work. Maybe you have 2 or 3 messages and the keyboard comes up. It won't scroll as you type or sending the next message

[lshannak]

Hi Guys, anyone has a solid update on this, its been 2 months so far and I still keep getting random crashes no consistently.
I tried everything on this thread but I feel this is really a huge issue because it makes the library not usable with the crashes that keep happening.
@hyouuu @SD10 @nathantannar4
If any one has a solution that will be very very helpful!

[nathantannar4]

@lshannak I haven't taken a deep look into this. I'm sorry this is causing frustration, but I wouldn't say it makes the library unusable. Its not a function that we require you to call.

Like Steven said it's not ideal but it's currently the best we have. Perhaps you can come up with something that works better for you specifically as I think a main pain point is that while this works for some, it doesn't for others and it's hard to find that one size fits all solution.

_{Sent with GitHawk}

[SD10]

@lshannak The issue #939 seems to be an issue not related to MessageKit. This is a common error when reloading rows/sections in a collectionView/tableView API. Please make sure you're updating your view correctly.

If you're confident everything is correct -- and the problem is MessageKit's scrollToBottom() method. Then you can subclass MessagesCollectionView and override the scrollToBottom() method to provide your own implementation that does not use the performBatchUpdates API

[hyouuu]

I think @lshannak has the same view as I did, that for MessageKit this is one of the basic functionalities so there shouldn't be a need to subclass to fix the issue ;) As stated in #939 I'm looking forward to how @zhongwuzw would help to improve this!

[SD10]

I think @lshannak has the same view as I did, that for MessageKit this is one of the basic functionalities so there shouldn't be a need to subclass to fix the issue ;)

@hyouuu I only partially agree here. The scrollToBottom() method isn't really MessageKit specific. It's more there for my internal use but the API is open for subclassing. The fact that Apple didn't even put a scrollToBottom method in UIKit says something

[hyouuu]

True all the hard to fix bugs I've faces originate from Apple's bugs, and they never fix them :/

[lshannak]

Guys thank you for all your work and help, after I stopped using scrollToBottom() things are looking good on my end so far.

[lshannak]

I implemented your suggested code @hyouuu and its working good so far, Other than that the library is a great help.
@SD10 Thank you for your help as well :)

[JulienKode]

Based on @hyouuu answer,
You can also implement something like

to be safe when there no content and you call scrollToBottom

    public func scrollToBottom(animated: Bool = false) {
        guard numberOfSections > 0 else { return }
        let lastSection = numberOfSections - 1
        let indexPath = IndexPath(row: 0, section: lastSection)
        scrollToItem(at: indexPath, at: UICollectionView.ScrollPosition.top, animated: animated)
    }

[Michele404]

Is there a real fix on this bug? I got a lot of crash by that

[joona0825]

Hello, it seems I have similar issue with this library so I'm investigating some issues about it. If this crash happens, which code and line does the debugger pointing at?

[hyouuu]

My side project has been using the method I suggested for over a year with a dozen of test users, and so far it's been performing well. If there's no objection I'll make a PR for it

[hyouuu]

So I kept the current method while adding the other method to give people choices atm. After people try out both methods we can declare a winner in the future. https://github.com/MessageKit/MessageKit/pull/1247

[hassanshafi]

I also encountered by the same issue, I wasn't able to fix this issue. i.e to insert multiple messages simultaneously, so after some research I found a solution.Steven is right this is not a message kit bug, It's a collectionview bug. So the solution is you just need to append the element in array in perform batch updates methods like this.It will not crash .I just checked by appending the dummy elements in array after retrieveing the all messages.Below is the following code .

self.messagesCollectionView.performBatchUpdates({
                self.chatArray.append(message)
                self.messagesCollectionView.insertSections([self.chatArray.count - 1])
                if self.chatArray.count >= 2 {
                    self.messagesCollectionView.reloadSections([self.chatArray.count - 2])
                    self.messagesCollectionView.reloadDataAndKeepOffset()

                }
            }, completion: { [weak self] _ in
                if self?.isLastSectionVisible() == true {
                    self?.messagesCollectionView.scrollToBottom(animated: true)
                }
            })