Skip to content

FirebaseInstallations: CreateInstallation API request error handling #4024

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.

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

Merged
merged 17 commits into from
Oct 9, 2019
Merged
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
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,14 @@ NS_ASSUME_NONNULL_BEGIN
@end

NS_ASSUME_NONNULL_END

/** Possible response HTTP codes for `CreateInstallation` API request. */
typedef NS_ENUM(NSInteger, FIRInstallationsRegistrationHTTPCode) {
FIRInstallationsRegistrationHTTPCodeSuccess = 201,
FIRInstallationsRegistrationHTTPCodeInvalidArgument = 400,
FIRInstallationsRegistrationHTTPCodeInvalidAPIKey = 401,
FIRInstallationsRegistrationHTTPCodeAPIKeyToProjectIDMismatch = 403,
FIRInstallationsRegistrationHTTPCodeProjectNotFound = 404,
FIRInstallationsRegistrationHTTPCodeTooManyRequests = 429,
FIRInstallationsRegistrationHTTPCodeServerInternalError = 500
};
13 changes: 13 additions & 0 deletions FirebaseInstallations/Source/Library/FIRInstallationsItem.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

@class FIRInstallationsStoredItem;
@class FIRInstallationsStoredAuthToken;
@class FIRInstallationsStoredRegistrationError;
@class FIRInstallationsStoredRegistrationParameters;

NS_ASSUME_NONNULL_BEGIN

Expand All @@ -43,6 +45,8 @@ NS_ASSUME_NONNULL_BEGIN
@property(nonatomic, nullable) FIRInstallationsStoredAuthToken *authToken;
@property(nonatomic, assign) FIRInstallationsStatus registrationStatus;

@property(nonatomic, nullable) FIRInstallationsStoredRegistrationError *registrationError;

- (instancetype)initWithAppID:(NSString *)appID firebaseAppName:(NSString *)firebaseAppName;

/**
Expand Down Expand Up @@ -77,6 +81,15 @@ NS_ASSUME_NONNULL_BEGIN
*/
+ (NSString *)generateFID;

/**
* Updates `registrationStatus` and `registrationError` accordingly.
* @param error The error for the Installation Registration API request.
* @param registrationParameters The parameters used for the Installation Registration API request.
*/
- (void)updateWithRegistrationError:(NSError *)error
registrationParameters:
(FIRInstallationsStoredRegistrationParameters *)registrationParameters;

@end

NS_ASSUME_NONNULL_END
14 changes: 14 additions & 0 deletions FirebaseInstallations/Source/Library/FIRInstallationsItem.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#import "FIRInstallationsStoredAuthToken.h"
#import "FIRInstallationsStoredItem.h"
#import "FIRInstallationsStoredRegistrationError.h"

@implementation FIRInstallationsItem

Expand Down Expand Up @@ -46,6 +47,7 @@ - (void)updateWithStoredItem:(FIRInstallationsStoredItem *)item {
self.refreshToken = item.refreshToken;
self.authToken = item.authToken;
self.registrationStatus = item.registrationStatus;
self.registrationError = item.registrationError;
}

