Skip to content

Remove FIRMessaging_FAIL macro, log result code #265

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

Merged
merged 2 commits into from
Sep 14, 2017
Merged

Remove FIRMessaging_FAIL macro, log result code #265

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ca7397
Remove FIRMessaging_FAIL macro, log result code
rsattar Sep 13, 2017
c253aa3
Remove commented-out line
rsattar Sep 14, 2017
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
2 changes: 1 addition & 1 deletion Example/Messaging/Tests/FIRMessagingFakeSocket.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ - (void)connectToHost:(NSString *)host
self.inStream = CFBridgingRelease(inputStreamRef);
self.outStream = CFBridgingRelease(outputStreamRef);
if (!self.inStream || !self.outStream) {
FIRMessaging_FAIL(@"cannot create a fake socket");
NSAssert(NO, @"Cannot create a fake socket");
return;
}

Expand Down
4 changes: 4 additions & 0 deletions Firebase/Messaging/FIRMMessageCode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,10 @@ typedef NS_ENUM(NSInteger, FIRMessagingMessageCode) {
kFIRMessagingMessageCodeRmq2PersistentStore004 = 13004, // I-FCM013004
kFIRMessagingMessageCodeRmq2PersistentStore005 = 13005, // I-FCM013005
kFIRMessagingMessageCodeRmq2PersistentStore006 = 13006, // I-FCM013006
kFIRMessagingMessageCodeRmq2PersistentStoreErrorCreatingDatabase = 13007, // I-FCM013007
kFIRMessagingMessageCodeRmq2PersistentStoreErrorOpeningDatabase = 13008, // I-FCM013008
kFIRMessagingMessageCodeRmq2PersistentStoreInvalidRmqDirectory = 13009, // I-FCM013009
kFIRMessagingMessageCodeRmq2PersistentStoreErrorCreatingTable = 13010, // I-FCM013010
// FIRMessagingRmqManager.m
kFIRMessagingMessageCodeRmqManager000 = 14000, // I-FCM014000
// FIRMessagingSecureSocket.m
Expand Down
12 changes: 0 additions & 12 deletions Firebase/Messaging/FIRMessagingDefines.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,6 @@
#endif // FIRMessaging_VERBOSE_LOGGING


// FIRMessaging_FAIL
#ifdef DEBUG
#define FIRMessaging_FAIL(format, ...) \
do { \
NSLog(format, ##__VA_ARGS__); \
__builtin_trap(); \
} while (false)
#else
#define FIRMessaging_FAIL(...) do { } while (0)
#endif


// WEAKIFY & STRONGIFY
// Helper macro.
#define _FIRMessaging_WEAKNAME(VAR) VAR ## _weak_
Expand Down
52 changes: 44 additions & 8 deletions Firebase/Messaging/FIRMessagingRmq2PersistentStore.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,14 @@
// table data handlers
typedef void(^FCMOutgoingRmqMessagesTableHandler)(int64_t rmqId, int8_t tag, NSData *data);

// Utility to create an NSString from a sqlite3 result code
NSString * _Nonnull FIRMessagingStringFromSQLiteResult(int result) {
const char *errorStr = sqlite3_errstr(result);
//NSString *errorString = [NSString stringWithCString:errorStr encoding:NSUTF8StringEncoding];
Copy link
Contributor

Choose a reason for hiding this comment

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

This commented line can probably be removed. But out of curiosity, why use a format instead of creating an NSString from the C string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch! Commented-out line removed. The main reason for going with the format was because I want to show it as "resultCode - errorMsg". Using initWithCString I'd end up creating a string with format anyway.

NSString *errorString = [NSString stringWithFormat:@"%d - %s", result, errorStr];
return errorString;
}

@interface FIRMessagingRmq2PersistentStore () {
sqlite3 *_database;
}
Expand Down Expand Up @@ -187,6 +195,7 @@ + (NSString *)pathForDatabase:(NSString *)dbName inDirectory:(FIRMessagingRmqDir
NSArray *paths;
NSArray *components;
NSString *dbNameWithExtension = [NSString stringWithFormat:@"%@.sqlite", dbName];
NSString *errorMessage;

switch (directory) {
case FIRMessagingRmqDirectoryDocuments:
Expand All @@ -206,7 +215,11 @@ + (NSString *)pathForDatabase:(NSString *)dbName inDirectory:(FIRMessagingRmqDir
break;

default:
FIRMessaging_FAIL(@"Invalid directory type %zd", directory);
errorMessage = [NSString stringWithFormat:@"Invalid directory type %zd", directory];
FIRMessagingLoggerError(kFIRMessagingMessageCodeRmq2PersistentStoreInvalidRmqDirectory,
@"%@",
errorMessage);
NSAssert(NO, errorMessage);
break;
}

Expand All @@ -219,9 +232,13 @@ - (void)createTableWithName:(NSString *)tableName command:(NSString *)command {
if (sqlite3_exec(_database, [createDatabase UTF8String], NULL, NULL, &error) != SQLITE_OK) {
// remove db before failing
[self removeDatabase];
FIRMessaging_FAIL(@"Couldn't create table: %@ %@",
kCreateTableOutgoingRmqMessages,
[NSString stringWithCString:error encoding:NSUTF8StringEncoding]);
NSString *errorMessage = [NSString stringWithFormat:@"Couldn't create table: %@ %@",
kCreateTableOutgoingRmqMessages,
[NSString stringWithCString:error encoding:NSUTF8StringEncoding]];
FIRMessagingLoggerError(kFIRMessagingMessageCodeRmq2PersistentStoreErrorCreatingTable,
@"%@",
errorMessage);
NSAssert(NO, errorMessage);
}
}

Expand Down Expand Up @@ -256,8 +273,17 @@ - (void)openDatabase:(NSString *)dbName {
BOOL didOpenDatabase = YES;
if (![fileManager fileExistsAtPath:path]) {
// We've to separate between different versions here because of backwards compatbility issues.
if (sqlite3_open([path UTF8String], &_database) != SQLITE_OK) {
FIRMessaging_FAIL(@"%@ Could not open rmq database: %@", kFCMRmqStoreTag, path);
int result = sqlite3_open([path UTF8String], &_database);
Copy link
Member

Choose a reason for hiding this comment

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

Since the next two blocks exactly match, probably should be factored into a function for code size and readability.

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, agreed. Just didn't want to change a larger block of code this close to release 😄

if (result != SQLITE_OK) {
NSString *errorString = FIRMessagingStringFromSQLiteResult(result);
NSString *errorMessage =
[NSString stringWithFormat:@"Could not open existing RMQ database at path %@, error: %@",
path,
errorString];
FIRMessagingLoggerError(kFIRMessagingMessageCodeRmq2PersistentStoreErrorOpeningDatabase,
@"%@",
errorMessage);
NSAssert(NO, errorMessage);
didOpenDatabase = NO;
return;
}
Expand All @@ -267,8 +293,18 @@ - (void)openDatabase:(NSString *)dbName {
[self createTableWithName:kTableLastRmqId command:kCreateTableLastRmqId];
[self createTableWithName:kTableS2DRmqIds command:kCreateTableS2DRmqIds];
} else {
if (sqlite3_open([path UTF8String], &_database) != SQLITE_OK) {
FIRMessaging_FAIL(@"%@ Could not open rmq database: %@", kFCMRmqStoreTag, path);
// Calling sqlite3_open should create the database, since the file doesn't exist.
int result = sqlite3_open([path UTF8String], &_database);
if (result != SQLITE_OK) {
NSString *errorString = FIRMessagingStringFromSQLiteResult(result);
NSString *errorMessage =
[NSString stringWithFormat:@"Could not create RMQ database at path %@, error: %@",
path,
errorString];
FIRMessagingLoggerError(kFIRMessagingMessageCodeRmq2PersistentStoreErrorCreatingDatabase,
@"%@",
errorMessage);
NSAssert(NO, errorMessage);
didOpenDatabase = NO;
} else {
[self updateDbWithStringRmqID];
Expand Down