diff --git a/src/IO.Ably.Shared.MsgPack/CustomSerialisers/GeneratedSerializers/IO_Ably_ConnectionDetailsMessageSerializer.cs b/src/IO.Ably.Shared.MsgPack/CustomSerialisers/GeneratedSerializers/IO_Ably_ConnectionDetailsMessageSerializer.cs index 250f33936..4b5648c42 100644 --- a/src/IO.Ably.Shared.MsgPack/CustomSerialisers/GeneratedSerializers/IO_Ably_ConnectionDetailsMessageSerializer.cs +++ b/src/IO.Ably.Shared.MsgPack/CustomSerialisers/GeneratedSerializers/IO_Ably_ConnectionDetailsMessageSerializer.cs @@ -35,7 +35,7 @@ public IO_Ably_ConnectionDetailsMessageSerializer(MsgPack.Serialization.Serializ } protected override void PackToCore(MsgPack.Packer packer, IO.Ably.ConnectionDetails objectTree) { - packer.PackMapHeader(7); + packer.PackMapHeader(8); this._serializer0.PackTo(packer, "clientId"); this._serializer0.PackTo(packer, objectTree.ClientId); this._serializer0.PackTo(packer, "connectionKey"); @@ -44,6 +44,8 @@ protected override void PackToCore(MsgPack.Packer packer, IO.Ably.ConnectionDeta this._serializer1.PackTo(packer, objectTree.ConnectionStateTtl); this._serializer0.PackTo(packer, "maxFrameSize"); this._serializer2.PackTo(packer, objectTree.MaxFrameSize); + this._serializer0.PackTo(packer, "maxIdleInterval"); + this._serializer1.PackTo(packer, objectTree.MaxIdleInterval); this._serializer0.PackTo(packer, "maxInboundRate"); this._serializer2.PackTo(packer, objectTree.MaxInboundRate); this._serializer0.PackTo(packer, "maxMessageSize"); @@ -112,6 +114,33 @@ protected override IO.Ably.ConnectionDetails UnpackFromCore(MsgPack.Unpacker unp result.MaxFrameSize = nullable2.Value; } unpacked = (unpacked + 1); + System.Nullable nullableMaxIdleInterval = default(System.Nullable); + if ((unpacked < itemsCount)) { + if ((unpacker.Read() == false)) { + throw MsgPack.Serialization.SerializationExceptions.NewMissingItem(4); + } + if (((unpacker.IsArrayHeader == false) + && (unpacker.IsMapHeader == false))) { + nullableMaxIdleInterval = this._serializer1.UnpackFrom(unpacker); + } + else { + MsgPack.Unpacker disposableMaxIdleInterval = default(MsgPack.Unpacker); + disposableMaxIdleInterval = unpacker.ReadSubtree(); + try { + nullableMaxIdleInterval = this._serializer1.UnpackFrom(disposableMaxIdleInterval); + } + finally { + if (((disposableMaxIdleInterval == null) + == false)) { + disposableMaxIdleInterval.Dispose(); + } + } + } + } + if (nullableMaxIdleInterval.HasValue) { + result.MaxIdleInterval = nullableMaxIdleInterval; + } + unpacked = (unpacked + 1); System.Nullable nullable3 = default(System.Nullable); if ((unpacked < itemsCount)) { nullable3 = MsgPack.Serialization.UnpackHelpers.UnpackNullableInt64Value(unpacker, typeof(IO.Ably.ConnectionDetails), "Int64 maxInboundRate"); @@ -230,7 +259,35 @@ protected override IO.Ably.ConnectionDetails UnpackFromCore(MsgPack.Unpacker unp } } else { - unpacker.Skip(); + if ((key == "maxIdleInterval")) { + System.Nullable nullableMaxIdle = default(System.Nullable); + if ((unpacker.Read() == false)) { + throw MsgPack.Serialization.SerializationExceptions.NewMissingItem(i); + } + if (((unpacker.IsArrayHeader == false) + && (unpacker.IsMapHeader == false))) { + nullableMaxIdle = this._serializer1.UnpackFrom(unpacker); + } + else { + MsgPack.Unpacker disposableMaxIdle = default(MsgPack.Unpacker); + disposableMaxIdle = unpacker.ReadSubtree(); + try { + nullableMaxIdle = this._serializer1.UnpackFrom(disposableMaxIdle); + } + finally { + if (((disposableMaxIdle == null) + == false)) { + disposableMaxIdle.Dispose(); + } + } + } + if (nullableMaxIdle.HasValue) { + result.MaxIdleInterval = nullableMaxIdle; + } + } + else { + unpacker.Skip(); + } } } } diff --git a/src/IO.Ably.Shared/AblyAuth.cs b/src/IO.Ably.Shared/AblyAuth.cs index 67cbbc7b3..1df81a98e 100644 --- a/src/IO.Ably.Shared/AblyAuth.cs +++ b/src/IO.Ably.Shared/AblyAuth.cs @@ -305,11 +305,27 @@ public virtual async Task RequestTokenAsync(TokenParams tokenParam bool shouldCatch = true; try { - var callbackResult = await authOptions.AuthCallback(tokenParams); + // RSA4c bounds an auth attempt by realtimeRequestTimeout. This one is awaited on + // the realtime workflow's single reader thread, so an authCallback that never + // returns would stall channel state, inbound messages and the idle monitor. + // + // Task.Run rather than awaiting the delegate directly: TimeoutAfter extends an + // already-created Task, so a callback whose body runs synchronously - the most + // ordinary shape in C# - would block before there is anything to bound. It + // cannot cancel the callback; an abandoned one runs to completion. + var callbackResult = await Task.Run(() => authOptions.AuthCallback(tokenParams)) + .TimeoutAfter(Options.RealtimeRequestTimeout, null); if (callbackResult == null) { - throw new AblyException("AuthCallback returned null", ErrorCodes.ClientAuthProviderRequestFailed); + // Covers a callback that returned null and one that timed out - TimeoutAfter + // yields null for both, and the wrapper below replaces the message anyway. + // ClientCallbackError, not ClientAuthProviderRequestFailed: the wrapper + // rethrows as 80019 per RSA4c1 with this as the cause, so 80019 here would + // duplicate its own parent. ably-js uses 40170 for the same case. + throw new AblyException( + $"AuthCallback returned null or did not return within {Options.RealtimeRequestTimeout.TotalSeconds}s", + ErrorCodes.ClientCallbackError); } if (callbackResult is TokenDetails) @@ -341,12 +357,17 @@ public virtual async Task RequestTokenAsync(TokenParams tokenParam : HttpStatusCode.Unauthorized; } + // RSA4c1 wants the cause "set to the underlying cause". The four argument overload + // assigns InnerException instead, which is not the spec's field and is not + // serialised as one. throw new AblyException( new ErrorInfo( "Error calling AuthCallback, token request failed. See inner exception for details.", ErrorCodes.ClientAuthProviderRequestFailed, statusCode, - ex), + href: null, + cause: (ex as AblyException)?.ErrorInfo ?? new ErrorInfo(ex.Message), + innerException: ex), ex); } } diff --git a/src/IO.Ably.Shared/AuthOptions.cs b/src/IO.Ably.Shared/AuthOptions.cs index 9ffa81271..c72acef36 100644 --- a/src/IO.Ably.Shared/AuthOptions.cs +++ b/src/IO.Ably.Shared/AuthOptions.cs @@ -13,6 +13,13 @@ public class AuthOptions /// /// The callback used to get a new or . /// AuthCallback is used by internally by .RequestTokenAsync. + /// + /// The callback is bounded by per RSA4c, + /// but cannot be cancelled: one that overruns is abandoned and keeps running, so a later auth + /// attempt may invoke it again concurrently and implementations must tolerate that. A result + /// returned after the bound is discarded, and each abandoned invocation holds a thread pool + /// worker until it returns. + /// /// public Func> AuthCallback { get; set; } diff --git a/src/IO.Ably.Shared/ClientOptions.cs b/src/IO.Ably.Shared/ClientOptions.cs index b93a57e4f..5c9e83591 100644 --- a/src/IO.Ably.Shared/ClientOptions.cs +++ b/src/IO.Ably.Shared/ClientOptions.cs @@ -295,6 +295,51 @@ public bool UseBinaryProtocol /// public TimeSpan ChannelRetryTimeout { get; set; } = Defaults.ChannelRetryTimeout; + private TimeSpan _realtimeRequestTimeout = Defaults.RealtimeRequestTimeout; + + /// + /// How long the library waits for Ably to answer before treating a realtime request as + /// having failed. Applies while establishing a connection, while awaiting a response to a + /// Heartbeat, Connect, Attach, Detach or Close, and as part of the RTN23a idle timeout. + /// Default: 10s. Must be a positive interval, and values beyond a minute or so are rarely + /// useful. Disabling the timeout is not supported - the timeouts this value drives are all + /// required to fire - so both Timeout.InfiniteTimeSpan and TimeSpan.MaxValue are rejected. + /// TO3l11 - https://sdk.ably.com/builds/ably/specification/main/features/#TO3l11. + /// + /// when set to zero, a negative value, or an + /// interval too large for the underlying timers to fire. + public TimeSpan RealtimeRequestTimeout + { + get => _realtimeRequestTimeout; + + set + { + // A non-positive value fails quietly rather than loudly: zero turns the RTN14c + // connect timeout into a hot reconnect loop, and a negative one stops the timer + // firing at all. Timeout.InfiniteTimeSpan is -1ms, so the lower bound catches it - + // which matters, because both Task.Delay and System.Threading.Timer accept it as + // genuine infinity and it would disable RTN14c, RTN12b, RTL4f, RTL5f and RSA4c. + // + // The upper bound is the tightest limit across the sinks this value reaches on every + // framework shipped: Task.Delay allows uint.MaxValue - 1 ms on .NET 6+ but only + // Int32.MaxValue on .NET Framework, Mono and Xamarin, and CountdownTimer casts to + // int for System.Threading.Timer. So Int32.MaxValue ms - a bound on the arithmetic, + // not a supported configuration, which is why the message names the useful range. + if (value <= TimeSpan.Zero || value.TotalMilliseconds > int.MaxValue) + { + throw new ArgumentOutOfRangeException( + nameof(RealtimeRequestTimeout), + value, + "RealtimeRequestTimeout must be a positive interval. The default is 10s; " + + "values beyond a minute or so are rarely useful. Disabling the timeout is " + + "not supported - the connect, close, attach, detach and auth timeouts it " + + "drives are all required to fire."); + } + + _realtimeRequestTimeout = value; + } + } + /// /// Timeout for opening an http request. /// Default: 4s. @@ -436,8 +481,6 @@ internal Func NowFunc internal bool SkipInternetCheck { get; set; } - internal TimeSpan RealtimeRequestTimeout { get; set; } = Defaults.RealtimeRequestTimeout; - /// /// Default constructor for ClientOptions. /// diff --git a/src/IO.Ably.Shared/Realtime/Connection.cs b/src/IO.Ably.Shared/Realtime/Connection.cs index 5eae16db9..2db827ad3 100644 --- a/src/IO.Ably.Shared/Realtime/Connection.cs +++ b/src/IO.Ably.Shared/Realtime/Connection.cs @@ -181,10 +181,13 @@ private void HandleNetworkStateChange(NetworkState state) /// recoveryKey. public string CreateRecoveryKey() { + // RTN16g3, which replaces RTN16g2 as of specification 6.1.0 - null in CLOSED, CLOSING + // and FAILED, and SUSPENDED is deliberately not among them. RTN8d and RTN9d keep the + // key through SUSPENDED because RTN14h always attempts a resume, so the connection is + // still recoverable there and the key has to be available to hand over. if (Key.IsEmpty() || InnerState.State == Realtime.ConnectionState.Closing || InnerState.State == Realtime.ConnectionState.Closed - || InnerState.State == Realtime.ConnectionState.Failed - || InnerState.State == Realtime.ConnectionState.Suspended) + || InnerState.State == Realtime.ConnectionState.Failed) { return string.Empty; } diff --git a/src/IO.Ably.Shared/Realtime/Presence.cs b/src/IO.Ably.Shared/Realtime/Presence.cs index 42358fd7c..fab293ca3 100644 --- a/src/IO.Ably.Shared/Realtime/Presence.cs +++ b/src/IO.Ably.Shared/Realtime/Presence.cs @@ -792,11 +792,22 @@ private void SendQueuedMessages() private void FailQueuedMessages(ErrorInfo reason) { + // RTL11 wants "an ErrorInfo indicating the failure". TaskWrapper has no branch for + // (false, null) and faults the task with a bare Exception, so the reason is coalesced + // here rather than at each call site. + var error = reason ?? ErrorInfo.ReasonUnknown; + while (!PendingPresenceQueue.IsEmpty) { if (PendingPresenceQueue.TryDequeue(out var queuedPresenceMessage)) { - queuedPresenceMessage.Callback?.Invoke(false, reason); + // Guarded like every other callback site: a throwing application callback would + // otherwise abort the rest of the queue and skip the RTP5a map clearing in + // ChannelDetachedOrFailed, which RealtimeChannels then swallows per channel. + ActionUtils.SafeExecute( + () => queuedPresenceMessage.Callback?.Invoke(false, error), + Logger, + nameof(FailQueuedMessages)); } } } diff --git a/src/IO.Ably.Shared/Realtime/RealtimeChannel.cs b/src/IO.Ably.Shared/Realtime/RealtimeChannel.cs index c9159ab2b..3aeb69e7d 100644 --- a/src/IO.Ably.Shared/Realtime/RealtimeChannel.cs +++ b/src/IO.Ably.Shared/Realtime/RealtimeChannel.cs @@ -28,12 +28,6 @@ internal class RealtimeChannel : EventEmitter, private readonly PushChannel _pushChannel; private int _retryCount = 0; - /// - /// True when the channel moves to the @ATTACHED@ state, and False - /// when the channel moves to the @DETACHING@ or @FAILED@ states. - /// - internal bool AttachResume { get; set; } - private int _decodeRecoveryInProgress; // We use interlocked exchange because it is a thread safe way to read a variable @@ -58,8 +52,6 @@ internal bool DecodeRecovery internal AblyRealtime RealtimeClient { get; } - private string PreviousConnectionId { get; set; } - private ConnectionState ConnectionState => Connection.State; private IConnectionManager ConnectionManager => RealtimeClient.ConnectionManager; @@ -147,68 +139,92 @@ internal RealtimeChannel( internal void ConnectionStateChanged(ConnectionStateChange connectionStateChange) { - var connectionRefreshed = PreviousConnectionId != Connection.Id; - if (connectionRefreshed) - { - PreviousConnectionId = Connection.Id; - } - switch (connectionStateChange.Current) { case ConnectionState.Connected: - if (State == ChannelState.Suspended) - { - Attach(); - } - if (State == ChannelState.Attaching && AttachedAwaiter.Waiting == false) + /* + * (RTN24) An update arrives on the connection we already hold, so it must not + * touch channel state at all - RTL3d governs *entering* CONNECTED. ably-js + * touches no channel on a mid-connection CONNECTED either. + */ + if (connectionStateChange.Event == ConnectionEvent.Update) { - Attach(null, emitUpdate: true); + break; } /* - * Connection state is only maintained server-side for a brief period, - * given by the connectionStateTtl in the connectionDetails (2 minutes at time of writing, see CD2f). - * If a client has been disconnected for longer than the connectionStateTtl - * it should clear the local connection state and any connection attempts should be made as for a fresh connection + * On entering CONNECTED, every channel that was attached or pending needs its + * ATTACH - or its DETACH - sent again; the previous transport will never answer. + * + * The two branches have different owners. RTL3d names ATTACHING, ATTACHED and + * SUSPENDED, and asks for an RTL4c attach. DETACHING is RTN19b instead, which + * requires the respective ATTACH or DETACH of any pending channel to be resent. + * ably-js splits it the same way, between checkPendingState and notifyState. * - * (RTN15g3) When a connection attempt succeeds after the connection state has been cleared in this way, - * channels that were previously ATTACHED, ATTACHING, or SUSPENDED must be automatically reattached, - * just as if the connection was a resume attempt which failed per RTN15c3 + * RTL3d1 requires all of this to be applied before CONNECTED reaches external + * listeners, which is why it lives here: Connection.NotifyUpdate runs the + * internal handlers, this among them, before the emit. * - * Given the above, if the channel is ATTACHED and the connection is fresh - * then set the channel to ATTACHING to trigger an ATTACH attempt + * Unconditional, deliberately. Whether the connection was resumed belongs + * inside the ATTACH, in the channelSerial RTL4c1 carries, rather than in a + * decision about whether to send one at all. ably-js reattaches unconditionally + * for the same reason. */ - if (State == ChannelState.Attached && connectionRefreshed) + switch (State) { - Attach(null, force: true, emitUpdate: false); - } - - if (State == ChannelState.Detaching && DetachedAwaiter.Waiting == false) - { - Detach(null, force: true, emitUpdate: true); + case ChannelState.Suspended: + case ChannelState.Attaching: + case ChannelState.Attached: + Attach(null, force: true, emitUpdate: false); + break; + case ChannelState.Detaching: + Detach(null, force: true, emitUpdate: true); + break; } break; case ConnectionState.Disconnected: AttachedAwaiter.Fail(new ErrorInfo("Connection is Disconnected")); DetachedAwaiter.Fail(new ErrorInfo("Connection is Disconnected")); + break; + case ConnectionState.Closing: + + // RTN11b - on connect() while CLOSING, channels "first transition to DETACHED, + // following RTL3b, and then reinitialize ... per RTN11d". Done at the connection + // transition rather than inside RTN11d so a close() with no connect() behind it + // is covered too. See DetachForConnectionGoingAway for the states involved. + DetachForConnectionGoingAway( + new ErrorInfo("Connection is closing", ErrorCodes.ChannelOperationFailed)); + break; case ConnectionState.Closed: - AttachedAwaiter.Fail(new ErrorInfo("Connection is closed")); - DetachedAwaiter.Fail(new ErrorInfo("Connection is closed")); - if (State == ChannelState.Attached || State == ChannelState.Attaching) - { - Detach(); - } + + // The same four states as CLOSING, for the same RTP5a reason. CLOSED is + // reachable without CLOSING: ConnectionSuspendedState.Close() queues + // SetClosedStateCommand directly. + DetachForConnectionGoingAway( + new ErrorInfo("Connection is closed", ErrorCodes.ChannelOperationFailed)); break; case ConnectionState.Suspended: AttachedAwaiter.Fail(new ErrorInfo("Connection is suspended")); DetachedAwaiter.Fail(new ErrorInfo("Connection is suspended")); - if (State == ChannelState.Attached || State == ChannelState.Attaching) + + // RTL3c names only ATTACHING and ATTACHED; DETACHING is ably-js parity rather + // than a spec requirement - propogateConnectionInterruption maps suspended over + // ['attaching','attached','detaching','suspended']. It matters because the lines + // above have just failed the awaiter that would have finished a detach, leaving + // the channel in DETACHING with no DETACHED coming and no callback. + // + // The connection's own reason is carried rather than a constant, as ably-js does + // by passing change.reason into notifyState. + if (State == ChannelState.Attached || State == ChannelState.Attaching || + State == ChannelState.Detaching) { - SetChannelState(ChannelState.Suspended, ErrorInfo.ReasonSuspended); + SetChannelState( + ChannelState.Suspended, + connectionStateChange.Reason ?? ErrorInfo.ReasonSuspended); } break; @@ -223,6 +239,39 @@ internal void ConnectionStateChanged(ConnectionStateChange connectionStateChange } } + /// + /// RTN11b/RTL3b - the connection is going away, so a channel that is attached or pending has + /// to detach. RTP5a clears the presence maps on entering DETACHED, which is why all four + /// pending states are covered and not just the two RTL3b names: a channel left SUSPENDED or + /// DETACHING carries its members from the abandoned connection into the next one. + /// + /// reported to anyone waiting on an attach or detach that this + /// transition abandons. Deliberately not carried into the channel state: RTL24 lists RTN11d, + /// RTL3a, RTL4g and RTL14 as the sources of RealtimeChannel#errorReason, and a connection + /// close is none of them. + private void DetachForConnectionGoingAway(ErrorInfo awaiterError) + { + AttachedAwaiter.Fail(awaiterError); + + if (State == ChannelState.Attached || State == ChannelState.Attaching || + State == ChannelState.Suspended) + { + DetachedAwaiter.Fail(awaiterError); + Detach(null, force: false, emitUpdate: false); + } + else if (State == ChannelState.Detaching) + { + // DetachedAwaiter is deliberately left alone: the detach the caller asked for is + // about to complete, so the DETACHED transition below finishes it as a success. + // ably-js resolves detach() here for the same reason. + SetChannelState(ChannelState.Detached); + } + else + { + DetachedAwaiter.Fail(awaiterError); + } + } + public void Attach(Action callback = null) { Attach(null, null, callback); @@ -305,11 +354,6 @@ ProtocolMessage CreateAttachMessage() message.SetModesAsFlags(Options.Modes); } - if (AttachResume) - { - message.SetFlag(ProtocolMessage.Flag.AttachResume); - } - return message; } } @@ -396,18 +440,17 @@ private void Detach(Action callback, bool force, bool emitUpdat { SetChannelState(ChannelState.Detaching, emitUpdate); - if (ConnectionState == ConnectionState.Closed || ConnectionState == ConnectionState.Connecting || - ConnectionState == ConnectionState.Suspended) - { - SetChannelState(ChannelState.Detached); - } - else if (ConnectionState != ConnectionState.Failed) + // RTL5l - "if the connection state is anything other than CONNECTED and none of the + // preceding channel state conditions apply, the channel transitions immediately to + // the DETACHED state". Tested as anything-but-CONNECTED rather than enumerated, so + // DISCONNECTED cannot fall through to the RTL6c2 queue. + if (ConnectionState != ConnectionState.Connected) { - SendMessage(new ProtocolMessage(ProtocolMessage.MessageAction.Detach, Name)); + SetChannelState(ChannelState.Detached); } else { - Logger.Warning($"#{Name}. Cannot send Detach messages when connection is in Failed State"); + SendMessage(new ProtocolMessage(ProtocolMessage.MessageAction.Detach, Name)); } } else @@ -664,8 +707,11 @@ private void HandleStateChange(ChannelState state, ErrorInfo error, ProtocolMess Logger.Debug($"HandleStateChange state change from {State} to {state}"); } - // RTP5a1 - if (state == ChannelState.Detached || state == ChannelState.Suspended || state == ChannelState.Failed) + // RTL15b2 - "If the channel enters the DETACHED or FAILED state, it must clear its + // channelSerial. (Unlike previous spec versions, it must not clear it when entering the + // SUSPENDED state)." Retaining it in SUSPENDED lets a channel suspended by an RTL4f + // attach timeout carry its serial on the reattach RTL4c1 asks for. + if (state == ChannelState.Detached || state == ChannelState.Failed) { Properties.ChannelSerial = null; } @@ -680,11 +726,9 @@ private void HandleStateChange(ChannelState state, ErrorInfo error, ProtocolMess break; case ChannelState.Detaching: AttachedAwaiter.Fail(new ErrorInfo("Channel transitioned to detaching", ErrorCodes.InternalError)); - AttachResume = false; break; case ChannelState.Attached: _retryCount = 0; - AttachResume = true; break; case ChannelState.Detached: /* RTL13a check for unexpected detach */ @@ -724,7 +768,6 @@ an attempt to reattach the channel should be made immediately */ break; case ChannelState.Failed: _retryCount = 0; - AttachResume = false; AttachedAwaiter.Fail(error); DetachedAwaiter.Fail(error); Presence.ChannelDetachedOrFailed(error); diff --git a/src/IO.Ably.Shared/Realtime/RealtimeChannels.cs b/src/IO.Ably.Shared/Realtime/RealtimeChannels.cs index eb7601a99..ccd4d46df 100644 --- a/src/IO.Ably.Shared/Realtime/RealtimeChannels.cs +++ b/src/IO.Ably.Shared/Realtime/RealtimeChannels.cs @@ -237,12 +237,24 @@ private void HandleInitialiseFailedChannelsCommand(RealtimeChannel channel) { switch (_realtimeClient.Connection.State) { + case ConnectionState.Closing: case ConnectionState.Closed: case ConnectionState.Failed: - /* (RTN11d) - * If the [Connection] state is FAILED, - * transitions all the channels to INITIALIZED */ + /* (RTN11d) From CLOSED or FAILED, every channel goes to INITIALIZED with its + * errorReason unset (RTL24). CLOSING is included because RTN11b routes connect() + * in that state through RTN11d. + * + * Passing no error is what unsets it: SetChannelState hands it to OnError, which + * assigns either way, and does so before the same-state early return. + * + * The connection half of RTN11d - Connection.errorReason and msgSerial - is done + * once in the workflow's ConnectCommand handler. */ channel.SetChannelState(ChannelState.Initialized); + + // RTN11d's "clear all internal connection data". RTL15b2 only nulls the serial + // for Detached and Failed, and on a close an ATTACHED channel is left in + // DETACHING with no DETACHED coming, so it needs clearing here. + channel.Properties.ChannelSerial = null; break; } } @@ -264,7 +276,10 @@ internal IDictionary GetChannelSerials() var channelSerials = new Dictionary(); foreach (var realtimeChannel in this) { - if (realtimeChannel.State == ChannelState.Attached) + // Gated on the serial, not on the channel state. RTL15b2 keeps the serial through + // SUSPENDED and RTN16i needs it in the recovery key, so a state test would drop it + // in exactly the state RTN16g3 hands a key out for. ably-js gates on the serial too. + if (realtimeChannel.Properties.ChannelSerial.IsNotEmpty()) { channelSerials[realtimeChannel.Name] = realtimeChannel.Properties.ChannelSerial; } diff --git a/src/IO.Ably.Shared/Realtime/Workflows/RealtimeCommands.cs b/src/IO.Ably.Shared/Realtime/Workflows/RealtimeCommands.cs index 84529ade3..22bd7ba23 100644 --- a/src/IO.Ably.Shared/Realtime/Workflows/RealtimeCommands.cs +++ b/src/IO.Ably.Shared/Realtime/Workflows/RealtimeCommands.cs @@ -154,21 +154,30 @@ private CloseConnectionCommand() internal class SetConnectingStateCommand : RealtimeCommand { - private SetConnectingStateCommand(bool clearConnectionKey, bool retryAuth) + private SetConnectingStateCommand(bool clearConnectionKey, bool retryAuth, bool? connectivityConfirmed) { ClearConnectionKey = clearConnectionKey; RetryAuth = retryAuth; + ConnectivityConfirmed = connectivityConfirmed; } public bool ClearConnectionKey { get; } public bool RetryAuth { get; } - public static SetConnectingStateCommand Create(bool clearConnectionKey = false, bool retryAuth = false) => new SetConnectingStateCommand(clearConnectionKey, retryAuth); + /// + /// The connectivity answer already obtained by the DISCONNECTED that queued this command, so + /// the RTN17j check is not paid for twice in one cycle. Null when there is no answer on hand + /// - a timer driven retry, or a caller's Connect() - and those take their own. + /// + public bool? ConnectivityConfirmed { get; } + + public static SetConnectingStateCommand Create(bool clearConnectionKey = false, bool retryAuth = false, bool? connectivityConfirmed = null) => + new SetConnectingStateCommand(clearConnectionKey, retryAuth, connectivityConfirmed); protected override string ExplainData() { - return string.Empty; + return ConnectivityConfirmed.HasValue ? $"ConnectivityConfirmed: {ConnectivityConfirmed}" : string.Empty; } } @@ -195,13 +204,12 @@ protected override string ExplainData() internal class SetDisconnectedStateCommand : RealtimeCommand { - private SetDisconnectedStateCommand(ErrorInfo error, bool retryInstantly, bool skipAttach, Exception exception, bool clearConnectionKey) + private SetDisconnectedStateCommand(ErrorInfo error, bool retryInstantly, bool skipAttach, Exception exception) { Error = error; RetryInstantly = retryInstantly; SkipAttach = skipAttach; Exception = exception; - ClearConnectionKey = clearConnectionKey; } public ErrorInfo Error { get; } @@ -212,45 +220,36 @@ private SetDisconnectedStateCommand(ErrorInfo error, bool retryInstantly, bool s public Exception Exception { get; } - public bool ClearConnectionKey { get; } - protected override string ExplainData() { return $"RetryInstantly: {RetryInstantly}" + "SkipAttach: " + SkipAttach + ((Error != null) ? " Error: " + Error : string.Empty) + - ((Exception != null) ? " Exception: " + Exception.Message : string.Empty) + - " ClearConnectionKey: " + ClearConnectionKey; + ((Exception != null) ? " Exception: " + Exception.Message : string.Empty); } public static SetDisconnectedStateCommand Create( ErrorInfo error, bool retryInstantly = false, bool skipAttach = false, - Exception exception = null, - bool clearConnectionKey = false) - => new SetDisconnectedStateCommand(error, retryInstantly, skipAttach, exception, clearConnectionKey); + Exception exception = null) + => new SetDisconnectedStateCommand(error, retryInstantly, skipAttach, exception); } internal class SetSuspendedStateCommand : RealtimeCommand { - private SetSuspendedStateCommand(ErrorInfo error, bool clearConnectionKey) + private SetSuspendedStateCommand(ErrorInfo error) { Error = error; - ClearConnectionKey = clearConnectionKey; } public ErrorInfo Error { get; } - public bool ClearConnectionKey { get; } - - public static SetSuspendedStateCommand Create(ErrorInfo error, bool clearConnectionKey = false) => new SetSuspendedStateCommand(error, clearConnectionKey); + public static SetSuspendedStateCommand Create(ErrorInfo error) => new SetSuspendedStateCommand(error); protected override string ExplainData() { - var message = (Error != null) ? " Error: " + Error : string.Empty; - message += " ClearConnectionKey:" + ClearConnectionKey; - return message; + return (Error != null) ? " Error: " + Error : string.Empty; } } @@ -485,24 +484,27 @@ protected override string ExplainData() } } + /// + /// A periodic tick asking the workflow to check whether the current transport has gone idle for + /// longer than RTN23a allows. Carries no state: the handler reads RealtimeState on the workflow + /// thread rather than the timer sampling it off-thread. + /// internal class HeartbeatMonitorCommand : RealtimeCommand { - private HeartbeatMonitorCommand(DateTimeOffset? confirmedAliveAt, TimeSpan connectionStateTtl) + private HeartbeatMonitorCommand(DateTimeOffset queuedAt) { - ConfirmedAliveAt = confirmedAliveAt; - ConnectionStateTtl = connectionStateTtl; + QueuedAt = queuedAt; } - public DateTimeOffset? ConfirmedAliveAt { get; } - - public TimeSpan ConnectionStateTtl { get; } + /// + /// When this tick was queued, which is the moment idleness is judged against. Reading the + /// clock in the handler would charge any wait behind a slow command to the transport. + /// + public DateTimeOffset QueuedAt { get; } - public static HeartbeatMonitorCommand Create(DateTimeOffset? confirmedAliveAt, TimeSpan connectionStateTtl) => - new HeartbeatMonitorCommand(confirmedAliveAt, connectionStateTtl); + public static HeartbeatMonitorCommand Create(DateTimeOffset queuedAt) => + new HeartbeatMonitorCommand(queuedAt); - protected override string ExplainData() - { - return $"ConfirmedAliveAt: {ConfirmedAliveAt}. ConnectionStateTtl {ConnectionStateTtl}"; - } + protected override string ExplainData() => $"QueuedAt: {QueuedAt:O}"; } } diff --git a/src/IO.Ably.Shared/Realtime/Workflows/RealtimeState.cs b/src/IO.Ably.Shared/Realtime/Workflows/RealtimeState.cs index 07515f861..85de01981 100644 --- a/src/IO.Ably.Shared/Realtime/Workflows/RealtimeState.cs +++ b/src/IO.Ably.Shared/Realtime/Workflows/RealtimeState.cs @@ -40,6 +40,14 @@ public ConnectionData(List fallbackHosts) public TimeSpan ConnectionStateTtl { get; internal set; } = Defaults.ConnectionStateTtl; + /// + /// The maximum period of inactivity the server promises in the server to client + /// direction, from the connectionDetails of the most recent Connected message. + /// Null when the server declines to make that promise, in which case no idle + /// timeout is applied (CD2h). + /// + public TimeSpan? MaxIdleInterval { get; internal set; } + /// /// Information relating to the transition to the current state, /// as an Ably ErrorInfo object. This contains an error code and @@ -72,19 +80,42 @@ public ConnectionStateChange UpdateState(ConnectionStateBase state, ILogger logg return new ConnectionStateChange(connectionEvent, oldState, newState, state.RetryIn, ErrorReason); } - public bool HasConnectionStateTtlPassed(Func now) - { - return ConfirmedAliveAt?.Add(ConnectionStateTtl) < now(); - } - - public void Update(ConnectionInfo info) + public void Update(ConnectionInfo info, bool isUpdate) { + // Guarded differently on purpose. connectionId is a top-level field and always + // meaningful, per RTN8b. connectionKey lives inside connectionDetails, and RTN21 + // scopes an override to "the attributes within ConnectionDetails" - so a CONNECTED + // carrying none overrides no key, and emptying it would leave a live connection with + // nothing to resume with under RTN15b. Clearing the key belongs to ClearKey and + // ClearKeyAndId, at the points that mean it. Id = info.ConnectionId; - Key = info.ConnectionKey; + + if (info.ConnectionKey.IsNotEmpty()) + { + Key = info.ConnectionKey; + } + if (info.ConnectionStateTtl.HasValue) { ConnectionStateTtl = info.ConnectionStateTtl.Value; } + + // RTN23a measures against the maxIdleInterval "sent in the connectionDetails of the + // most recent CONNECTED message received on that transport", so the promise belongs + // to the transport that carried it. Hence the two cases, which isUpdate separates: + // + // - A CONNECTED starting a new transport must not inherit the old threshold. An + // omitted field is Ably declining to promise anything, so detection stands down. + // Strictly unspecified - RTN23a says the field "will be sent" and CD2h licenses + // arbitrary inactivity for an explicit 0 - but failing open matches CD2h's + // outcome for 0, and ably-js. + // - A CONNECTED arriving while already CONNECTED is an RTN24 update on the + // transport we already hold, so an omitted field is not a withdrawal and the + // previous value stands. ably-js keeps it with the same guard. + if (isUpdate == false || info.MaxIdleInterval.HasValue) + { + MaxIdleInterval = info.MaxIdleInterval; + } } public void ClearKeyAndId() diff --git a/src/IO.Ably.Shared/Realtime/Workflows/RealtimeWorkflow.cs b/src/IO.Ably.Shared/Realtime/Workflows/RealtimeWorkflow.cs index c6fcdb02e..0daf40084 100644 --- a/src/IO.Ably.Shared/Realtime/Workflows/RealtimeWorkflow.cs +++ b/src/IO.Ably.Shared/Realtime/Workflows/RealtimeWorkflow.cs @@ -34,6 +34,11 @@ internal sealed class RealtimeWorkflow : IQueueCommand, IDisposable // way of figuring out when processing has finished private volatile bool _processingCommand; private bool _heartbeatMonitorDisconnectRequested; + + // Null until first asked. See ProtocolHeartbeatsNotRequestedByCaller. + private bool? _protocolHeartbeatsNotRequested; + + private bool _warnedIdleCheckInactive; private bool _disposedValue; private AblyRealtime Client { get; } @@ -108,7 +113,7 @@ public void Start() { while (true) { - QueueCommand(HeartbeatMonitorCommand.Create(Connection.ConfirmedAliveAt, Connection.ConnectionStateTtl).TriggeredBy("AblyRealtime.HeartbeatMonitor()")); + QueueCommand(HeartbeatMonitorCommand.Create(Now()).TriggeredBy("AblyRealtime.HeartbeatMonitor()")); await Task.Delay(Client.Options.HeartbeatMonitorDelay, _heartbeatMonitorCancellationTokenSource.Token); } }, @@ -196,7 +201,8 @@ private void DelayCommandHandler(TimeSpan delay, RealtimeCommand cmd) => internal async Task> ProcessCommand(RealtimeCommand command) { - bool shouldLogCommand = !((command is EmptyCommand) || (command is ListCommand)); + // Ticks every second, so logging each one would bury everything else at Debug. + bool shouldLogCommand = !((command is EmptyCommand) || (command is ListCommand) || (command is HeartbeatMonitorCommand)); try { if (Logger.IsDebug && shouldLogCommand) @@ -233,7 +239,7 @@ internal async Task> ProcessCommand(RealtimeCommand State.Connection.CurrentStateObject?.AbortTimer(); return Enumerable.Empty(); case HeartbeatMonitorCommand cmd: - return await HandleHeartbeatMonitorCommand(cmd); + return HandleHeartbeatMonitorCommand(cmd); default: var next = await ProcessCommandInner(command); return new[] @@ -251,31 +257,96 @@ internal async Task> ProcessCommand(RealtimeCommand } } - private async Task> HandleHeartbeatMonitorCommand(HeartbeatMonitorCommand command) + /// + /// RTN23a - a transport silent for longer than maxIdleInterval plus realtimeRequestTimeout + /// is treated as dead and disconnected. Any inbound message counts as activity, not just + /// Heartbeats, which is why ProcessMessage refreshes the timestamp rather than the Heartbeat + /// handler. Data we send does not count. + /// + private IEnumerable HandleHeartbeatMonitorCommand(HeartbeatMonitorCommand command) { - if (!command.ConfirmedAliveAt.HasValue) + var connection = State.Connection; + + // Only meaningful while Connected: elsewhere there is no live transport, and + // ConfirmedAliveAt may still hold a previous transport's timestamp. + if (connection.State != ConnectionState.Connected) { + _heartbeatMonitorDisconnectRequested = false; + + // Re-armed per transport, since maxIdleInterval is a per-transport promise. + _warnedIdleCheckInactive = false; return Enumerable.Empty(); } - TimeSpan delta = Now() - command.ConfirmedAliveAt.Value; - if (delta > command.ConnectionStateTtl) + // RTN23b - without protocol heartbeats Ably may satisfy maxIdleInterval with websocket + // ping frames, which this library cannot observe, leaving nothing to measure. + if (ProtocolHeartbeatsNotRequestedByCaller()) { - if (!_heartbeatMonitorDisconnectRequested) - { - _heartbeatMonitorDisconnectRequested = true; - return new RealtimeCommand[] { SetDisconnectedStateCommand.Create(ErrorInfo.ReasonDisconnected).TriggeredBy(command) }; - } + return Enumerable.Empty(); } - else + + // No promised idle period to measure against. + var maxIdleInterval = connection.MaxIdleInterval; + if (maxIdleInterval.HasValue == false || maxIdleInterval.Value <= TimeSpan.Zero) { - if (_heartbeatMonitorDisconnectRequested) + if (_warnedIdleCheckInactive == false) { - _heartbeatMonitorDisconnectRequested = false; + _warnedIdleCheckInactive = true; + + // Logged because the two causes differ: absent means no CONNECTED carried the + // field, zero is CD2h's explicit "arbitrarily-long levels of inactivity". + Logger.Debug( + maxIdleInterval.HasValue + ? "Ably set maxIdleInterval to 0, so it guarantees no inactivity limit. Idle connection detection is off." + : "No maxIdleInterval received from Ably, so idle connection detection is off."); } + + return Enumerable.Empty(); + } + + if (connection.ConfirmedAliveAt.HasValue == false) + { + return Enumerable.Empty(); + } + + // Measured to when the tick was queued, not to now. The workflow is a single reader, so + // work queued ahead of the tick - an inbound AUTH awaiting the application's + // authCallback - would otherwise be charged to the transport. + var idleFor = command.QueuedAt - connection.ConfirmedAliveAt.Value; + + // A window that cannot be represented can never elapse. maxIdleInterval is unbounded off + // the wire, and an OverflowException here would be logged and dropped by the command + // loop, silently killing detection for the life of the connection. + if (maxIdleInterval.Value >= TimeSpan.MaxValue - Client.Options.RealtimeRequestTimeout) + { + return Enumerable.Empty(); + } + + var allowedIdleTime = maxIdleInterval.Value + Client.Options.RealtimeRequestTimeout; + + if (idleFor <= allowedIdleTime) + { + _heartbeatMonitorDisconnectRequested = false; + return Enumerable.Empty(); } - return Enumerable.Empty(); + if (_heartbeatMonitorDisconnectRequested) + { + return Enumerable.Empty(); + } + + _heartbeatMonitorDisconnectRequested = true; + + var error = ErrorInfo.NoActivityFrom(idleFor); + Logger.Warning($"{error.Message} The limit was {allowedIdleTime.TotalSeconds:0.#}s."); + + // RTN15a - a transport we have given up on counts as disconnected unexpectedly, so + // RTN15h3's immediate reconnect applies. Requested explicitly because NoActivityFrom + // carries 408, which the instant retry check does not recognise on its own. + return new RealtimeCommand[] + { + SetDisconnectedStateCommand.Create(error, retryInstantly: true).TriggeredBy(command), + }; } /// @@ -289,6 +360,17 @@ private async Task ProcessCommandInner(RealtimeCommand command) switch (command) { case ConnectCommand _: + + // RTN11d - connect() out of CLOSED or FAILED starts afresh. The channel half, + // back to INITIALIZED with errorReason unset, is done per channel by the command + // queued below; Id and Key are already emptied on entering CLOSED or FAILED. + if (State.Connection.State == ConnectionState.Closed || + State.Connection.State == ConnectionState.Failed) + { + State.Connection.ErrorReason = null; + State.Connection.MessageSerial = 0; + } + var nextCommand = ConnectionManager.Connect(); var initFailedChannelsOnConnect = ChannelCommand.CreateForAllChannels(InitialiseFailedChannelsOnConnect.Create().TriggeredBy(command)); @@ -333,7 +415,20 @@ private async Task ProcessCommandInner(RealtimeCommand command) if (State.Connection.CurrentStateObject.CanSend || cmd.Force) { var sendResult = SendMessage(cmd.ProtocolMessage, cmd.Callback); - if (sendResult.IsFailure && State.Connection.CurrentStateObject.CanQueue && Client.Options.QueueMessages) + + // Never queue a message already awaiting an ACK. One instance in both queues + // would be sent twice on reconnect, and SendMessage's second MsgSerial + // assignment would renumber the copy WaitingForAck reads live, leaving a hole + // in the sequence RTN7b requires to be unique and serially incrementing. + // + // Unreachable today only by coincidence - AckRequired implies CanSend, and + // CanQueue is false in CONNECTED - so the invariant is stated rather than + // left to three unrelated facts. ably-js keeps it deliberately, via + // MessageQueue's sendAttempted flag. + if (sendResult.IsFailure && + cmd.ProtocolMessage.AckRequired == false && + State.Connection.CurrentStateObject.CanQueue && + Client.Options.QueueMessages) { Logger.Debug("Failed to send message. Queuing it."); State.PendingMessages.Add(new MessageAndCallback( @@ -381,9 +476,7 @@ private async Task ProcessCommandInner(RealtimeCommand command) } catch (AblyException e) { - return SetDisconnectedStateCommand.Create( - e.ErrorInfo, - clearConnectionKey: true) + return SetDisconnectedStateCommand.Create(e.ErrorInfo) .TriggeredBy(cmd); } } @@ -409,20 +502,10 @@ async Task AttemptANewConnection() } case HandleConnectingDisconnectedCommand cmd: - if (State.ShouldSuspend(Now)) - { - return SetSuspendedStateCommand.Create( - cmd.Error ?? ErrorInfo.ReasonSuspended, - clearConnectionKey: true) - .TriggeredBy(cmd); - } - else - { - return SetDisconnectedStateCommand.Create( - cmd.Error ?? ErrorInfo.ReasonDisconnected, - clearConnectionKey: true) - .TriggeredBy(cmd); - } + + // Suspending is decided in the SetDisconnectedStateCommand handler, for every path. + return SetDisconnectedStateCommand.Create(cmd.Error ?? ErrorInfo.ReasonDisconnected) + .TriggeredBy(cmd); case HandleConnectingErrorCommand cmd: var error = cmd.Error ?? cmd.Exception?.ErrorInfo ?? ErrorInfo.ReasonUnknown; @@ -435,17 +518,7 @@ async Task AttemptANewConnection() if (error.IsRetryableStatusCode()) { - if (State.ShouldSuspend(Now)) - { - return SetSuspendedStateCommand.Create( - error, - clearConnectionKey: true) - .TriggeredBy(cmd); - } - - return SetDisconnectedStateCommand.Create( - error, - clearConnectionKey: true) + return SetDisconnectedStateCommand.Create(error) .TriggeredBy(cmd); } else @@ -559,6 +632,79 @@ ErrorInfo GetErrorInfoFromTransportException(Exception ex, ErrorInfo @default) return EmptyCommand.Instance; } + /// + /// Whether the caller's own transportParams entry has displaced ours, leaving RTN23b's + /// protocol heartbeats unrequested. RTN23b guarantees them only for exactly + /// `heartbeats=true`; anything else lets Ably use transport-level pings, which + /// ClientWebSocket does not surface. + /// + /// true when protocol heartbeats have not been requested. + private bool ProtocolHeartbeatsNotRequestedByCaller() + { + // Answered once - TransportParams is fixed at client construction. + if (_protocolHeartbeatsNotRequested.HasValue) + { + return _protocolHeartbeatsNotRequested.Value; + } + + _protocolHeartbeatsNotRequested = ComputeProtocolHeartbeatsNotRequested(); + return _protocolHeartbeatsNotRequested.Value; + } + + private bool ComputeProtocolHeartbeatsNotRequested() + { + var transportParams = Client.Options.TransportParams; + if (transportParams == null) + { + return false; + } + + // Case-insensitive because DictionaryExtensions.Merge drops our own heartbeats param on a + // case-insensitive key match, so "Heartbeats" reaches the wire in place of ours. + var callerEntry = transportParams.FirstOrDefault(x => x.Key.EqualsTo("heartbeats")); + if (callerEntry.Key == null) + { + return false; + } + + // Two conditions, because Merge has already dropped ours on a case-insensitive key match: + // - the value must be "true"; RTN23b guarantees protocol heartbeats only for that, and + // treats false or unspecified as permission to use any transport-level mechanism. + // - the key must be exactly "heartbeats"; any other spelling is a param Ably ignores, + // which reads as unspecified. + string value; + try + { + value = callerEntry.Value?.ToString(); + } + catch (Exception ex) + { + // Unguarded, a throwing ToString would escape every tick and be dropped by the + // command loop, killing RTN23a silently. TransportParams.ConvertValue guards it too. + Logger.Error($"Could not read transportParams['{callerEntry.Key}'] as a string.", ex); + value = null; + } + + var keyIsExact = callerEntry.Key.EqualsTo("heartbeats", caseSensitive: true); + var valueIsTrue = value.EqualsTo("true"); + var disabled = keyIsExact == false || valueIsTrue == false; + + if (disabled) + { + // Named separately because the two halves need different fixes. + var reason = keyIsExact + ? $"transportParams sets heartbeats to '{value}', not 'true'." + : $"transportParams sets '{callerEntry.Key}'; Ably only reads 'heartbeats'."; + + Logger.Warning( + $"{reason} Ably may then keep this connection alive with websocket pings, which " + + "this library cannot see, so idle connection detection is off and a silently " + + "dropped connection will not be detected. Set heartbeats to 'true' to enable it."); + } + + return disabled; + } + private void SetNewHostInState(string newHost) { if (IsFallbackHost()) @@ -599,12 +745,31 @@ private void HandleConnectedCommand(SetConnectedStateCommand cmd) { var info = new ConnectionInfo(cmd.Message); - // recover is used when set via clientOptions#recover initially, resume will be used for all subsequent requests. - var isConnectionResumeOrRecoverAttempt = State.Connection.Key.IsNotEmpty() || Client.Options.Recover.IsNotEmpty(); - - var failedResumeOrRecover = State.Connection.Id != info.ConnectionId && cmd.Message.Error != null; // RTN15c7, RTN16d - - State.Connection.Update(info); // RTN16d, RTN15e + // Whether this Connected continues the message serial sequence we are already part of. + // One that does not must restart at zero per RTN15c7, renumbering anything still + // awaiting an ACK. Three ways to continue: + // + // - an RTN24 update, which arrives on the connection we already hold; + // - a successful resume (RTN15c6), judged on the connectionId alone. RTN15c6's "and no + // error property" describes what Ably sends, while the reset belongs to RTN15c7, + // which is keyed on "a new connectionId". ably-js discriminates the same way, on + // connIdChanged; + // - a successful recover, which deliberately adopts a previous connection's counter. + // RTN16f initialises it from the recovery key, which carries no connectionId, so + // success is judged by the absence of an error. + // + // Broader than testing for an error on the message: a resume the server refuses is + // answered with a new connectionId and, often, no error at all - exactly the case that + // most needs a new sequence. + // + // Must be evaluated before Update below, which overwrites the id being compared, and + // before Options.Recover is cleared for RTN16k. + var isRecoverAttempt = Client.Options.Recover.IsNotEmpty(); + var connectionContinues = cmd.IsUpdate || + (isRecoverAttempt && cmd.Message.Error == null) || + (State.Connection.Id.IsNotEmpty() && State.Connection.Id == info.ConnectionId); + + State.Connection.Update(info, cmd.IsUpdate); // RTN16d, RTN15e, RTN23a if (info.ClientId.IsNotEmpty()) { @@ -621,25 +786,17 @@ private void HandleConnectedCommand(SetConnectedStateCommand cmd) Client.Options.Recover = null; // RTN16k, explicitly setting null so it won't be used for subsequent connection requests - // RTN15c7 - if (isConnectionResumeOrRecoverAttempt && failedResumeOrRecover) + // RTN15c7, RTN11d - a connection that is not a continuation of the one we held + // restarts the message serial sequence at zero. + if (connectionContinues == false) { State.Connection.MessageSerial = 0; } - // RTN15g3, RTN15c6, RTN15c7, RTN16l - for resume/recovered or when connection ttl passed, re-attach channels - if (State.Connection.HasConnectionStateTtlPassed(Now) || isConnectionResumeOrRecoverAttempt) - { - foreach (var channel in Channels) - { - if (channel.State == ChannelState.Attaching || channel.State == ChannelState.Attached || channel.State == ChannelState.Suspended) - { - ((RealtimeChannel)channel).Attach(null, null, null, true); // state changes as per RTL2g - } - } - } - - SendPendingMessagesOnConnected(failedResumeOrRecover); // RTN19a + // The RTL3d reattach lives in RealtimeChannel.ConnectionStateChanged, not here: that + // handler runs inside NotifyUpdate's internal handlers, so the channel transitions land + // before CONNECTED reaches external listeners, as RTL3d1 requires. + SendPendingMessagesOnConnected(connectionContinues); // RTN19a } private void HandlePingTimer(PingTimerCommand cmd) @@ -719,22 +876,39 @@ private async Task HandleSetStateCommand(RealtimeCommand comman State.Connection.ClearKey(); } - // RTN15g - If a client has been disconnected for longer - // than the connectionStateTtl, it should not attempt to resume. - if (State.Connection.HasConnectionStateTtlPassed(Now)) - { - State.Connection.ClearKeyAndId(); - } - var defaultRealtimeHost = Client.Options.FullRealtimeHost(); - // Always retry on defaultPrimaryHost first when connecting command triggered by Disconnected/Suspended state timeout. + // RTN17 - every attempt considers a fallback, including the timer driven + // ones. Excluding those would lock a client out once the immediate retry + // budget is spent, since every remaining attempt is timer driven and so + // pinned to the primary. + // + // Asked speculatively, which costs nothing: GetHost only reads state, and + // RTN17i is its job - it returns to the primary whenever the last host + // was a fallback. + var candidateHost = AttemptsHelpers.GetHost(State, defaultRealtimeHost); var connectingHost = defaultRealtimeHost; - // Otherwise use host fallbacks if connecting command triggered by other commands - if (cmd.TriggeredByMessage.Contains("OnTimeOut()") == false) + // RTN17j - the connectivity check comes before the decision to use an + // alternative host. If the internet is unreachable the problem is not this + // host, so stay on the primary rather than working through fallbacks that + // cannot answer either. + // + // The answer is carried on the command, so it cannot outlive the decision + // it was taken for or be picked up by a CONNECTING another path queued. + // Held on the workflow it would have no bound, because the command loop + // abandons a nested batch once its depth guard trips. + // + // Note HandleConnectingTokenError reaches CreateTransport through + // AttemptANewConnection without a check - an RTN17j hole this does not + // close. + var alreadyConfirmed = cmd.ConnectivityConfirmed; + + if (candidateHost == defaultRealtimeHost || + alreadyConfirmed == true || + (alreadyConfirmed == null && await Client.RestClient.CanConnectToAbly())) { - connectingHost = AttemptsHelpers.GetHost(State, defaultRealtimeHost); + connectingHost = candidateHost; } SetNewHostInState(connectingHost); @@ -773,14 +947,31 @@ private async Task HandleSetStateCommand(RealtimeCommand comman case SetFailedStateCommand cmd: - ClearAckQueueAndFailMessages(ErrorInfo.ReasonFailed); - var error = TransformIfTokenErrorAndNotRetryable(); var failedState = new ConnectionFailedState(ConnectionManager, error, Logger); - SetState(failedState); - State.Connection.ClearKeyAndId(); // RTN8c, RTN9c - ConnectionManager.DestroyTransport(); + // RTN7e - the queued messages are failed with "an error representing the + // reason for the state change", taken off the state object so it is this + // transition's reason even if SetState early-returns. In the finally, after + // the transition, so a publisher's callback sees the state it is being told + // about and a throwing transition cannot strand the messages uncalled. + // ably-js orders it the same way: enactStateChange then failQueuedMessages. + // + // RTN8d, RTN9d - the key and id go the other way round, before the + // transition, because SetState emits the state change and with no + // SynchronizationContext that emit is inline. Nothing between here and the + // emit reads either field. + State.Connection.ClearKeyAndId(); // RTN8d, RTN9d + + try + { + SetState(failedState); + } + finally + { + ClearAckQueueAndFailMessages(failedState.Error); + ConnectionManager.DestroyTransport(); + } ErrorInfo TransformIfTokenErrorAndNotRetryable() { @@ -797,11 +988,23 @@ ErrorInfo TransformIfTokenErrorAndNotRetryable() break; case SetDisconnectedStateCommand cmd: - if (cmd.ClearConnectionKey) + // RTN14e - measured here because this is the one place every path into + // DISCONNECTED converges. Checking only the two connection-attempt failure + // handlers misses the token and auth retry paths, which do not pass through + // either: a client whose token source keeps failing would loop CONNECTING and + // DISCONNECTED indefinitely without ever suspending. + // + // Ordered before CheckInstantRetryFlag so that suspending beats retrying. + // + // SkipAttach is excluded: the caller has already queued the next command, so + // diverting would emit SUSPENDED and then immediately CONNECTING. + if (cmd.SkipAttach == false && State.ShouldSuspend(Now)) { - State.Connection.ClearKey(); + return SetSuspendedStateCommand.Create(cmd.Error ?? ErrorInfo.ReasonSuspended) + .TriggeredBy(command); } + bool? connectivityAnswer = null; var retryInstantly = await CheckInstantRetryFlag(); var disconnectedState = new ConnectionDisconnectedState(ConnectionManager, cmd.Error, Logger) @@ -810,6 +1013,14 @@ ErrorInfo TransformIfTokenErrorAndNotRetryable() Exception = cmd.Exception, }; + if (cmd.SkipAttach) + { + // RTN14d - retryIn must be the delay actually waited. skipAttach means + // the caller has already queued the next command, so there is no wait, + // and StartTimer, which records the real figure, never runs. + disconnectedState.RetryIn = TimeSpan.Zero; + } + SetState(disconnectedState, skipTimer: cmd.SkipAttach); // RTN7d @@ -831,7 +1042,21 @@ ErrorInfo TransformIfTokenErrorAndNotRetryable() if (retryInstantly) { - return SetConnectingStateCommand.Create().TriggeredBy(command); + State.AttemptsInfo.RecordInstantRetry(); + + // Handed to the command returned on the next line, and only to that one. + // Both this handler and the CONNECTING one need to know whether the + // internet is reachable - one to decide whether to retry now, the other + // whether to accept a fallback - and both were asking, on the workflow's + // single reader thread, one after the other. Two checks at up to + // MaxHttpOpenTimeout each is a loop held for twice as long as it needs to + // be on every failing attempt, and it eats into the RTN14e budget this + // series worked to make punctual. + // + // Carried on the command so it cannot go stale or be consumed by anything + // else. A timer driven retry carries no answer and takes its own check. + return SetConnectingStateCommand.Create(connectivityConfirmed: connectivityAnswer) + .TriggeredBy(command); } async Task CheckInstantRetryFlag() @@ -841,9 +1066,42 @@ async Task CheckInstantRetryFlag() return true; } - if ((cmd.Error != null && cmd.Error.IsRetryableStatusCode()) || cmd.Exception != null) + // RTN17j sanctions reconnecting immediately, rather than waiting out the + // disconnected retry timeout, to work through the fallback domains. It + // does not sanction doing so without end, and every failed attempt + // produces another DISCONNECTED carrying an exception that qualifies + // again - so the traversal is bounded by the number of domains to + // traverse. Past that we are in RTN14d, where attempts are periodic and + // spaced per RTB1, and host selection continues at that slower pace. + var domainCount = 1 + State.Connection.FallbackHosts.Count; + if (State.AttemptsInfo.InstantRetryCount >= domainCount) { - return await Client.RestClient.CanConnectToAbly(); + return false; + } + + // RTN15a and RTN15h3 - an unexpected transport drop or a non-token + // DISCONNECTED both earn an immediate reconnect. The first two tests + // cover the drop, the third the DISCONNECTED, which carries no status + // code of its own. + // + // Token errors are excluded because RTN15h3 is the "error other than a + // token error" clause: RTN15h2 owns them and has already queued its own + // CONNECTING behind this command, so granting a retry here too gives two + // overlapping attempts. + // + // Gated on the connectivity check, because the retry this grants is + // where host selection happens and RTN17j requires a check before an + // alternative host is used. + var reconnectImmediately = cmd.Exception != null + || (cmd.Error != null && cmd.Error.IsRetryableStatusCode()) + || (State.Connection.State == ConnectionState.Connected + && cmd.Error?.IsTokenError != true); + + if (reconnectImmediately) + { + // Remembered so the CONNECTING behind this command does not repeat it. + connectivityAnswer = await Client.RestClient.CanConnectToAbly(); + return connectivityAnswer.Value; } return false; @@ -857,8 +1115,8 @@ async Task CheckInstantRetryFlag() var connectedTransport = transport?.State == TransportState.Connected; var closingState = new ConnectionClosingState(ConnectionManager, connectedTransport, Logger); + State.Connection.ClearKeyAndId(); // RTN8d, RTN9d - before the emit SetState(closingState); - State.Connection.ClearKeyAndId(); // RTN8c, RTN9c if (connectedTransport) { @@ -871,32 +1129,44 @@ async Task CheckInstantRetryFlag() case SetSuspendedStateCommand cmd: - if (cmd.ClearConnectionKey) + var suspendedState = new ConnectionSuspendedState(ConnectionManager, cmd.Error, Logger); + + // RTN7e and the teardown - see the note on the FAILED case. + try { - State.Connection.ClearKey(); + SetState(suspendedState); } + finally + { + ClearAckQueueAndFailMessages(suspendedState.Error); - ClearAckQueueAndFailMessages(ErrorInfo.ReasonSuspended); - - var suspendedState = new ConnectionSuspendedState(ConnectionManager, cmd.Error, Logger); - SetState(suspendedState); - State.Connection.ClearKeyAndId(); // RTN8c, RTN9c + // Needed here as well as in the DISCONNECTED handler, which diverts to + // this case before reaching its own DestroyTransport. A surviving + // transport keeps its listener for up to suspendedRetryTimeout. + ConnectionManager.DestroyTransport(); + } break; case SetClosedStateCommand cmd: - ClearAckQueueAndFailMessages(ErrorInfo.ReasonClosed); - var closedState = new ConnectionClosedState(ConnectionManager, cmd.Error, Logger) { Exception = cmd.Exception, }; - SetState(closedState); - State.Connection.ClearKeyAndId(); // RTN8c, RTN9c + // RTN7e, RTN8d, RTN9d and the teardown - see the note on the FAILED case. + State.Connection.ClearKeyAndId(); // RTN8d, RTN9d - before the emit - ConnectionManager.DestroyTransport(); + try + { + SetState(closedState); + } + finally + { + ClearAckQueueAndFailMessages(closedState.Error); + ConnectionManager.DestroyTransport(); + } break; } @@ -927,6 +1197,8 @@ public void SetState(ConnectionStateBase newState, bool skipTimer = false) Logger.Debug(message); } + var notified = false; + try { if (newState.IsUpdate == false) @@ -954,13 +1226,28 @@ public void SetState(ConnectionStateBase newState, bool skipTimer = false) Logger.Debug($"xx {newState.State}: Skipping attaching."); } + notified = true; UpdateStateAndNotifyConnection(newState); } - catch (AblyException ex) + catch (Exception ex) { - Logger.Error("Error attaching to context", ex); - - UpdateStateAndNotifyConnection(newState); + // Everything, not just AblyException: anything else thrown by StartTimer or the + // state object would reach the command loop, which logs and drops it, leaving the + // connection with no transport, no timer and no state change emitted. The transition + // is still completed below and the exception still rethrown. + Logger.Error($"Error attaching to context while changing state to {newState.State}", ex); + + // Only if the notify has not already happened. A throw during the transition lands + // here after the state change has been emitted - StartTimer is one source, and a + // negative retry timeout reaches System.Threading.Timer. Not a channel's + // ConnectionStateChanged, which RealtimeChannels guards per channel. Re-emitting is harmless for an ordinary transition, which the + // same-state check swallows, but an RTN24 update has no such check and was emitted + // twice. The flag is set before the call so a throw from inside it does not trigger + // a second attempt either. + if (notified == false) + { + UpdateStateAndNotifyConnection(newState); + } newState.AbortTimer(); @@ -977,26 +1264,42 @@ private void UpdateStateAndNotifyConnection(ConnectionStateBase newState) } } - private void SendPendingMessagesOnConnected(bool failedResumeOrRecover) + private void SendPendingMessagesOnConnected(bool connectionContinues) { - // RTN19a1 - if (failedResumeOrRecover) + if (connectionContinues) { - foreach (var messageAndCallback in State.WaitingForAck) + // RTN19a2 - the same connection is still expecting the serials these messages were + // originally given, so resend them unchanged and leave them awaiting their ACK. + foreach (var message in State.WaitingForAck.Select(x => x.Message)) { - State.PendingMessages.Add(new MessageAndCallback( - messageAndCallback.Message, - messageAndCallback.Callback, - messageAndCallback.Logger)); + ConnectionManager.SendToTransport(message); } } else { - // RTN19a2 - successful resume, msgSerial doesn't change - foreach (var message in State.WaitingForAck.Select(x => x.Message)) - { - ConnectionManager.SendToTransport(message); - } + // RTN19a1, RTN19a2 - a different connection means a fresh serial sequence, so + // requeue rather than resend. The loop below hands each message to SendMessage, + // which assigns a serial from the counter that HandleConnectedCommand has just + // reset and re-registers it for its ACK. + // + // Resending these unchanged would leave the server's sequence sitting at the old + // high water mark while ours restarted at zero, and Ably silently discards a + // message whose serial is below what it has already seen - no ACK, no NACK, so the + // publish callback would never be called at all. + // + // WaitingForAck is cleared because SendMessage re-registers each message as it + // goes; stale entries would hold serials of the old sequence that the next ACK also + // matches, running their callbacks twice. + // + // Inserted at the front, not appended: PendingMessages is the RTL6c2 queue and + // already holds anything published while disconnected, which happened *after* these. + // Appending would give the newer messages the lower serials and reverse publish + // order. ably-js prepends for the same reason. + State.PendingMessages.InsertRange( + 0, + State.WaitingForAck.Select(x => new MessageAndCallback(x.Message, x.Callback, x.Logger))); + + State.WaitingForAck.Clear(); } if (Logger.IsDebug && State.PendingMessages.Count > 0) @@ -1016,15 +1319,30 @@ private void SendPendingMessagesOnConnected(bool failedResumeOrRecover) State.PendingMessages.Clear(); } + /// + /// RTN7e - when the connection enters SUSPENDED, CLOSED or FAILED, everything that has not + /// been acknowledged has failed and must be reported as such. + /// private void ClearAckQueueAndFailMessages(ErrorInfo error) { + var messageError = error ?? ErrorInfo.ReasonUnknown; + foreach (var item in State.WaitingForAck.Where(x => x.Callback != null)) { - var messageError = error ?? ErrorInfo.ReasonUnknown; item.SafeExecute(false, messageError); } State.WaitingForAck.Clear(); + + // RTN7e covers RTL6c2 as well as RTL6c1: a message submitted via either "should be + // considered failed ... and removed from any RTN19a retry queue". PendingMessages is the + // RTL6c2 queue. + foreach (var item in State.PendingMessages.Where(x => x.Callback != null)) + { + item.SafeExecute(false, messageError); + } + + State.PendingMessages.Clear(); } public void QueueAck(ProtocolMessage message, Action callback) diff --git a/src/IO.Ably.Shared/Transport/ConnectionAttemptsInfo.cs b/src/IO.Ably.Shared/Transport/ConnectionAttemptsInfo.cs index 032796196..2ce7544b9 100644 --- a/src/IO.Ably.Shared/Transport/ConnectionAttemptsInfo.cs +++ b/src/IO.Ably.Shared/Transport/ConnectionAttemptsInfo.cs @@ -23,10 +23,23 @@ public ConnectionAttemptsInfo(Func now = null) internal bool TriedToRenewToken { get; private set; } + /// + /// How many times we have skipped the disconnected retry timeout and reconnected straight + /// away since last being connected. RTN17j sanctions retrying immediately to work through the + /// fallback domains, but the traversal must be bounded or RTB1 is never reached. + /// + internal int InstantRetryCount { get; private set; } + public void Reset() { Attempts.Clear(); TriedToRenewToken = false; + InstantRetryCount = 0; + } + + public void RecordInstantRetry() + { + InstantRetryCount++; } public void RecordTokenRetry() @@ -82,10 +95,16 @@ private void RecordAttemptFailure(ConnectionState state, ErrorInfo error) private void RecordAttemptFailure(ConnectionState state, Exception ex) { - if (Attempts.Any()) + // Mirrors the ErrorInfo overload above, including the empty-collection case, which is + // the normal one here: the only caller passing an exception is the transport dropping out + // of CONNECTED, and entering CONNECTED has just cleared the collection. Dropping it would + // leave FirstAttempt null, delaying the RTN14e clock, and DisconnectedCount at zero, + // which feeds RTN17 host selection. + var attempt = Attempts.LastOrDefault() ?? new ConnectionAttempt(_now()); + attempt.FailedStates.Add(new AttemptFailedState(state, ex)); + if (Attempts.Count == 0) { - var attempt = Attempts.Last(); - attempt.FailedStates.Add(new AttemptFailedState(state, ex)); + Attempts.Add(attempt); } } } diff --git a/src/IO.Ably.Shared/Transport/ConnectionInfo.cs b/src/IO.Ably.Shared/Transport/ConnectionInfo.cs index d7f47269b..da36eb03b 100644 --- a/src/IO.Ably.Shared/Transport/ConnectionInfo.cs +++ b/src/IO.Ably.Shared/Transport/ConnectionInfo.cs @@ -33,6 +33,7 @@ public ConnectionInfo(ProtocolMessage message) ConnectionId = message.ConnectionId; ClientId = message.ConnectionDetails?.ClientId; ConnectionStateTtl = message.ConnectionDetails?.ConnectionStateTtl; + MaxIdleInterval = message.ConnectionDetails?.MaxIdleInterval; ConnectionKey = message.ConnectionDetails?.ConnectionKey; } @@ -41,6 +42,12 @@ public ConnectionInfo(ProtocolMessage message) /// public TimeSpan? ConnectionStateTtl { get; private set; } + /// + /// The maximum period of inactivity the server will allow in the server to client + /// direction before it sends a Heartbeat or transport level ping. See CD2h. + /// + public TimeSpan? MaxIdleInterval { get; private set; } + /// /// contains the client ID assigned to the connection. /// diff --git a/src/IO.Ably.Shared/Transport/ConnectionManager.cs b/src/IO.Ably.Shared/Transport/ConnectionManager.cs index d2a2869f2..ed699c7f8 100644 --- a/src/IO.Ably.Shared/Transport/ConnectionManager.cs +++ b/src/IO.Ably.Shared/Transport/ConnectionManager.cs @@ -172,12 +172,14 @@ internal async Task OnAuthUpdated(TokenDetails tokenDetails, bool wait) { while (true) { - var (success, newState) = await waiter.Wait(Defaults.RealtimeRequestTimeout); + // Options rather than Defaults, since TO3l11 makes realtimeRequestTimeout a + // client option. + var (success, newState) = await waiter.Wait(Options.RealtimeRequestTimeout); if (success == false) { throw new AblyException( new ErrorInfo( - $"Connection state didn't change after Auth updated within {Defaults.RealtimeRequestTimeout}", + $"Connection state didn't change after Auth updated within {Options.RealtimeRequestTimeout}", 40140)); } diff --git a/src/IO.Ably.Shared/Transport/States/Connection/ConnectionClosingState.cs b/src/IO.Ably.Shared/Transport/States/Connection/ConnectionClosingState.cs index 7a7a854ab..f6b63c906 100644 --- a/src/IO.Ably.Shared/Transport/States/Connection/ConnectionClosingState.cs +++ b/src/IO.Ably.Shared/Transport/States/Connection/ConnectionClosingState.cs @@ -10,7 +10,6 @@ internal class ConnectionClosingState : ConnectionStateBase { public override ErrorInfo DefaultErrorInfo => ErrorInfo.ReasonClosed; - private const int CloseTimeout = 1000; private readonly bool _connectedTransport; private readonly ICountdownTimer _timer; @@ -62,7 +61,9 @@ public override void StartTimer() { if (_connectedTransport) { - _timer.Start(TimeSpan.FromMilliseconds(CloseTimeout), OnTimeOut); + // RTN12b - the wait for the CLOSED message is realtimeRequestTimeout, which TO3l11 + // makes a client option. + _timer.Start(Context.DefaultTimeout, OnTimeOut); } } diff --git a/src/IO.Ably.Shared/Transport/States/Connection/ConnectionConnectedState.cs b/src/IO.Ably.Shared/Transport/States/Connection/ConnectionConnectedState.cs index c61f182f2..3d03f136b 100644 --- a/src/IO.Ably.Shared/Transport/States/Connection/ConnectionConnectedState.cs +++ b/src/IO.Ably.Shared/Transport/States/Connection/ConnectionConnectedState.cs @@ -55,6 +55,9 @@ public override async Task OnMessageReceived(ProtocolMessage message, Real return true; } + // RTN15h3's immediate reconnect is granted by the SetDisconnectedStateCommand + // handler, which recognises a disconnect arriving while connected. Not requested + // here, which would bypass the RTN17j connectivity check. Context.ExecuteCommand(SetDisconnectedStateCommand.Create(message.Error).TriggeredBy("ConnectedState.OnMessageReceived()")); return true; case ProtocolMessage.MessageAction.Error: diff --git a/src/IO.Ably.Shared/Transport/States/Connection/ConnectionDisconnectedState.cs b/src/IO.Ably.Shared/Transport/States/Connection/ConnectionDisconnectedState.cs index a8d4bd624..e5aae50dc 100644 --- a/src/IO.Ably.Shared/Transport/States/Connection/ConnectionDisconnectedState.cs +++ b/src/IO.Ably.Shared/Transport/States/Connection/ConnectionDisconnectedState.cs @@ -49,14 +49,74 @@ public override void AbortTimer() // RTN14d public override void StartTimer() { + if (RetryInstantly) + { + // RTN14d - there is no wait, so say so rather than advertising the nominal timeout. + RetryIn = TimeSpan.Zero; + return; + } + + var state = Context.Connection.RealtimeClient?.State; var retryInterval = Context.RetryTimeout.TotalMilliseconds; - var noOfAttempts = Context.Connection.RealtimeClient?.State?.AttemptsInfo?.NumberOfAttempts ?? 0 + 1; // First attempt should start with 1 instead of 0. - var retryIn = TimeSpan.FromMilliseconds(ReconnectionStrategy.GetRetryTime(retryInterval, noOfAttempts)); - if (RetryInstantly == false) + // RTB1a's coefficient sequence is indexed from the first attempt, so a count of zero + // would apply 2/3 where the first retry should get 1. On a drop straight out of + // CONNECTED there is no recorded attempt, because entering CONNECTED cleared them. + var noOfAttempts = Math.Max(state?.AttemptsInfo?.NumberOfAttempts ?? 0, 1); + + var retryIn = ClampToStateTtl( + TimeSpan.FromMilliseconds(ReconnectionStrategy.GetRetryTime(retryInterval, noOfAttempts)), + state); + + // RTN14d - retryIn must be "the time in milliseconds until the next connection attempt", + // so report the delay we are about to wait rather than the nominal + // disconnectedRetryTimeout, which ignores the RTB1 coefficient, its jitter and the clamp + // above. SetState calls this before emitting, so this is what the application sees. + RetryIn = retryIn; + + _timer.Start(retryIn, OnTimeOut); + } + + /// + /// RTN14e requires the move to SUSPENDED once the connection state ttl has elapsed, and that + /// decision is taken when an attempt fails. Sleeping past the deadline would delay SUSPENDED + /// by however long we slept, unbounded because disconnectedRetryTimeout is a client option. + /// Waking at the deadline lets the attempt fail there and be converted, keeping one timer. + /// + private TimeSpan ClampToStateTtl(TimeSpan retryIn, RealtimeState state) + { + var firstAttempt = state?.AttemptsInfo?.FirstAttempt; + if (firstAttempt.HasValue == false) { - _timer.Start(retryIn, OnTimeOut); + return retryIn; } + + var elapsed = Context.Connection.Now() - firstAttempt.Value; + + // Both operands clamped before subtracting: a backwards clock step makes elapsed + // negative, and a ttl near TimeSpan.MaxValue then overflows. SetState calls StartTimer + // inside a catch that only handles AblyException, so the throw would be dropped and the + // whole transition abandoned - no DISCONNECTED, no transport, no timer. + if (elapsed < TimeSpan.Zero) + { + elapsed = TimeSpan.Zero; + } + + if (state.Connection.ConnectionStateTtl >= TimeSpan.MaxValue - elapsed) + { + return retryIn; + } + + var remaining = state.Connection.ConnectionStateTtl - elapsed; + + if (remaining <= TimeSpan.Zero || remaining >= retryIn) + { + // The deadline has passed, so the next attempt converts to SUSPENDED whenever it + // happens, or it is further out than the backoff and there is nothing to clamp. + return retryIn; + } + + return remaining; } private void OnTimeOut() diff --git a/src/IO.Ably.Shared/Transport/States/Connection/ConnectionStateBase.cs b/src/IO.Ably.Shared/Transport/States/Connection/ConnectionStateBase.cs index 5c1b0a612..011221f28 100644 --- a/src/IO.Ably.Shared/Transport/States/Connection/ConnectionStateBase.cs +++ b/src/IO.Ably.Shared/Transport/States/Connection/ConnectionStateBase.cs @@ -26,7 +26,7 @@ protected ConnectionStateBase(IConnectionContext context, ILogger logger) public Exception Exception { get; set; } - public TimeSpan? RetryIn { get; protected set; } + public TimeSpan? RetryIn { get; internal set; } public virtual bool CanQueue => false; diff --git a/src/IO.Ably.Shared/Transport/TransportParams.cs b/src/IO.Ably.Shared/Transport/TransportParams.cs index 357a0943b..8b329573f 100644 --- a/src/IO.Ably.Shared/Transport/TransportParams.cs +++ b/src/IO.Ably.Shared/Transport/TransportParams.cs @@ -187,6 +187,12 @@ public Dictionary GetParams() result["format"] = UseBinaryProtocol ? "msgpack" : "json"; result["echo"] = EchoMessages.ToString().ToLower(); + // RTN23b - ask Ably to keep the connection alive with Heartbeat protocol messages rather + // than a transport level mechanism. ClientWebSocket cannot observe an incoming websocket + // ping frame, so protocol messages are the only activity RTN23a can measure. Overridable + // through ClientOptions.TransportParams, which take precedence in the merge below. + result["heartbeats"] = "true"; + // RTN15b - resume connection using connectionKey if (ConnectionKey.IsNotEmpty()) { diff --git a/src/IO.Ably.Shared/Types/ConnectionDetails.cs b/src/IO.Ably.Shared/Types/ConnectionDetails.cs index 095533fe3..772d40f5c 100644 --- a/src/IO.Ably.Shared/Types/ConnectionDetails.cs +++ b/src/IO.Ably.Shared/Types/ConnectionDetails.cs @@ -32,6 +32,16 @@ public class ConnectionDetails [JsonProperty("maxFrameSize")] public long MaxFrameSize { get; set; } + /// + /// The maximum length of time that the server will allow no activity to occur in the + /// server to client direction. After such a period of inactivity the server will send a + /// Heartbeat or a transport level ping. A value of zero means the server allows + /// arbitrarily long levels of inactivity and no idle timeout should be applied. + /// See CD2h - https://sdk.ably.com/builds/ably/specification/main/features/#CD2h. + /// + [JsonProperty("maxIdleInterval")] + public TimeSpan? MaxIdleInterval { get; set; } + /// /// Max inbound rate. /// diff --git a/src/IO.Ably.Shared/Types/ErrorInfo.cs b/src/IO.Ably.Shared/Types/ErrorInfo.cs index 03dfa9b59..cc8b4ea54 100644 --- a/src/IO.Ably.Shared/Types/ErrorInfo.cs +++ b/src/IO.Ably.Shared/Types/ErrorInfo.cs @@ -28,6 +28,18 @@ public class ErrorInfo internal const string ReasonPropertyName = "message"; internal const string HrefBase = "https://help.ably.io/error/"; + /// + /// The error used when a transport is disconnected for seeing no activity from Ably for + /// longer than RTN23a allows. The observed idle time is included to make it diagnosable. + /// + /// how long the transport has been idle. + /// a Disconnected ErrorInfo describing the idle period. + internal static ErrorInfo NoActivityFrom(TimeSpan idleFor) => + new ErrorInfo( + $"No activity from Ably for {idleFor.TotalSeconds:0.#}s, assuming the connection has dropped.", + ErrorCodes.Disconnected, + HttpStatusCode.RequestTimeout); + /// /// Ably error code (see https://github.com/ably/ably-common/blob/main/protocol/errors.json). /// diff --git a/src/IO.Ably.Tests.Shared/Infrastructure/AblyRealtimeSpecs.cs b/src/IO.Ably.Tests.Shared/Infrastructure/AblyRealtimeSpecs.cs index ab3dfaad3..5c8ffd080 100644 --- a/src/IO.Ably.Tests.Shared/Infrastructure/AblyRealtimeSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Infrastructure/AblyRealtimeSpecs.cs @@ -96,6 +96,8 @@ internal AblyRealtime GetRealtimeClient(Action optionsAction, Fun protected FakeTransport LastCreatedTransport => FakeTransportFactory.LastCreatedTransport; + protected List CreatedTransports => FakeTransportFactory.CreatedTransports; + internal AblyRealtime GetClientWithFakeTransport(Action optionsAction = null, Func> handleRequestFunc = null) { var options = new ClientOptions(ValidKey) { TransportFactory = FakeTransportFactory }; diff --git a/src/IO.Ably.Tests.Shared/Infrastructure/FakeTransportFactory.cs b/src/IO.Ably.Tests.Shared/Infrastructure/FakeTransportFactory.cs index 8a0ee9b6f..860e84a0d 100644 --- a/src/IO.Ably.Tests.Shared/Infrastructure/FakeTransportFactory.cs +++ b/src/IO.Ably.Tests.Shared/Infrastructure/FakeTransportFactory.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using IO.Ably.Transport; namespace IO.Ably.Tests.Realtime @@ -7,11 +8,18 @@ public class FakeTransportFactory : ITransportFactory { public FakeTransport LastCreatedTransport { get; set; } + /// + /// Every transport created, oldest first. LastCreatedTransport alone cannot answer questions + /// about a sequence of attempts - how many were made, or whether each carried a resume. + /// + public List CreatedTransports { get; } = new List(); + public Action InitialiseFakeTransport = obj => { }; public ITransport CreateTransport(TransportParams parameters) { LastCreatedTransport = new FakeTransport(parameters); + CreatedTransports.Add(LastCreatedTransport); InitialiseFakeTransport(LastCreatedTransport); return LastCreatedTransport; } diff --git a/src/IO.Ably.Tests.Shared/Realtime/ChannelSandboxSpecs.cs b/src/IO.Ably.Tests.Shared/Realtime/ChannelSandboxSpecs.cs index 5aa8c564b..670ad3df0 100644 --- a/src/IO.Ably.Tests.Shared/Realtime/ChannelSandboxSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Realtime/ChannelSandboxSpecs.cs @@ -116,62 +116,6 @@ public async Task WhenAttachingAChannelWithInsufficientPermissions_ShouldSetItTo result.Error.StatusCode.Should().Be(HttpStatusCode.Unauthorized); } - [Theory] - [ProtocolData] - [Trait("spec", "RTL4j")] - public async Task WhenChannelIsAlreadyAttached_AndReAttachIsForcedByChangingChannelOptions_ShouldPassAttachResumeFlagInAttachMessage(Protocol protocol) - { - var sentMessages = new List(); - var client = await GetRealtimeClient(protocol, (options, _) => - { - var optionsTransportFactory = new TestTransportFactory - { - OnMessageSent = sentMessages.Add, - }; - options.TransportFactory = optionsTransportFactory; - }); - - var channel = client.Channels.Get("Test"); - await channel.AttachAsync(); - - var result = await channel.SetOptionsAsync(new ChannelOptions().WithModes(ChannelMode.Publish)); - - result.IsSuccess.Should().BeTrue(); - var attachMessages = sentMessages.Where(x => x.Action == ProtocolMessage.MessageAction.Attach).ToList(); - attachMessages.Should().HaveCount(2); - attachMessages.First().Flags.Should().BeNull(); - attachMessages.Last().HasFlag(ProtocolMessage.Flag.AttachResume).Should().BeTrue(); - } - - [Theory] - [ProtocolData] - [Trait("spec", "RTL4j2")] - public async Task TestAttachResume_And_RewindParam(Protocol protocol) - { - var client = await GetRealtimeClient(protocol); - var client1 = await GetRealtimeClient(protocol, (opts, _) => opts.AutoConnect = false); - var client2 = await GetRealtimeClient(protocol, (opts, _) => opts.AutoConnect = false); - - var channel = client.Channels.Get("Test"); - await channel.PublishAsync("test", "test"); - - var channelWithAttachResume = client1.Channels.Get("Test", new ChannelOptions().WithRewind(1)) as RealtimeChannel; - channelWithAttachResume.AttachResume = true; - var channel1Messages = new List(); - channelWithAttachResume.Subscribe(channel1Messages.Add); - await channelWithAttachResume.WaitForAttachedState(); - var channelWithoutAttachResume = client2.Channels.Get("Test", new ChannelOptions().WithRewind(1)); - - var channel2Messages = new List(); - channelWithoutAttachResume.Subscribe(channel2Messages.Add); - await channelWithoutAttachResume.WaitForAttachedState(); - - await Task.Delay(2000); - - channel2Messages.Should().HaveCount(1); - channel1Messages.Should().BeEmpty(); - } - [Theory] [ProtocolData] [Trait("spec", "RTL4k1")] diff --git a/src/IO.Ably.Tests.Shared/Realtime/ChannelSpecs.cs b/src/IO.Ably.Tests.Shared/Realtime/ChannelSpecs.cs index 863ba6628..17ecbc7c7 100644 --- a/src/IO.Ably.Tests.Shared/Realtime/ChannelSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Realtime/ChannelSpecs.cs @@ -388,6 +388,65 @@ public async Task WhenChannelIsSuspended_WhenConnectionBecomesConnectedAttemptAt completed.Should().BeTrue("channel should have become Suspended again"); } + [Fact] + [Trait("spec", "RTL3d1")] + [Trait("spec", "RTL3d")] + public async Task WhenConnectedIsEmittedExternally_TheRTL3dTransitionsShouldAlreadyBeApplied() + { + // RTL3d1: "The RTL3d channel state transitions must be applied before the CONNECTED + // connection state change is emitted to external listeners." Nothing implements it + // directly - it holds because Connection.NotifyUpdate runs the internal handlers, + // where RealtimeChannels fans the change out to every channel, before the emit. + // + // An ATTACHED channel on a successful resume is the case that needs it, and the one + // a conditional reattach would miss. With no SynchronizationContext - the default, + // and what every server-side host has - NotifyExternalClients invokes inline, so + // anything done after SetState is visible to the application too late. + var client = await GetConnectedClient(opts => + opts.DisconnectedRetryTimeout = TimeSpan.FromMinutes(10)); + + var channel = (RealtimeChannel)client.Channels.Get("test".AddRandomSuffix()); + channel.SetChannelState(ChannelState.Attached); + await client.ProcessCommands(); + + client.ExecuteCommand(SetDisconnectedStateCommand.Create(ErrorInfo.ReasonDisconnected)); + await client.WaitForState(ConnectionState.Disconnected); + await client.ProcessCommands(); + + ChannelState? seenByExternalListener = null; + client.Connection.On(ConnectionEvent.Connected, _ => seenByExternalListener = channel.State); + + // The same connectionId the client already holds, so this is an RTN15c6 resume. + client.Workflow.QueueCommand(SetConnectedStateCommand.Create(ConnectedProtocolMessage, false)); + await client.WaitForState(ConnectionState.Connected); + await client.ProcessCommands(); + + seenByExternalListener.Should().Be(ChannelState.Attaching); + } + + [Fact] + [Trait("spec", "RTL3c")] + public async Task WhenConnectionIsSuspended_ADetachingChannelShouldNotBeStranded() + { + // RTL3c names only ATTACHING and ATTACHED, so DETACHING is ably-js parity rather than + // a spec requirement - propogateConnectionInterruption maps suspended over + // ['attaching','attached','detaching','suspended']. It matters because the handler + // fails the DetachedAwaiter first, leaving the channel with no DETACHED coming. + var (client, channel) = await GetClientAndChannel(); + + ((RealtimeChannel)channel).SetChannelState(ChannelState.Detaching); + await client.ProcessCommands(); + + client.Workflow.QueueCommand(SetSuspendedStateCommand.Create(new ErrorInfo("why it suspended", 12345))); + await client.WaitForState(ConnectionState.Suspended); + await client.ProcessCommands(); + + channel.State.Should().Be(ChannelState.Suspended); + + // And the connection's own reason, not a constant - ably-js passes change.reason. + channel.ErrorReason.Message.Should().Be("why it suspended"); + } + [Theory] [InlineData(ChannelState.Attached)] [InlineData(ChannelState.Attaching)] @@ -399,10 +458,12 @@ public async Task WhenConnectionIsSuspended_AttachingOrAttachedChannelsShouldTra ((RealtimeChannel)channel).SetChannelState(state); - client.Close(); - + // Driven straight to SUSPENDED, with no Close() first: CLOSING detaches channels per + // RTN11b/RTL3b, so closing would race the suspend for the outcome, and RTL3c is + // about entering SUSPENDED anyway. client.Workflow.QueueCommand(SetSuspendedStateCommand.Create(null)); await client.WaitForState(ConnectionState.Suspended); + await client.ProcessCommands(); // Assert channel.State.Should().Be(ChannelState.Suspended); diff --git a/src/IO.Ably.Tests.Shared/Realtime/ChannelsSpecs.cs b/src/IO.Ably.Tests.Shared/Realtime/ChannelsSpecs.cs index 722feb98b..bf5fae462 100644 --- a/src/IO.Ably.Tests.Shared/Realtime/ChannelsSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Realtime/ChannelsSpecs.cs @@ -329,20 +329,6 @@ public async Task WithExistingChannel_Get_WithNewChannelOptionsButDifferentModes ex.Message.Should().Contain("SetOptions"); } - [Theory] - [InlineData(ChannelState.Initialized, false)] - [InlineData(ChannelState.Attached, true)] - [InlineData(ChannelState.Detaching, false)] - [InlineData(ChannelState.Failed, false)] - [Trait("spec", "RTL4j1")] - public async Task WhenChannelMovesToState_AttachResumeShouldHaveCorrectValue(ChannelState state, bool expectedAttachResumeValue) - { - var client = GetRealtimeClient(); - var channel = client.Channels.Get("Test") as RealtimeChannel; - channel.SetChannelState(state); - channel.AttachResume.Should().Be(expectedAttachResumeValue); - } - public ChannelsSpecs(ITestOutputHelper output) : base(output) { diff --git a/src/IO.Ably.Tests.Shared/Realtime/ConnectionSandBoxSpecs.cs b/src/IO.Ably.Tests.Shared/Realtime/ConnectionSandBoxSpecs.cs index e7e303bca..c51051622 100644 --- a/src/IO.Ably.Tests.Shared/Realtime/ConnectionSandBoxSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Realtime/ConnectionSandBoxSpecs.cs @@ -960,12 +960,16 @@ public async Task WhenDisconnectedMessageContainsTokenError_IfTokenRenewFails_Sh [Theory] [ProtocolData] - [Trait("spec", "RTN15g")] - [Trait("spec", "RTN15g1")] - // "RTN15g2" It can't implement that spec item because RTN23a is not even implemented - [Trait("spec", "RTN15g3")] - public async Task WhenDisconnectedPastTTL_ShouldNotResume_ShouldClearConnectionStateAndAttemptNewConnection(Protocol protocol) + [Trait("spec", "RTN14h")] + [Trait("spec", "RTN8d")] + [Trait("spec", "RTN9d")] + [Trait("spec", "RTN15c6")] + public async Task WhenDisconnectedPastTTL_ShouldStillResume_AndReattachChannels(Protocol protocol) { + // RTN14h, which replaces RTN15g as of specification 6.1.0 - the client always attempts + // the resume and lets the server decide whether continuity survives. Against a live + // endpoint the reconnect comes back with the connectionId we were holding, which is only + // possible because the resume param went out (RTN15b1). var client = await GetRealtimeClient(protocol, (options, _) => { options.RealtimeRequestTimeout = TimeSpan.FromMilliseconds(1000); @@ -974,14 +978,11 @@ public async Task WhenDisconnectedPastTTL_ShouldNotResume_ShouldClearConnectionS await client.WaitForState(ConnectionState.Connected); - client.State.Connection.ConnectionStateTtl = TimeSpan.FromSeconds(1); string initialConnectionId = client.Connection.Id; - TimeSpan connectionStateTtl = client.Connection.ConnectionStateTtl; - - var aliveAt1 = client.Connection.ConfirmedAliveAt; - var aliveAt2 = aliveAt1; + string initialConnectionKey = client.Connection.Key; - // RTN15g3 ATTACHED, ATTACHING, or SUSPENDED must be automatically reattached + // RTL3d - channels that were ATTACHED, ATTACHING or SUSPENDED are reattached on + // entering CONNECTED regardless of whether the resume succeeded. var channels = new List { client.Channels.Get("attached".AddRandomSuffix()) as RealtimeChannel, @@ -997,25 +998,21 @@ public async Task WhenDisconnectedPastTTL_ShouldNotResume_ShouldClearConnectionS channels[1].State.Should().Be(ChannelState.Initialized); // set attaching later channels[2].State.Should().Be(ChannelState.Suspended); - DateTime disconnectedAt = DateTime.MinValue; - DateTime reconnectedAt = DateTime.MinValue; string newConnectionId = string.Empty; await WaitFor(60000, done => { - client.Connection.Once(ConnectionEvent.Disconnected, change2 => + client.Connection.Once(ConnectionEvent.Disconnected, _ => { - disconnectedAt = DateTime.UtcNow; + // RTN8d, RTN9d - DISCONNECTED is not a terminal state, so both survive. + client.Connection.Id.Should().Be(initialConnectionId); + client.Connection.Key.Should().Be(initialConnectionKey); + channels[1].Attach(); - client.Connection.Once(ConnectionEvent.Connecting, change3 => + client.Connection.Once(ConnectionEvent.Connected, _ => { - reconnectedAt = DateTime.UtcNow; - client.Connection.Once(ConnectionEvent.Connected, change4 => - { - newConnectionId = client.Connection.Id; - aliveAt2 = client.Connection.ConfirmedAliveAt; - done(); - }); + newConnectionId = client.Connection.Id; + done(); }); }); @@ -1023,15 +1020,10 @@ await WaitFor(60000, done => client.Workflow.QueueCommand(SetDisconnectedStateCommand.Create(ErrorInfo.ReasonDisconnected)); }); - var reconnectedInTime = reconnectedAt - disconnectedAt; - - var (lowerBound, _) = ReconnectionStrategyTest.Bounds(1, 5000); - reconnectedInTime.TotalMilliseconds.Should().BeGreaterThan(lowerBound); - + // RTN15c6 - the server still held the connection, so the resume succeeded and the + // connectionId comes back unchanged. initialConnectionId.Should().NotBeNullOrEmpty(); - initialConnectionId.Should().NotBe(newConnectionId); - connectionStateTtl.Should().Be(TimeSpan.FromSeconds(1)); - aliveAt1.Value.Should().BeBefore(aliveAt2.Value); + newConnectionId.Should().Be(initialConnectionId); await channels[0].WaitForAttachedState(); await channels[1].WaitForAttachedState(); diff --git a/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionFailureSpecs.cs b/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionFailureSpecs.cs index 32794695d..18c74b4fe 100644 --- a/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionFailureSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionFailureSpecs.cs @@ -183,13 +183,10 @@ public async Task WhenTransportFails_ShouldTransitionToDisconnectedAndEmitErrorW await client.WaitForState(ConnectionState.Connecting); - client.Connection.On((args) => + ConnectionStateChange firstChange = null; + client.Connection.Once(ConnectionEvent.Disconnected, args => { - args.Current.Should().Be(ConnectionState.Disconnected); - args.Previous.Should().Be(ConnectionState.Connecting); - args.Event.Should().Be(ConnectionEvent.Disconnected); - args.RetryIn.Should().Be(options.DisconnectedRetryTimeout); - args.Reason.Should().NotBeNull(); + firstChange = args; Done(); }); @@ -199,6 +196,17 @@ public async Task WhenTransportFails_ShouldTransitionToDisconnectedAndEmitErrorW LastCreatedTransport.Listener.OnTransportEvent(LastCreatedTransport.Id, TransportState.Closing, new Exception()); WaitOne(); + + firstChange.Should().NotBeNull(); + firstChange.Previous.Should().Be(ConnectionState.Connecting); + firstChange.Event.Should().Be(ConnectionEvent.Disconnected); + firstChange.Reason.Should().NotBeNull(); + + // RTN15a - a transport disconnected unexpectedly is treated as a non-token DISCONNECTED + // per RTN15h3, which reconnects immediately. So RTN14d's "time until the next connection + // attempt" is zero here, not the disconnectedRetryTimeout. The backoff reporting itself + // is covered deterministically by DisconnectedStateSpecs. + firstChange.RetryIn.Should().Be(TimeSpan.Zero); } [Fact(Skip = "Requires a SandBox Spec")] diff --git a/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionFailuresOnceConnectedSpecs.cs b/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionFailuresOnceConnectedSpecs.cs index e0da14480..c93e32e72 100644 --- a/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionFailuresOnceConnectedSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionFailuresOnceConnectedSpecs.cs @@ -4,6 +4,7 @@ using System.Diagnostics; using System.Linq; using System.Net; +using System.Threading; using System.Threading.Tasks; using IO.Ably.Realtime; @@ -214,6 +215,44 @@ public async Task WhenTransportCloses_ShouldResumeConnection() LastCreatedTransport.Should().NotBeSameAs(firstTransport); } + // UTS: realtime/unit/RTN15b/successful-resume-0 + [Fact] + [Trait("spec", "RTN15b")] + [Trait("spec", "RTN15c6")] + public async Task WhenTheTransportDropsAndTheResumeSucceeds_ShouldKeepTheConnectionId() + { + // RTN15b - the reconnect carries the connectionKey in the resume query param. RTN15c6 - + // the server signals a successful resume by answering with the same connectionId, and + // may hand back a refreshed connectionKey with it. + var client = await SetupConnectedClient(); + + var connectionId = client.Connection.Id; + var connectionKey = client.Connection.Key; + connectionId.Should().NotBeNullOrEmpty(); + connectionKey.Should().NotBeNullOrEmpty(); + + // An unexpected transport drop, so the next attempt is a resume rather than a fresh + // connection. + LastCreatedTransport.Listener.OnTransportEvent(LastCreatedTransport.Id, TransportState.Closed); + await client.WaitForState(ConnectionState.Connecting); + await client.ProcessCommands(); + + LastCreatedTransport.Parameters.GetParams() + .Should().ContainKey("resume") + .WhoseValue.Should().Be(connectionKey); + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Connected) + { + ConnectionId = connectionId, + ConnectionDetails = new ConnectionDetails { ConnectionKey = "connectionKey-updated" }, + }); + await client.WaitForState(ConnectionState.Connected); + await client.ProcessCommands(); + + client.Connection.Id.Should().Be(connectionId); + client.Connection.Key.Should().Be("connectionKey-updated"); + } + [Fact] [Trait("spec", "RTN15a")] public async Task AckMessagesAreSentWhenConnectionIsDroppedAndNotResumed() @@ -254,6 +293,192 @@ public async Task AckMessagesAreResentWhenConnectionIsDroppedAndResumed() client.State.WaitingForAck.Should().HaveCount(2); } + [Fact] + [Trait("spec", "RTN15h2")] + [Trait("spec", "RTN15h3")] + public async Task WithTokenError_ShouldNotAlsoGrantTheNonTokenImmediateReconnect() + { + // RTN15h3's immediate reconnect is for "an error other than a token error". RTN15h2 owns + // token errors and reconnects of its own accord, so granting a retry here as well gives + // two overlapping attempts - and the second reaches FAILED where RTN15h2 requires + // DISCONNECTED. + var client = await SetupConnectedClient(ConnectedClientErrors.FailRenewal); + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Disconnected) + { + Error = _tokenErrorInfo, + }); + + await client.ProcessCommands(); + + client.State.AttemptsInfo.InstantRetryCount.Should().Be(0); + client.Connection.State.Should().Be(ConnectionState.Disconnected); + } + + // UTS: realtime/unit/RTN15h3/non-token-error-resume-0 + [Fact] + [Trait("spec", "RTN15h3")] + public async Task WithNonTokenDisconnected_ShouldReconnectImmediately() + { + var client = await GetConnectedClient(opts => + opts.DisconnectedRetryTimeout = TimeSpan.FromMinutes(10)); + + var originalId = client.Connection.Id; + var connectionKey = client.Connection.Key; + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Disconnected) + { + Error = new ErrorInfo("Something else went wrong", 50000), + }); + + await client.ProcessCommands(); + + // Immediately, not in ten minutes. + client.State.AttemptsInfo.InstantRetryCount.Should().Be(1); + client.Connection.State.Should().Be(ConnectionState.Connecting); + + // RTN15h3 asks for a reconnect *with a resume attempt*, so follow it through: the new + // attempt carries the key, and a CONNECTED bearing the same id keeps the connection. + LastCreatedTransport.Parameters.GetParams() + .Should().ContainKey("resume") + .WhoseValue.Should().Be(connectionKey); + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Connected) + { + ConnectionId = originalId, + ConnectionDetails = new ConnectionDetails { ConnectionKey = connectionKey }, + }); + await client.WaitForState(ConnectionState.Connected); + await client.ProcessCommands(); + + client.Connection.Id.Should().Be(originalId); + } + + [Fact] + [Trait("spec", "RTN15h3")] + [Trait("spec", "RTN17j")] + public async Task WhenAConnectionSucceeds_ShouldClearTheImmediateRetryBudget() + { + // The budget that bounds RTN17j's traversal is per failure run, cleared by + // UpdateAttemptState's Connected case. Without that a client which spent its retries once + // would never get an immediate reconnect again, leaving RTN15h3 unimplemented from the + // second disconnect onwards. + var client = await GetConnectedClient(opts => + opts.DisconnectedRetryTimeout = TimeSpan.FromMinutes(10)); + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Disconnected) + { + Error = new ErrorInfo("Something else went wrong", 50000), + }); + await client.ProcessCommands(); + + client.State.AttemptsInfo.InstantRetryCount.Should().Be(1); + + client.FakeProtocolMessageReceived(ConnectedProtocolMessage); + await client.WaitForState(ConnectionState.Connected); + await client.ProcessCommands(); + + client.State.AttemptsInfo.InstantRetryCount.Should().Be(0); + } + + [Fact] + [Trait("spec", "RSA4c")] + public async Task WithASynchronouslyBlockingAuthCallback_ShouldStillBoundTheAttempt() + { + // RSA4c - TimeoutAfter extends an already-created Task, so the callback has to be invoked + // through Task.Run to be bounded at all. A callback whose body runs synchronously - the + // most ordinary C# shape - would otherwise block before there is anything to bound and + // hold the workflow's single reader thread for as long as it takes. + var released = new ManualResetEventSlim(false); + var client = GetClientWithFakeTransport(opts => + { + opts.Key = ValidKey; + opts.RealtimeRequestTimeout = TimeSpan.FromMilliseconds(200); + opts.AuthCallback = _ => + { + released.Wait(TimeSpan.FromSeconds(30)); + return Task.FromResult(new TokenDetails("blocked")); + }; + }); + + try + { + var sw = Stopwatch.StartNew(); + var error = await Assert.ThrowsAsync(() => client.Auth.AuthorizeAsync()); + sw.Stop(); + + // The specific failure, not merely any failure - a bare IsFaulted check would also + // have been satisfied by an unrelated fast fault. + error.ErrorInfo.Code.Should().Be(ErrorCodes.ClientAuthProviderRequestFailed); + error.ErrorInfo.Cause.Should().NotBeNull(); + error.ErrorInfo.Cause.Code.Should().Be(ErrorCodes.ClientCallbackError); + + // Comfortably inside the 30s the callback blocks for, and comfortably outside the + // 200ms bound so a loaded run does not flake. + sw.Elapsed.Should().BeLessThan(TimeSpan.FromSeconds(5)); + } + finally + { + released.Set(); + } + } + + [Fact] + [Trait("spec", "RSA4c1")] + public async Task WhenTheAuthCallbackFails_ShouldSetTheCauseNotOnlyTheInnerException() + { + // RSA4c1 wants an ErrorInfo "with code 80019, statusCode 401, and cause set to the + // underlying cause". The four argument overload assigns InnerException instead, which is + // not the spec's field and is not serialised as one. + var client = GetClientWithFakeTransport(opts => + { + opts.Key = ValidKey; + opts.AuthCallback = _ => throw new AblyException(new ErrorInfo("the underlying cause", 40100)); + }); + + var error = await Assert.ThrowsAsync(() => client.Auth.AuthorizeAsync()); + + error.ErrorInfo.Code.Should().Be(ErrorCodes.ClientAuthProviderRequestFailed); + error.ErrorInfo.StatusCode.Should().Be(HttpStatusCode.Unauthorized); + error.ErrorInfo.Cause.Should().NotBeNull(); + error.ErrorInfo.Cause.Message.Should().Be("the underlying cause"); + } + + [Theory] + [InlineData(0)] + [InlineData(-1)] + [Trait("spec", "TO3l11")] + public void WithANonPositiveRealtimeRequestTimeout_ShouldReject(int seconds) + { + var options = new ClientOptions(ValidKey); + + Assert.Throws( + () => options.RealtimeRequestTimeout = TimeSpan.FromSeconds(seconds)); + } + + [Fact] + [Trait("spec", "TO3l11")] + public void WithARealtimeRequestTimeoutTooLargeForATimer_ShouldReject() + { + // TimeSpan.MaxValue is the idiomatic "never time out", and it reaches Task.Delay through + // TimeoutAfter, which rejects anything over uint.MaxValue - 1 ms - surfacing as a code-0 + // error out of Authorize(). Rejected up front instead. + var options = new ClientOptions(ValidKey); + + Assert.Throws( + () => options.RealtimeRequestTimeout = TimeSpan.MaxValue); + + // uint.MaxValue - 1 is Task.Delay's limit on .NET 6+ only: it is above what net46, Mono + // and Xamarin accept, and above what CountdownTimer's cast to int can carry into + // System.Threading.Timer on any framework. + Assert.Throws( + () => options.RealtimeRequestTimeout = TimeSpan.FromMilliseconds(uint.MaxValue - 1)); + + // Just inside the limit is still allowed. + options.RealtimeRequestTimeout = TimeSpan.FromMilliseconds(int.MaxValue); + options.RealtimeRequestTimeout.TotalMilliseconds.Should().Be(int.MaxValue); + } + [Flags] private enum ConnectedClientErrors { diff --git a/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionFallbackSpecs.cs b/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionFallbackSpecs.cs index 3100d12f7..a20cb0ed3 100644 --- a/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionFallbackSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionFallbackSpecs.cs @@ -110,6 +110,117 @@ public async Task WhenPreviousAttemptFailed_ShouldGoToDefaultHostFirst() realtimeHosts.Last().Should().Be(Defaults.RealtimeHost); } + [Fact] + [Trait("spec", "RTN17")] + [Trait("spec", "RTN17j")] + public async Task WhenTheImmediateRetriesAreSpent_ShouldStillReachAFallbackHost() + { + // RTN17 - every attempt considers a fallback, including the timer driven ones. Skipping + // them would lock a client out entirely once the immediate retry budget is spent, since + // every remaining attempt is timer driven and would be pinned to the primary. + var client = await GetConnectedClient(opts => opts.DisconnectedRetryTimeout = TimeSpan.FromMilliseconds(10)); + + var hostsTried = new List(); + FakeTransportFactory.InitialiseFakeTransport = t => hostsTried.Add(t.Parameters.Host); + + // Spend the immediate retry budget and then some, so the later attempts are all + // timer driven. + var domainCount = 1 + client.State.Connection.FallbackHosts.Count; + for (var i = 0; i < domainCount + 3; i++) + { + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Disconnected) + { + Error = new ErrorInfo { StatusCode = HttpStatusCode.GatewayTimeout }, + }); + + await client.ProcessCommands(); + + if (client.Connection.State != ConnectionState.Connecting) + { + await Task.Delay(50); + } + } + + client.State.AttemptsInfo.InstantRetryCount.Should().Be(domainCount); + hostsTried.Should().Contain(x => client.State.Connection.FallbackHosts.Contains(x)); + } + + [Fact] + [Trait("spec", "RTN17j")] + public async Task WhenAnImmediateRetryIsGranted_ShouldCheckConnectivityOnceForTheCycle() + { + // RTN17j asks for a connectivity check before an alternative host is used. Two decisions + // in the same cycle need the answer - whether to retry now, and whether to accept a + // fallback - and asking twice holds the workflow's single reader thread for up to two + // MaxHttpOpenTimeouts on every failing attempt. + var response = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(Defaults.InternetCheckOkMessage), + }; + + var handler = new FakeHttpMessageHandler(response); + var client = GetClientWithFakeTransportAndMessageHandler(messageHandler: handler); + client.Options.SkipInternetCheck = false; + + await client.ConnectClient(); + await client.ProcessCommands(); + + handler.Requests.Clear(); + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Disconnected) + { + Error = new ErrorInfo { StatusCode = HttpStatusCode.GatewayTimeout }, + }); + + await client.ProcessCommands(); + await client.WaitForState(ConnectionState.Connecting); + await client.ProcessCommands(); + + var checks = handler.Requests + .Count(x => x.RequestUri.ToString().EqualsTo(Defaults.InternetCheckUrl)); + + checks.Should().Be(1); + } + + [Fact] + [Trait("spec", "RTN17j")] + public async Task WhenAConnectingCommandIsAbandoned_ShouldNotLeaveAnAnswerBehindForALaterAttempt() + { + // The answer is carried on the command so it cannot outlive the decision it was taken + // for. Held on the workflow it would have no bound: the command loop abandons a nested + // batch once its depth guard trips, so a dropped CONNECTING would leave the answer set + // for a later attempt to consume without any check of its own - which is exactly what + // RTN17j requires before using an alternative host. + var response = new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent(Defaults.InternetCheckOkMessage), + }; + + var handler = new FakeHttpMessageHandler(response); + var client = GetClientWithFakeTransportAndMessageHandler(messageHandler: handler); + client.Options.SkipInternetCheck = false; + + await client.ConnectClient(); + await client.ProcessCommands(); + + // Grant an immediate retry, then abandon the CONNECTING it produced by consuming it + // directly rather than letting the loop deliver it. + var next = await client.Workflow.ProcessCommand(SetDisconnectedStateCommand.Create( + new ErrorInfo { StatusCode = HttpStatusCode.GatewayTimeout })); + + next.Should().ContainSingle().Which.Should().BeOfType(); + + handler.Requests.Clear(); + + // A fresh attempt that carries no answer of its own must take its own check. + await client.Workflow.ProcessCommand(SetConnectingStateCommand.Create()); + + var checks = handler.Requests + .Count(x => x.RequestUri.ToString().EqualsTo(Defaults.InternetCheckUrl)); + + checks.Should().Be(1); + } + [Fact] [Trait("spec", "RTN17e")] public async Task WithFallbackHost_ShouldMakeRestRequestsOnSameHost() diff --git a/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionParameterSpecs.cs b/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionParameterSpecs.cs index 740708b2a..3fef85a4e 100644 --- a/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionParameterSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionParameterSpecs.cs @@ -52,6 +52,37 @@ public async Task WithEchoInClientOptions_ShouldSetTransportEchoCorrectly(bool e .WhoseValue.Should().Be(echo.ToString().ToLower()); } + // UTS: realtime/unit/RTN23a/heartbeats-true-query-param-0 + [Fact] + [Trait("spec", "RTN23b")] + public async Task ShouldRequestProtocolHeartbeats() + { + // RTN23b - ClientWebSocket cannot observe an incoming websocket ping frame, so protocol + // messages are the only activity RTN23a can measure. + _ = await GetConnectedClient(); + + LastCreatedTransport.Parameters.GetParams() + .Should().ContainKey("heartbeats") + .WhoseValue.Should().Be("true"); + } + + [Fact] + [Trait("spec", "RTN23b")] + [Trait("spec", "RTC1f1")] + public async Task WithHeartbeatsInTransportParams_ShouldLetTheCallerOverrideIt() + { + // RTN23b makes the param optional, so a caller can still turn it off. Pinned because it + // works by virtue of how AdditionalParameters are merged rather than anything explicit. + _ = await GetConnectedClient(options => options.TransportParams = new Dictionary + { + { "heartbeats", false }, + }); + + LastCreatedTransport.Parameters.GetParams() + .Should().ContainKey("heartbeats") + .WhoseValue.Should().Be("false"); + } + [Fact] [Trait("spec", "RTN2d")] public async Task WithClientId_ShouldSetTransportClientIdCorrectly() diff --git a/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionRecoverySpecs.cs b/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionRecoverySpecs.cs index 25d400647..bc4d7027a 100644 --- a/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionRecoverySpecs.cs +++ b/src/IO.Ably.Tests.Shared/Realtime/ConnectionSpecs/ConnectionRecoverySpecs.cs @@ -34,7 +34,7 @@ public async Task CreateRecoveryKey_ShouldReturnSerializedConnectionKeyAndMsgSer } [Fact] - [Trait("spec", "RTN16g2")] + [Trait("spec", "RTN16g3")] public async Task CreateRecoveryKey_ShouldReturnNullRecoveryKeyForNullConnectionKeyOrWhenStateIsClosed() { var client = GetClientWithFakeTransport(); @@ -49,6 +49,43 @@ public async Task CreateRecoveryKey_ShouldReturnNullRecoveryKeyForNullConnection client.Connection.CreateRecoveryKey().Should().BeNullOrEmpty(); } + [Fact] + [Trait("spec", "RTN16g3")] + [Trait("spec", "RTN14h")] + public async Task CreateRecoveryKey_ShouldReturnAKeyWhileSuspended() + { + // RTN16g3 lists CLOSED, CLOSING and FAILED, and deliberately not SUSPENDED: RTN8d and + // RTN9d keep the key there because RTN14h always attempts a resume, so the connection + // is still recoverable and the key can be handed to another client. + var client = GetClientWithFakeTransport(); + client.FakeProtocolMessageReceived(ConnectedProtocolMessage); + await client.WaitForState(ConnectionState.Connected); + + // An attached channel carrying a channelSerial, because RTN16i makes those the point of + // the key. Without one the assertion below holds even if they are dropped. + var channel = (RealtimeChannel)client.Channels.Get("test"); + channel.Attach(); + await client.ProcessCommands(); + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Attached) + { + Channel = "test", + ChannelSerial = "serial-abc", + }); + await client.ProcessCommands(); + + var connectedKey = client.Connection.CreateRecoveryKey(); + connectedKey.Should().Contain("serial-abc"); + + client.Workflow.QueueCommand(SetSuspendedStateCommand.Create(ErrorInfo.ReasonSuspended)); + await client.WaitForState(ConnectionState.Suspended); + await client.ProcessCommands(); + + // RTL15b2 keeps the serial through SUSPENDED, so the key is unchanged - connectionKey, + // msgSerial and channelSerials all still present. + channel.State.Should().Be(ChannelState.Suspended); + client.Connection.CreateRecoveryKey().Should().Be(connectedKey); + } + [Fact] [Trait("spec", "RTN16m")] [System.Obsolete] diff --git a/src/IO.Ably.Tests.Shared/Realtime/ConnectionStateTests/ClosingStateSpecs.cs b/src/IO.Ably.Tests.Shared/Realtime/ConnectionStateTests/ClosingStateSpecs.cs index 1ca7d1139..85c82b985 100644 --- a/src/IO.Ably.Tests.Shared/Realtime/ConnectionStateTests/ClosingStateSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Realtime/ConnectionStateTests/ClosingStateSpecs.cs @@ -1,3 +1,4 @@ +using System; using System.Threading.Tasks; using FluentAssertions; using IO.Ably.Realtime; @@ -96,6 +97,23 @@ public async Task ShouldHandleInboundDisconnectedMessageAndGoToDisconnectedState _context.ShouldQueueCommand(); } + [Fact] + [Trait("spec", "RTN12b")] + [Trait("spec", "TO3l11")] + public void StartTimer_ShouldWaitRealtimeRequestTimeout() + { + // RTN12b names the duration: "If the CLOSED ProtocolMessage is not received within + // realtimeRequestTimeout, the transport will be disconnected and the connection will + // automatically transition to the CLOSED state". The test below covers what happens when + // the timer fires; this pins how long it waits. + _context.DefaultTimeout = TimeSpan.FromMilliseconds(1234); + + var state = GetState(connectedTransport: true); + state.StartTimer(); + + _timer.LastDelay.Should().Be(TimeSpan.FromMilliseconds(1234)); + } + [Fact] [Trait("spec", "RTN12b")] public async Task AfterTimeoutExpires_ShouldForceStateToClosed() diff --git a/src/IO.Ably.Tests.Shared/Realtime/ConnectionStateTests/DisconnectedStateSpecs.cs b/src/IO.Ably.Tests.Shared/Realtime/ConnectionStateTests/DisconnectedStateSpecs.cs index 5bd5e8ae9..6de28f028 100644 --- a/src/IO.Ably.Tests.Shared/Realtime/ConnectionStateTests/DisconnectedStateSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Realtime/ConnectionStateTests/DisconnectedStateSpecs.cs @@ -1,3 +1,4 @@ +using System; using System.Threading.Tasks; using FluentAssertions; using IO.Ably.Realtime; @@ -105,6 +106,45 @@ public async Task AfterAnInterval_ShouldRetryConnection() _context.ShouldQueueCommand(); } + [Fact] + [Trait("spec", "RTN14d")] + public void StartTimer_ShouldReportTheDelayItActuallyWaits() + { + // RTN14d - retryIn is "the time in milliseconds until the next connection attempt", so + // the value handed to the application is the one the timer was started with, RTB1 backoff + // and jitter included. + _state.StartTimer(); + + _timer.LastDelay.Should().BeGreaterThan(TimeSpan.Zero); + _state.RetryIn.Should().Be(_timer.LastDelay); + } + + [Fact] + [Trait("spec", "RTN14d")] + [Trait("spec", "RTB1")] + public void StartTimer_ShouldApplyTheBackoffAndJitter() + { + _state.StartTimer(); + + // RTB1a's coefficient is 1 for the first retry and RTB1b's jitter is 0.8 to 1.0. + var nominal = _context.RetryTimeout; + _timer.LastDelay.Should().BeGreaterOrEqualTo(TimeSpan.FromMilliseconds(nominal.TotalMilliseconds * 0.8)); + _timer.LastDelay.Should().BeLessOrEqualTo(nominal); + } + + [Fact] + [Trait("spec", "RTN14d")] + public void WhenRetryingInstantly_ShouldReportNoWaitAndNotStartTheTimer() + { + var state = GetState(); + state.RetryInstantly = true; + + state.StartTimer(); + + state.RetryIn.Should().Be(TimeSpan.Zero); + _timer.StartedWithAction.Should().BeFalse(); + } + private ConnectionDisconnectedState GetState(ErrorInfo error = null) { return new ConnectionDisconnectedState(_context, error, _timer, Logger); diff --git a/src/IO.Ably.Tests.Shared/Realtime/ProtocolMessageTests.cs b/src/IO.Ably.Tests.Shared/Realtime/ProtocolMessageTests.cs index d78469ad5..8c7d5f018 100644 --- a/src/IO.Ably.Tests.Shared/Realtime/ProtocolMessageTests.cs +++ b/src/IO.Ably.Tests.Shared/Realtime/ProtocolMessageTests.cs @@ -11,6 +11,44 @@ namespace IO.Ably.Tests.Shared.Realtime { public class ProtocolMessageTests { + [Fact] + [Trait("spec", "CD2h")] + public void ConnectionDetails_ShouldDeserialiseMaxIdleIntervalAsMilliseconds() + { + // Pinned against a literal payload rather than a round-trip, which would not catch the + // unit being wrong. 15000 on the wire is 15 seconds. + const string connected = @"{ + ""action"": 4, + ""connectionId"": ""abc"", + ""connectionDetails"": { + ""connectionKey"": ""key"", + ""connectionStateTtl"": 120000, + ""maxIdleInterval"": 15000 + } + }"; + + var message = JsonHelper.Deserialize(connected); + + message.ConnectionDetails.MaxIdleInterval.Should().Be(TimeSpan.FromSeconds(15)); + message.ConnectionDetails.ConnectionStateTtl.Should().Be(TimeSpan.FromSeconds(120)); + } + + [Fact] + [Trait("spec", "CD2h")] + public void ConnectionDetails_WithoutMaxIdleInterval_ShouldLeaveItNull() + { + // Absent is meaningfully different from zero, so it must not deserialise to zero. + const string connected = @"{ + ""action"": 4, + ""connectionId"": ""abc"", + ""connectionDetails"": { ""connectionKey"": ""key"" } + }"; + + var message = JsonHelper.Deserialize(connected); + + message.ConnectionDetails.MaxIdleInterval.Should().BeNull(); + } + [Fact] [Trait("spec", "TR3")] public void ProtocolMessageFlagHaveCorrectValues() diff --git a/src/IO.Ably.Tests.Shared/Realtime/RealtimeWorkflowSpecs.cs b/src/IO.Ably.Tests.Shared/Realtime/RealtimeWorkflowSpecs.cs index 4d07db7e8..b015cf91c 100644 --- a/src/IO.Ably.Tests.Shared/Realtime/RealtimeWorkflowSpecs.cs +++ b/src/IO.Ably.Tests.Shared/Realtime/RealtimeWorkflowSpecs.cs @@ -1,10 +1,13 @@ using System; using System.Collections.Generic; +using System.Linq; +using System.Net; using System.Threading.Tasks; using IO.Ably.Realtime; using IO.Ably.Realtime.Workflow; using IO.Ably.Tests.Infrastructure; +using IO.Ably.Tests.Realtime; using IO.Ably.Transport; using IO.Ably.Types; @@ -70,15 +73,29 @@ public class ConnectingStateSpecs : AblyRealtimeSpecs [Trait("spec", "RTN14g")] public async Task WithInboundErrorMessage_WhenNotTokenErrorAndChannelsEmpty_GoesToFailed() { - var client = GetRealtimeClient(opts => opts.RealtimeHost = "non-default.ably.io"); // Force no fallback + // A custom host means no fallbacks. The transport is faked and held short of + // Connected, and realtimeRequestTimeout put out of reach, so neither a real DNS + // failure nor the CONNECTING timeout can reach Disconnected before the injected + // error does. + FakeTransportFactory.InitialiseFakeTransport = + transport => transport.OnConnectChangeStateToConnected = false; + + var client = GetClientWithFakeTransport(opts => + { + opts.RealtimeHost = "non-default.ably.io"; + opts.RealtimeRequestTimeout = TimeSpan.FromMinutes(1); + }); await client.WaitForState(ConnectionState.Connecting); // Arrange ErrorInfo targetError = new ErrorInfo("test", 123); - // Act - client.Workflow.ProcessCommand(ProcessMessageCommand.Create(new ProtocolMessage(ProtocolMessage.MessageAction.Error) { Error = targetError })); + // Queued and drained rather than fired and forgotten: an un-awaited ProcessCommand + // can have its continuation delayed past the wait below under a parallel suite. + client.ExecuteCommand(ProcessMessageCommand.Create( + new ProtocolMessage(ProtocolMessage.MessageAction.Error) { Error = targetError })); + await client.ProcessCommands(); // Assert await client.WaitForState(ConnectionState.Failed); @@ -92,27 +109,112 @@ public ConnectingStateSpecs(ITestOutputHelper output) public class ConnectingCommandSpecs : AblyRealtimeSpecs { + // UTS: realtime/unit/RTN14h/resume-after-ttl-0 [Fact] - public async Task WithInboundErrorMessageWhenItCanUseFallBack_ShouldClearsConnectionKey() + [Trait("spec", "RTN14h")] + [Trait("spec", "RTN15b1")] + public async Task AfterSuspendedAndAFailedAttempt_EveryReconnectionShouldCarryResume() { - // Arrange - var client = GetRealtimeClient(options => + // RTN14h: "Reconnection attempts in this state should continue to attempt to + // resume, regardless of how long it has been since the client was last connected." + var client = await GetConnectedClient(); + var key = client.State.Connection.Key; + var id = client.State.Connection.Id; + key.Should().NotBeNullOrEmpty(); + + client.ExecuteCommand(SetSuspendedStateCommand.Create(ErrorInfo.ReasonSuspended)); + await client.ProcessCommands(); + + // RTN8d, RTN9d - retained, because SUSPENDED is not one of the terminal states. + client.State.Connection.Key.Should().Be(key); + client.State.Connection.Id.Should().Be(id); + + client.ExecuteCommand(SetConnectingStateCommand.Create()); + await client.ProcessCommands(); + + // A second and third attempt, because the clause is "reconnection attempts" plural - + // one attempt carrying a resume does not show that every later one does. + for (var i = 0; i < 2; i++) + { + client.ExecuteCommand(HandleConnectingErrorCommand.Create( + new ErrorInfo("boom", 50000, System.Net.HttpStatusCode.InternalServerError))); + await client.ProcessCommands(); + client.ExecuteCommand(SetConnectingStateCommand.Create()); + await client.ProcessCommands(); + } + + var reconnects = CreatedTransports.Skip(1).ToList(); + reconnects.Should().HaveCountGreaterOrEqualTo(3); + foreach (var transport in reconnects) + { + transport.Parameters.GetParams() + .Should().Contain(new KeyValuePair("resume", key)); // RTN15b1 + } + } + + [Fact] + [Trait("spec", "RTN14h")] + [Trait("spec", "RTN15b1")] + public async Task AfterAFailedAttempt_TheNextReconnectionShouldStillCarryResume() + { + // RTN14h's "continue to attempt to resume" applies from the second attempt onwards, + // not just after a suspend. + // + // Deliberately never passes through SUSPENDED, and asserts as much: once AttemptsInfo + // has recorded a suspend, ShouldSuspend returns true and the DISCONNECTED handler + // diverts straight back to SUSPENDED without reaching the path under test. + var client = await GetConnectedClient(opts => + opts.DisconnectedRetryTimeout = TimeSpan.FromMinutes(10)); + + var key = client.State.Connection.Key; + key.Should().NotBeNullOrEmpty(); + + client.ExecuteCommand(HandleConnectingErrorCommand.Create( + new ErrorInfo("boom", 50000, System.Net.HttpStatusCode.InternalServerError))); + await client.ProcessCommands(); + + client.Connection.State.Should().NotBe(ConnectionState.Suspended); + client.State.Connection.Key.Should().Be(key); + + // A 500 earns the RTN15h3 instant retry, so the next transport is already built by + // the time the commands settle, and must carry the resume. + LastCreatedTransport.Parameters.GetParams() + .Should().Contain(new KeyValuePair("resume", key)); + } + + [Fact] + [Trait("spec", "RTN14h")] + public async Task WithInboundErrorMessageWhenItCanUseFallBack_ShouldKeepConnectionKey() + { + // RTN14h - a retryable inbound ERROR must not cost the key, because the reconnection + // attempt has to continue to attempt to resume. The client is driven through + // CONNECTED first so there is a real key at stake. + var client = await GetConnectedClient(options => { options.RealtimeRequestTimeout = TimeSpan.FromSeconds(60); options.DisconnectedRetryTimeout = TimeSpan.FromSeconds(60); }); - await client.WaitForState(ConnectionState.Connecting); + var key = client.State.Connection.Key; + key.Should().NotBeNullOrEmpty(); + + // Back to CONNECTING, where a retryable inbound ERROR is a failed attempt rather + // than RTN15j's fatal connection error. + client.ExecuteCommand(SetDisconnectedStateCommand.Create(ErrorInfo.ReasonDisconnected)); + await client.ProcessCommands(); + client.ExecuteCommand(SetConnectingStateCommand.Create()); + await client.ProcessCommands(); + + client.State.Connection.Key.Should().Be(key); var messageWithError = new ProtocolMessage(ProtocolMessage.MessageAction.Error) { Error = new ErrorInfo("test", 123, System.Net.HttpStatusCode.InternalServerError), }; - // Act await client.ProcessMessage(messageWithError); - client.State.Connection.Key.Should().BeEmpty(); + client.State.Connection.Key.Should().Be(key); } [Fact] @@ -148,6 +250,45 @@ public async Task WhenDisconnectedWithFallback_ShouldRetryConnectionImmediately( states.Should().BeEquivalentTo(new[] { ConnectionState.Disconnected, ConnectionState.Connecting }); } + [Fact] + [Trait("spec", "RTN17j")] + [Trait("spec", "RTN14d")] + public async Task InstantRetries_ShouldBeBoundedByTheNumberOfDomains() + { + // RTN17j sanctions reconnecting immediately to work through the fallback domains, but + // the traversal is bounded - every failure carries an exception and so qualifies + // again, which would mean RTB1 is never reached. + var client = GetClientWithFakeTransport(opts => opts.DisconnectedRetryTimeout = TimeSpan.FromMinutes(10)); + await client.WaitForState(ConnectionState.Connecting); + await client.ProcessCommands(); + + var domainCount = 1 + client.State.Connection.FallbackHosts.Count; + domainCount.Should().BeGreaterThan(1); + + var connectingCount = 0; + client.Connection.On(x => + { + if (x.Current == ConnectionState.Connecting) + { + connectingCount++; + } + }); + + // Each failure carries an exception, so each one qualifies for an instant retry. + for (var i = 0; i < domainCount + 3; i++) + { + client.ExecuteCommand(SetDisconnectedStateCommand.Create( + ErrorInfo.ReasonDisconnected, exception: new Exception("transport gone"))); + await client.ProcessCommands(); + } + + // One per domain. The long retry timeout means nothing else can have produced a + // CONNECTING, so anything past the bound went to the timer instead. + connectingCount.Should().Be(domainCount); + client.State.AttemptsInfo.InstantRetryCount.Should().Be(domainCount); + client.Connection.State.Should().Be(ConnectionState.Disconnected); + } + [Fact] public async Task ShouldCreateTransport() { @@ -570,12 +711,1276 @@ void Callback(bool ack, ErrorInfo err) Assert.True(callbacks.TrueForAll(c => ReferenceEquals(c.Item2, error))); // Error } + [Fact] + [Trait("spec", "RTN7b")] + [Trait("spec", "RTN19a")] + public async Task WhenAnAckedMessageFailsToSend_ShouldHoldItInOneQueueOnly() + { + // A message can be awaiting an ACK or queued for a later connection, never both. + // RTN19a resends WaitingForAck on reconnect, so a message in both queues is sent + // twice and the second msgSerial assignment renumbers the copy WaitingForAck holds, + // leaving a hole in the sequence Ably is tracking. + // + // No caller produces this shape today - AckRequired implies CanSend and so + // CONNECTED, where CanQueue is false - so the invariant is asserted directly. + var client = GetClientWithFakeTransport(); + await client.WaitForState(ConnectionState.Connecting); + await client.ProcessCommands(); + + // CONNECTING can queue, so forcing an ack-requiring message there reaches both + // branches. A throwing write is what SendToTransport turns into a failure. + LastCreatedTransport.SetSendAction(_ => throw new Exception("socket gone")); + + var message = new ProtocolMessage(ProtocolMessage.MessageAction.Message, "test"); + client.ExecuteCommand(SendMessageCommand.Create(message, (_, __) => { }, force: true)); + + await client.ProcessCommands(); + + client.State.WaitingForAck.Should().HaveCount(1); + client.State.PendingMessages.Should().BeEmpty(); + } + + [Fact] + [Trait("spec", "RTN7b")] + [Trait("spec", "RTN19a")] + public async Task WhenANonAckedMessageFailsToSend_ShouldStillQueueItForTheNextConnection() + { + // The other half: a message not awaiting an ACK is tracked nowhere else, so dropping + // it here would lose it silently. CLOSE is what the two force:true callers send. + var client = GetClientWithFakeTransport(); + await client.WaitForState(ConnectionState.Connecting); + await client.ProcessCommands(); + + LastCreatedTransport.SetSendAction(_ => throw new Exception("socket gone")); + + var message = new ProtocolMessage(ProtocolMessage.MessageAction.Close); + client.ExecuteCommand(SendMessageCommand.Create(message, (_, __) => { }, force: true)); + + await client.ProcessCommands(); + + client.State.WaitingForAck.Should().BeEmpty(); + client.State.PendingMessages.Should().HaveCount(1); + } + + // UTS: realtime/unit/RTN7e/error-represents-reason-4 + [Theory] + [InlineData(ConnectionState.Failed)] + [InlineData(ConnectionState.Suspended)] + [InlineData(ConnectionState.Closed)] + [Trait("spec", "RTN7e")] + public async Task WhenTheConnectionFailsAMessage_ShouldReportTheReasonForTheStateChange( + ConnectionState state) + { + // RTN7e: the callback "should be called with an error representing the reason for + // the state change" - this transition's reason, not a module level constant. + var client = await GetConnectedClient(); + var reason = new ErrorInfo("the actual reason", 12345); + + var errors = new List(); + client.ExecuteCommand(SendMessageCommand.Create( + new ProtocolMessage(ProtocolMessage.MessageAction.Message, "test"), + (_, error) => errors.Add(error))); + await client.ProcessCommands(); + + RealtimeCommand command = state switch + { + ConnectionState.Failed => SetFailedStateCommand.Create(reason), + ConnectionState.Suspended => SetSuspendedStateCommand.Create(reason), + _ => SetClosedStateCommand.Create(reason), + }; + + client.ExecuteCommand(command); + await client.ProcessCommands(); + + errors.Should().ContainSingle(); + errors[0].Message.Should().Be("the actual reason"); + errors[0].Code.Should().Be(12345); + + // The point of the clause: the error handed to the publisher is the reason the + // connection itself is reporting, not a separately minted one. + client.Connection.ErrorReason.Should().NotBeNull(); + client.Connection.ErrorReason.Code.Should().Be(errors[0].Code); + } + + [Fact] + [Trait("spec", "RTN7e")] + public async Task WhenTheConnectionFailsAMessage_ShouldHaveAlreadyEnteredTheNewState() + { + // An application inspecting the connection from inside its publish callback sees the + // state it is being told about, not the previous one. ably-js orders + // enactStateChange before failQueuedMessages for the same reason. + var client = await GetConnectedClient(); + + var seen = new List(); + client.ExecuteCommand(SendMessageCommand.Create( + new ProtocolMessage(ProtocolMessage.MessageAction.Message, "test"), + (_, __) => seen.Add(client.Connection.State))); + await client.ProcessCommands(); + + client.ExecuteCommand(SetSuspendedStateCommand.Create(new ErrorInfo("gone", 1))); + await client.ProcessCommands(); + + seen.Should().Equal(new[] { ConnectionState.Suspended }); + } + + [Theory] + [InlineData(ConnectionState.Suspended)] + [Trait("spec", "RTN7e")] + public async Task WhenTheTransitionThrows_ShouldStillFailTheMessage(ConnectionState state) + { + // The connection enters the state before the throw, so RTN7e applies whether or not + // the transition threw - hence the finally. Reachable from application code: + // SuspendedRetryTimeout is public and unvalidated, and a negative value makes + // System.Threading.Timer throw. + // + // SUSPENDED only, because ConnectionSuspendedState.StartTimer is the one state whose + // timer takes a caller-supplied delay. + var client = await GetConnectedClient(opts => + opts.SuspendedRetryTimeout = TimeSpan.FromSeconds(-5)); + + var errors = new List(); + client.ExecuteCommand(SendMessageCommand.Create( + new ProtocolMessage(ProtocolMessage.MessageAction.Message, "test"), + (_, error) => errors.Add(error))); + await client.ProcessCommands(); + client.State.WaitingForAck.Should().HaveCount(1); + + RealtimeCommand command = state switch + { + ConnectionState.Failed => SetFailedStateCommand.Create(new ErrorInfo("gone", 1)), + ConnectionState.Suspended => SetSuspendedStateCommand.Create(new ErrorInfo("gone", 1)), + _ => SetClosedStateCommand.Create(new ErrorInfo("gone", 1)), + }; + + client.ExecuteCommand(command); + await client.ProcessCommands(); + + client.Connection.State.Should().Be(state); + errors.Should().ContainSingle(); + client.State.WaitingForAck.Should().BeEmpty(); + } + + [Fact] + [Trait("spec", "RTN7e")] + public async Task WhenAlreadyInTheState_ShouldNotReportThePreviousReason() + { + // SetState early-returns when already in the target state, before + // Connection.ErrorReason is updated - so the reason is read off the state object + // being entered, which cannot go stale. + var client = await GetConnectedClient(); + + client.ExecuteCommand(SetSuspendedStateCommand.Create(new ErrorInfo("first reason", 1))); + await client.ProcessCommands(); + + var errors = new List(); + client.ExecuteCommand(SendMessageCommand.Create( + new ProtocolMessage(ProtocolMessage.MessageAction.Message, "test"), + (_, error) => errors.Add(error), + force: true)); + await client.ProcessCommands(); + + client.ExecuteCommand(SetSuspendedStateCommand.Create(new ErrorInfo("second reason", 2))); + await client.ProcessCommands(); + + errors.Should().ContainSingle(); + errors[0].Message.Should().Be("second reason"); + } + public AckProtocolTests(ITestOutputHelper output) : base(output) { } } + [Trait("spec", "RTN11d")] + public class ReinitialiseOnConnectSpecs : AblyRealtimeSpecs + { + [Theory] + [InlineData(ConnectionState.Closed)] + [InlineData(ConnectionState.Failed)] + public async Task WhenConnectingFromClosedOrFailed_ShouldReinitialiseEverythingRTN11dNames(ConnectionState from) + { + var client = await GetClientWithHistory(); + var channel = (RealtimeChannel)client.Channels.Get("test"); + + await MoveTo(client, from); + + // Preconditions: there is something to clear on all four counts. + channel.State.Should().NotBe(ChannelState.Initialized); + channel.ErrorReason.Should().NotBeNull(); + client.Connection.ErrorReason.Should().NotBeNull(); + client.State.Connection.MessageSerial.Should().Be(3); + + client.Connect(); + await client.ProcessCommands(); + + channel.State.Should().Be(ChannelState.Initialized); + channel.ErrorReason.Should().BeNull(); + client.Connection.ErrorReason.Should().BeNull(); + client.State.Connection.MessageSerial.Should().Be(0); + } + + [Fact] + public async Task WhenConnectingFromClosed_ShouldResetTheSerialAtConnectTime() + { + // RTN11d asks for the reset at connect(), not at the next CONNECTED. Not framed + // around Connection.RecoveryKey, which returns empty for the whole of that window. + var client = await GetClientWithHistory(); + await MoveTo(client, ConnectionState.Closed); + + client.Connect(); + await client.ProcessCommands(); + + client.State.Connection.MessageSerial.Should().Be(0); + } + + [Fact] + public async Task WhenConnectingFromDisconnected_ShouldLeaveTheSerialAlone() + { + // RTN11d applies only to CLOSED and FAILED. From DISCONNECTED the connection may + // still be resumable, and RTN19a2 needs the sequence intact to resend with it. + var client = await GetClientWithHistory(); + await MoveTo(client, ConnectionState.Disconnected); + + client.Connect(); + await client.ProcessCommands(); + + client.State.Connection.MessageSerial.Should().Be(3); + } + + [Fact] + [Trait("spec", "RTN11b")] + [Trait("spec", "RTL3b")] + public async Task WhenTheConnectionCloses_ShouldDetachChannelsBeforeReinitialisingThem() + { + // RTN11b: "the client should ensure that all channels first transition to DETACHED, + // following RTL3b, and then reinitialize channels per RTN11d". The DETACHED branch + // is the only caller of Presence.ChannelDetachedOrFailed, so skipping it would carry + // presence members from the abandoned connection into the next one. + var client = await GetConnectedClient(); + var channel = (RealtimeChannel)client.Channels.Get("test"); + channel.SetChannelState(ChannelState.Attached); + await client.ProcessCommands(); + + var states = new List(); + channel.On(args => states.Add(args.Current)); + + client.Close(); + await client.WaitForState(ConnectionState.Closing); + await client.ProcessCommands(); + + states.Should().Contain(ChannelState.Detached); + + client.Connect(); + await client.ProcessCommands(); + + channel.State.Should().Be(ChannelState.Initialized); + } + + [Fact] + [Trait("spec", "RTN11b")] + [Trait("spec", "RTL4f")] + public async Task WhenTheConnectionCloses_ShouldNotLetAStaleAwaiterSuspendAReinitialisedChannel() + { + // The awaiters are failed on CLOSING, so an attach still in flight cannot keep its + // timer and later apply RTL4f's SUSPENDED to a channel RTN11d has already reset. + var client = await GetConnectedClient(opts => + opts.RealtimeRequestTimeout = TimeSpan.FromMilliseconds(200)); + + var channel = (RealtimeChannel)client.Channels.Get("test"); + channel.Attach(); + await client.ProcessCommands(); + channel.State.Should().Be(ChannelState.Attaching); + + client.Close(); + await client.WaitForState(ConnectionState.Closing); + await client.ProcessCommands(); + + channel.AttachedAwaiter.Waiting.Should().BeFalse(); + + client.Connect(); + await client.ProcessCommands(); + channel.State.Should().Be(ChannelState.Initialized); + + // Long enough that the original attach timer would have fired. + await Task.Delay(500); + + channel.State.Should().NotBe(ChannelState.Suspended); + } + + [Theory] + [InlineData(ChannelState.Attached)] + [InlineData(ChannelState.Suspended)] + [InlineData(ChannelState.Detaching)] + [Trait("spec", "RTN11b")] + [Trait("spec", "RTP5a")] + public async Task WhenTheConnectionCloses_ShouldClearPresenceForEveryPendingChannel( + ChannelState from) + { + // RTP5a clears the presence maps on entering DETACHED, so any channel left in + // another state would carry its members from the abandoned connection into the next + // one - hence all four pending states, not just the two RTL3b names. + var client = await GetConnectedClient(); + var channel = (RealtimeChannel)client.Channels.Get("test"); + channel.SetChannelState(ChannelState.Attached); + await client.ProcessCommands(); + + channel.Presence.MembersMap.Put( + new PresenceMessage(PresenceAction.Enter, "ghost") { ConnectionId = "old" }); + channel.Presence.InternalMembersMap.Put( + new PresenceMessage(PresenceAction.Enter, "ghost") { ConnectionId = "old" }); + + channel.SetChannelState(from); + await client.ProcessCommands(); + channel.Presence.MembersMap.Values.Should().NotBeEmpty(); + + client.Close(); + await client.WaitForState(ConnectionState.Closing); + await client.ProcessCommands(); + + channel.State.Should().Be(ChannelState.Detached); + channel.Presence.MembersMap.Values.Should().BeEmpty(); + channel.Presence.InternalMembersMap.Values.Should().BeEmpty(); + } + + [Theory] + [InlineData(ChannelState.Attached)] + [InlineData(ChannelState.Suspended)] + [InlineData(ChannelState.Detaching)] + [Trait("spec", "RTP5a")] + public async Task WhenTheConnectionClosesFromSuspended_ShouldStillClearPresence(ChannelState from) + { + // ConnectionSuspendedState.Close() queues SetClosedStateCommand directly, so CLOSING + // never happens and the CLOSED branch has to cover the same RTP5a teardown. + var client = await GetConnectedClient(); + var channel = (RealtimeChannel)client.Channels.Get("test"); + channel.SetChannelState(ChannelState.Attached); + await client.ProcessCommands(); + + channel.Presence.MembersMap.Put( + new PresenceMessage(PresenceAction.Enter, "ghost") { ConnectionId = "old" }); + channel.Presence.InternalMembersMap.Put( + new PresenceMessage(PresenceAction.Enter, "ghost") { ConnectionId = "old" }); + + client.ExecuteCommand(SetSuspendedStateCommand.Create(ErrorInfo.ReasonSuspended)); + await client.WaitForState(ConnectionState.Suspended); + channel.SetChannelState(from); + await client.ProcessCommands(); + + client.Close(); + await client.WaitForState(ConnectionState.Closed); + await client.ProcessCommands(); + + channel.State.Should().Be(ChannelState.Detached); + channel.Presence.MembersMap.Values.Should().BeEmpty(); + channel.Presence.InternalMembersMap.Values.Should().BeEmpty(); + } + + [Fact] + [Trait("spec", "RTL24")] + public async Task AfterACleanClose_ShouldNotStampAFabricatedChannelError() + { + // RTL24 lists RTN11d, RTL3a, RTL4g and RTL14 as the sources of channel errorReason. A + // clean close is none of them, so the ErrorInfo minted for RTL11's presence callbacks + // must not reach the channel. ably-js passes change.reason, which is null here. + var client = await GetConnectedClient(); + var channel = (RealtimeChannel)client.Channels.Get("test"); + channel.SetChannelState(ChannelState.Attached); + await client.ProcessCommands(); + + client.Close(); + await client.WaitForState(ConnectionState.Closed); + await client.ProcessCommands(); + + channel.State.Should().Be(ChannelState.Detached); + channel.ErrorReason.Should().BeNull(); + } + + [Fact] + [Trait("spec", "RTL5e")] + public async Task WhenAPendingDetachCompletesOnClose_ShouldReportSuccess() + { + // The detach the caller asked for does complete - CLOSING takes the channel to + // DETACHED - so the awaiter must report success, not failure. ably-js resolves + // detach() on that transition too. + var client = await GetConnectedClient(); + var channel = (RealtimeChannel)client.Channels.Get("test"); + channel.SetChannelState(ChannelState.Attached); + await client.ProcessCommands(); + + var detach = channel.DetachAsync(); + await client.ProcessCommands(); + channel.State.Should().Be(ChannelState.Detaching); + + client.Close(); + await client.WaitForState(ConnectionState.Closing); + await client.ProcessCommands(); + + channel.State.Should().Be(ChannelState.Detached); + (await detach).IsSuccess.Should().BeTrue(); + } + + // UTS: realtime/unit/RTL5l/detach-attached-when-disconnected-1 + [Fact] + [Trait("spec", "RTL5l")] + public async Task WhenDetachingWhileDisconnected_ShouldDetachImmediately() + { + // RTL5l is "anything other than CONNECTED", which includes DISCONNECTED - where an + // enumerated check would park the DETACH in the RTL6c2 queue and never call back. + var client = await GetConnectedClient(opts => + opts.DisconnectedRetryTimeout = TimeSpan.FromMinutes(10)); + + var channel = (RealtimeChannel)client.Channels.Get("test"); + channel.SetChannelState(ChannelState.Attached); + await client.ProcessCommands(); + + client.ExecuteCommand(SetDisconnectedStateCommand.Create(ErrorInfo.ReasonDisconnected)); + await client.WaitForState(ConnectionState.Disconnected); + await client.ProcessCommands(); + + var detach = await channel.DetachAsync(); + + detach.IsSuccess.Should().BeTrue(); + channel.State.Should().Be(ChannelState.Detached); + client.State.PendingMessages.Should().BeEmpty(); + + // "No DETACH message was sent (transport is unavailable)" - checked on the wire, + // not only on the outbound queue. + LastCreatedTransport.SentMessages + .Select(x => x.Original) + .Where(x => x != null) + .Should().NotContain(x => x.Action == ProtocolMessage.MessageAction.Detach); + } + + [Theory] + [InlineData(ConnectionState.Closing)] + [InlineData(ConnectionState.Closed)] + [InlineData(ConnectionState.Failed)] + [Trait("spec", "RTN8d")] + [Trait("spec", "RTN9d")] + public async Task WhenATerminalStateIsEmitted_TheKeyAndIdShouldAlreadyBeNull( + ConnectionState state) + { + // RTN8d and RTN9d: both are null in CLOSED, CLOSING and FAILED. SetState is what + // emits - inline, when no SynchronizationContext is installed - so the clear has to + // precede it. + // + // Read from inside the listener deliberately: asserting after the commands settle + // passes either way. Same shape as the RTL3d1 ordering requirement one clause over. + var client = await GetConnectedClient(); + client.State.Connection.Key.Should().NotBeEmpty(); + + string keyAtEmit = null; + string idAtEmit = null; + client.Connection.On(state.ToConnectionEvent(), _ => + { + keyAtEmit = client.Connection.Key; + idAtEmit = client.Connection.Id; + }); + + client.ExecuteCommand(state switch + { + ConnectionState.Closing => SetClosingStateCommand.Create(), + ConnectionState.Closed => SetClosedStateCommand.Create(), + _ => SetFailedStateCommand.Create(new ErrorInfo("gone", 1)), + }); + await client.ProcessCommands(); + + keyAtEmit.Should().BeNullOrEmpty(); + idAtEmit.Should().BeNullOrEmpty(); + } + + [Theory] + [InlineData(ConnectionState.Failed)] + [InlineData(ConnectionState.Closed)] + [InlineData(ConnectionState.Suspended)] + [Trait("spec", "RTN7e")] + [Trait("spec", "RTN8d")] + [Trait("spec", "RTN9d")] + [Trait("spec", "RTN14h")] + public async Task WhenTheTransitionThrows_ShouldStillCompleteTheTeardown( + ConnectionState state) + { + // The teardown must complete even when SetState rethrows: otherwise a live transport + // survives with its listener attached, behind an RTN23a monitor gated on Connected, + // and RTN7e's failure of the ack queue is skipped. + // + // All three states are driven because the key and id differ between them: RTN8d and + // RTN9d name only CLOSED, CLOSING and FAILED, while SUSPENDED retains them for + // RTN14h's next resume. + var client = await GetConnectedClient(); + client.State.WaitingForAck.Add(new MessageAndCallback(new ProtocolMessage(), null)); + + // Connection.NotifyUpdate invokes internal handlers unguarded, so throwing from one + // lands after the connection has entered the state and before the teardown. A plain + // Exception, not an AblyException, so the workflow's catch does not divert to FAILED + // and hide the state under test. + client.Connection.InternalStateChanged += (_, change) => + { + if (change.Current == state) + { + throw new Exception("thrown from a state change handler"); + } + }; + + client.ExecuteCommand(state switch + { + ConnectionState.Failed => SetFailedStateCommand.Create(new ErrorInfo("gone", 1)), + ConnectionState.Closed => SetClosedStateCommand.Create(), + _ => SetSuspendedStateCommand.Create(new ErrorInfo("gone", 1)), + }); + await client.ProcessCommands(); + + client.Connection.State.Should().Be(state); + client.State.WaitingForAck.Should().BeEmpty(); + client.ConnectionManager.Transport.Should().BeNull(); + + if (state == ConnectionState.Suspended) + { + client.State.Connection.Key.Should().NotBeEmpty(); + client.State.Connection.Id.Should().NotBeEmpty(); + } + else + { + client.State.Connection.Key.Should().BeEmpty(); + client.State.Connection.Id.Should().BeEmpty(); + } + } + + [Fact] + [Trait("spec", "RTN21")] + [Trait("spec", "RTN15b")] + [Trait("spec", "RTN8b")] + public async Task WhenAConnectedCarriesNoConnectionDetails_ShouldKeepTheKeyAndTakeTheId() + { + // The two fields are guarded differently on purpose. connectionId is top-level and + // always meaningful per RTN8b - ConnectionIdSpecs pins that a CONNECTED carrying only + // an id sets it. The key lives inside connectionDetails, which is what RTN21 scopes + // its override to, so a message carrying none must leave the key alone or a live + // connection is left with nothing to resume with under RTN15b. + var client = await GetConnectedClient(); + client.State.Connection.Key.Should().Be("connectionKey"); + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Connected) + { + ConnectionId = "a-different-id", + }); + + await client.ProcessCommands(); + + client.State.Connection.Key.Should().Be("connectionKey"); + client.State.Connection.Id.Should().Be("a-different-id"); + } + + private async Task GetClientWithHistory() + { + var client = await GetConnectedClient(); + var channel = (RealtimeChannel)client.Channels.Get("test"); + channel.SetChannelState(ChannelState.Attached); + + for (var i = 0; i < 3; i++) + { + client.ExecuteCommand(SendMessageCommand.Create( + new ProtocolMessage(ProtocolMessage.MessageAction.Message, "test"))); + } + + await client.ProcessCommands(); + client.State.Connection.MessageSerial.Should().Be(3); + return client; + } + + private static async Task MoveTo(AblyRealtime client, ConnectionState state) + { + var error = new ErrorInfo("something went wrong", 50000); + + switch (state) + { + case ConnectionState.Closed: + // Closed carries no error of its own, so put one on the channel and the + // connection first - RTN11d has to clear both. + client.ExecuteCommand(SetFailedStateCommand.Create(error)); + await client.ProcessCommands(); + client.ExecuteCommand(SetClosedStateCommand.Create()); + break; + case ConnectionState.Failed: + client.ExecuteCommand(SetFailedStateCommand.Create(error)); + break; + default: + client.ExecuteCommand(SetDisconnectedStateCommand.Create(error)); + break; + } + + await client.ProcessCommands(); + } + + public ReinitialiseOnConnectSpecs(ITestOutputHelper output) + : base(output) + { + } + } + + [Trait("spec", "RTN19a2")] + public class ConnectionContinuitySpecs : AblyRealtimeSpecs + { + [Fact] + [Trait("spec", "RTN15c6")] + public async Task OnASuccessfulResume_ShouldKeepTheSerialSequence() + { + var client = await GetClientWithOneUnackedMessage(); + + await Reconnect(client, connectionId: "1"); + + // The same connection is still expecting the serial this message was given. + client.State.Connection.MessageSerial.Should().Be(3); + SentSerials(client).Should().Equal(2L); + } + + [Fact] + [Trait("spec", "RTN24")] + public async Task OnAnUpdate_ShouldKeepTheSerialSequence() + { + var client = await GetClientWithOneUnackedMessage(); + + await Reconnect(client, connectionId: "1", isUpdate: true); + + client.State.Connection.MessageSerial.Should().Be(3); + } + + [Fact] + [Trait("spec", "RTN15c7")] + public async Task OnAFreshConnectionWithoutAnError_ShouldRestartTheSerialSequence() + { + // A resume the server refuses is answered with a new connectionId and, often, no + // error at all - which is precisely when the sequence must restart. + var client = await GetClientWithOneUnackedMessage(); + + await Reconnect(client, connectionId: "different"); + + SentSerials(client).Should().Equal(0L); + client.State.Connection.MessageSerial.Should().Be(1); + } + + // UTS: realtime/unit/RTN15c7/failed-resume-new-id-0 + [Fact] + [Trait("spec", "RTN15c7")] + public async Task OnAFailedResume_ShouldRestartTheSerialSequence() + { + var client = await GetClientWithOneUnackedMessage(); + var originalId = client.State.Connection.Id; + + // Through DISCONNECTED first, as a refused resume actually arrives. Reconnecting + // straight from CONNECTED trips UpdateState's same-state early return, which drops + // the error before it reaches Connection.ErrorReason. + client.ExecuteCommand(SetDisconnectedStateCommand.Create(ErrorInfo.ReasonDisconnected)); + await client.WaitForState(ConnectionState.Disconnected); + await client.ProcessCommands(); + + await Reconnect( + client, + connectionId: "different", + error: new ErrorInfo("resume failed", 80008), + connectionKey: "connectionKey-2"); + + SentSerials(client).Should().Equal(0L); + client.State.Connection.MessageSerial.Should().Be(1); + + // The rest of what a refused resume looks like: a new id and key, the reason on the + // connection, and CONNECTED all the same. + client.State.Connection.Id.Should().Be("different").And.NotBe(originalId); + client.State.Connection.Key.Should().Be("connectionKey-2"); + client.Connection.ErrorReason.Should().NotBeNull(); + client.Connection.ErrorReason.Code.Should().Be(80008); + client.Connection.State.Should().Be(ConnectionState.Connected); + } + + [Fact] + public async Task WhenRestartingTheSequence_ShouldNotLeaveStaleEntriesAwaitingAck() + { + // Requeued messages are re-registered for their ACK as they are sent, so the original + // entries must go: they hold serials from the abandoned sequence that a later ACK + // would also match, running the same callback twice. + var client = await GetClientWithOneUnackedMessage(); + + await Reconnect(client, connectionId: "different"); + + client.State.WaitingForAck.Should().HaveCount(1); + client.State.WaitingForAck.Single().Message.MsgSerial.Should().Be(0); + } + + // UTS: realtime/unit/RTN16f/recover-initializes-msgserial-0 + [Fact] + [Trait("spec", "RTN16f")] + public async Task OnASuccessfulRecover_ShouldKeepTheRecoveredSerial() + { + // RTN16f initialises the counter from the recovery key. A recover deliberately adopts + // another connection's sequence, so a successful one must not restart it, even though + // the connectionId is one we have never seen. + var client = GetClientWithFakeTransport(options => options.Recover = + "{\"connectionKey\":\"uniqueKey\",\"msgSerial\":45,\"channelSerials\":{}}"); + // WaitForState returns as soon as the state is set, which happens before + // ConnectionManager.CreateTransport runs - and that is what applies the recovered + // serial. Drain the queue so the precondition is established rather than raced. + await client.WaitForState(ConnectionState.Connecting); + await client.ProcessCommands(); + client.State.Connection.MessageSerial.Should().Be(45); + + await Reconnect(client, connectionId: "recovered"); + + client.State.Connection.MessageSerial.Should().Be(45); + + // And it is the serial actually put on the wire, which is what RTN16f is for. + client.ExecuteCommand(SendMessageCommand.Create( + new ProtocolMessage(ProtocolMessage.MessageAction.Message, "test"))); + await client.ProcessCommands(); + SentSerials(client).Should().Equal(45L); + } + + [Fact] + [Trait("spec", "RTN16f")] + [Trait("spec", "RTN15c7")] + public async Task OnAFailedRecover_ShouldRestartTheSerialSequence() + { + // RTN16f: "If the recover fails, the counter should be reset to 0 per RTN15c7." + var client = GetClientWithFakeTransport(options => options.Recover = + "{\"connectionKey\":\"uniqueKey\",\"msgSerial\":45,\"channelSerials\":{}}"); + // WaitForState returns as soon as the state is set, which happens before + // ConnectionManager.CreateTransport runs - and that is what applies the recovered + // serial. Drain the queue so the precondition is established rather than raced. + await client.WaitForState(ConnectionState.Connecting); + await client.ProcessCommands(); + client.State.Connection.MessageSerial.Should().Be(45); + + await Reconnect(client, connectionId: "different", error: new ErrorInfo("unable to recover", 80008)); + + client.State.Connection.MessageSerial.Should().Be(0); + } + + private async Task GetClientWithOneUnackedMessage() + { + var client = await GetConnectedClient(); + + for (var i = 0; i < 3; i++) + { + client.ExecuteCommand(SendMessageCommand.Create( + new ProtocolMessage(ProtocolMessage.MessageAction.Message, "test"))); + } + + // Acknowledge the first two, so the sequence is deliberately not zero based and a + // restart is distinguishable from a resume. + client.ExecuteCommand(ProcessMessageCommand.Create( + new ProtocolMessage(ProtocolMessage.MessageAction.Ack) { MsgSerial = 0, Count = 2 })); + await client.ProcessCommands(); + + client.State.Connection.MessageSerial.Should().Be(3); + client.State.WaitingForAck.Should().HaveCount(1); + client.State.WaitingForAck.Single().Message.MsgSerial.Should().Be(2); + + LastCreatedTransport.SentMessages.Clear(); + return client; + } + + private static async Task Reconnect( + AblyRealtime client, string connectionId, bool isUpdate = false, ErrorInfo error = null, string connectionKey = "connectionKey") + { + await client.Workflow.ProcessCommand(SetConnectedStateCommand.Create( + new ProtocolMessage(ProtocolMessage.MessageAction.Connected) + { + ConnectionId = connectionId, + ConnectionDetails = new ConnectionDetails { ConnectionKey = connectionKey }, + Error = error, + }, + isUpdate)); + await client.ProcessCommands(); + } + + private IEnumerable SentSerials(AblyRealtime client) => + LastCreatedTransport.SentMessages + .Select(x => x.Original) + .Where(x => x != null && x.Action == ProtocolMessage.MessageAction.Message) + .Select(x => x.MsgSerial) + .ToList(); + + public ConnectionContinuitySpecs(ITestOutputHelper output) + : base(output) + { + } + } + + [Trait("spec", "RTN24")] + public class ConnectedUpdateSpecs : AblyRealtimeSpecs + { + [Fact] + [Trait("spec", "RTN15c7")] + [Trait("spec", "RTL3d")] + public async Task AfterAFailedResume_ShouldReattachAnAttachedChannel() + { + // RTN15c7 - a resume the server refused, answered with a connectionId we were not + // holding. RTL3d reattaches regardless. + var client = await GetConnectedClient(); + var channel = (RealtimeChannel)client.Channels.Get("test"); + channel.SetChannelState(ChannelState.Attached); + await client.ProcessCommands(); + + client.ExecuteCommand(SetDisconnectedStateCommand.Create(ErrorInfo.ReasonDisconnected)); + await client.ProcessCommands(); + client.ExecuteCommand(SetConnectingStateCommand.Create()); + await client.ProcessCommands(); + + // RTN14h - the reconnection attempt still carries the resume. + client.State.Connection.Key.Should().NotBeEmpty(); + LastCreatedTransport.SentMessages.Clear(); + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Connected) + { + ConnectionId = "brand-new", + ConnectionDetails = new ConnectionDetails { ConnectionKey = "newKey" }, + }); + await client.ProcessCommands(); + + channel.State.Should().Be(ChannelState.Attaching); + LastCreatedTransport.SentMessages + .Select(x => x.Original) + .Where(x => x != null) + .Select(x => x.Action) + .Should().Contain(ProtocolMessage.MessageAction.Attach); + } + + [Fact] + [Trait("spec", "RTL3d")] + public async Task OnAnUpdate_ShouldNotReattachAttachedChannels() + { + // RTL3d applies on entering CONNECTED. An RTN24 update arrives on the connection we + // already hold, so reattaching would be a spurious round trip per channel. + var client = await GetConnectedClient(); + var channel = (RealtimeChannel)client.Channels.Get("test"); + channel.SetChannelState(ChannelState.Attached); + await client.ProcessCommands(); + + LastCreatedTransport.SentMessages.Clear(); + var states = new List(); + channel.On(x => states.Add(x.Current)); + + // Same connectionId, delivered as an update - the shape RTC8a reauth produces. + await client.Workflow.ProcessCommand(SetConnectedStateCommand.Create( + new ProtocolMessage(ProtocolMessage.MessageAction.Connected) + { + ConnectionId = "1", + ConnectionDetails = new ConnectionDetails { ConnectionKey = "connectionKey" }, + }, + isUpdate: true)); + await client.ProcessCommands(); + + channel.State.Should().Be(ChannelState.Attached); + states.Should().NotContain(ChannelState.Attaching); + LastCreatedTransport.SentMessages + .Select(x => x.Original?.Action) + .Should().NotContain(ProtocolMessage.MessageAction.Attach); + } + + public ConnectedUpdateSpecs(ITestOutputHelper output) + : base(output) + { + } + } + + [Trait("spec", "RTN23a")] + public class IdleTimeoutSpecs : AblyRealtimeSpecs + { + private static readonly TimeSpan PromisedMaxIdleInterval = TimeSpan.FromSeconds(15); + private static readonly TimeSpan RequestTimeout = TimeSpan.FromSeconds(10); + + // 15s promised by the server plus the 10s realtimeRequestTimeout. + private static readonly TimeSpan AllowedIdleTime = PromisedMaxIdleInterval + RequestTimeout; + + private readonly Now _now = new Now(); + + [Fact] + [Trait("spec", "TO3l11")] + public async Task ShouldUseTheConfiguredRealtimeRequestTimeout() + { + // TO3l11 makes realtimeRequestTimeout a client option, and RTN23a derives its + // threshold from it, so a caller changing it has to move the idle timeout with it. + var client = await GetConnectedClient(PromisedMaxIdleInterval, TimeSpan.FromSeconds(30)); + + // Past the default 25s window but inside the 45s this client asked for. + _now.Reset(_now.Value.Add(TimeSpan.FromSeconds(40))); + (await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value))).Should().BeEmpty(); + + _now.Reset(_now.Value.Add(TimeSpan.FromSeconds(10))); + (await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value))) + .Should().ContainSingle().Which.Should().BeOfType(); + } + + [Fact] + public async Task WhenIdleForLongerThanAllowed_ShouldDisconnect() + { + var client = await GetConnectedClient(PromisedMaxIdleInterval); + + _now.Reset(_now.Value.Add(AllowedIdleTime.Add(TimeSpan.FromSeconds(1)))); + + var commands = await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value)); + + var disconnect = commands.Should().ContainSingle() + .Which.Should().BeOfType().Subject; + disconnect.Error.Code.Should().Be(ErrorCodes.Disconnected); + disconnect.Error.StatusCode.Should().Be(HttpStatusCode.RequestTimeout); + + // RTN15a - the idle disconnect asks to reconnect at once rather than waiting out + // the disconnected retry timeout. + disconnect.RetryInstantly.Should().BeTrue(); + } + + // UTS: realtime/unit/RTN23a/idle-timeout-reconnect-1 + [Fact] + [Trait("spec", "RTN23a")] + [Trait("spec", "RTN15a")] + public async Task WhenTheIdleTimeoutFires_ShouldDisconnectAndReconnect() + { + // The UTS case asserts the whole cycle, not just the decision to disconnect: + // connecting, connected, disconnected, connecting, connected, with a second + // connection attempt and a new connectionId. The test above pins what the monitor + // decides; this one pins what the client does with it. + var client = await GetConnectedClient(PromisedMaxIdleInterval); + + var states = new List(); + client.Connection.On(args => states.Add(args.Current)); + + CreatedTransports.Should().HaveCount(1); + + _now.Reset(_now.Value.Add(AllowedIdleTime.Add(TimeSpan.FromSeconds(1)))); + foreach (var command in await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value))) + { + client.ExecuteCommand(command); + } + + await client.WaitForState(ConnectionState.Connecting); + await client.ProcessCommands(); + + // RTN15a's immediate reconnect built a second transport. + CreatedTransports.Should().HaveCount(2); + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Connected) + { + ConnectionId = "connection-id-2", + ConnectionDetails = new ConnectionDetails + { + ConnectionKey = "key-2", + MaxIdleInterval = PromisedMaxIdleInterval, + }, + }); + await client.WaitForState(ConnectionState.Connected); + await client.ProcessCommands(); + + states.Should().ContainInOrder( + ConnectionState.Disconnected, ConnectionState.Connecting, ConnectionState.Connected); + client.Connection.Id.Should().Be("connection-id-2"); + } + + [Fact] + public async Task WhenIdleForExactlyTheAllowedTime_ShouldNotDisconnect() + { + var client = await GetConnectedClient(PromisedMaxIdleInterval); + + _now.Reset(_now.Value.Add(AllowedIdleTime)); + + var commands = await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value)); + + commands.Should().BeEmpty(); + } + + [Fact] + [Trait("spec", "CD2h")] + public async Task WhenMaxIdleIntervalIsZero_ShouldNeverDisconnect() + { + // A zero maxIdleInterval means Ably allows arbitrarily long inactivity. + var client = await GetConnectedClient(TimeSpan.Zero); + + _now.Reset(_now.Value.Add(TimeSpan.FromHours(1))); + + var commands = await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value)); + + commands.Should().BeEmpty(); + } + + [Fact] + [Trait("spec", "CD2h")] + public async Task WhenMaxIdleIntervalIsAbsent_ShouldNeverDisconnect() + { + // Ably declining to send the field is Ably declining to make the promise. + var client = await GetConnectedClient(null); + + _now.Reset(_now.Value.Add(TimeSpan.FromHours(1))); + + var commands = await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value)); + + commands.Should().BeEmpty(); + } + + [Fact] + public async Task WhenNotConnected_ShouldNotDisconnect() + { + // ConfirmedAliveAt can still be carrying a timestamp from a previous transport, so + // acting on it outside Connected risks tearing down a healthy new one. + var client = await GetConnectedClient(PromisedMaxIdleInterval); + + await client.Workflow.ProcessCommand(SetDisconnectedStateCommand.Create(ErrorInfo.ReasonDisconnected)); + _now.Reset(_now.Value.Add(TimeSpan.FromHours(1))); + + var commands = await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value)); + + commands.Should().BeEmpty(); + } + + // UTS: realtime/unit/RTN23a/any-message-resets-timer-3 + [Fact] + public async Task AnyReceivedMessage_NotOnlyHeartbeat_ShouldResetTheIdleTimer() + { + var client = await GetConnectedClient(PromisedMaxIdleInterval); + + // Most of the way through the allowed window, then a message that is deliberately + // not a Heartbeat - RTN23a counts any received message as a sign of activity. + _now.Reset(_now.Value.Add(TimeSpan.FromSeconds(20))); + client.ExecuteCommand(ProcessMessageCommand.Create( + new ProtocolMessage(ProtocolMessage.MessageAction.Attached) { Channel = "test" })); + await client.ProcessCommands(); + + // Another 20s on from that message is still inside the 25s window. + _now.Reset(_now.Value.Add(TimeSpan.FromSeconds(20))); + + var commands = await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value)); + + commands.Should().BeEmpty(); + + // Still a single attempt at this point, per the case. + CreatedTransports.Should().HaveCount(1); + + // And once the window really does elapse with no activity, the cycle runs. + _now.Reset(_now.Value.Add(AllowedIdleTime.Add(TimeSpan.FromSeconds(1)))); + foreach (var command in await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value))) + { + client.ExecuteCommand(command); + } + + await client.WaitForState(ConnectionState.Connecting); + await client.ProcessCommands(); + CreatedTransports.Should().HaveCount(2); + } + + [Fact] + [Trait("spec", "RTN23a")] + [Trait("spec", "CD2h")] + public async Task WhenANewTransportOmitsMaxIdleInterval_ShouldStopMeasuring() + { + // RTN23a measures against the interval "sent in the connectionDetails of the most + // recent CONNECTED message received on that transport", so a CONNECTED starting a new + // transport must not inherit the previous threshold. An omitted field is Ably + // declining to promise anything, which CD2h treats as arbitrarily long inactivity. + var client = await GetConnectedClient(PromisedMaxIdleInterval); + + // A real reconnect, so the CONNECTED lands on a new transport rather than becoming + // an RTN24 update on the one we already hold. + client.ExecuteCommand(SetDisconnectedStateCommand.Create(ErrorInfo.ReasonDisconnected)); + await client.ProcessCommands(); + client.ExecuteCommand(SetConnectingStateCommand.Create()); + await client.ProcessCommands(); + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Connected) + { + ConnectionId = "2", + ConnectionDetails = new ConnectionDetails { ConnectionKey = "anotherKey" }, + }); + + await client.WaitForState(ConnectionState.Connected); + await client.ProcessCommands(); + + client.State.Connection.MaxIdleInterval.Should().BeNull(); + + _now.Reset(_now.Value.Add(AllowedIdleTime).Add(TimeSpan.FromSeconds(1))); + (await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value))) + .Should().BeEmpty(); + } + + [Fact] + [Trait("spec", "RTN23a")] + [Trait("spec", "RTN24")] + public async Task WhenAnUpdateOmitsMaxIdleInterval_ShouldKeepMeasuring() + { + // An RTN24 update arrives on the transport we already hold, which is still bound by + // whatever it promised, so an omitted field is not a withdrawal. ably-js keeps the + // previous value here with its if (maxPromisedIdle) guard, nulling the field per + // transport instead. Nulling it here would disarm RTN23a on a live transport. + var client = await GetConnectedClient(PromisedMaxIdleInterval); + var transportBefore = LastCreatedTransport; + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Connected) + { + ConnectionId = "1", + ConnectionDetails = new ConnectionDetails { ConnectionKey = "connectionKey" }, + }); + + await client.ProcessCommands(); + + // Same transport - nothing reconnected. + LastCreatedTransport.Should().BeSameAs(transportBefore); + client.State.Connection.MaxIdleInterval.Should().Be(PromisedMaxIdleInterval); + + _now.Reset(_now.Value.Add(AllowedIdleTime).Add(TimeSpan.FromSeconds(1))); + (await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value))) + .Should().ContainSingle().Which.Should().BeOfType(); + } + + [Fact] + [Trait("spec", "RTN23a")] + public async Task WhenTheThresholdCannotBeRepresented_ShouldNotThrow() + { + // The wire value is an unbounded integer of milliseconds, so the sum can overflow. A + // throw would be logged and dropped by the command loop, and every later tick would + // throw too - killing idle detection silently for the life of the connection. + var client = await GetConnectedClient(TimeSpan.MaxValue - TimeSpan.FromSeconds(1)); + + _now.Reset(_now.Value.Add(TimeSpan.FromDays(1))); + + var commands = await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value)); + + commands.Should().BeEmpty(); + client.Connection.State.Should().Be(ConnectionState.Connected); + } + + [Theory] + [InlineData("Heartbeats", false)] + [InlineData("heartbeats", false)] + [InlineData("heartbeats", "no")] + [InlineData("heartbeats", 0)] + [InlineData("heartbeats", null)] + + // A differently-cased key with a true value still leaves nothing on the wire asking for + // protocol heartbeats: Merge drops our correctly-cased entry, and only that spelling is + // the param Ably reads. + [InlineData("Heartbeats", true)] + [InlineData("HEARTBEATS", "true")] + [Trait("spec", "RTN23b")] + public async Task WhenTheCallerDoesNotAskForProtocolHeartbeats_ShouldStandDown( + string key, object value) + { + // Two halves, both needed. Merge drops our own heartbeats param on a + // case-insensitive key match, so "Heartbeats" reaches the wire in place of ours. And + // RTN23b guarantees protocol heartbeats only for the literal "true": "if it is false + // or unspecified, the server is permitted to use any transport-level mechanism" - + // which this library cannot observe. + var client = GetClientWithFakeTransport(opts => + { + opts.NowFunc = _now.ValueFn; + opts.RealtimeRequestTimeout = RequestTimeout; + opts.HeartbeatMonitorDelay = (int)TimeSpan.FromMinutes(10).TotalMilliseconds; + opts.TransportParams = new Dictionary { { key, value } }; + }); + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Connected) + { + ConnectionId = "1", + ConnectionDetails = new ConnectionDetails + { + ConnectionKey = "connectionKey", + MaxIdleInterval = PromisedMaxIdleInterval, + }, + }); + + await client.WaitForState(ConnectionState.Connected); + + _now.Reset(_now.Value.Add(AllowedIdleTime).Add(TimeSpan.FromSeconds(1))); + (await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value))) + .Should().BeEmpty(); + } + + [Theory] + [InlineData("heartbeats", true)] + [InlineData("heartbeats", "true")] + [InlineData("heartbeats", "TRUE")] + [Trait("spec", "RTN23b")] + public async Task WhenTheCallerAsksForProtocolHeartbeats_ShouldStayArmed(string key, object value) + { + // Standing down is the guard's default outcome for any caller value, so the armed + // branch is the one an edit could lose silently. This pins it. + var client = GetClientWithFakeTransport(opts => + { + opts.NowFunc = _now.ValueFn; + opts.RealtimeRequestTimeout = RequestTimeout; + opts.HeartbeatMonitorDelay = (int)TimeSpan.FromMinutes(10).TotalMilliseconds; + opts.TransportParams = new Dictionary { { key, value } }; + }); + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Connected) + { + ConnectionId = "1", + ConnectionDetails = new ConnectionDetails + { + ConnectionKey = "connectionKey", + MaxIdleInterval = PromisedMaxIdleInterval, + }, + }); + + await client.WaitForState(ConnectionState.Connected); + + _now.Reset(_now.Value.Add(AllowedIdleTime).Add(TimeSpan.FromSeconds(1))); + (await client.Workflow.ProcessCommand(HeartbeatMonitorCommand.Create(_now.Value))) + .Should().ContainSingle().Which.Should().BeOfType(); + } + + [Fact] + [Trait("spec", "RTN21")] + [Trait("spec", "RTN15b")] + public async Task WhenAConnectedCarriesNoConnectionDetails_ShouldKeepTheConnectionKey() + { + // RTN21 scopes the override to "the attributes within ConnectionDetails", so a + // CONNECTED carrying none overrides nothing and the key has to survive. + var client = await GetConnectedClient(PromisedMaxIdleInterval); + client.State.Connection.Key.Should().Be("connectionKey"); + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Connected) + { + ConnectionId = "1", + }); + + await client.ProcessCommands(); + + client.State.Connection.Key.Should().Be("connectionKey"); + } + + private async Task GetConnectedClient( + TimeSpan? maxIdleInterval, TimeSpan? requestTimeout = null) + { + var client = GetClientWithFakeTransport(opts => + { + opts.NowFunc = _now.ValueFn; + opts.RealtimeRequestTimeout = requestTimeout ?? RequestTimeout; + + // These tests drive the monitor tick explicitly and assert on what that tick + // decides, so the workflow's own once-a-second loop is pushed out of reach; on a + // loaded run it would fire first and the explicit tick would find the disconnect + // already requested. + opts.HeartbeatMonitorDelay = (int)TimeSpan.FromMinutes(10).TotalMilliseconds; + }); + + client.FakeProtocolMessageReceived(new ProtocolMessage(ProtocolMessage.MessageAction.Connected) + { + ConnectionId = "1", + ConnectionDetails = new ConnectionDetails + { + ConnectionKey = "connectionKey", + MaxIdleInterval = maxIdleInterval, + }, + }); + + await client.WaitForState(ConnectionState.Connected); + return client; + } + + public IdleTimeoutSpecs(ITestOutputHelper output) + : base(output) + { + } + } + public RealtimeWorkflowSpecs(ITestOutputHelper output) : base(output) { diff --git a/src/IO.Ably.Tests.Shared/Utils/ReconnectionStrategyTest.cs b/src/IO.Ably.Tests.Shared/Utils/ReconnectionStrategyTest.cs index 556446afa..1cee06fa4 100644 --- a/src/IO.Ably.Tests.Shared/Utils/ReconnectionStrategyTest.cs +++ b/src/IO.Ably.Tests.Shared/Utils/ReconnectionStrategyTest.cs @@ -8,8 +8,11 @@ namespace IO.Ably.Tests.Shared.Utils { public class ReconnectionStrategyTest { + // UTS: realtime/unit/RTB1/disconnected-retry-delay-0 [Fact] [Trait("spec", "RTB1")] + [Trait("spec", "RTB1a")] + [Trait("spec", "RTB1b")] public void ShouldCalculateRetryTimeoutsUsingBackOffAndJitter() { var retryAttempts = new[] { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10 };