Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 6 additions & 5 deletions Source/ARTJsonLikeEncoder.m
Original file line number Diff line number Diff line change
Expand Up @@ -1129,11 +1129,12 @@ - (ARTErrorInfo *)decodeErrorInfo:(NSData *)artError statusCode:(NSInteger)statu
status:[(statusNumber ?: @(statusCode)) intValue]
message:message ?: [NSString stringWithFormat:@"HTTP request failed with status code %ld", statusCode]];
} else {
// We expect `decodedError` as a dictionary from Ably REST API, but in case user sets custom authUrl in the auth options, it can be anything
// We'll address this in an upcoming proper fix for https://github.com/ably/ably-cocoa/issues/2135
return [ARTErrorInfo createWithCode:statusCode * 100
status:statusCode
message:[NSString stringWithFormat:@"HTTP request failed with status code %ld", statusCode]];
// We expect `decodedError` as a dictionary from Ably REST API
// In case it's something else we set the decoding error
*error = [ARTErrorInfo createWithCode:ARTClientCodeErrorInvalidType
status:0
message:[NSString stringWithFormat:@"Failed to decode error dictionary from the provided error data."]];
return nil;
}
}

Expand Down
27 changes: 15 additions & 12 deletions Source/ARTRealtime.m
Original file line number Diff line number Diff line change
@@ -1,13 +1,8 @@
//
// ARTRealtime.m
//
//

#import "ARTRealtime+Private.h"
#import "ARTRealtime+WrapperSDKProxy.h"

