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

1817 revise rtn14d terminal errors #1818

Closed
wants to merge 5 commits into from
Closed

1817 revise rtn14d terminal errors #1818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c77f95f
Improve readability of error mapping
lawrence-forooghian Nov 3, 2023
9a81bc5
Make creation of ARTRealtimeTransportError a bit more readable
lawrence-forooghian Nov 3, 2023
c16c7e9
Document validity of ARTRealtimeTransportError badResponseCode
lawrence-forooghian Nov 3, 2023
373211c
Explain approach of fallback host test
lawrence-forooghian Nov 6, 2023
561ea45
Treat all transport errors as recoverable to see what tests fail
lawrence-forooghian Nov 3, 2023
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
30 changes: 10 additions & 20 deletions Source/ARTRealtime.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1629,21 +1629,10 @@ - (void)realtimeTransportFailed:(id<ARTRealtimeTransport>)transport withError:(A
return;
}
}

switch (transportError.type) {
case ARTRealtimeTransportErrorTypeBadResponse:
case ARTRealtimeTransportErrorTypeOther: {
ARTErrorInfo *const errorInfo = [ARTErrorInfo createFromNSError:transportError.error];
ARTConnectionStateChangeMetadata *const metadata = [[ARTConnectionStateChangeMetadata alloc] initWithErrorInfo:errorInfo];
[self transition:ARTRealtimeFailed withMetadata:metadata];
break;
}
default: {
ARTErrorInfo *error = [ARTErrorInfo createFromNSError:transportError.error];
ARTConnectionStateChangeMetadata *const metadata = [[ARTConnectionStateChangeMetadata alloc] initWithErrorInfo:error];
[self transitionToDisconnectedOrSuspendedWithMetadata:metadata];
}
}

ARTErrorInfo *const errorInfo = [ARTErrorInfo createFromNSError:transportError.error];
ARTConnectionStateChangeMetadata *const metadata = [[ARTConnectionStateChangeMetadata alloc] initWithErrorInfo:errorInfo];
[self transitionToDisconnectedOrSuspendedWithMetadata:metadata];
}

- (void)realtimeTransportNeverConnected:(id<ARTRealtimeTransport>)transport {
Expand All @@ -1654,7 +1643,7 @@ - (void)realtimeTransportNeverConnected:(id<ARTRealtimeTransport>)transport {

ARTErrorInfo *const errorInfo = [ARTErrorInfo createWithCode:ARTClientCodeErrorTransport message:@"Transport never connected"];
ARTConnectionStateChangeMetadata *const metadata = [[ARTConnectionStateChangeMetadata alloc] initWithErrorInfo:errorInfo];
[self transition:ARTRealtimeFailed withMetadata:metadata];
[self transitionToDisconnectedOrSuspendedWithMetadata:metadata];
}

- (void)realtimeTransportRefused:(id<ARTRealtimeTransport>)transport withError:(ARTRealtimeTransportError *)error {
Expand All @@ -1666,15 +1655,16 @@ - (void)realtimeTransportRefused:(id<ARTRealtimeTransport>)transport withError:(
if (error && error.type == ARTRealtimeTransportErrorTypeRefused) {
ARTErrorInfo *const errorInfo = [ARTErrorInfo createWithCode:ARTClientCodeErrorTransport message:[NSString stringWithFormat:@"Connection refused using %@", error.url]];
ARTConnectionStateChangeMetadata *const metadata = [[ARTConnectionStateChangeMetadata alloc] initWithErrorInfo:errorInfo];
[self transition:ARTRealtimeFailed withMetadata:metadata];
[self transitionToDisconnectedOrSuspendedWithMetadata:metadata];
}
else if (error) {
ARTErrorInfo *const errorInfo = [ARTErrorInfo createFromNSError:error.error];
ARTConnectionStateChangeMetadata *const metadata = [[ARTConnectionStateChangeMetadata alloc] initWithErrorInfo:errorInfo];
[self transition:ARTRealtimeFailed withMetadata:metadata];
[self transitionToDisconnectedOrSuspendedWithMetadata:metadata];
}
else {
[self transition:ARTRealtimeFailed withMetadata:[[ARTConnectionStateChangeMetadata alloc] init]];
ARTConnectionStateChangeMetadata *const metadata = [[ARTConnectionStateChangeMetadata alloc] init];
[self transitionToDisconnectedOrSuspendedWithMetadata:metadata];
}
}

Expand All @@ -1686,7 +1676,7 @@ - (void)realtimeTransportTooBig:(id<ARTRealtimeTransport>)transport {

ARTErrorInfo *const errorInfo = [ARTErrorInfo createWithCode:ARTClientCodeErrorTransport message:@"Transport too big"];
ARTConnectionStateChangeMetadata *const metadata = [[ARTConnectionStateChangeMetadata alloc] initWithErrorInfo:errorInfo];
[self transition:ARTRealtimeFailed withMetadata:metadata];
[self transitionToDisconnectedOrSuspendedWithMetadata:metadata];
}

- (void)realtimeTransportSetMsgSerial:(id<ARTRealtimeTransport>)transport msgSerial:(int64_t)msgSerial {
Expand Down
24 changes: 17 additions & 7 deletions Source/ARTWebSocketTransport.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,20 +282,30 @@ - (void)webSocket:(id<ARTWebSocket>)webSocket didCloseWithCode:(NSInteger)code r
[_delegate realtimeTransportDisconnected:self withError:nil];
break;
case ARTWsRefuse:
case ARTWsPolicyValidation:
[_delegate realtimeTransportRefused:self withError:[[ARTRealtimeTransportError alloc] initWithError:[ARTErrorInfo createWithCode:code message:reason] type:ARTRealtimeTransportErrorTypeRefused url:self.websocketURL]];
case ARTWsPolicyValidation: {
ARTErrorInfo *const errorInfo = [ARTErrorInfo createWithCode:code message:reason];
ARTRealtimeTransportError *const error = [[ARTRealtimeTransportError alloc] initWithError:errorInfo
type:ARTRealtimeTransportErrorTypeRefused
url:self.websocketURL];
[_delegate realtimeTransportRefused:self withError:error];
break;
}
case ARTWsTooBig:
[_delegate realtimeTransportTooBig:self];
break;
case ARTWsNoUtf8:
case ARTWsCloseProtocolError:
case ARTWsUnexpectedCondition:
case ARTWsExtension:
case ARTWsTlsError:
case ARTWsTlsError: {
// Failed
[_delegate realtimeTransportFailed:self withError:[[ARTRealtimeTransportError alloc] initWithError:[ARTErrorInfo createWithCode:code message:reason] type:ARTRealtimeTransportErrorTypeOther url:self.websocketURL]];
ARTErrorInfo *const errorInfo = [ARTErrorInfo createWithCode:code message:reason];
ARTRealtimeTransportError *const error = [[ARTRealtimeTransportError alloc] initWithError:errorInfo
type:ARTRealtimeTransportErrorTypeOther
url:self.websocketURL];
[_delegate realtimeTransportFailed:self withError:error];
break;
}
default:
NSAssert(true, @"WebSocket close: unknown code");
break;
Expand All @@ -317,11 +327,11 @@ - (ARTRealtimeTransportError *)classifyError:(NSError *)error {

if ([error.domain isEqualToString:@"com.squareup.SocketRocket"] && error.code == 504) {
type = ARTRealtimeTransportErrorTypeTimeout;
} else if ([error.domain isEqualToString:(NSString *)kCFErrorDomainCFNetwork]) {
} else if (error.domain == (NSString *)kCFErrorDomainCFNetwork) {
type = ARTRealtimeTransportErrorTypeHostUnreachable;
} else if ([error.domain isEqualToString:@"NSPOSIXErrorDomain"] && (error.code == 57 || error.code == 50)) {
} else if (error.domain == NSPOSIXErrorDomain && (error.code == ENOTCONN || error.code == ENETDOWN)) {
type = ARTRealtimeTransportErrorTypeNoInternet;
} else if ([error.domain isEqualToString:ARTSRWebSocketErrorDomain] && error.code == 2132) {
} else if (error.domain == ARTSRWebSocketErrorDomain && error.code == 2132) {
id status = error.userInfo[ARTSRHTTPResponseErrorKey];
if (status) {
return [[ARTRealtimeTransportError alloc] initWithError:error
Expand Down
3 changes: 3 additions & 0 deletions Source/PrivateHeaders/Ably/ARTRealtimeTransport.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@ typedef NS_ENUM(NSUInteger, ARTRealtimeTransportState) {

@property (nonatomic) NSError *error;
@property (nonatomic) ARTRealtimeTransportErrorType type;
/**
This meaning of this property is only defined if the error is of type `ARTRealtimeTransportErrorTypeBadResponse`.
*/
@property (nonatomic) NSInteger badResponseCode;
@property (nonatomic) NSURL *url;

Expand Down
10 changes: 6 additions & 4 deletions Test/Test Utilities/TestUtilities.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ enum FakeNetworkResponse {
var error: NSError {
switch self {
case .noInternet:
return NSError(domain: NSPOSIXErrorDomain, code: 50, userInfo: [NSLocalizedDescriptionKey: "network is down", NSLocalizedFailureReasonErrorKey: AblyTestsErrorDomain + ".FakeNetworkResponse"])
return NSError(domain: NSPOSIXErrorDomain, code: Int(ENETDOWN), userInfo: [NSLocalizedDescriptionKey: "network is down", NSLocalizedFailureReasonErrorKey: AblyTestsErrorDomain + ".FakeNetworkResponse"])
case .hostUnreachable:
return NSError(domain: kCFErrorDomainCFNetwork as String, code: 2, userInfo: [NSLocalizedDescriptionKey: "host unreachable", NSLocalizedFailureReasonErrorKey: AblyTestsErrorDomain + ".FakeNetworkResponse"])
case .requestTimeout:
Expand Down Expand Up @@ -1658,9 +1658,11 @@ extension ARTWebSocketTransport {
}

func simulateIncomingError() {
let error = NSError(domain: ARTAblyErrorDomain, code: 0, userInfo: [NSLocalizedDescriptionKey:"Fail test"])
let webSocketDelegate = self as ARTWebSocketDelegate
webSocketDelegate.webSocket?(self.websocket!, didFailWithError: error)
// Simulate receiving an ERROR ProtocolMessage, which should put a client into the FAILED state (per RTN15i)
let protocolMessage = ARTProtocolMessage()
protocolMessage.action = .error
protocolMessage.error = ARTErrorInfo.create(withCode: 50000 /* arbitrarily chosen */, message: "Fail test")
receive(protocolMessage)
}
}

Expand Down
49 changes: 29 additions & 20 deletions Test/Tests/RealtimeClientConnectionTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3670,52 +3670,61 @@ class RealtimeClientConnectionTests: XCTestCase {
testUsesAlternativeHostOnResponse(.hostInternalError(code: 501), channelName: test.uniqueChannelName())
}

func test__100__Connection__Host_Fallback__should_move_to_disconnected_when_there_s_no_internet__with_NSPOSIXErrorDomain_with_code_57() throws {
func test__100__Connection__Host_Fallback__should_move_to_disconnected_when_there_s_no_internet__with_NSPOSIXErrorDomain_with_code_ENOTCONN() throws {
let test = Test()
try testMovesToDisconnectedWithNetworkingError(NSError(domain: "NSPOSIXErrorDomain", code: 57, userInfo: [NSLocalizedDescriptionKey: "shouldn't matter"]), for: test)
try testMovesToDisconnectedWithNetworkingError(NSError(domain: NSPOSIXErrorDomain, code: Int(ENOTCONN), userInfo: [NSLocalizedDescriptionKey: "shouldn't matter"]), for: test)
}

func test__101__Connection__Host_Fallback__should_move_to_disconnected_when_there_s_no_internet__with_NSPOSIXErrorDomain_with_code_50() throws {
func test__101__Connection__Host_Fallback__should_move_to_disconnected_when_there_s_no_internet__with_NSPOSIXErrorDomain_with_code_ENETDOWN() throws {
let test = Test()
try testMovesToDisconnectedWithNetworkingError(NSError(domain: "NSPOSIXErrorDomain", code: 50, userInfo: [NSLocalizedDescriptionKey: "shouldn't matter"]), for: test)
try testMovesToDisconnectedWithNetworkingError(NSError(domain: NSPOSIXErrorDomain, code: Int(ENETDOWN), userInfo: [NSLocalizedDescriptionKey: "shouldn't matter"]), for: test)
}

func test__102__Connection__Host_Fallback__should_move_to_disconnected_when_there_s_no_internet__with_any_kCFErrorDomainCFNetwork() throws {
let test = Test()
try testMovesToDisconnectedWithNetworkingError(NSError(domain: "kCFErrorDomainCFNetwork", code: 1337, userInfo: [NSLocalizedDescriptionKey: "shouldn't matter"]), for: test)
try testMovesToDisconnectedWithNetworkingError(NSError(domain: kCFErrorDomainCFNetwork as String, code: 1337, userInfo: [NSLocalizedDescriptionKey: "shouldn't matter"]), for: test)
}

func test__090__Connection__Host_Fallback__should_not_use_an_alternative_host_when_the_client_receives_a_bad_request() {
let test = Test()
func test__090__Connection__Host_Fallback__should_not_use_an_alternative_host_when_the_client_receives_a_bad_request() throws {
let options = ARTClientOptions(key: "xxxx:xxxx")
options.autoConnect = false
options.disconnectedRetryTimeout = 1.0 // so that the test doesn't have to wait a long time to observe a retry
options.testOptions.realtimeRequestTimeout = 1.0
let transportFactory = TestProxyTransportFactory()
options.testOptions.transportFactory = transportFactory
let client = ARTRealtime(options: options)
let channel = client.channels.get(test.uniqueChannelName())

transportFactory.fakeNetworkResponse = .host400BadRequest

var urlConnections = [URL]()
transportFactory.networkConnectEvent = { transport, url in
if client.internal.transport !== transport {
return
let dataGatherer = DataGatherer(description: "Observe emitted state changes and transport connection attempts") { submit in
var stateChanges: [ARTConnectionStateChange] = []
var urlConnections = [URL]()

client.connection.on { stateChange in
stateChanges.append(stateChange)
if (stateChanges.count == 3) {
submit((stateChanges: stateChanges, urlConnections: urlConnections))
}
}

transportFactory.networkConnectEvent = { transport, url in
if client.internal.transport !== transport {
return
}
urlConnections.append(url)
}
urlConnections.append(url)
}

client.connect()
defer { client.dispose(); client.close() }

waitUntil(timeout: testTimeout) { done in
channel.publish(nil, data: "message") { _ in
done()
}
}
let data = try dataGatherer.waitForData(timeout: testTimeout)

XCTAssertEqual(urlConnections.count, 1)
XCTAssertTrue(NSRegularExpression.match(urlConnections[0].absoluteString, pattern: "//realtime.ably.io"))
// We expect the first connection attempt to fail due to the .fakeNetworkResponse configured above. This error does not meet the criteria for trying a fallback host, and so should not provoke the use of a fallback host. Hence the connection should transition to DISCONNECTED, and then subsequently retry, transitioning back to CONNECTING. We should see that there were two connection attempts, both to the primary host.

XCTAssertEqual(data.stateChanges.map(\.current), [.connecting, .disconnected, .connecting])
XCTAssertEqual(data.urlConnections.count, 2)
XCTAssertTrue(data.urlConnections.allSatisfy { url in NSRegularExpression.match(url.absoluteString, pattern: "//realtime.ably.io") })
}

// RTN17a
Expand Down
Loading