Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add RTN15a #789

Merged
merged 8 commits into from
Sep 13, 2018
Merged

Add RTN15a #789

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
daa6a7f
Add suspendImmediateReconnection
funkyboy Aug 23, 2018
eef66ae
Add implementation of RTN15a
funkyboy Aug 23, 2018
bace71d
Fix tests that were depending on a low value for disconnectedRetryTim…
funkyboy Aug 23, 2018
bf04b99
Remove unneeded setup of disconnectedRetryTimeout values for tests
funkyboy Aug 23, 2018
2af1414
Clarify RTN14e test
funkyboy Sep 5, 2018
df6be9e
Remove leftover disconnectedRetryTimeout test settings
funkyboy Sep 5, 2018
8bab329
Add explicit test for RTN15a
funkyboy Sep 6, 2018
ca19426
Tweak values for RTN15a test
funkyboy Sep 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Source/ARTRealtime+Private.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ NS_ASSUME_NONNULL_BEGIN

@property (readonly, getter=getClientOptions) ARTClientOptions *options;

/// Suspend the behavior defined in RTN15a, that is trying to immediately reconnect after a disconnection
@property (readwrite, assign, nonatomic) BOOL suspendImmediateReconnection;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this flag? If the client isn't respecting the spec then it should not be a flag to be activated only for the test suite, right? Maybe I'm missing something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec says If a Connection transport is disconnected unexpectedly or if a token expires, then the Connection manager will immediately attempt to reconnect and restore the connection state. so the client should immediately attempt to reconnect always and it doesn't. It's a bug then, where iOS does not retry a connection when it fails immediately. It waits 15s now right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ricardopereira correct, now it waits 15s.

The flag is needed because sometimes you need a "disconnect and stay in the disconnected for x seconds" case. For example when you are testing that there's a new connection id if you connect after (ttl + idle interval). Without this flag you'd have an immediate connection retry.
We had to do something similar in Java.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, understood.


@end

