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

Consistent "Invalid number of sections" and "Index out of range" errors when using typing indicator #1788

Open
monstermac77 opened this issue Apr 14, 2023 · 13 comments
Labels

Comments

@monstermac77
Copy link
Contributor

monstermac77 commented Apr 14, 2023

Describe the bug
All attempts to add a typing indicator to our MessagesViewController have resulted in fatal crashes. We get "Invalid number of sections" or "Index out of range" errors. It seems a number of people have had similar issues with the typing indicator (#1387, #1787) , despite us all basing our code off of the working example, AdvancedExampleViewController.

We are using a custom cell implementation, we believe we have taken the additional steps necessary to handle typing indicators for custom cells (found here), namely adding code to return typingIndicatorSizeCalculator in our custom cell size calculator and the typing indicator cell itself in the cellForItemAt function.

To Reproduce
Take an existing, currently functional, implementation of MessageKit with a custom cell. Then add the following at some point in the view's lifecycle (which attempts to add the typing indicator to the view):

self.setTypingIndicatorViewHidden(false, animated: true)

Running it at this stage will obviously crash, as you haven't updated the necessary functions to handle the fact that there is now an extra section that will appear in certain cases. So update these functions:

While the class you've inherited has the isTypingIndicatorHidden case in numberOfSections handled already, you would have had to implement it in your view controller as well in order to conform to MessagesDataSource, so rather than just returning messages.count, you'll need to add 1 in the case that the typing indicator is being shown.

func numberOfSections(in messagesCollectionView: MessageKit.MessagesCollectionView) -> Int {
   let sections = messages.count
   return isTypingIndicatorHidden ? sections : sections + 1
}

As mentioned above, you'll also need to make sure you return the typing indicator cell when it's being shown, rather than trying to paint a message cell.

override func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
        guard let dataSource = messagesCollectionView.messagesDataSource else{fatalError("No data for messages.")} 
        if isSectionReservedForTypingIndicator(indexPath.section) {
            return dataSource.typingIndicator(at: indexPath, in: messagesCollectionView)
        }

      ....

}

And lastly, in order to calculate the correct size for the typing indicator, you'll need to return the correct calculator in your custom MessagesCollectionViewFlowLayout.

override open func cellSizeCalculatorForItem(at indexPath: IndexPath) -> CellSizeCalculator {
        let message = messagesDataSource.messageForItem(at: indexPath, in: messagesCollectionView)
        if isSectionReservedForTypingIndicator(indexPath.section) {
            return typingIndicatorSizeCalculator
        }
    ...
}

Those are the only modifications that should be needed, and specifically no modifications to messageForItem should be needed either, because we're not pretending to have an extra message, we're just telling the view to paint an extra row with a certain cell.

Expected behavior
The typing indicator should just show up when you open the view.

What actually happens

Invalid update: invalid number of sections. The number of sections contained in the collection view after the update (2) must be equal to the number of sections contained in the collection view before the update (0), plus or minus the number of sections inserted or deleted (1 inserted, 0 deleted).

Which really has me confounded. To make things simpler, my chat view has no messages in it, so it should just be the typing indicator. No messages are being added during this process either. What should be happening is that we start with 0 sections, then setTypingIndicatorViewHidden is called which runs a messagesCollectionView.insertSections([section]) (which you can find in performUpdatesForTypingIndicatorVisability()), which adds a single section. Now, if numberOfSections is called at any point in this process, it's very simple: it'll return 0 sections if the typing indicator is hidden, and 1 if it's not. So we should be going from 0, adding 1, and getting to 1. Somehow, though, the crash claims we got to 2 sections. Regardless of the number of messages, this was the same error. n messages would result in it expecting n + 2, even though it only ever should expect n + 1. This, I'm pretty sure, is a bug.

In an attempt to fix this, we tried just removing the + 1 from our numberOfSections function, but this just caused messageForItem to break immediately with an Index out of range error. The problem was, even after using the debugger, we couldn't really figure out where messageForItem was being called. The only evident place was in cellForItemAt, but in the case of the typing indicator cell, it never should get called because isSectionReservedForTypingIndicator causes it to return the typing indicator cell before it gets to that line.

In a desperate attempt to get this to work, I gave this a whirl: numberOfSections returning just the message count, and compensating for it by providing a dummy "Message" object when it asks for the message at that row. That looks like:

    func messageForItem(at indexPath: IndexPath, in messagesCollectionView: MessageKit.MessagesCollectionView) -> MessageKit.MessageType {
        if isTypingIndicatorHidden{
            return messages[indexPath.section]
        }else{
            if indexPath.section != messages.count{
                return messages[indexPath.section]
            }else{
                return Message.init(id: 0, chatID: 0, coursicleID: 0, userName: "This is an abuse", userPhoto: "SAD.jpg", type: .text, data: JSON(), sent: Date(), reactions: [], state: .delivered)
            }
        }
    }

