lib fails all the user's channels on transition to connecting/disconnected if queueMessages is disabled?

[SimonWoolf]

ably-cocoa/Source/ARTRealtime.m

Lines 701 to 739 in f0e4018

	if ([self shouldSendEvents]) {
	[self sendQueuedMessages];
	// For every Channel
	for (ARTRealtimeChannelInternal *channel in self.channels.nosyncIterable) {
	[channel sendQueuedMessages];
	}
	} else if (![self shouldQueueEvents]) {
	ARTStatus *channelStatus = status;
	if (!channelStatus) {
	channelStatus = stateChange.reason ? [ARTStatus state:ARTStateError info:stateChange.reason] : [self defaultError];
	}
	[self failQueuedMessages:channelStatus];
	// If there's a channels.release() going on waiting on this channel
	// to detach, doing those operations on it here would fire its event listener and
	// immediately remove the channel from the channels dictionary, thus
	// invalidating the iterator and causing a crashing.
	//
	// So copy the channels and operate on them later, when we're done using the iterator.
	NSMutableArray<ARTRealtimeChannelInternal *> * const channels = [[NSMutableArray alloc] init];
	for (ARTRealtimeChannelInternal *channel in self.channels.nosyncIterable) {
	[channels addObject:channel];
	}
	for (ARTRealtimeChannelInternal *channel in channels) {
	if (stateChange.current == ARTRealtimeClosing) {
	//do nothing. Closed state is coming.
	}
	else if (stateChange.current == ARTRealtimeClosed) {
	[channel detachChannel:[ARTStatus state:ARTStateOk]];
	}
	else if (stateChange.current == ARTRealtimeSuspended) {
	[channel setSuspended:channelStatus];
	}
	else {
	[channel setFailed:channelStatus];
	}
	}
	}

This is in transitionSideEffects. The implemented logic appears to be that if sendEvents and queueEvents are both false, and it's not in the closing, closed, or suspended states, then it should set all the channels to failed. Perhaps it's assuming that the only remaining state that it can be in in that branch is the failed state; if so this is not a correct assumption because queueMessages can be disabled, which will make shouldSendEvents and shouldQueueEvents both return false in states like connecting and disconnected.

If I'm reading the code right, the effect of this is to make the queueMessages client option completely unusable, all subsequent operations on any channels that had already been accessed will then fail even once the lib has reconnected after the first time it goes into the connecting or disconnected states

[QuintinWillison]

Internal Breadcrumbs: Slack, Intercom

Expected behaviours, as supplied by @SimonWoolf via Slack: RTL3 and TO3g.

[tomczoink]

Agent Tom Camp linked Freshdesk ticket 106205 for this issue.