@interface ARTRealtime (Private)
Expand Down
8 changes: 7 additions & 1 deletion Source/ARTRealtime.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ - (void)setRetryIn:(NSTimeInterval)retryIn;
@implementation ARTRealtime {
BOOL _resuming;
BOOL _renewingToken;
BOOL _suspendImmediateReconnection;
ARTEventEmitter<ARTEvent *, ARTErrorInfo *> *_pingEventEmitter;
NSDate *_startedReconnection;
NSDate *_lastActivity;
Expand Down Expand Up @@ -505,7 +506,12 @@ - (ARTEventListener *)transitionSideEffects:(ARTConnectionStateChange *)stateCha

[self.transport close];
_transport = nil;
[stateChange setRetryIn:self.options.disconnectedRetryTimeout];
NSTimeInterval retryInterval = self.options.disconnectedRetryTimeout;
// RTN15a - retry immediately if client was connected
if (stateChange.previous == ARTRealtimeConnected && !_suspendImmediateReconnection) {
retryInterval = 0.1;
}
[stateChange setRetryIn:retryInterval];
stateChangeEventListener = [self unlessStateChangesBefore:stateChange.retryIn do:^{
[weakSelf transition:ARTRealtimeConnecting];
_connectionRetryFromDisconnectedListener = nil;
Expand Down
1 change: 0 additions & 1 deletion Spec/RealtimeClient.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1439,7 +1439,6 @@ class RealtimeClient: QuickSpec {

it("moves to DISCONNECTED on an unexpected normal WebSocket close") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 0.3
let client = ARTRealtime(options: options)
defer { client.dispose(); client.close() }

Expand Down
5 changes: 0 additions & 5 deletions Spec/RealtimeClientChannel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,6 @@ class RealtimeClientChannel: QuickSpec {
// RTL2f
it("ChannelStateChange will contain a resumed boolean attribute with value @true@ if the bit flag RESUMED was included") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.0
options.tokenDetails = getTestTokenDetails(ttl: 5.0)
let client = ARTRealtime(options: options)
defer { client.dispose(); client.close() }
Expand Down Expand Up @@ -833,7 +832,6 @@ class RealtimeClientChannel: QuickSpec {

it("DISCONNECTED") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 0.1
let client = ARTRealtime(options: options)
defer { client.dispose(); client.close() }

Expand Down Expand Up @@ -1491,7 +1489,6 @@ class RealtimeClientChannel: QuickSpec {

it("DISCONNECTED") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 0.1
let client = ARTRealtime(options: options)
defer { client.dispose(); client.close() }

Expand Down Expand Up @@ -1750,7 +1747,6 @@ class RealtimeClientChannel: QuickSpec {
context("the message should be queued and delivered as soon as the connection state returns to CONNECTED if the connection is") {
let options = AblyTests.commonAppSetup()
options.useTokenAuth = true
options.disconnectedRetryTimeout = 0.3
options.autoConnect = false
var client: ARTRealtime!
var channel: ARTRealtimeChannel!
Expand Down Expand Up @@ -1897,7 +1893,6 @@ class RealtimeClientChannel: QuickSpec {
// RTL6c4
context("will result in an error if the") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 0.1
options.suspendedRetryTimeout = 0.3
options.autoConnect = false
var client: ARTRealtime!
Expand Down
27 changes: 7 additions & 20 deletions Spec/RealtimeClientConnection.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,6 @@ class RealtimeClientConnection: QuickSpec {
it("should emit events for state changes") {
let options = AblyTests.commonAppSetup()
options.autoConnect = false
options.disconnectedRetryTimeout = 0.0

let client = ARTRealtime(options: options)
defer { client.dispose(); client.close() }
Expand Down Expand Up @@ -975,7 +974,6 @@ class RealtimeClientConnection: QuickSpec {

it("should reset msgSerial serially if the connection does not resume") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.0
options.clientId = "tester"
let client = AblyTests.newRealtime(options)
defer { client.dispose(); client.close() }
Expand Down Expand Up @@ -1136,7 +1134,6 @@ class RealtimeClientConnection: QuickSpec {
it("lost connection state") {
let options = AblyTests.commonAppSetup()
options.autoConnect = false
options.disconnectedRetryTimeout = 0.1
let client = ARTRealtime(options: options)
client.setTransport(TestProxyTransport.self)
client.connect()
Expand Down Expand Up @@ -1918,7 +1915,6 @@ class RealtimeClientConnection: QuickSpec {
it("should not emit error with a renewable token") {
let options = AblyTests.commonAppSetup()
options.autoConnect = false
options.disconnectedRetryTimeout = 0.1
options.authCallback = { tokenParams, callback in
getTestTokenDetails(key: options.key, capability: tokenParams.capability, ttl: tokenParams.ttl as! TimeInterval?, completion: callback)
}
Expand Down Expand Up @@ -2104,6 +2100,7 @@ class RealtimeClientConnection: QuickSpec {
ARTDefault.setRealtimeRequestTimeout(0.1)

let client = ARTRealtime(options: options)
client.suspendImmediateReconnection = true
defer {
client.connection.off()
client.close()
Expand Down Expand Up @@ -2142,6 +2139,7 @@ class RealtimeClientConnection: QuickSpec {
// RTN14e
it("connection state has been in the DISCONNECTED state for more than the default connectionStateTtl should change the state to SUSPENDED") {
let options = AblyTests.commonAppSetup()
// to not wait the defaul 15s before reconnecting
options.disconnectedRetryTimeout = 0.1
options.suspendedRetryTimeout = 0.5
options.autoConnect = false
Expand All @@ -2160,6 +2158,7 @@ class RealtimeClientConnection: QuickSpec {
ARTDefault.setRealtimeRequestTimeout(0.1)

let client = ARTRealtime(options: options)
client.suspendImmediateReconnection = true
defer { client.dispose(); client.close() }

waitUntil(timeout: testTimeout) { done in
Expand All @@ -2179,6 +2178,7 @@ class RealtimeClientConnection: QuickSpec {

it("on CLOSE the connection should stop connection retries") {
let options = AblyTests.commonAppSetup()
// to avoid waiting for the default 15s before trying a reconnection
options.disconnectedRetryTimeout = 0.1
options.suspendedRetryTimeout = 0.5
options.autoConnect = false
Expand All @@ -2197,6 +2197,7 @@ class RealtimeClientConnection: QuickSpec {
ARTDefault.setRealtimeRequestTimeout(0.1)

let client = ARTRealtime(options: options)
client.suspendImmediateReconnection = true
defer { client.dispose(); client.close() }

waitUntil(timeout: testTimeout) { done in
Expand Down Expand Up @@ -2235,7 +2236,6 @@ class RealtimeClientConnection: QuickSpec {
it("should not receive published messages until the connection reconnects successfully") {
let options = AblyTests.commonAppSetup()
options.autoConnect = false
options.disconnectedRetryTimeout = 1.0

let client1 = ARTRealtime(options: options)
defer { client1.close() }
Expand Down Expand Up @@ -2290,7 +2290,6 @@ class RealtimeClientConnection: QuickSpec {
// RTN15b1, RTN15b2
it("resume is the private connection key and connection_serial is the most recent ProtocolMessage#connectionSerial received") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 0.1
let client = AblyTests.newRealtime(options)
defer { client.dispose(); client.close() }

Expand Down Expand Up @@ -2318,7 +2317,6 @@ class RealtimeClientConnection: QuickSpec {
// RTN15c1
it("CONNECTED ProtocolMessage with the same connectionId as the current client, and no error") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.0
let client = AblyTests.newRealtime(options)
defer { client.dispose(); client.close() }
let channel = client.channels.get("test")
Expand Down Expand Up @@ -2346,7 +2344,6 @@ class RealtimeClientConnection: QuickSpec {
// RTN15c2
it("CONNECTED ProtocolMessage with the same connectionId as the current client and an non-fatal error") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.0
let client = AblyTests.newRealtime(options)
defer { client.dispose(); client.close() }
let channel = client.channels.get("test")
Expand Down Expand Up @@ -2406,7 +2403,6 @@ class RealtimeClientConnection: QuickSpec {
// RTN15c3
it("CONNECTED ProtocolMessage with a new connectionId and an error") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.0
let client = AblyTests.newRealtime(options)
defer { client.dispose(); client.close() }
let channel = client.channels.get("test")
Expand Down Expand Up @@ -2445,7 +2441,6 @@ class RealtimeClientConnection: QuickSpec {
// RTN15c4
it("ERROR ProtocolMessage indicating a fatal error in the connection") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.0
let client = AblyTests.newRealtime(options)
defer { client.dispose(); client.close() }
let channel = client.channels.get("test")
Expand Down Expand Up @@ -2477,7 +2472,6 @@ class RealtimeClientConnection: QuickSpec {

it("should resume the connection after an auth renewal") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.0
options.tokenDetails = getTestTokenDetails(ttl: 5.0)
let client = AblyTests.newRealtime(options)
defer { client.dispose(); client.close() }
Expand Down Expand Up @@ -2555,7 +2549,6 @@ class RealtimeClientConnection: QuickSpec {
// RTN15d
it("should recover from disconnection and messages should be delivered once the connection is resumed") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.0

let client1 = ARTRealtime(options: options)
defer { client1.close() }
Expand Down Expand Up @@ -2600,7 +2593,6 @@ class RealtimeClientConnection: QuickSpec {
it("the connection#key may change and will be provided in the first CONNECTED ProtocolMessage#connectionDetails") {
let options = AblyTests.commonAppSetup()
options.autoConnect = false
options.disconnectedRetryTimeout = 1.0

let client = ARTRealtime(options: options)
client.setTransport(TestProxyTransport.self)
Expand Down Expand Up @@ -2636,7 +2628,6 @@ class RealtimeClientConnection: QuickSpec {
// RTN15f
it("ACK and NACK responses for published messages can only ever be received on the transport connection on which those messages were sent") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.5
let client = AblyTests.newRealtime(options)
defer { client.dispose(); client.close() }
let channel = client.channels.get("test")
Expand Down Expand Up @@ -2684,6 +2675,7 @@ class RealtimeClientConnection: QuickSpec {

it("uses a new connection") {
client = AblyTests.newRealtime(options)
client.suspendImmediateReconnection = true
client.connect()
defer { client.close() }

Expand Down Expand Up @@ -2713,6 +2705,7 @@ class RealtimeClientConnection: QuickSpec {
// RTN15g3
it("reattaches to the same channels after a new connection has been established") {
client = AblyTests.newRealtime(options)
client.suspendImmediateReconnection = true
defer { client.close() }
let channelName = "test-reattach-after-ttl"
let channel = client.channels.get(channelName)
Expand Down Expand Up @@ -2749,7 +2742,6 @@ class RealtimeClientConnection: QuickSpec {
// RTN15g2
context("when connection (ttl + idle interval) period has NOT passed since last activity") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 5.0
var client: ARTRealtime!
var connectionId = ""

Expand Down Expand Up @@ -2784,7 +2776,6 @@ class RealtimeClientConnection: QuickSpec {

it("if the token is renewable then error should not be emitted") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.0
options.autoConnect = false
options.authCallback = { tokenParams, callback in
getTestTokenDetails(key: options.key, capability: tokenParams.capability, ttl: TimeInterval(60 * 60), completion: callback)
Expand Down Expand Up @@ -2838,7 +2829,6 @@ class RealtimeClientConnection: QuickSpec {
// RTN15h2
it("should transition to disconnected when the token renewal fails and the error should be emitted") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.0
options.autoConnect = false
let tokenTtl = 5.0
let tokenDetails = getTestTokenDetails(key: options.key, capability: nil, ttl: tokenTtl)!
Expand Down Expand Up @@ -3678,7 +3668,6 @@ class RealtimeClientConnection: QuickSpec {
// RTN19a
it("should resend any ProtocolMessage that is awaiting a ACK/NACK") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 0.1
let client = AblyTests.newRealtime(options)
defer { client.dispose(); client.close() }
let channel = client.channels.get("test")
Expand Down Expand Up @@ -3709,7 +3698,6 @@ class RealtimeClientConnection: QuickSpec {
// RTN19b
it("should resent the ATTACH message if there are any pending channels") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 0.1
let client = AblyTests.newRealtime(options)
defer { client.dispose(); client.close() }
let channel = client.channels.get("test")
Expand Down Expand Up @@ -3742,7 +3730,6 @@ class RealtimeClientConnection: QuickSpec {
// RTN19b
it("should resent the DETACH message if there are any pending channels") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.0
let client = AblyTests.newRealtime(options)
defer { client.dispose(); client.close() }
let channel = client.channels.get("test")
Expand Down
4 changes: 0 additions & 4 deletions Spec/RealtimeClientPresence.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@ class RealtimeClientPresence: QuickSpec {
let membersCount = 110

let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.0
var clientSecondary: ARTRealtime!
defer { clientSecondary.dispose(); clientSecondary.close() }

Expand Down Expand Up @@ -690,7 +689,6 @@ class RealtimeClientPresence: QuickSpec {
}
defer { clientMembers?.dispose(); clientMembers?.close() }

options.disconnectedRetryTimeout = 1.0
options.tokenDetails = getTestTokenDetails(key: options.key!, ttl: 5.0)
let client = AblyTests.newRealtime(options)
defer { client.dispose(); client.close() }
Expand Down Expand Up @@ -3171,7 +3169,6 @@ class RealtimeClientPresence: QuickSpec {
// RTP16b
it("all presence messages will be queued and delivered as soon as the connection state returns to CONNECTED") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.0
let client = ARTRealtime(options: options)
defer { client.dispose(); client.close() }
let channel = client.channels.get("test")
Expand Down Expand Up @@ -3199,7 +3196,6 @@ class RealtimeClientPresence: QuickSpec {
// RTP16b
it("all presence messages will be lost if queueMessages has been explicitly set to false") {
let options = AblyTests.commonAppSetup()
options.disconnectedRetryTimeout = 1.0
options.queueMessages = false
let client = ARTRealtime(options: options)
defer { client.dispose(); client.close() }
Expand Down