#import "ARTRealtimeChannel+Private.h"
#import "ARTStatus.h"
#import "ARTErrorInfo+Private.h"
#import "ARTDefault.h"
#import "ARTRest+Private.h"
#import "ARTAuth+Private.h"
Expand Down Expand Up @@ -1073,10 +1068,14 @@ - (void)onConnectionTimeOut {

ARTErrorInfo *error;
if (self.auth.authorizing_nosync && (self.options.authUrl || self.options.authCallback)) {
error = [ARTErrorInfo createWithCode:ARTErrorAuthConfiguredProviderFailure status:ARTStateConnectionFailed message:@"timed out"];
error = [ARTErrorInfo createWithCode:ARTErrorAuthConfiguredProviderFailure
status:ARTHttpStatusCodeFromErrorCode(ARTErrorUnauthorized) // RSA4c1
message:@"timed out"];
}
else {
error = [ARTErrorInfo createWithCode:ARTErrorConnectionTimedOut status:ARTStateConnectionFailed message:@"timed out"];
error = [ARTErrorInfo createWithCode:ARTErrorConnectionTimedOut
status:ARTStateConnectionFailed
message:@"timed out"];
}
switch (self.connection.state_nosync) {
case ARTRealtimeConnected: {
Expand Down Expand Up @@ -1195,19 +1194,23 @@ - (void)handleTokenAuthError:(NSError *)error {
else if (self.options.authUrl || self.options.authCallback) {
if (error.code == ARTErrorForbidden /* RSA4d */) {
ARTErrorInfo *errorInfo = [ARTErrorInfo createWithCode:ARTErrorAuthConfiguredProviderFailure
status:error.artStatusCode
message:error.description];
status:ARTHttpStatusCodeFromErrorCode(ARTErrorForbidden)
message:error.description
underlyingError:error];
ARTConnectionStateChangeParams *const params = [[ARTConnectionStateChangeParams alloc] initWithErrorInfo:errorInfo];
[self performTransitionToState:ARTRealtimeFailed withParams:params];
} else {
ARTErrorInfo *errorInfo = [ARTErrorInfo createWithCode:ARTErrorAuthConfiguredProviderFailure status:ARTStateConnectionFailed message:error.description];
ARTErrorInfo *errorInfo = [ARTErrorInfo createWithCode:ARTErrorAuthConfiguredProviderFailure
status:ARTHttpStatusCodeFromErrorCode(ARTErrorUnauthorized)
message:error.description
underlyingError:error];
switch (self.connection.state_nosync) {
case ARTRealtimeConnected:
// RSA4c3
[self.connection setErrorReason:errorInfo];
break;
default: {
// RSA4c
// RSA4c1, RSA4c2
ARTConnectionStateChangeParams *const params = [[ARTConnectionStateChangeParams alloc] initWithErrorInfo:errorInfo];
[self performTransitionToDisconnectedOrSuspendedWithParams:params];
break;
Expand Down
67 changes: 44 additions & 23 deletions Source/ARTRest.m
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
#import "ARTRest+Private.h"

#import "ARTErrorInfo+Private.h"
#import "ARTChannel+Private.h"
#import "ARTRestChannels+Private.h"
#import "ARTDataQuery+Private.h"
Expand Down Expand Up @@ -335,6 +335,32 @@ - (NSString *)agentIdentifierWithWrapperSDKAgents:(nullable NSDictionary<NSStrin
return [ARTClientInformation agentIdentifierWithAdditionalAgents:additionalAgents];
}

// Checks if the response contains any Ably header
- (BOOL)isAblyResponse:(NSHTTPURLResponse *)response {
for (NSString* key in response.allHeaderFields) {
if ([key.lowercaseString containsString:@"-ably"]) {
return true;
}
}
return false;
}

// Checks if the response contains any expected content type
- (BOOL)hasValidContentType:(NSHTTPURLResponse *)response {
NSString *contentType = [response.allHeaderFields objectForKey:@"Content-Type"];
for (NSString *mimeType in [self->_encoders.allValues valueForKeyPath:@"mimeType"]) {
if ([contentType containsString:mimeType]) {
return true;
}
}
return false;
}

// Checks if the auth request was made with the user's authUrl
- (BOOL)isCustomAuthRequest:(NSURLRequest *)request {
return [request.URL.host isEqualToString:self.options.authUrl.host];
}

/**
originalRequestId is used only for fallback requests. It should never be used to execute request by yourself, it's passed from within below method.
*/
Expand Down Expand Up @@ -383,40 +409,34 @@ - (NSString *)agentIdentifierWithWrapperSDKAgents:(nullable NSDictionary<NSStrin
}
}


ARTLogDebug(self.logger, @"RS:%p executing request %@", self, request);
__block NSObject<ARTCancellable> *task;
task = [self.httpExecutor executeRequest:request completion:^(NSHTTPURLResponse *response, NSData *data, NSError *error) {
// Error messages in plaintext and HTML format (only if the URL request is different than `options.authUrl` and we don't have an error already)
if (error == nil && data != nil && data.length != 0 && ![request.URL.host isEqualToString:[self.options.authUrl host]]) {
NSString *contentType = [response.allHeaderFields objectForKey:@"Content-Type"];

BOOL validContentType = NO;
for (NSString *mimeType in [self->_encoders.allValues valueForKeyPath:@"mimeType"]) {
if ([contentType containsString:mimeType]) {
validContentType = YES;
break;
}
}

if (!validContentType) {
NSString *plain = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding];
if (error == nil && data != nil && data.length != 0 && ![self isCustomAuthRequest:request]) {
Copy link
Copy Markdown
Collaborator

@lawrence-forooghian lawrence-forooghian Dec 4, 2025

Choose a reason for hiding this comment

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

I haven't looked at this PR in detail but my overall thoughts are that we should not have a single method that's used for both authURL requests and normal Ably REST requests; the handling of the two is sufficiently different that it just ends up making one very complicated method, and there are other behaviours that this method implements that don't apply to auth URL requests, e.g. fallback hosts. It's also kind of odd to look at the request's URL to check whether you were making a request to the authURL; we should know this upfront given that we're the ones creating the request!

if it helps, here's how the WIP ably-swift does it:

I'm not saying we should copy that exactly but I think it would be good not to try and handle everything in one place.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'll do refactoring in a different PR.

if (![self hasValidContentType:response]) {
// Construct artificial error
NSString *plain = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding];
error = [ARTErrorInfo createWithCode:response.statusCode * 100
status:response.statusCode
message:[plain art_shortString] requestId:requestId];
message:[plain art_shortString]
requestId:requestId];
data = nil; // Discard data; format is unreliable.
ARTLogError(self.logger, @"Request %@ failed with %@", request, error);
}
}

if (response.statusCode >= 400) {
if (data) {
if (data && data.length != 0) {
NSError *decodeError = nil;
ARTErrorInfo *dataError = [self->_encoders[response.MIMEType] decodeErrorInfo:data
statusCode:response.statusCode
error:&decodeError];
if ([self shouldRenewToken:&dataError] && [request isKindOfClass:[NSMutableURLRequest class]]) {
ARTErrorInfo *dataError = nil;
id<ARTEncoder> encoder = self->_encoders[response.MIMEType];

if (encoder && [self isAblyResponse:response]) {
dataError = [encoder decodeErrorInfo:data statusCode:response.statusCode error:&decodeError];
}

if (dataError && [self shouldRenewToken:&dataError] && [request isKindOfClass:[NSMutableURLRequest class]]) {
ARTLogDebug(self.logger, @"RS:%p retry request %@", self, request);
// Make a single attempt to reissue the token and resend the request
if (self->_tokenErrorRetries < 1) {
Expand All @@ -432,10 +452,11 @@ - (NSString *)agentIdentifierWithWrapperSDKAgents:(nullable NSDictionary<NSStrin
}
}
if (!error) {
// Return error with HTTP StatusCode if ARTErrorStatusCode does not exist
// Construct artificial error
NSString *plain = data != nil && data.length != 0 ? [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding] : [NSString stringWithFormat:@"HTTP request failed with status code %ld", response.statusCode];
error = [ARTErrorInfo createWithCode:response.statusCode * 100
status:response.statusCode
message:[[NSString alloc] initWithData:data ?: [NSData data] encoding:NSUTF8StringEncoding]
message:[plain art_shortString]
requestId:requestId];
}

Expand Down
30 changes: 17 additions & 13 deletions Source/ARTStatus.m
Original file line number Diff line number Diff line change
Expand Up @@ -15,41 +15,46 @@

NSString *const ARTAblyMessageNoMeansToRenewToken = @"no means to renew the token is provided (either an API key, authCallback or authUrl)";

NSInteger getStatusFromCode(NSInteger code) {
return code / 100;
NSInteger ARTHttpStatusCodeFromErrorCode(ARTErrorCode errorCode) {
return errorCode / 100;
}

@implementation ARTErrorInfo

+ (ARTErrorInfo *)createWithCode:(NSInteger)code message:(NSString *)message requestId:(nullable NSString *)requestId {
return [ARTErrorInfo createWithCode:code status:getStatusFromCode(code) message:message requestId:requestId];
return [ARTErrorInfo createWithCode:code status:ARTHttpStatusCodeFromErrorCode(code) message:message requestId:requestId];
}

+ (ARTErrorInfo *)createWithCode:(NSInteger)code message:(NSString *)message requestId:(nullable NSString *)requestId additionalUserInfo:(nullable NSDictionary<NSString *, id> *)additionalUserInfo {
return [ARTErrorInfo createWithCode:code status:getStatusFromCode(code) message:message requestId:requestId additionalUserInfo:additionalUserInfo];
return [ARTErrorInfo createWithCode:code status:ARTHttpStatusCodeFromErrorCode(code) message:message requestId:requestId additionalUserInfo:additionalUserInfo underlyingError:nil];
}

+ (ARTErrorInfo *)createWithCode:(NSInteger)code message:(NSString *)message {
return [ARTErrorInfo createWithCode:code status:getStatusFromCode(code) message:message requestId:nil];
return [ARTErrorInfo createWithCode:code status:ARTHttpStatusCodeFromErrorCode(code) message:message requestId:nil];
}

+ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message underlyingError:(NSError *)underlyingError {
return [ARTErrorInfo createWithCode:code status:status message:message requestId:nil additionalUserInfo:nil underlyingError:underlyingError];
}

+ (ARTErrorInfo *)createWithCode:(NSInteger)code message:(NSString *)message additionalUserInfo:(nullable NSDictionary<NSString *, id> *)additionalUserInfo {
return [ARTErrorInfo createWithCode:code status:getStatusFromCode(code) message:message requestId:nil additionalUserInfo:additionalUserInfo];
return [ARTErrorInfo createWithCode:code status:ARTHttpStatusCodeFromErrorCode(code) message:message requestId:nil additionalUserInfo:additionalUserInfo underlyingError:nil];
}

+ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message additionalUserInfo:(nullable NSDictionary<NSString *, id> *)additionalUserInfo {
return [ARTErrorInfo createWithCode:code status:status message:message requestId:nil additionalUserInfo:additionalUserInfo];
return [ARTErrorInfo createWithCode:code status:status message:message requestId:nil additionalUserInfo:additionalUserInfo underlyingError:nil];
}

+ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message requestId:(nullable NSString *)requestId {
return [ARTErrorInfo createWithCode:code status:status message:message requestId:requestId additionalUserInfo:nil];
return [ARTErrorInfo createWithCode:code status:status message:message requestId:requestId additionalUserInfo:nil underlyingError:nil];
}

+ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message requestId:(nullable NSString *)requestId additionalUserInfo:(nullable NSDictionary<NSString *, id> *)additionalUserInfo {
+ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message requestId:(nullable NSString *)requestId additionalUserInfo:(nullable NSDictionary<NSString *, id> *)additionalUserInfo underlyingError:(NSError *)underlyingError {
NSMutableDictionary *userInfo = [NSMutableDictionary new];
userInfo[ARTErrorInfoStatusCodeKey] = [NSNumber numberWithInteger:status];
userInfo[NSLocalizedDescriptionKey] = message;
userInfo[ARTErrorInfoRequestIdKey] = requestId;
userInfo[NSUnderlyingErrorKey] = underlyingError;

// Add any additional userInfo values
if (additionalUserInfo) {
Expand Down Expand Up @@ -132,9 +137,9 @@ - (NSInteger)statusCode {

- (NSString *)description {
if (self.reason != nil) {
return [NSString stringWithFormat:@"Error %ld - %@ (reason: %@)", (long)self.code, self.message ?: @"<Empty Message>", self.reason];
return [NSString stringWithFormat:@"Error %ld (status: %ld) - %@ (reason: %@)", (long)self.code, (long)self.statusCode, self.message ?: @"<Empty Message>", self.reason];
} else {
return [NSString stringWithFormat:@"Error %ld - %@", (long)self.code, self.message ?: @"<Empty Message>"];
return [NSString stringWithFormat:@"Error %ld (status: %ld) - %@", (long)self.code, (long)self.statusCode, self.message ?: @"<Empty Message>"];
}
}

Expand All @@ -153,7 +158,6 @@ - (NSString *)requestId {
@end



@implementation ARTStatus

- (instancetype)init {
Expand Down Expand Up @@ -184,7 +188,7 @@ - (NSString *)description {

#pragma mark private

-(void) setErrorInfo:(ARTErrorInfo *)errorInfo {
- (void)setErrorInfo:(ARTErrorInfo *)errorInfo {
_errorInfo = errorInfo;
}

Expand Down
3 changes: 3 additions & 0 deletions Source/PrivateHeaders/Ably/ARTErrorInfo+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ NS_ASSUME_NONNULL_BEGIN
@end
#endif

// Gets an HTTP status code from error code by taking first three digits. Doesn't perform any checks if the result is a valid status code.
NSInteger ARTHttpStatusCodeFromErrorCode(ARTErrorCode errorCode);

NS_ASSUME_NONNULL_END
5 changes: 4 additions & 1 deletion Source/include/Ably/ARTStatus.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,9 @@ FOUNDATION_EXPORT NSString *const ARTAblyMessageNoMeansToRenewToken;
/// :nodoc:
+ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message;

/// :nodoc:
+ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(nullable NSString *)message underlyingError:(nullable NSError *)error;

/// :nodoc:
+ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message additionalUserInfo:(nullable NSDictionary<NSString *, id> *)additionalUserInfo;

Expand All @@ -250,7 +253,7 @@ FOUNDATION_EXPORT NSString *const ARTAblyMessageNoMeansToRenewToken;
+ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message requestId:(nullable NSString *)requestId;

/// :nodoc:
+ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message requestId:(nullable NSString *)requestId additionalUserInfo:(nullable NSDictionary<NSString *, id> *)additionalUserInfo;
+ (ARTErrorInfo *)createWithCode:(NSInteger)code status:(NSInteger)status message:(NSString *)message requestId:(nullable NSString *)requestId additionalUserInfo:(nullable NSDictionary<NSString *, id> *)additionalUserInfo underlyingError:(nullable NSError *)underlyingError;

/// :nodoc:
+ (ARTErrorInfo *)createFromNSException:(NSException *)error requestId:(nullable NSString *)requestId;
Expand Down
21 changes: 21 additions & 0 deletions Test/AblyTests/Tests/AuthTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,27 @@ class AuthTests: XCTestCase {

XCTAssertEqual(realtime.connection.state, ARTRealtimeConnectionState.connected)
}

// RSA4c2 (with 404, see https://github.com/ably/ably-cocoa/issues/2135)
func test__040__Token__authentication_method__if_a_request_by_a_realtime_client_to_an_authUrl_results_in_an_HTTP_404_the_client_library_should_transition_to_the_DISCONNECTED_state() throws {
let test = Test()
let options = try AblyTests.clientOptions(for: test)
options.autoConnect = false
options.authUrl = URL(string: "https://api.restful-api.dev/objects/\(UUID())")! // random address to get 404
let realtime = ARTRealtime(options: options)
defer { realtime.dispose(); realtime.close() }

waitUntil(timeout: testTimeout) { done in
realtime.connection.once(.disconnected) { stateChange in
XCTAssertEqual(stateChange.reason?.code, ARTErrorCode.authConfiguredProviderFailure.intValue)
XCTAssertEqual(stateChange.reason?.statusCode, 401)
XCTAssertEqual(stateChange.reason?.cause?.statusCode, 404)
XCTAssertEqual(realtime.connection.errorReason, stateChange.reason)
done()
}
realtime.connect()
}
}

// RSA15

Expand Down
9 changes: 4 additions & 5 deletions Test/AblyTests/Tests/UtilitiesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ class UtilitiesTests: XCTestCase {
}
}

func test__Utilities__JSON_Encoder__should_decode_rest_error_response_with_only_error_field() throws {
func test__Utilities__JSON_Encoder__should_not_decode_rest_http_error_if_it_doesnt_contains_error_dictionary_under_error_key() throws {
let test = Test()
beforeEach__Utilities__JSON_Encoder()

Expand All @@ -263,15 +263,14 @@ class UtilitiesTests: XCTestCase {
guard let error = error as? ARTErrorInfo else {
fail("Should be ARTErrorInfo"); done(); return
}
XCTAssertTrue(error.code == 40400)
XCTAssertTrue(error.statusCode == 404)
XCTAssertTrue(error.message == "HTTP request failed with status code 404")
XCTAssertTrue(error.code == ARTClientCodeError.invalidType.rawValue)
XCTAssertTrue(error.message == "Failed to decode error dictionary from the provided error data.")
done()
})
}
}

func test__Utilities__JSON_Encoder__should_decode_rest_error_response_with_complete_error_info() throws {
func test__Utilities__JSON_Encoder__should_decode_rest_http_error_if_it_contains_error_dictionary_under_error_key() throws {
let test = Test()
beforeEach__Utilities__JSON_Encoder()

Expand Down
Loading