From f1708c0b3074c2d97ffebf97c6611396e34e359d Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 19 Jun 2018 17:01:46 -0400 Subject: [PATCH 1/4] Improve logging around deserialization failures. --- SignalServiceKit/src/Storage/OWSStorage.m | 7 ++++--- SignalServiceKit/src/Util/OWSAnalytics.m | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/SignalServiceKit/src/Storage/OWSStorage.m b/SignalServiceKit/src/Storage/OWSStorage.m index c8f9aa026..ffb2991e3 100644 --- a/SignalServiceKit/src/Storage/OWSStorage.m +++ b/SignalServiceKit/src/Storage/OWSStorage.m @@ -230,8 +230,8 @@ NSString *const kNSUserDefaults_DatabaseExtensionVersionMap = @"kNSUserDefaults_ cannotDecodeObjectOfClassName:(NSString *)name originalClasses:(NSArray *)classNames { - DDLogError(@"%@ Could not decode object: %@", self.logTag, name); - OWSProdError([OWSAnalyticsEvents storageErrorCouldNotDecodeClass]); + OWSProdLogAndFail(@"%@ Could not decode object: %@", self.logTag, name); + OWSProdCritical([OWSAnalyticsEvents storageErrorCouldNotDecodeClass]); return [OWSUnknownDBObject class]; } @@ -458,7 +458,8 @@ NSString *const kNSUserDefaults_DatabaseExtensionVersionMap = @"kNSUserDefaults_ return [unarchiver decodeObjectForKey:@"root"]; } @catch (NSException *exception) { // Sync log in case we bail. - OWSProdError([OWSAnalyticsEvents storageErrorDeserialization]); + OWSProdLogAndFail(@"%@ error deserializing object: %@", self.logTag, collection); + OWSProdCritical([OWSAnalyticsEvents storageErrorDeserialization]); @throw exception; } }; diff --git a/SignalServiceKit/src/Util/OWSAnalytics.m b/SignalServiceKit/src/Util/OWSAnalytics.m index 191208f7a..e819a8673 100755 --- a/SignalServiceKit/src/Util/OWSAnalytics.m +++ b/SignalServiceKit/src/Util/OWSAnalytics.m @@ -399,7 +399,7 @@ NSString *NSStringForOWSAnalyticsSeverity(OWSAnalyticsSeverity severity) - (BOOL)isSeverityAsync:(OWSAnalyticsSeverity)severity { - return severity == OWSAnalyticsSeverityCritical; + return severity != OWSAnalyticsSeverityCritical; } #pragma mark - Logging From 21a9ce3b2054359970eaf334339257944ea073ca Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 20 Jun 2018 13:51:42 -0400 Subject: [PATCH 2/4] Ensure TSRecipient can be deserialized. --- SignalServiceKit/src/Storage/TSRecipient.h | 6 ++++-- SignalServiceKit/src/Storage/TSRecipient.m | 8 -------- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/SignalServiceKit/src/Storage/TSRecipient.h b/SignalServiceKit/src/Storage/TSRecipient.h index 5d2c3c74d..32febfdd6 100644 --- a/SignalServiceKit/src/Storage/TSRecipient.h +++ b/SignalServiceKit/src/Storage/TSRecipient.h @@ -1,12 +1,14 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // +#import "TSYapDatabaseObject.h" + NS_ASSUME_NONNULL_BEGIN // Some lingering TSRecipient records in the wild causing crashes. // This is a stop gap until a proper cleanup happens. -@interface TSRecipient : NSObject +@interface TSRecipient : TSYapDatabaseObject @end diff --git a/SignalServiceKit/src/Storage/TSRecipient.m b/SignalServiceKit/src/Storage/TSRecipient.m index 6d9b8eeff..00470f401 100644 --- a/SignalServiceKit/src/Storage/TSRecipient.m +++ b/SignalServiceKit/src/Storage/TSRecipient.m @@ -8,14 +8,6 @@ NS_ASSUME_NONNULL_BEGIN @implementation TSRecipient -- (void)encodeWithCoder:(nonnull NSCoder *)aCoder { - return; -} - -- (nullable instancetype)initWithCoder:(nonnull NSCoder *)aDecoder { - return nil; -} - @end NS_ASSUME_NONNULL_END From 2e6b4899a7785a3e12be7ae56fecdaa6191f8b21 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 20 Jun 2018 14:30:50 -0400 Subject: [PATCH 3/4] Remove TSRecipient. --- SignalServiceKit/src/Storage/OWSStorage.m | 2 ++ SignalServiceKit/src/Storage/TSRecipient.h | 15 --------------- SignalServiceKit/src/Storage/TSRecipient.m | 13 ------------- 3 files changed, 2 insertions(+), 28 deletions(-) delete mode 100644 SignalServiceKit/src/Storage/TSRecipient.h delete mode 100644 SignalServiceKit/src/Storage/TSRecipient.m diff --git a/SignalServiceKit/src/Storage/OWSStorage.m b/SignalServiceKit/src/Storage/OWSStorage.m index ffb2991e3..386a6148a 100644 --- a/SignalServiceKit/src/Storage/OWSStorage.m +++ b/SignalServiceKit/src/Storage/OWSStorage.m @@ -212,6 +212,8 @@ NSString *const kNSUserDefaults_DatabaseExtensionVersionMap = @"kNSUserDefaults_ - (void)encodeWithCoder:(NSCoder *)aCoder { + OWSRaiseException( + @"OWSStorageExceptionName_SaveToUnknownCollection", @"Tried to save object from unknown collection"); } @end diff --git a/SignalServiceKit/src/Storage/TSRecipient.h b/SignalServiceKit/src/Storage/TSRecipient.h deleted file mode 100644 index 32febfdd6..000000000 --- a/SignalServiceKit/src/Storage/TSRecipient.h +++ /dev/null @@ -1,15 +0,0 @@ -// -// Copyright (c) 2018 Open Whisper Systems. All rights reserved. -// - -#import "TSYapDatabaseObject.h" - -NS_ASSUME_NONNULL_BEGIN - -// Some lingering TSRecipient records in the wild causing crashes. -// This is a stop gap until a proper cleanup happens. -@interface TSRecipient : TSYapDatabaseObject - -@end - -NS_ASSUME_NONNULL_END diff --git a/SignalServiceKit/src/Storage/TSRecipient.m b/SignalServiceKit/src/Storage/TSRecipient.m deleted file mode 100644 index 00470f401..000000000 --- a/SignalServiceKit/src/Storage/TSRecipient.m +++ /dev/null @@ -1,13 +0,0 @@ -// -// Copyright (c) 2018 Open Whisper Systems. All rights reserved. -// - -#import "TSRecipient.h" - -NS_ASSUME_NONNULL_BEGIN - -@implementation TSRecipient - -@end - -NS_ASSUME_NONNULL_END From b88254244a8022303e82f89a0eed8d6c2382ed81 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 20 Jun 2018 16:39:53 -0400 Subject: [PATCH 4/4] Respond to CR. --- Pods | 2 +- SignalServiceKit/src/Util/OWSAnalytics.m | 19 +++++++++++++------ 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/Pods b/Pods index 9f0128211..5caa906cf 160000 --- a/Pods +++ b/Pods @@ -1 +1 @@ -Subproject commit 9f01282113e10d50ff87e76ee5fa8d2c09d19f7f +Subproject commit 5caa906cfd9c24f464247747004cc8335b96040c diff --git a/SignalServiceKit/src/Util/OWSAnalytics.m b/SignalServiceKit/src/Util/OWSAnalytics.m index e819a8673..121831c96 100755 --- a/SignalServiceKit/src/Util/OWSAnalytics.m +++ b/SignalServiceKit/src/Util/OWSAnalytics.m @@ -340,7 +340,7 @@ NSString *NSStringForOWSAnalyticsSeverity(OWSAnalyticsSeverity severity) } }; - if ([self isSeverityAsync:severity]) { + if ([self shouldReportAsync:severity]) { dispatch_async(self.serialQueue, addEvent); } else { dispatch_sync(self.serialQueue, addEvent); @@ -383,12 +383,19 @@ NSString *NSStringForOWSAnalyticsSeverity(OWSAnalyticsSeverity severity) // Log the event. NSString *logString = [NSString stringWithFormat:@"%s:%d %@", location, line, eventName]; if (!parameters) { - LOG_MAYBE([self isSeverityAsync:severity], LOG_LEVEL_DEF, logFlag, 0, nil, location, @"%@", logString); + LOG_MAYBE([self shouldReportAsync:severity], LOG_LEVEL_DEF, logFlag, 0, nil, location, @"%@", logString); } else { - LOG_MAYBE( - [self isSeverityAsync:severity], LOG_LEVEL_DEF, logFlag, 0, nil, location, @"%@ %@", logString, parameters); + LOG_MAYBE([self shouldReportAsync:severity], + LOG_LEVEL_DEF, + logFlag, + 0, + nil, + location, + @"%@ %@", + logString, + parameters); } - if (![self isSeverityAsync:severity]) { + if (![self shouldReportAsync:severity]) { [DDLog flushLog]; } @@ -397,7 +404,7 @@ NSString *NSStringForOWSAnalyticsSeverity(OWSAnalyticsSeverity severity) [self addEvent:eventName severity:severity properties:eventProperties]; } -- (BOOL)isSeverityAsync:(OWSAnalyticsSeverity)severity +- (BOOL)shouldReportAsync:(OWSAnalyticsSeverity)severity { return severity != OWSAnalyticsSeverityCritical; }