This does achieve the desired result (although the cell isn't animating), but it causes a bunch of downstream errors, because we can no longer assume we have a "real" message when our messageForItem is called.

Screenshots
Here's the stack trace when you get the invalid number of sections error:
image

Here's the stack trace when you get the Index out of range error on messageForItem:
image

Goal
Why is the number of sections after the update 1 more than it should be after calling setTypingIndicatorViewHidden()? What is the standard way of adding typing indicators to message views with custom cells?

This is an issue that has been brought up many times (#1108, #1387, #1787, #1324), so my hope is that we can come up with a good path forward for those using custom cells who want to add a typing indicator.

Environment

  • What version of MessageKit are you using?
    • 4.1.1 (latest release)
  • What version of iOS are you running on?
    • 16.4 (current)
  • What version of Swift are you running on?
    • 5.8 (current)

Update: I was able to solve a small part of this problem, the Index out of range that I refer to. Basically I just swapped the order of two blocks inside cellSizeCalculatorForItem. You have to make sure that you don't try getting the messageForItem on the typing indicator's section, so you short circuit it before calling messageForItem. Of course, this doesn't solve the primary issue, which is the invalid number of sections detailed above.

override open func cellSizeCalculatorForItem(at indexPath: IndexPath) -> CellSizeCalculator {
        if isSectionReservedForTypingIndicator(indexPath.section) {
            return typingIndicatorSizeCalculator
        }
        let message = messagesDataSource.messageForItem(at: indexPath, in: messagesCollectionView)
    ...
}
@Robert-Vaccaro
Copy link

Robert-Vaccaro commented Apr 17, 2023

@monstermac77
This may not be the correct or best solution but I found a work around for this issue.

From what I understand, the typingIndicator takes up a section in MessagesCollectionView which means it will be one more than your messages array length.

messages is the array of messages that I fill MessagesCollectionView with

I first updated messageForItem to this, to handle the error for when the indicator is not hidden.

func messageForItem(
    at indexPath: IndexPath,
    in messagesCollectionView: MessagesCollectionView) -> MessageType {
        messagesCollectionView.layer.zPosition = 9
        if messages.count == indexPath.section && messages.count > 0 && indexPath.section > 0 {
            return messages[indexPath.section-1]
        }
        return messages[indexPath.section]
    }

Then once I received a new message from another user, I call

self.setTypingIndicatorViewHidden(true, message: newMessage)

I had to update the function be this:

func setTypingIndicatorViewHidden(_ isHidden: Bool, message:Message? = nil, performUpdates updates: (() -> Void)? = nil) {
    setTypingIndicatorViewHidden(isHidden, animated: true, whilePerforming: updates) { [weak self] success in
        if success {
            if message != nil {
                self?.messages.append(message!)
            }
            self?.messagesCollectionView.reloadData()
            self?.messagesCollectionView.scrollToBottom(animated: true)
        }
    }
}

If anyone finds a better solution or wants to improve upon this, please feel free, this is the only work around I could find.

@uberexpertsApp
Copy link

facing the same issue:

*** Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Invalid update: invalid number of sections. The number of sections contained in the collection view after the update (41) must be equal to the number of sections contained in the collection view before the update (41), plus or minus the number of sections inserted or deleted (0 inserted, 1 deleted).'

@Kaspik
Copy link
Member

Kaspik commented Apr 30, 2023

I was looking at this in another Issue and I think I found the reason for that specific one (PR here - #1792). Is it possible something similar is happening for you?

@josmdddeveloper
Copy link

I've had this problem for years, tried everything, but didn't work.

If you want no crashes, use the title method with "is writting...".

MessageKit should remove the animated typing indicator as it never worked with sockets in production. It only creates us confusion and hours of trying workarounds that never works.

@monstermac77
Copy link
Contributor Author

@Kaspik that very well could be a fix. @josmdddeveloper did you try that fix?

@Robert-Vaccaro @uberexpertsApp or any of you guys, have you tried putting all of these insertions and deletions of sections in a beginUpdates() and endUpdates() block to ensure internal consistency of the number of sections?

@fukemy
Copy link

fukemy commented Jun 26, 2023

I decided to use TypingIndicator is a View between ChatMessageContainer and InputKeyboard, It's work better to me, I think so:

  1. No more handle cell, index, ...
  2. Easy to using Api without access CollectionView data state..
  3. No more crash to wasted time to solve
  4. Code performance, ... the typing state is not affect to message data...

@monstermac77
Copy link
Contributor Author

@fukemy Awesome. Do you have some sample code you could provide? That would be very, very appreciated. We're still struggling with this a lot.

@fukemy
Copy link

fukemy commented Jun 26, 2023

@monstermac77 , just look at those lines of MessageKit

private func setupSubviews() {
view.addSubviews(messagesCollectionView, inputContainerView)
}
private func setupConstraints() {
messagesCollectionView.translatesAutoresizingMaskIntoConstraints = false
/// Constraints of inputContainerView are managed by keyboardManager
inputContainerView.translatesAutoresizingMaskIntoConstraints = false
NSLayoutConstraint.activate([
messagesCollectionView.topAnchor.constraint(equalTo: view.topAnchor),
messagesCollectionView.bottomAnchor.constraint(equalTo: view.bottomAnchor),
messagesCollectionView.leadingAnchor.constraint(equalTo: view.safeAreaLayoutGuide.leadingAnchor),
messagesCollectionView.trailingAnchor.constraint(equalTo: view.safeAreaLayoutGuide.trailingAnchor),
])
}
private func setupDelegates() {
messagesCollectionView.delegate = self
messagesCollectionView.dataSource = self
}
private func setupInputBar(for kind: MessageInputBarKind) {
inputContainerView.subviews.forEach { $0.removeFromSuperview() }
func pinViewToInputContainer(_ view: UIView) {
view.translatesAutoresizingMaskIntoConstraints = false
inputContainerView.addSubviews(view)
NSLayoutConstraint.activate([
view.topAnchor.constraint(equalTo: inputContainerView.topAnchor),
view.bottomAnchor.constraint(equalTo: inputContainerView.bottomAnchor),
view.leadingAnchor.constraint(equalTo: inputContainerView.leadingAnchor),
view.trailingAnchor.constraint(equalTo: inputContainerView.trailingAnchor),
])
}
switch kind {
case .messageInputBar:
pinViewToInputContainer(messageInputBar)
case .custom(let view):
pinViewToInputContainer(view)
}
}

They add the InputBar + MessageView programmatically then setup the constraint later. For my case, you just clear all the constraint then add the TypingIndicator view, then re-setup the constraint.

Note that the TypingIndicator must exactly between the ChatMessage and InputKeyboard.
Default height of TypingIndicator is 0, so by play with the height + some animation, you can show TypingIndicator as well

@monstermac77
Copy link
Contributor Author

Thank you so much to @Zandor300 and @Kaspik for addressing this issue in 4.2.0. We just got a chance to test and unfortunately we are still having this issue. It seems like it's especially prevalent on iOS 16. We see it more often on physical devices than simulators, but you can reproduce it on a simulator.

I've attached a video of us reproducing the error. This is on a physical device: an iPhone 13 Pro Max running iOS 16.6.1. We're also able to regularly produce the error on an iPhone 12 mini running iOS 16.3.1. We have a hard time producing it on our iPhone 12 running 17.1.1, iPhone 13 mini running iOS 17.1.2, and iPhone 15 Pro running iOS 17.1.2 (we've seen some crashes on those, but they might not be related to this issue).

Video of reproduction: messageKitTypingCrash.mp4
Full stack trace: iphone13ProStackTrace.txt

Thanks so much for your help. We're going to try pushing this into production briefly to see how it goes and gather more crash data.

@fukemy
Copy link

fukemy commented Dec 15, 2023

lol I wonder how can you spent half year for PR about this problem, while you can find other way to solve this

@erwanmace
Copy link
Contributor

I am getting the same issue as well...

the suggestion to insert a view between the collectionview and the inputbar is a nice workaround but still a workaround. Ideally, the issue should be fixed in MessageKit itself.

Moreover the inserted view would not scroll with the message so less than ideal solution.

Screenshot 2023-12-19 at 12 32 02 PM

@erwanmace
Copy link
Contributor

Hi @monstermac77 any update on this issue?
I believe you were planning to push something in production according to your last message?
Thanks.

@fukemy
Copy link

fukemy commented Jan 26, 2024

ok, if you dont want to play around with modified code, lets see the better lib that based on messagekit here: https://github.com/ekazaev/ChatLayout
Just tried this then give feedback about the indicator crash problem if it's gone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants