From 9ca7397f3fb3ff467be5acf30444702592455db9 Mon Sep 17 00:00:00 2001 From: Rizwan Sattar Date: Wed, 13 Sep 2017 15:37:23 -0700 Subject: [PATCH 1/2] Remove FIRMessaging_FAIL macro, log result code This removes the `FIRMessaging_FAIL` macro which was using `__builtin_trap()`, and replaced with `NSAssert` calls. These `NSAssert` calls may not get called in release builds, and so we also log them with FIRLogger error messages. The RMQ database open error result code is now parsed and included in the error message to help us identify causes for #199 --- .../Messaging/Tests/FIRMessagingFakeSocket.m | 2 +- Firebase/Messaging/FIRMMessageCode.h | 4 ++ Firebase/Messaging/FIRMessagingDefines.h | 12 ----- .../FIRMessagingRmq2PersistentStore.m | 52 ++++++++++++++++--- 4 files changed, 49 insertions(+), 21 deletions(-) diff --git a/Example/Messaging/Tests/FIRMessagingFakeSocket.m b/Example/Messaging/Tests/FIRMessagingFakeSocket.m index 2b0b47702d8..cffe69a9193 100644 --- a/Example/Messaging/Tests/FIRMessagingFakeSocket.m +++ b/Example/Messaging/Tests/FIRMessagingFakeSocket.m @@ -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; } diff --git a/Firebase/Messaging/FIRMMessageCode.h b/Firebase/Messaging/FIRMMessageCode.h index 6afc1bb8a78..46505ecdf6f 100644 --- a/Firebase/Messaging/FIRMMessageCode.h +++ b/Firebase/Messaging/FIRMMessageCode.h @@ -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 diff --git a/Firebase/Messaging/FIRMessagingDefines.h b/Firebase/Messaging/FIRMessagingDefines.h index 36448edf948..62399b3f7b8 100644 --- a/Firebase/Messaging/FIRMessagingDefines.h +++ b/Firebase/Messaging/FIRMessagingDefines.h @@ -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_ diff --git a/Firebase/Messaging/FIRMessagingRmq2PersistentStore.m b/Firebase/Messaging/FIRMessagingRmq2PersistentStore.m index 9edac40f7d6..4cbcba20977 100644 --- a/Firebase/Messaging/FIRMessagingRmq2PersistentStore.m +++ b/Firebase/Messaging/FIRMessagingRmq2PersistentStore.m @@ -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]; + NSString *errorString = [NSString stringWithFormat:@"%d - %s", result, errorStr]; + return errorString; +} + @interface FIRMessagingRmq2PersistentStore () { sqlite3 *_database; } @@ -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: @@ -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; } @@ -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); } } @@ -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); + 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; } @@ -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]; From c253aa33ce57333c46e0b72dbe9031f9c7a63b56 Mon Sep 17 00:00:00 2001 From: Rizwan Sattar Date: Wed, 13 Sep 2017 17:38:32 -0700 Subject: [PATCH 2/2] Remove commented-out line --- Firebase/Messaging/FIRMessagingRmq2PersistentStore.m | 1 - 1 file changed, 1 deletion(-) diff --git a/Firebase/Messaging/FIRMessagingRmq2PersistentStore.m b/Firebase/Messaging/FIRMessagingRmq2PersistentStore.m index 4cbcba20977..a85298ce08b 100644 --- a/Firebase/Messaging/FIRMessagingRmq2PersistentStore.m +++ b/Firebase/Messaging/FIRMessagingRmq2PersistentStore.m @@ -105,7 +105,6 @@ // 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]; NSString *errorString = [NSString stringWithFormat:@"%d - %s", result, errorStr]; return errorString; }