- (FIRInstallationsStoredItem *)storedItem {
Expand All @@ -54,6 +56,7 @@ - (FIRInstallationsStoredItem *)storedItem {
storedItem.refreshToken = self.refreshToken;
storedItem.authToken = self.authToken;
storedItem.registrationStatus = self.registrationStatus;
storedItem.registrationError = self.registrationError;
return storedItem;
}

Expand Down Expand Up @@ -99,4 +102,15 @@ + (NSString *)base64URLEncodedStringWithData:(NSData *)data {
return string;
}

- (void)updateWithRegistrationError:(NSError *)error
registrationParameters:
(FIRInstallationsStoredRegistrationParameters *)registrationParameters {
self.registrationStatus = FIRInstallationStatusRegistrationFailed;
self.registrationError = [[FIRInstallationsStoredRegistrationError alloc]
initWithRegistrationParameters:registrationParameters
APIError:error];
self.authToken = nil;
self.refreshToken = nil;
}

@end
8 changes: 8 additions & 0 deletions FirebaseInstallations/Source/Library/FIRInstallationsLogger.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,17 @@ extern NSString *const kFIRInstallationsMessageCodeAPIResponseParsingAuthTokenSu
extern NSString *const kFIRInstallationsMessageCodeNewGetInstallationOperationCreated;
extern NSString *const kFIRInstallationsMessageCodeNewGetAuthTokenOperationCreated;
extern NSString *const kFIRInstallationsMessageCodeNewDeleteInstallationOperationCreated;
extern NSString *const kFIRInstallationsMessageCodeInvalidFirebaseConfiguration;

// FIRInstallationsStoredItem.m
extern NSString *const kFIRInstallationsMessageCodeInstallationCoderVersionMismatch;

// FIRInstallationsStoredAuthToken.m
extern NSString *const kFIRInstallationsMessageCodeAuthTokenCoderVersionMismatch;

// FIRInstallationsStoredRegistrationError.m
extern NSString *const kFIRInstallationsMessageCodeRegistrationErrorCoderVersionMismatch;
extern NSString *const kFIRInstallationsMessageCodeRegistrationErrorFailedToDecode;

// FIRInstallationsStoredRegistrationParameters.m
extern NSString *const kFIRInstallationsMessageCodeRegistrationParametersCoderVersionMismatch;
9 changes: 9 additions & 0 deletions FirebaseInstallations/Source/Library/FIRInstallationsLogger.m
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,18 @@
NSString *const kFIRInstallationsMessageCodeNewGetInstallationOperationCreated = @"I-FIS002000";
NSString *const kFIRInstallationsMessageCodeNewGetAuthTokenOperationCreated = @"I-FIS002001";
NSString *const kFIRInstallationsMessageCodeNewDeleteInstallationOperationCreated = @"I-FIS002002";
NSString *const kFIRInstallationsMessageCodeInvalidFirebaseConfiguration = @"I-FIS002003";

// FIRInstallationsStoredItem.m
NSString *const kFIRInstallationsMessageCodeInstallationCoderVersionMismatch = @"I-FIS003000";

// FIRInstallationsStoredAuthToken.m
NSString *const kFIRInstallationsMessageCodeAuthTokenCoderVersionMismatch = @"I-FIS004000";

// FIRInstallationsStoredRegistrationError.m
NSString *const kFIRInstallationsMessageCodeRegistrationErrorCoderVersionMismatch = @"I-FIS005000";
NSString *const kFIRInstallationsMessageCodeRegistrationErrorFailedToDecode = @"I-FIS005001";

// FIRInstallationsStoredRegistrationParameters.m
NSString *const kFIRInstallationsMessageCodeRegistrationParametersCoderVersionMismatch =
@"I-FIS006000";
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@ NS_ASSUME_NONNULL_BEGIN
*/
@interface FIRInstallationsAPIService : NSObject

@property(nonatomic, readonly) NSString *APIKey;
@property(nonatomic, readonly) NSString *projectID;

/**
* The default initializer.
* @param APIKey The Firebase project API key (see `FIROptions.APIKey`).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ - (instancetype)initWithResponse:(NSHTTPURLResponse *)response data:(nullable NS

@interface FIRInstallationsAPIService ()
@property(nonatomic, readonly) NSURLSession *URLSession;
@property(nonatomic, readonly) NSString *APIKey;
@property(nonatomic, readonly) NSString *projectID;
@end

NS_ASSUME_NONNULL_END
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,13 @@
#import "FIRInstallationsLogger.h"
#import "FIRInstallationsSingleOperationPromiseCache.h"
#import "FIRInstallationsStore.h"
#import "FIRInstallationsStoredAuthToken.h"
#import "FIRSecureStorage.h"

#import "FIRInstallationsHTTPError.h"
#import "FIRInstallationsStoredAuthToken.h"
#import "FIRInstallationsStoredRegistrationError.h"
#import "FIRInstallationsStoredRegistrationParameters.h"

const NSNotificationName FIRInstallationIDDidChangeNotification =
@"FIRInstallationIDDidChangeNotification";
NSString *const kFIRInstallationIDDidChangeNotificationAppNameKey =
Expand Down Expand Up @@ -135,9 +139,21 @@ - (instancetype)initWithGoogleAppID:(NSString *)appID
__PRETTY_FUNCTION__, self.appName);

FBLPromise<FIRInstallationsItem *> *installationItemPromise =
[self getStoredInstallation].recover(^id(NSError *error) {
return [self createAndSaveFID];
});
[self getStoredInstallation]
.recover(^id(NSError *error) {
return [self createAndSaveFID];
})
.then(^id(FIRInstallationsItem *installation) {
// Validate if a previous registration attempt failed with an error requiring Firebase
// configuration changes.
if (installation.registrationStatus == FIRInstallationStatusRegistrationFailed &&
[self areInstallationRegistrationParametersEqualToCurrent:
installation.registrationError.registrationParameters]) {
return installation.registrationError.APIError;
}

return installation;
});

// Initiate registration process on success if needed, but return the installation without waiting
// for it.
Expand All @@ -156,6 +172,7 @@ - (instancetype)initWithGoogleAppID:(NSString *)appID
switch (installation.registrationStatus) {
case FIRInstallationStatusUnregistered:
case FIRInstallationStatusRegistered:
case FIRInstallationStatusRegistrationFailed:
isValid = YES;
break;

Expand Down Expand Up @@ -201,6 +218,14 @@ - (instancetype)initWithGoogleAppID:(NSString *)appID
});
}

- (BOOL)areInstallationRegistrationParametersEqualToCurrent:
(FIRInstallationsStoredRegistrationParameters *)parameters {
NSString *APIKey = self.APIService.APIKey;
NSString *projectID = self.APIService.projectID;
return (parameters.APIKey == APIKey || [parameters.APIKey isEqual:APIKey]) &&
(parameters.projectID == projectID || [parameters.projectID isEqual:projectID]);
Copy link
Contributor

Choose a reason for hiding this comment

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

parameters.APIKey == APIKey
Is this for comparing the address to be the same as well?

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 use parameters.APIKey == APIKey to cover the case when they are both nil. [parameters.APIKey isEqual:APIKey] returns NO in this case. It is the easiest way to cover all equality cases I came up with, but I am open for suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

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

nice! I didn't know that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I didn't pay attention to it until the integration tests failures. Basically if e.g. parameters.APIKey == nil then any message to it will return nil which is casted to NO.

}

#pragma mark - FID registration

- (FBLPromise<FIRInstallationsItem *> *)registerInstallationIfNeeded:
Expand All @@ -212,11 +237,15 @@ - (instancetype)initWithGoogleAppID:(NSString *)appID

case FIRInstallationStatusUnknown:
case FIRInstallationStatusUnregistered:
case FIRInstallationStatusRegistrationFailed:
// Registration required. Proceed.
break;
}

return [self.APIService registerInstallation:installation]
.recover(^id(NSError *error) {
return [self handleRegistrationRequestError:error installation:installation];
})
.then(^id(FIRInstallationsItem *registeredInstallation) {
// Expected successful result: @[FIRInstallationsItem *registeredInstallation, NSNull]
return [FBLPromise all:@[
Expand All @@ -234,6 +263,56 @@ - (instancetype)initWithGoogleAppID:(NSString *)appID
});
}

- (FBLPromise<FIRInstallationsItem *> *)handleRegistrationRequestError:(NSError *)error
installation:
(FIRInstallationsItem *)installation {
if ([self doesRegistrationErrorRequireConfigChange:error]) {
FIRLogError(kFIRLoggerInstallations, kFIRInstallationsMessageCodeInvalidFirebaseConfiguration,
@"Firebase Installation registration failed for app with name: %@, error: "
@"%@\nPlease make sure you use valid GoogleService-Info.plist",
self.appName, error);

FIRInstallationsItem *failedInstallation = [installation copy];
[failedInstallation updateWithRegistrationError:error
registrationParameters:[self currentRegistrationParameters]];

// Save the error and then fail with the API error.
return
[self.installationsStore saveInstallation:failedInstallation].then(^NSError *(id result) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If registration returns with error, why do we still save such installation item?

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 added a design doc link explaining the details (sorry I should've added it before). TL;DR: we would like to store the registration parameters and the error received, so we can return the error without sending the API request. We will send the API request again only if the parameters changed.

Copy link
Contributor

Choose a reason for hiding this comment

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

what if the error is because network is not available?

Copy link
Contributor Author

@maksymmalyhin maksymmalyhin Oct 9, 2019

Choose a reason for hiding this comment

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

In this case [self doesRegistrationErrorRequireConfigChange:error] at line # 269 will return NO so the error will not be saved and the installation status will stay "unregistered".

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it thanks!

return error;
});
}

FBLPromise *errorPromise = [FBLPromise pendingPromise];
[errorPromise reject:error];
return errorPromise;
}

- (BOOL)doesRegistrationErrorRequireConfigChange:(NSError *)error {
FIRInstallationsHTTPError *HTTPError = (FIRInstallationsHTTPError *)error;
if (![HTTPError isKindOfClass:[FIRInstallationsHTTPError class]]) {
return NO;
}

switch (HTTPError.HTTPResponse.statusCode) {
// These are the errors that require Firebase configuration change.
case FIRInstallationsRegistrationHTTPCodeInvalidArgument:
case FIRInstallationsRegistrationHTTPCodeInvalidAPIKey:
case FIRInstallationsRegistrationHTTPCodeAPIKeyToProjectIDMismatch:
case FIRInstallationsRegistrationHTTPCodeProjectNotFound:
return YES;

default:
return NO;
}
}

- (FIRInstallationsStoredRegistrationParameters *)currentRegistrationParameters {
return [[FIRInstallationsStoredRegistrationParameters alloc]
initWithAPIKey:self.APIService.APIKey
projectID:self.APIService.projectID];
}

#pragma mark - Auth Token

- (FBLPromise<FIRInstallationsItem *> *)getAuthTokenForcingRefresh:(BOOL)forceRefresh {
Expand Down Expand Up @@ -321,6 +400,7 @@ - (instancetype)initWithGoogleAppID:(NSString *)appID
switch (installation.registrationStatus) {
case FIRInstallationStatusUnknown:
case FIRInstallationStatusUnregistered:
case FIRInstallationStatusRegistrationFailed:
// The installation is not registered, so it is safe to be deleted as is, so return early.
return [FBLPromise resolvedWith:installation];
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,7 @@ typedef NS_ENUM(NSInteger, FIRInstallationsStatus) {
FIRInstallationStatusUnregistered,
/// The Firebase Installation has successfully been registered with FIS.
FIRInstallationStatusRegistered,
/// The Firebase Installation registration with FIS failed. Firebase configuration changes are
/// required.
FIRInstallationStatusRegistrationFailed,
};
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ typedef NS_ENUM(NSInteger, FIRInstallationsAuthTokenStatus) {
@property(nullable, copy) NSDate *expirationDate;

/// The version of local storage.
@property(nonatomic) NSInteger storageVersion;
@property(nonatomic, readonly) NSInteger storageVersion;

@end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,15 @@

@implementation FIRInstallationsStoredAuthToken

- (NSInteger)storageVersion {
return kFIRInstallationsStoredAuthTokenStorageVersion;
}

- (nonnull id)copyWithZone:(nullable NSZone *)zone {
FIRInstallationsStoredAuthToken *clone = [[FIRInstallationsStoredAuthToken alloc] init];
clone.status = self.status;
clone.token = [self.token copy];
clone.expirationDate = self.expirationDate;
clone.storageVersion = self.storageVersion;
return clone;
}

Expand All @@ -41,7 +44,7 @@ - (void)encodeWithCoder:(nonnull NSCoder *)aCoder {
[aCoder encodeObject:self.token forKey:kFIRInstallationsStoredAuthTokenTokenKey];
[aCoder encodeObject:self.expirationDate
forKey:kFIRInstallationsStoredAuthTokenExpirationDateKey];
[aCoder encodeInteger:kFIRInstallationsStoredAuthTokenStorageVersion
[aCoder encodeInteger:self.storageVersion
forKey:kFIRInstallationsStoredAuthTokenStorageVersionKey];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
#import "FIRInstallationsStatus.h"

@class FIRInstallationsStoredAuthToken;
@class FIRInstallationsStoredRegistrationError;

NS_ASSUME_NONNULL_BEGIN

Expand All @@ -40,8 +41,10 @@ NS_ASSUME_NONNULL_BEGIN
@property(nonatomic, nullable) FIRInstallationsStoredAuthToken *authToken;
@property(nonatomic) FIRInstallationsStatus registrationStatus;

@property(nonatomic, nullable) FIRInstallationsStoredRegistrationError *registrationError;

/// The version of local storage.
@property(nonatomic) NSInteger storageVersion;
@property(nonatomic, readonly) NSInteger storageVersion;
@end

NS_ASSUME_NONNULL_END
Loading