Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
45 changes: 26 additions & 19 deletions FirebaseMessaging/Sources/FIRMessaging.m
Original file line number Diff line number Diff line change
Expand Up @@ -578,14 +578,14 @@ - (NSString *)FCMToken {
- (void)retrieveFCMTokenForSenderID:(nonnull NSString *)senderID
completion:(nonnull FIRMessagingFCMTokenFetchCompletion)completion {
if (!senderID.length) {
FIRMessagingLoggerError(kFIRMessagingMessageCodeSenderIDNotSuppliedForTokenFetch,
@"Sender ID not supplied. It is required for a token fetch, "
@"to identify the sender.");
NSString *description = @"Couldn't fetch token because a Sender ID was not supplied. A valid "
@"Sender ID is required to fetch an FCM token";
FIRMessagingLoggerError(kFIRMessagingMessageCodeSenderIDNotSuppliedForTokenFetch, @"%@",
description);
if (completion) {
NSString *description = @"Couldn't fetch token because a Sender ID was not supplied. A valid "
@"Sender ID is required to fetch an FCM token";
NSError *error = [NSError fcm_errorWithCode:FIRMessagingErrorInvalidRequest
userInfo:@{NSLocalizedDescriptionKey : description}];
NSError *error = [NSError
messagingErrorWithCode:(FIRMessagingInternalErrorCode)FIRMessagingErrorInvalidRequest
Copy link
Contributor

Choose a reason for hiding this comment

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

Casting to FIRMessagingInternalErrorCode doesn't look nice and leads to a couple questions:

  1. If we don't care of the actual code why we require it in the method interface
  2. If we do care of the code, then after casting from an unrelated enum the code may actually change meaning.

I wonder if there is a reason for this type casting?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. I guess we don't have to require in the method interface, so should I just use NSUInteger?
  2. This was the old structure where somehow we kept two list of error codes, an internal and an external. My guess is adding an external requires API change, also external one is the one will be exposed to external API. I also notice the internal one makes a copy of the external one for easy casting.

I think we should deprecate the external error codes as they don't really mean anything to users, especially once failure reason is set and shown, the error doesn't even show the error code anymore, which is much clearer to users.
WDYT?

Copy link
Contributor

@maksymmalyhin maksymmalyhin Apr 30, 2020

Choose a reason for hiding this comment

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

Specific codes for specific errors can be useful for the case when developers can actually react to them. Taking this into account I would suggest to reduce amount of public error codes to the cases that can be handled differently. Ideally, the public codes should go through API review.

As for NSUInteger, I'm a bit hesitant doing it since usually error codes are meaningful within an error domain. Can we just use a code from FIRMessagingInternalErrorCode enum so we don't have to cast and can be sure that the code is interpreted properly in the proper domain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean using internal code, but has a public code with the same value to represent the meaning of it?

Because if we use internal code, we are not actually using the public one at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated to use internal one only (renamed to FIRMessagingErrorCode)

failureReason:description];
completion(nil, error);
}
return;
Expand All @@ -610,13 +610,14 @@ - (void)retrieveFCMTokenForSenderID:(nonnull NSString *)senderID
- (void)deleteFCMTokenForSenderID:(nonnull NSString *)senderID
completion:(nonnull FIRMessagingDeleteFCMTokenCompletion)completion {
if (!senderID.length) {
FIRMessagingLoggerError(kFIRMessagingMessageCodeSenderIDNotSuppliedForTokenDelete,
@"Sender ID not supplied. It is required to delete an FCM token.");
NSString *description = @"Couldn't delete token because a Sender ID was not supplied. A "
@"valid Sender ID is required to delete an FCM token";
FIRMessagingLoggerError(kFIRMessagingMessageCodeSenderIDNotSuppliedForTokenDelete, @"%@",
description);
if (completion) {
NSString *description = @"Couldn't delete token because a Sender ID was not supplied. A "
@"valid Sender ID is required to delete an FCM token";
NSError *error = [NSError fcm_errorWithCode:FIRMessagingErrorInvalidRequest
userInfo:@{NSLocalizedDescriptionKey : description}];
NSError *error = [NSError
messagingErrorWithCode:(FIRMessagingInternalErrorCode)FIRMessagingErrorInvalidRequest
failureReason:description];
completion(error);
}
return;
Expand Down Expand Up @@ -786,10 +787,13 @@ - (void)subscribeToTopic:(NSString *)topic
[strongSelf.pubsub subscribeToTopic:normalizeTopic handler:completion];
return;
}
FIRMessagingLoggerError(kFIRMessagingMessageCodeMessaging009,
@"Cannot parse topic name %@. Will not subscribe.", topic);
NSString *failureReason =
[NSString stringWithFormat:@"Cannot parse topic name: '%@'. Will not subscribe.", topic];
FIRMessagingLoggerError(kFIRMessagingMessageCodeMessaging009, @"%@", failureReason);
if (completion) {
completion([NSError fcm_errorWithCode:FIRMessagingErrorInvalidTopicName userInfo:nil]);
completion([NSError
messagingErrorWithCode:(FIRMessagingInternalErrorCode)FIRMessagingErrorInvalidTopicName
failureReason:failureReason]);
}
}];
}
Expand Down Expand Up @@ -824,10 +828,13 @@ - (void)unsubscribeFromTopic:(NSString *)topic
[strongSelf.pubsub unsubscribeFromTopic:normalizeTopic handler:completion];
return;
}
FIRMessagingLoggerError(kFIRMessagingMessageCodeMessaging011,
@"Cannot parse topic name %@. Will not unsubscribe.", topic);
NSString *failureReason =
[NSString stringWithFormat:@"Cannot parse topic name: '%@'. Will not unsubscribe.", topic];
FIRMessagingLoggerError(kFIRMessagingMessageCodeMessaging011, @"%@", failureReason);
if (completion) {
completion([NSError fcm_errorWithCode:FIRMessagingErrorInvalidTopicName userInfo:nil]);
completion([NSError
messagingErrorWithCode:(FIRMessagingInternalErrorCode)FIRMessagingErrorInvalidTopicName
failureReason:failureReason]);
}
}];
}
Expand Down
27 changes: 14 additions & 13 deletions FirebaseMessaging/Sources/FIRMessagingClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,11 @@ - (void)updateSubscriptionWithToken:(NSString *)token
shouldDelete:shouldDelete
handler:completion];
} else {
FIRMessagingLoggerDebug(kFIRMessagingMessageCodeRegistrar000,
@"Device check in error, no auth credentials found");
NSError *error = [NSError errorWithFCMErrorCode:kFIRMessagingErrorCodeMissingDeviceID];
NSString *failureReason = @"Device ID and checkin info is not found. Will not proceed with "
@"subscription/unsubscription.";
FIRMessagingLoggerDebug(kFIRMessagingMessageCodeRegistrar000, @"%@", failureReason);
NSError *error = [NSError messagingErrorWithCode:kFIRMessagingErrorCodeMissingDeviceID
failureReason:failureReason];
handler(error);
}
}
Expand Down Expand Up @@ -267,10 +269,9 @@ - (void)retryConnectionImmediately:(BOOL)immediately {
- (void)connectWithHandler:(FIRMessagingConnectCompletionHandler)handler {
if (self.isConnected) {
NSError *error =
[NSError fcm_errorWithCode:kFIRMessagingErrorCodeAlreadyConnected
userInfo:@{
NSLocalizedFailureReasonErrorKey : @"FIRMessaging is already connected",
}];
[NSError messagingErrorWithCode:kFIRMessagingErrorCodeAlreadyConnected
failureReason:
@"FIRMessaging is already connected. Will not try to connect again."];
handler(error);
return;
}
Expand All @@ -290,12 +291,13 @@ - (void)connect {
self.stayConnected = YES;
if (![[FIRInstanceID instanceID] tryToLoadValidCheckinInfo]) {
// Checkin info is not available. This may be due to the checkin still being fetched.
NSString *failureReason = @"Failed to connect to MCS. No deviceID and secret found.";
if (self.connectHandler) {
NSError *error = [NSError errorWithFCMErrorCode:kFIRMessagingErrorCodeMissingDeviceID];
NSError *error = [NSError messagingErrorWithCode:kFIRMessagingErrorCodeMissingDeviceID
failureReason:failureReason];
self.connectHandler(error);
}
FIRMessagingLoggerDebug(kFIRMessagingMessageCodeClient009,
@"Failed to connect to MCS. No deviceID and secret found.");
FIRMessagingLoggerDebug(kFIRMessagingMessageCodeClient009, @"%@", failureReason);
// Return for now. If checkin is, in fact, retrieved, the
// |kFIRMessagingCheckinFetchedNotification| will be fired.
return;
Expand Down Expand Up @@ -492,9 +494,8 @@ - (void)scheduleConnectRetry {
// disconnect before issuing a callback
[self disconnectWithTryToConnectLater:YES];
NSError *error =
[NSError errorWithDomain:@"No internet available, cannot connect to FIRMessaging"
code:kFIRMessagingErrorCodeNetwork
userInfo:nil];
[NSError messagingErrorWithCode:(FIRMessagingInternalErrorCode)FIRMessagingErrorNetwork
failureReason:@"No internet available, cannot connect to FIRMessaging"];
if (handler) {
handler(error);
self.connectHandler = nil;
Expand Down
11 changes: 7 additions & 4 deletions FirebaseMessaging/Sources/FIRMessagingDataMessageManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,9 @@ - (void)sendDataMessageStanza:(NSMutableDictionary *)dataMessage {
NSString *event __unused = [NSString stringWithFormat:@"Queued message: %@", [stanza id_p]];
FIRMessagingLoggerDebug(kFIRMessagingMessageCodeDataMessageManager007, @"%@", event);
} else {
[self willSendDataMessageFail:stanza withMessageId:msgId error:kFIRMessagingErrorCodeNetwork];
[self willSendDataMessageFail:stanza
withMessageId:msgId
error:(FIRMessagingInternalErrorCode)FIRMessagingErrorNetwork];
return;
}
}
Expand Down Expand Up @@ -404,10 +406,11 @@ - (void)willSendDataMessageSuccess:(GtalkDataMessageStanza *)stanza
- (void)willSendDataMessageFail:(GtalkDataMessageStanza *)stanza
withMessageId:(NSString *)messageId
error:(FIRMessagingInternalErrorCode)errorCode {
FIRMessagingLoggerDebug(kFIRMessagingMessageCodeDataMessageManager011,
@"Send message fail: %@ error: %lu", messageId, (unsigned long)errorCode);
NSString *failureReason = [NSString
stringWithFormat:@"Send message fail: %@ error: %lu", messageId, (unsigned long)errorCode];
FIRMessagingLoggerDebug(kFIRMessagingMessageCodeDataMessageManager011, @"%@", failureReason);

NSError *error = [NSError errorWithFCMErrorCode:errorCode];
NSError *error = [NSError messagingErrorWithCode:errorCode failureReason:failureReason];
if ([self.delegate respondsToSelector:@selector(willSendDataMessageWithID:error:)]) {
[self.delegate willSendDataMessageWithID:messageId error:error];
}
Expand Down
28 changes: 20 additions & 8 deletions FirebaseMessaging/Sources/FIRMessagingPubSub.m
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,10 @@ - (void)subscribeWithToken:(NSString *)token
options:(NSDictionary *)options
handler:(FIRMessagingTopicOperationCompletion)handler {
if (!self.client) {
handler([NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubFIRMessagingNotSetup]);
handler([NSError
messagingErrorWithCode:kFIRMessagingErrorCodePubSubClientNotSetup
failureReason:@"Firebase Messaging Client does not exist. Firebase Messaging was "
@"not setup property and subscription failed."]);
return;
}

Expand All @@ -73,9 +76,12 @@ - (void)subscribeWithToken:(NSString *)token
}

if (![[self class] isValidTopicWithPrefix:topic]) {
FIRMessagingLoggerError(kFIRMessagingMessageCodePubSub000,
@"Invalid FIRMessaging Pubsub topic %@", topic);
handler([NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubInvalidTopic]);
NSString *failureReason =
[NSString stringWithFormat:@"Invalid subscription topic :'%@'", topic];
FIRMessagingLoggerError(kFIRMessagingMessageCodePubSub000, @"%@", failureReason);
handler([NSError
messagingErrorWithCode:(FIRMessagingInternalErrorCode)FIRMessagingErrorInvalidTopicName
failureReason:failureReason]);
return;
}

Expand All @@ -102,7 +108,10 @@ - (void)unsubscribeWithToken:(NSString *)token
options:(NSDictionary *)options
handler:(FIRMessagingTopicOperationCompletion)handler {
if (!self.client) {
handler([NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubFIRMessagingNotSetup]);
handler([NSError
messagingErrorWithCode:kFIRMessagingErrorCodePubSubClientNotSetup
failureReason:@"Firebase Messaging Client does not exist. Firebase Messaging was "
@"not setup property and subscription failed."]);
return;
}
token = [token copy];
Expand All @@ -112,9 +121,12 @@ - (void)unsubscribeWithToken:(NSString *)token
}

if (![[self class] isValidTopicWithPrefix:topic]) {
FIRMessagingLoggerError(kFIRMessagingMessageCodePubSub002,
@"Invalid FIRMessaging Pubsub topic %@", topic);
handler([NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubInvalidTopic]);
NSString *failureReason =
[NSString stringWithFormat:@"Invalid topic name : '%@' for unsubscription.", topic];
FIRMessagingLoggerError(kFIRMessagingMessageCodePubSub002, @"%@", failureReason);
handler([NSError
messagingErrorWithCode:(FIRMessagingInternalErrorCode)FIRMessagingErrorInvalidTopicName
failureReason:failureReason]);
return;
}
if (![self verifyPubSubOptions:options]) {
Expand Down
28 changes: 19 additions & 9 deletions FirebaseMessaging/Sources/FIRMessagingTopicOperation.m
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,10 @@ - (void)setFinished:(BOOL)finished {

- (void)start {
if (self.isCancelled) {
NSError *error =
[NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubOperationIsCancelled];
NSError *error = [NSError
messagingErrorWithCode:kFIRMessagingErrorCodePubSubOperationIsCancelled
failureReason:
@"Failed to start the pubsub service as the topic operation is cancelled."];
[self finishWithError:error];
return;
}
Expand All @@ -148,7 +150,8 @@ - (void)finishWithError:(NSError *)error {
- (void)cancel {
[super cancel];
[self.dataTask cancel];
NSError *error = [NSError errorWithFCMErrorCode:kFIRMessagingErrorCodePubSubOperationIsCancelled];
NSError *error = [NSError messagingErrorWithCode:kFIRMessagingErrorCodePubSubOperationIsCancelled
failureReason:@"The topic operation is cancelled."];
[self finishWithError:error];
}

Expand Down Expand Up @@ -210,16 +213,23 @@ - (void)performSubscriptionChange {
}
NSString *response = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding];
if (response.length == 0) {
FIRMessagingLoggerDebug(kFIRMessagingMessageCodeTopicOperationEmptyResponse,
@"Invalid registration response - zero length.");
[self finishWithError:[NSError errorWithFCMErrorCode:kFIRMessagingErrorCodeUnknown]];
NSString *failureReason = @"Invalid registration response - zero length.";
FIRMessagingLoggerDebug(kFIRMessagingMessageCodeTopicOperationEmptyResponse, @"%@",
failureReason);
[self finishWithError:[NSError messagingErrorWithCode:(FIRMessagingInternalErrorCode)
FIRMessagingErrorUnknown
failureReason:failureReason]];
return;
}
NSArray *parts = [response componentsSeparatedByString:@"="];
if (![parts[0] isEqualToString:@"token"] || parts.count <= 1) {
FIRMessagingLoggerDebug(kFIRMessagingMessageCodeTopicOption002,
@"Invalid registration response %@", response);
[self finishWithError:[NSError errorWithFCMErrorCode:kFIRMessagingErrorCodeUnknown]];
NSString *failureReason = [NSString
stringWithFormat:@"Invalid registration response :'%@'. It is missing 'token' field.",
response];
FIRMessagingLoggerDebug(kFIRMessagingMessageCodeTopicOption002, @"%@", failureReason);
[self finishWithError:[NSError messagingErrorWithCode:(FIRMessagingInternalErrorCode)
FIRMessagingErrorUnknown
failureReason:failureReason]];
return;
}
[self finishWithError:nil];
Expand Down
34 changes: 12 additions & 22 deletions FirebaseMessaging/Sources/NSError+FIRMessaging.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,54 +16,44 @@

#import <Foundation/Foundation.h>

#import <FirebaseMessaging/FIRMessaging.h>

NS_ASSUME_NONNULL_BEGIN

FOUNDATION_EXPORT NSString *const kFIRMessagingDomain;

typedef NS_ENUM(NSUInteger, FIRMessagingInternalErrorCode) {
// Unknown error.
kFIRMessagingErrorCodeUnknown = 0,

// HTTP related errors.
kFIRMessagingErrorCodeAuthentication = 1,
kFIRMessagingErrorCodeNoAccess = 2,
kFIRMessagingErrorCodeTimeout = 3,
kFIRMessagingErrorCodeNetwork = 4,

// Another operation is in progress.
kFIRMessagingErrorCodeOperationInProgress = 5,

// Failed to perform device check in.
kFIRMessagingErrorCodeRegistrarFailedToCheckIn = 6,

kFIRMessagingErrorCodeInvalidRequest = 7,

kFIRMessagingErrorInvalidTopicName = 8,

// FIRMessaging generic errors
kFIRMessagingErrorCodeMissingDeviceID = 501,

// upstream send errors
// Upstream send errors
kFIRMessagingErrorServiceNotAvailable = 1001,
kFIRMessagingErrorInvalidParameters = 1002,
kFIRMessagingErrorMissingTo = 1003,
kFIRMessagingErrorSave = 1004,
kFIRMessagingErrorSizeExceeded = 1005,
// Future Send Errors

// MCS errors
// Already connected with MCS
kFIRMessagingErrorCodeAlreadyConnected = 2001,

// PubSub errors
kFIRMessagingErrorCodePubSubAlreadySubscribed = 3001,
kFIRMessagingErrorCodePubSubAlreadyUnsubscribed = 3002,
kFIRMessagingErrorCodePubSubInvalidTopic = 3003,
kFIRMessagingErrorCodePubSubFIRMessagingNotSetup = 3004,
kFIRMessagingErrorCodePubSubClientNotSetup = 3004,
kFIRMessagingErrorCodePubSubOperationIsCancelled = 3005,
};

@interface NSError (FIRMessaging)

@property(nonatomic, readonly) FIRMessagingInternalErrorCode fcmErrorCode;

+ (NSError *)errorWithFCMErrorCode:(FIRMessagingInternalErrorCode)fcmErrorCode;
+ (NSError *)fcm_errorWithCode:(NSInteger)code userInfo:(NSDictionary *)userInfo;
+ (NSError *)messagingErrorWithCode:(FIRMessagingInternalErrorCode)fcmErrorCode
failureReason:(NSString *)failureReason;

@end

NS_ASSUME_NONNULL_END
15 changes: 5 additions & 10 deletions FirebaseMessaging/Sources/NSError+FIRMessaging.m
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,11 @@

@implementation NSError (FIRMessaging)

- (FIRMessagingInternalErrorCode)fcmErrorCode {
return (FIRMessagingInternalErrorCode)self.code;
}

+ (NSError *)errorWithFCMErrorCode:(FIRMessagingInternalErrorCode)fcmErrorCode {
return [NSError errorWithDomain:kFIRMessagingDomain code:fcmErrorCode userInfo:nil];
}

+ (NSError *)fcm_errorWithCode:(NSInteger)code userInfo:(NSDictionary *)userInfo {
return [NSError errorWithDomain:kFIRMessagingDomain code:code userInfo:userInfo];
+ (NSError *)messagingErrorWithCode:(FIRMessagingInternalErrorCode)fcmErrorCode
failureReason:(NSString *)failureReason {
NSMutableDictionary *userInfo = [NSMutableDictionary dictionary];
userInfo[NSLocalizedFailureReasonErrorKey] = failureReason;
return [NSError errorWithDomain:kFIRMessagingDomain code:fcmErrorCode userInfo:userInfo];
}

@end
Loading