When using `useChannel` channel never detaches even when all listeners have unsubscribed

[thenano]

It looks like the detach on this line never runs, because channel.listeners.length always evaluates as 1.

Please forgive me if my investigation is way off and I have looked in the wrong place. From what I could tell, listeners in the Channel is actually a function, defined by EventEmitter, which would return any listeners when called without and event, or the event specific listeners when called with an event. Apart from that, it looks like the channel's subscriptions are actually held in the channel's subscriptions attribute, which is also an instance of EventEmitter, so this would be the right place to look for remaining subscriptions? It looks to me that to ensure there are no more listeners in the channel and we can safely detach, one would have to check all any/events/anyOnce/eventsOnce subscriptions? Something like:

if (channel.subscriptions.any.length === 0 && channel.subscriptions.anyOnce.length === 0 && Object.keys(channel.subscriptions.events).length === 0 && Object.keys(channel.subscriptions.eventsOnce).length === 0) {
  await channel.detach();
}

or some sort of !some loop on all subscriptions?

Happy to do a pull request is any of this makes sense? Apologies if I'm doing something wrong.

┆Issue is synchronized with this Jira Bug by Unito

[sync-by-unito[bot]]

➤ Automation for Jira commented:

The link to the corresponding Jira issue is https://ably.atlassian.net/browse/SDK-4054

[VeskeR]

Hello @thenano !

Thank you for bringing this up. The issue here is that, as you've correctly noticed, channel.listeners is a method that comes from the EventEmitter class, but it's never actually called on this line. Instead it accesses length property on listeners, which just returns the number of parameters expected by the function (which is always 1 listeners).

This is a bug and will be fixed. I'll let you know when we've released a fix.

[thenano]

Thanks very much for the reply @VeskeR I just also want to point out that, as far as I can tell, even if useChannel is changed to call the listeners method instead of just accessing length, I don't think it will correctly check for all listeners, since the method either returns any listeners when no parameter is passed, or just the listeners for the specified event when that is passed? As far as I understand, we'd want to check for listeners for all events plus any listeners? And perhaps even for eventsOnce and anyOnce listeners?

[zameschua]

Hey @VeskeR , I think I ran into the same issue.

I was using the imperative API previously and was manually detaching the channels. When switched from the imperative API to react hooks recently, the following error appears in production:

Maximum number of channels per connection (200) exceeded

I traced it back to the same line of code. For my app, we use ChannelProvider and useChannel() for multiple different channels, depending on the screen being rendered. So subscribes and unsubscribes happen quite often when screens mount and unmount.

Let me know if there's anyway I can help debug the issue more or patch it on my end first.

Version that I'm on: "ably": "^2.0.3"

[zameschua]

Hey @VeskeR or @sacOO7,

Is there any way this bug can be prioritized? It's currently breaking in production. (Sorry for being annoying)