From e168db79aa40a04b3636afe57d28386040085bca Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 21 Jul 2017 15:38:44 -0400 Subject: [PATCH 1/6] Instrument errors in message manager. // FREEBIE --- .../src/Messages/TSMessagesManager.m | 83 +++++++++++++------ SignalServiceKit/src/Util/OWSAnalytics.h | 18 ++++ 2 files changed, 75 insertions(+), 26 deletions(-) diff --git a/SignalServiceKit/src/Messages/TSMessagesManager.m b/SignalServiceKit/src/Messages/TSMessagesManager.m index 2a8568f96..49ab1dd92 100644 --- a/SignalServiceKit/src/Messages/TSMessagesManager.m +++ b/SignalServiceKit/src/Messages/TSMessagesManager.m @@ -42,6 +42,23 @@ NS_ASSUME_NONNULL_BEGIN +#define kOWSProdAssertParameterEnvelopeIsLegacy @"envelope_is_legacy" +#define kOWSProdAssertParameterEnvelopeDescription @"envelope_description" +#define kOWSProdAssertParameterEnvelopeEncryptedLength @"encrypted_length" + +#define AnalyticsParametersFromEnvelope(__envelope) \ + ^{ \ + NSData *__encryptedData = __envelope.hasContent ? __envelope.content : __envelope.legacyMessage; \ + return (@{ \ + kOWSProdAssertParameterEnvelopeIsLegacy : @(__envelope.hasLegacyMessage), \ + kOWSProdAssertParameterEnvelopeDescription : [self descriptionForEnvelopeType:__envelope], \ + kOWSProdAssertParameterEnvelopeEncryptedLength : @(__encryptedData.length), \ + }); \ + } + +#define OWSProdErrorWEnvelope(__analyticsEventName, __envelope) \ + OWSProdErrorWParams(__analyticsEventName, AnalyticsParametersFromEnvelope(__envelope)) + @interface TSMessagesManager () @property (nonatomic, readonly) id callMessageHandler; @@ -135,39 +152,37 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark - Debugging -- (NSString *)descriptionForEnvelope:(OWSSignalServiceProtosEnvelope *)envelope +- (NSString *)descriptionForEnvelopeType:(OWSSignalServiceProtosEnvelope *)envelope { OWSAssert(envelope != nil); - NSString *envelopeType; switch (envelope.type) { case OWSSignalServiceProtosEnvelopeTypeReceipt: - envelopeType = @"DeliveryReceipt"; - break; + return @"DeliveryReceipt"; case OWSSignalServiceProtosEnvelopeTypeUnknown: // Shouldn't happen - OWSAssert(NO); - envelopeType = @"Unknown"; - break; + OWSProdFail(@"message_manager_error_envelope_type_unknown"); + return @"Unknown"; case OWSSignalServiceProtosEnvelopeTypeCiphertext: - envelopeType = @"SignalEncryptedMessage"; - break; + return @"SignalEncryptedMessage"; case OWSSignalServiceProtosEnvelopeTypeKeyExchange: // Unsupported - OWSAssert(NO); - envelopeType = @"KeyExchange"; - break; + OWSProdFail(@"message_manager_error_envelope_type_key_exchange"); + return @"KeyExchange"; case OWSSignalServiceProtosEnvelopeTypePrekeyBundle: - envelopeType = @"PreKeyEncryptedMessage"; - break; + return @"PreKeyEncryptedMessage"; default: // Shouldn't happen - OWSAssert(NO); - envelopeType = @"Other"; - break; + OWSProdFail(@"message_manager_error_envelope_type_other"); + return @"Other"; } +} + +- (NSString *)descriptionForEnvelope:(OWSSignalServiceProtosEnvelope *)envelope +{ + OWSAssert(envelope != nil); return [NSString stringWithFormat:@"", - envelopeType, + [self descriptionForEnvelopeType:envelope], envelope.source, (unsigned int)envelope.sourceDevice, envelope.timestamp, @@ -189,7 +204,9 @@ NS_ASSUME_NONNULL_BEGIN } else if (content.hasNullMessage) { return [NSString stringWithFormat:@"", content.nullMessage]; } else { - OWSAssert(NO); + // Don't fire an analytics event; if we ever add a new content type, we'd generate a ton of + // analytics traffic. + OWSFail(@"Unknown content type."); return @"UnknownContent"; } } @@ -233,9 +250,11 @@ NS_ASSUME_NONNULL_BEGIN [description appendString:@"ContactRequest"]; } else if (syncMessage.request.type == OWSSignalServiceProtosSyncMessageRequestTypeGroups) { [description appendString:@"GroupRequest"]; + } else if (syncMessage.request.type == OWSSignalServiceProtosSyncMessageRequestTypeBlocked) { + [description appendString:@"BlockedRequest"]; } else { // Shouldn't happen - OWSAssert(NO); + OWSFail(@"Unknown sync message request type"); [description appendString:@"UnknownRequest"]; } } else if (syncMessage.hasBlocked) { @@ -248,7 +267,7 @@ NS_ASSUME_NONNULL_BEGIN [description appendString:verifiedString]; } else { // Shouldn't happen - OWSAssert(NO); + OWSFail(@"Unknown sync message type"); [description appendString:@"Unknown"]; } @@ -295,6 +314,7 @@ NS_ASSUME_NONNULL_BEGIN envelope.source, (unsigned int)envelope.sourceDevice, error); + OWSProdError(@"message_manager_error_could_not_handle_secure_message"); } completion(); }]; @@ -312,6 +332,7 @@ NS_ASSUME_NONNULL_BEGIN envelope.source, (unsigned int)envelope.sourceDevice, error); + OWSProdError(@"message_manager_error_could_not_handle_prekey_bundle"); } completion(); }]; @@ -336,6 +357,7 @@ NS_ASSUME_NONNULL_BEGIN } } @catch (NSException *exception) { DDLogError(@"Received an incorrectly formatted protocol buffer: %@", exception.debugDescription); + OWSProdFailWNSException(@"message_manager_error_invalid_protocol_message", exception); } completion(); @@ -367,15 +389,18 @@ NS_ASSUME_NONNULL_BEGIN NSData *encryptedData = messageEnvelope.hasContent ? messageEnvelope.content : messageEnvelope.legacyMessage; if (!encryptedData) { - DDLogError(@"Skipping message envelope which had no encrypted data."); + OWSProdFail(@"message_manager_error_message_envelope_has_no_content"); completion(nil); return; } NSUInteger kMaxEncryptedDataLength = 250 * 1024; if (encryptedData.length > kMaxEncryptedDataLength) { - DDLogError(@"Skipping message envelope with oversize encrypted data: %lu.", - (unsigned long)encryptedData.length); + OWSProdErrorWParams(@"message_manager_error_oversize_message", ^{ + return (@{ + @"message_size" : @(encryptedData.length), + }); + }); completion(nil); return; } @@ -424,7 +449,7 @@ NS_ASSUME_NONNULL_BEGIN // DEPRECATED - Remove after all clients have been upgraded. NSData *encryptedData = preKeyEnvelope.hasContent ? preKeyEnvelope.content : preKeyEnvelope.legacyMessage; if (!encryptedData) { - DDLogError(@"Skipping message envelope which had no encrypted data"); + OWSProdFail(@"message_manager_error_prekey_bundle_envelope_has_no_content"); completion(nil); return; } @@ -816,7 +841,7 @@ NS_ASSUME_NONNULL_BEGIN NSData *groupId = dataMessage.hasGroup ? dataMessage.group.id : nil; if (!groupId) { - OWSAssert(groupId); + OWSFail(@"Group info request is missing group id."); return; } @@ -1009,24 +1034,30 @@ NS_ASSUME_NONNULL_BEGIN __block TSErrorMessage *errorMessage; [self.dbConnection readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { if ([exception.name isEqualToString:NoSessionException]) { + OWSProdErrorWEnvelope(@"message_manager_error_no_session", envelope); errorMessage = [TSErrorMessage missingSessionWithEnvelope:envelope withTransaction:transaction]; } else if ([exception.name isEqualToString:InvalidKeyException]) { + OWSProdErrorWEnvelope(@"message_manager_error_invalid_key", envelope); errorMessage = [TSErrorMessage invalidKeyExceptionWithEnvelope:envelope withTransaction:transaction]; } else if ([exception.name isEqualToString:InvalidKeyIdException]) { + OWSProdErrorWEnvelope(@"message_manager_error_invalid_key_id", envelope); errorMessage = [TSErrorMessage invalidKeyExceptionWithEnvelope:envelope withTransaction:transaction]; } else if ([exception.name isEqualToString:DuplicateMessageException]) { // Duplicate messages are dismissed return; } else if ([exception.name isEqualToString:InvalidVersionException]) { + OWSProdErrorWEnvelope(@"message_manager_error_invalid_message_version", envelope); errorMessage = [TSErrorMessage invalidVersionWithEnvelope:envelope withTransaction:transaction]; } else if ([exception.name isEqualToString:UntrustedIdentityKeyException]) { // Should no longer get here, since we now record the new identity for incoming messages. + OWSProdErrorWEnvelope(@"message_manager_error_untrusted_identity_key_exception", envelope); OWSFail(@"%@ Failed to trust identity on incoming message from: %@.%d", self.tag, envelope.source, envelope.sourceDevice); return; } else { + OWSProdErrorWEnvelope(@"message_manager_error_corrupt_message", envelope); errorMessage = [TSErrorMessage corruptedMessageWithEnvelope:envelope withTransaction:transaction]; } diff --git a/SignalServiceKit/src/Util/OWSAnalytics.h b/SignalServiceKit/src/Util/OWSAnalytics.h index ffddb8818..53eb1c964 100755 --- a/SignalServiceKit/src/Util/OWSAnalytics.h +++ b/SignalServiceKit/src/Util/OWSAnalytics.h @@ -56,6 +56,9 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() #define kOWSProdAssertParameterNSErrorDomain @"nserror_domain" #define kOWSProdAssertParameterNSErrorCode @"nserror_code" #define kOWSProdAssertParameterNSErrorDescription @"nserror_description" +#define kOWSProdAssertParameterNSExceptionName @"nsexception_name" +#define kOWSProdAssertParameterNSExceptionReason @"nsexception_reason" +#define kOWSProdAssertParameterNSExceptionClassName @"nsexception_classname" // These methods should be used to assert errors for which we want to fire analytics events. // @@ -121,9 +124,21 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() }); \ } +#define AnalyticsParametersFromNSException(__exception) \ + ^{ \ + return (@{ \ + kOWSProdAssertParameterNSExceptionName : __exception.name, \ + kOWSProdAssertParameterNSExceptionReason : __exception.reason, \ + kOWSProdAssertParameterNSExceptionClassName : NSStringFromClass([__exception class]), \ + }); \ + } + #define OWSProdFailWNSError(__analyticsEventName, __nserror) \ OWSProdFailWParams(__analyticsEventName, AnalyticsParametersFromNSError(__nserror)) +#define OWSProdFailWNSException(__analyticsEventName, __exception) \ + OWSProdFailWParams(__analyticsEventName, AnalyticsParametersFromNSException(__exception)) + #define OWSProdEventWParams(__severityLevel, __analyticsEventName, __parametersBlock) \ { \ NSDictionary *__eventParameters \ @@ -150,4 +165,7 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() #define OWSProdErrorWNSError(__analyticsEventName, __nserror) \ OWSProdErrorWParams(__analyticsEventName, AnalyticsParametersFromNSError(__nserror)) +#define OWSProdErrorWNSException(__analyticsEventName, __exception) \ + OWSProdErrorWParams(__analyticsEventName, AnalyticsParametersFromNSException(__exception)) + NS_ASSUME_NONNULL_END From 19c0a7ad7cf7df9c23d997eb87e5d59215e7498a Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 21 Jul 2017 16:22:07 -0400 Subject: [PATCH 2/6] Instrument errors in message sender. // FREEBIE --- .../src/Messages/OWSMessageSender.m | 37 ++++++++----------- .../src/Messages/TSMessagesManager.m | 9 ++++- .../src/Storage/TSStorageManager.m | 2 +- SignalServiceKit/src/Util/OWSAnalytics.h | 23 +++++++++--- 4 files changed, 42 insertions(+), 29 deletions(-) diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 1ac44d17a..3376de51d 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -186,7 +186,7 @@ NSUInteger const OWSSendMessageOperationMaxRetries = 4; _successHandler = ^{ typeof(self) strongSelf = weakSelf; if (!strongSelf) { - OWSCAssert(NO); + OWSProdCFail(@"message_sender_error_send_operation_did_not_complete"); return; } @@ -200,7 +200,7 @@ NSUInteger const OWSSendMessageOperationMaxRetries = 4; _failureHandler = ^(NSError *_Nonnull error) { typeof(self) strongSelf = weakSelf; if (!strongSelf) { - OWSCAssert(NO); + OWSProdCFail(@"message_sender_error_send_operation_did_not_complete"); return; } @@ -474,7 +474,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; [TSAttachmentStream fetchObjectWithUniqueID:message.attachmentIds.firstObject]; if (!attachmentStream) { - DDLogError(@"%@ Unable to find local saved attachment to upload.", self.tag); + OWSProdError(@"message_sender_error_could_not_load_attachment"); NSError *error = OWSErrorMakeFailedToSendOutgoingMessageError(); // Not finding local attachment is a terminal failure. [error setIsRetryable:NO]; @@ -539,7 +539,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; NSError *error; [attachmentStream writeData:dataCopy error:&error]; if (error) { - DDLogError(@"%@ Failed to write data for outgoing attachment with error:%@", self.tag, error); + OWSProdErrorWNSError(@"message_sender_error_could_not_write_attachment", error); return failureHandler(error); } @@ -575,7 +575,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; if (recipients.count == 0 && !*error) { // error should be set in contactsUpater, but just in case. - DDLogError(@"%@ Unknown error finding contacts", self.tag); + OWSProdError(@"message_sender_error_could_not_find_contacts_1"); *error = OWSErrorMakeFailedToSendOutgoingMessageError(); } @@ -598,7 +598,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; if (recipients.count == 0) { if (!error) { - DDLogError(@"%@ Unknown error finding contacts", self.tag); + OWSProdError(@"message_sender_error_could_not_find_contacts_2"); error = OWSErrorMakeFailedToSendOutgoingMessageError(); } // If no recipients were found, there's no reason to retry. It will just fail again. @@ -652,7 +652,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; [self unregisteredRecipient:recipient message:message thread:thread]; } - DDLogError(@"%@ contact lookup failed with error: %@", self.tag, error); + OWSProdErrorWNSError(@"message_sender_error_could_not_find_contacts_3", error); // No need to repeat trying to find a failure. Apart from repeatedly failing, it would also cause us // to print redundant error messages. [error setIsRetryable:NO]; @@ -678,10 +678,8 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; success:successHandler failure:failureHandler]; } else { - DDLogError(@"%@ Unexpected unhandlable message: %@", self.tag, message); - // Neither a group nor contact thread? This should never happen. - OWSAssert(NO); + OWSFail(@"%@ Unknown message type: %@", self.tag, NSStringFromClass([message class])); NSError *error = OWSErrorMakeFailedToSendOutgoingMessageError(); [error setIsRetryable:NO]; @@ -851,7 +849,6 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; DDLogWarn(@"%@ Failed to update prekeys with the server: %@", self.tag, error); }]; - DDLogError(@"%@ Message send failed due to repeated inability to update prekeys.", self.tag); NSError *error = OWSErrorMakeMessageSendDisabledDueToPreKeyUpdateFailuresError(); [error setIsRetryable:YES]; return failureHandler(error); @@ -859,8 +856,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; if (remainingAttempts <= 0) { // We should always fail with a specific error. - DDLogError(@"%@ Unexpected generic failure.", self.tag); - OWSAssert(NO); + OWSProdFail(@"message_sender_error_generic_send_failure"); NSError *error = OWSErrorMakeFailedToSendOutgoingMessageError(); [error setIsRetryable:YES]; @@ -897,7 +893,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; PreKeyBundle *newKeyBundle = exception.userInfo[TSInvalidPreKeyBundleKey]; if (![newKeyBundle isKindOfClass:[PreKeyBundle class]]) { - OWSFail(@"%@ unexpected TSInvalidPreKeyBundleKey: %@", self.tag, newKeyBundle); + OWSProdFail(@"message_sender_error_unexpected_key_bundle"); failureHandler(error); return; } @@ -905,14 +901,14 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; NSData *newIdentityKeyWithVersion = newKeyBundle.identityKey; if (![newIdentityKeyWithVersion isKindOfClass:[NSData class]]) { - OWSFail(@"%@ unexpected TSInvalidRecipientKey: %@", self.tag, newIdentityKeyWithVersion); + OWSProdFail(@"message_sender_error_invalid_identity_key_type"); failureHandler(error); return; } // TODO migrate to storing the full 33 byte representation of the identity key. if (newIdentityKeyWithVersion.length != kIdentityKeyLength) { - OWSFail(@"%@ unexpected key length: %lu", self.tag, (unsigned long)newIdentityKeyWithVersion.length); + OWSProdFail(@"message_sender_error_invalid_identity_key_length"); failureHandler(error); return; } @@ -1017,7 +1013,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; NSDictionary *serializedResponse = [NSJSONSerialization JSONObjectWithData:responseData options:0 error:&error]; if (error) { - DDLogError(@"%@ Failed to serialize response of mismatched devices: %@", self.tag, error); + OWSProdErrorWNSError(@"message_sender_error_could_not_parse_mismatched_devices_json", error); [error setIsRetryable:YES]; return failureHandler(error); } @@ -1057,8 +1053,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; dispatch_async([OWSDispatch sessionStoreQueue], ^{ if (extraDevices.count < 1 && missingDevices.count < 1) { - DDLogError(@"%@ No missing or extra devices in %s", self.tag, __PRETTY_FUNCTION__); - OWSAssert(NO); + OWSProdFail(@"message_sender_error_no_missing_or_extra_devices"); } if (extraDevices && extraDevices.count > 0) { @@ -1221,7 +1216,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; dispatch_semaphore_signal(sema); } failure:^(NSURLSessionDataTask *task, NSError *error) { - DDLogError(@"Server replied on PreKeyBundle request with error: %@", error); + DDLogError(@"Server replied to PreKeyBundle request with error: %@", error); NSHTTPURLResponse *response = (NSHTTPURLResponse *)task.response; if (response.statusCode == 404) { // Can't throw exception from within callback as it's probabably a different thread. @@ -1292,7 +1287,7 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; NSDictionary *jsonDict = [MTLJSONAdapter JSONDictionaryFromModel:messageParams error:&error]; if (error) { - DDLogError(@"Error while making JSON dictionary of message: %@", error.debugDescription); + OWSProdErrorWNSError(@"message_send_error_could_not_serialize_message_json", error); return nil; } diff --git a/SignalServiceKit/src/Messages/TSMessagesManager.m b/SignalServiceKit/src/Messages/TSMessagesManager.m index 49ab1dd92..cbcc0df95 100644 --- a/SignalServiceKit/src/Messages/TSMessagesManager.m +++ b/SignalServiceKit/src/Messages/TSMessagesManager.m @@ -57,7 +57,14 @@ NS_ASSUME_NONNULL_BEGIN } #define OWSProdErrorWEnvelope(__analyticsEventName, __envelope) \ - OWSProdErrorWParams(__analyticsEventName, AnalyticsParametersFromEnvelope(__envelope)) + { \ + DDLogError(@"%s:%d %@: %@", \ + __PRETTY_FUNCTION__, \ + __LINE__, \ + __analyticsEventName, \ + [self descriptionForEnvelope:__envelope]); \ + OWSProdErrorWParams(__analyticsEventName, AnalyticsParametersFromEnvelope(__envelope)) \ + } @interface TSMessagesManager () diff --git a/SignalServiceKit/src/Storage/TSStorageManager.m b/SignalServiceKit/src/Storage/TSStorageManager.m index 539aaf33d..2bd9cfc65 100644 --- a/SignalServiceKit/src/Storage/TSStorageManager.m +++ b/SignalServiceKit/src/Storage/TSStorageManager.m @@ -353,7 +353,7 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; BOOL shouldHavePassword = [NSFileManager.defaultManager fileExistsAtPath:[self dbPath]]; if (shouldHavePassword) { - OWSProdErrorWNSError(@"storage_error_could_not_load_database_second_attempt", keyFetchError); + OWSProdError(@"storage_error_could_not_load_database_second_attempt"); } // Try to reset app by deleting database. diff --git a/SignalServiceKit/src/Util/OWSAnalytics.h b/SignalServiceKit/src/Util/OWSAnalytics.h index 53eb1c964..1e3dedb13 100755 --- a/SignalServiceKit/src/Util/OWSAnalytics.h +++ b/SignalServiceKit/src/Util/OWSAnalytics.h @@ -67,8 +67,7 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() // // parametersBlock is of type OWSProdAssertParametersBlock. // The "C" variants (e.g. OWSProdAssert() vs. OWSProdCAssert() should be used in free functions, -// where there is no self. -// +// where there is no self. They can also be used in blocks to avoid capturing a reference to self. #define OWSProdAssertWParamsTemplate(__value, __analyticsEventName, __parametersBlock, __assertMacro) \ { \ if (!(BOOL)(__value)) { \ @@ -134,10 +133,16 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() } #define OWSProdFailWNSError(__analyticsEventName, __nserror) \ - OWSProdFailWParams(__analyticsEventName, AnalyticsParametersFromNSError(__nserror)) + { \ + DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, error.debugDescription); \ + OWSProdFailWParams(__analyticsEventName, AnalyticsParametersFromNSError(__nserror)) \ + } #define OWSProdFailWNSException(__analyticsEventName, __exception) \ - OWSProdFailWParams(__analyticsEventName, AnalyticsParametersFromNSException(__exception)) + { \ + DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, __exception); \ + OWSProdFailWParams(__analyticsEventName, AnalyticsParametersFromNSException(__exception)) \ + } #define OWSProdEventWParams(__severityLevel, __analyticsEventName, __parametersBlock) \ { \ @@ -163,9 +168,15 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() #define OWSProdCFail(__analyticsEventName) OWSProdCFailWParams(__analyticsEventName, nil) #define OWSProdErrorWNSError(__analyticsEventName, __nserror) \ - OWSProdErrorWParams(__analyticsEventName, AnalyticsParametersFromNSError(__nserror)) + { \ + DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, error.debugDescription); \ + OWSProdErrorWParams(__analyticsEventName, AnalyticsParametersFromNSError(__nserror)) \ + } #define OWSProdErrorWNSException(__analyticsEventName, __exception) \ - OWSProdErrorWParams(__analyticsEventName, AnalyticsParametersFromNSException(__exception)) + { \ + DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, __exception); \ + OWSProdErrorWParams(__analyticsEventName, AnalyticsParametersFromNSException(__exception)) \ + } NS_ASSUME_NONNULL_END From 7da5df594f6d107ba09007d3423726d7709d0843 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Fri, 21 Jul 2017 17:12:00 -0400 Subject: [PATCH 3/6] Instrument errors in storage manager. // FREEBIE --- .../src/Storage/TSStorageManager.m | 27 +++++++++++-------- SignalServiceKit/src/Util/OWSAnalytics.h | 4 +-- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/SignalServiceKit/src/Storage/TSStorageManager.m b/SignalServiceKit/src/Storage/TSStorageManager.m index 2bd9cfc65..8475ec405 100644 --- a/SignalServiceKit/src/Storage/TSStorageManager.m +++ b/SignalServiceKit/src/Storage/TSStorageManager.m @@ -85,7 +85,11 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; - (nullable Class)unarchiver:(NSKeyedUnarchiver *)unarchiver cannotDecodeObjectOfClassName:(NSString *)name originalClasses:(NSArray *)classNames { - DDLogError(@"[OWSUnarchiverDelegate] Ignoring unknown class name: %@. Was the class definition deleted?", name); + OWSProdErrorWParams(@"storage_error_could_not_decode_class", ^{ + return (@{ + @"class_name" : name, + }); + }); return [OWSUnknownObject class]; } @@ -180,13 +184,14 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; return [unarchiver decodeObjectForKey:@"root"]; } @catch (NSException *exception) { // Sync log in case we bail. - DDLogError(@"%@ Unarchiving key:%@ from collection:%@ and data %@ failed with error: %@", - self.tag, - key, - collection, - data, - exception.reason); - DDLogError(@"%@ Raising exception.", self.tag); + OWSProdErrorWParams(@"storage_error_deserialization", ^{ + return (@{ + @"collection" : collection, + kOWSProdAssertParameterNSExceptionName : exception.name, + kOWSProdAssertParameterNSExceptionReason : exception.reason, + kOWSProdAssertParameterNSExceptionClassName : NSStringFromClass([exception class]), + }); + }); @throw exception; } }; @@ -255,7 +260,7 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; BOOL success = [ressourceURL setResourceValues:resourcesAttrs error:&error]; if (error || !success) { - DDLogError(@"Error while removing files from backup: %@", error.description); + OWSProdErrorWNSError(@"storage_error_file_protecion", error); return; } } @@ -372,14 +377,14 @@ static NSString *keychainDBPassAccount = @"TSDatabasePass"; NSError *keySetError; [SAMKeychain setPassword:newDBPassword forService:keychainService account:keychainDBPassAccount error:&keySetError]; if (keySetError) { - DDLogError(@"%@ Setting DB password failed with error: %@", self.tag, keySetError); + OWSProdErrorWNSError(@"storage_error_could_not_store_database_password", keySetError); [self deletePasswordFromKeychain]; [NSException raise:TSStorageManagerExceptionNameDatabasePasswordUnwritable format:@"Setting DB password failed with error: %@", keySetError]; } else { - DDLogError(@"Succesfully set new DB password."); + DDLogWarn(@"Succesfully set new DB password."); } return newDBPassword; diff --git a/SignalServiceKit/src/Util/OWSAnalytics.h b/SignalServiceKit/src/Util/OWSAnalytics.h index 1e3dedb13..dc74ccd5e 100755 --- a/SignalServiceKit/src/Util/OWSAnalytics.h +++ b/SignalServiceKit/src/Util/OWSAnalytics.h @@ -134,7 +134,7 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() #define OWSProdFailWNSError(__analyticsEventName, __nserror) \ { \ - DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, error.debugDescription); \ + DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, __nserror.debugDescription); \ OWSProdFailWParams(__analyticsEventName, AnalyticsParametersFromNSError(__nserror)) \ } @@ -169,7 +169,7 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() #define OWSProdErrorWNSError(__analyticsEventName, __nserror) \ { \ - DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, error.debugDescription); \ + DDLogError(@"%s:%d %@: %@", __PRETTY_FUNCTION__, __LINE__, __analyticsEventName, __nserror.debugDescription); \ OWSProdErrorWParams(__analyticsEventName, AnalyticsParametersFromNSError(__nserror)) \ } From 117bca7c483a8e59c3c9be0ac26b0affa5d9b58d Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 24 Jul 2017 10:22:12 -0400 Subject: [PATCH 4/6] Instrument errors in app delegate. // FREEBIE --- Signal/src/AppDelegate.m | 21 +++++++++---------- .../src/Messages/TSMessagesManager.m | 3 +++ SignalServiceKit/src/Util/OWSAnalytics.h | 11 +++++----- 3 files changed, 19 insertions(+), 16 deletions(-) diff --git a/Signal/src/AppDelegate.m b/Signal/src/AppDelegate.m index 178f6b89a..d6c131d7c 100644 --- a/Signal/src/AppDelegate.m +++ b/Signal/src/AppDelegate.m @@ -263,7 +263,7 @@ static NSString *const kURLHostVerifyPrefix = @"verify"; - (void)application:(UIApplication *)application didFailToRegisterForRemoteNotificationsWithError:(NSError *)error { - DDLogError(@"%@ Failed to register for remote notifications with error %@", self.tag, error); + OWSProdErrorWNSError(@"app_delegate_error_failed_to_register_for_remote_notifications", error); #ifdef DEBUG DDLogWarn(@"%@ We're in debug mode. Faking success for remote registration with a fake push identifier", self.tag); [PushManager.sharedManager.pushNotificationFutureSource trySetResult:[[NSMutableData dataWithLength:32] copy]]; @@ -334,25 +334,24 @@ static NSString *const kURLHostVerifyPrefix = @"verify"; NSError *typeError; [url getResourceValue:&utiType forKey:NSURLTypeIdentifierKey error:&typeError]; if (typeError) { - DDLogError( - @"%@ Determining type of picked document at url: %@ failed with error: %@", self.tag, url, typeError); - OWSAssert(NO); + OWSFail( + @"%@ Determining type of picked document at url: %@ failed with error: %@", self.tag, url, typeError); + return NO; } if (!utiType) { - DDLogDebug(@"%@ falling back to default filetype for picked document at url: %@", self.tag, url); - OWSAssert(NO); + OWSFail(@"%@ falling back to default filetype for picked document at url: %@", self.tag, url); utiType = (__bridge NSString *)kUTTypeData; + return NO; } NSNumber *isDirectory; NSError *isDirectoryError; [url getResourceValue:&isDirectory forKey:NSURLIsDirectoryKey error:&isDirectoryError]; if (isDirectoryError) { - DDLogError(@"%@ Determining if picked document at url: %@ was a directory failed with error: %@", - self.tag, - url, - isDirectoryError); - OWSAssert(NO); + OWSFail(@"%@ Determining if picked document at url: %@ was a directory failed with error: %@", + self.tag, + url, + isDirectoryError); return NO; } else if ([isDirectory boolValue]) { DDLogInfo(@"%@ User picked directory at url: %@", self.tag, url); diff --git a/SignalServiceKit/src/Messages/TSMessagesManager.m b/SignalServiceKit/src/Messages/TSMessagesManager.m index cbcc0df95..745d69021 100644 --- a/SignalServiceKit/src/Messages/TSMessagesManager.m +++ b/SignalServiceKit/src/Messages/TSMessagesManager.m @@ -43,6 +43,7 @@ NS_ASSUME_NONNULL_BEGIN #define kOWSProdAssertParameterEnvelopeIsLegacy @"envelope_is_legacy" +#define kOWSProdAssertParameterEnvelopeHasContent @"has_content" #define kOWSProdAssertParameterEnvelopeDescription @"envelope_description" #define kOWSProdAssertParameterEnvelopeEncryptedLength @"encrypted_length" @@ -51,6 +52,7 @@ NS_ASSUME_NONNULL_BEGIN NSData *__encryptedData = __envelope.hasContent ? __envelope.content : __envelope.legacyMessage; \ return (@{ \ kOWSProdAssertParameterEnvelopeIsLegacy : @(__envelope.hasLegacyMessage), \ + kOWSProdAssertParameterEnvelopeHasContent : @(__envelope.hasContent), \ kOWSProdAssertParameterEnvelopeDescription : [self descriptionForEnvelopeType:__envelope], \ kOWSProdAssertParameterEnvelopeEncryptedLength : @(__encryptedData.length), \ }); \ @@ -162,6 +164,7 @@ NS_ASSUME_NONNULL_BEGIN - (NSString *)descriptionForEnvelopeType:(OWSSignalServiceProtosEnvelope *)envelope { OWSAssert(envelope != nil); + switch (envelope.type) { case OWSSignalServiceProtosEnvelopeTypeReceipt: return @"DeliveryReceipt"; diff --git a/SignalServiceKit/src/Util/OWSAnalytics.h b/SignalServiceKit/src/Util/OWSAnalytics.h index dc74ccd5e..b3abfa4df 100755 --- a/SignalServiceKit/src/Util/OWSAnalytics.h +++ b/SignalServiceKit/src/Util/OWSAnalytics.h @@ -117,18 +117,19 @@ typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)() #define AnalyticsParametersFromNSError(__nserror) \ ^{ \ return (@{ \ - kOWSProdAssertParameterNSErrorDomain : __nserror.domain, \ + kOWSProdAssertParameterNSErrorDomain : (__nserror.domain ?: @"unknown"), \ kOWSProdAssertParameterNSErrorCode : @(__nserror.code), \ - kOWSProdAssertParameterNSErrorDescription : __nserror.description, \ + kOWSProdAssertParameterNSErrorDescription : (__nserror.description ?: @"unknown"), \ }); \ } #define AnalyticsParametersFromNSException(__exception) \ ^{ \ return (@{ \ - kOWSProdAssertParameterNSExceptionName : __exception.name, \ - kOWSProdAssertParameterNSExceptionReason : __exception.reason, \ - kOWSProdAssertParameterNSExceptionClassName : NSStringFromClass([__exception class]), \ + kOWSProdAssertParameterNSExceptionName : (__exception.name ?: @"unknown"), \ + kOWSProdAssertParameterNSExceptionReason : (__exception.reason ?: @"unknown"), \ + kOWSProdAssertParameterNSExceptionClassName : \ + (__exception ? NSStringFromClass([__exception class]) : @"unknown"), \ }); \ } From 9587aab37b097cf33cf8b54b87ca369522e03448 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 24 Jul 2017 11:06:19 -0400 Subject: [PATCH 5/6] Instrument network errors. // FREEBIE --- .../Migrations/OWS103EnableVideoCalling.m | 4 + Signal/src/environment/VersionMigrations.m | 31 ++--- .../src/Account/TSAccountManager.m | 14 +- .../src/Account/TSPreKeyManager.m | 16 ++- .../src/Contacts/ContactsUpdater.m | 78 +++++------ .../Attachments/OWSAttachmentsProcessor.m | 123 +++++++++--------- .../src/Messages/OWSMessageSender.m | 3 + .../API/OWSDeviceProvisioningCodeService.m | 7 +- .../API/OWSDeviceProvisioningService.m | 7 +- .../src/Network/API/OWSDevicesService.m | 10 +- .../src/Network/API/TSNetworkManager.h | 2 + .../src/Network/API/TSNetworkManager.m | 5 + 12 files changed, 172 insertions(+), 128 deletions(-) diff --git a/Signal/src/environment/Migrations/OWS103EnableVideoCalling.m b/Signal/src/environment/Migrations/OWS103EnableVideoCalling.m index b3f2e71bf..678ac2055 100644 --- a/Signal/src/environment/Migrations/OWS103EnableVideoCalling.m +++ b/Signal/src/environment/Migrations/OWS103EnableVideoCalling.m @@ -34,6 +34,10 @@ static NSString *const OWS103EnableVideoCallingMigrationId = @"103"; [self save]; } failure:^(NSURLSessionDataTask *task, NSError *error) { + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError( + @"error_enable_video_calling_request_failed", error); + } DDLogError(@"%@ failed with error: %@", self.tag, error); }]; }]; diff --git a/Signal/src/environment/VersionMigrations.m b/Signal/src/environment/VersionMigrations.m index d72a4333e..4ba182cbf 100644 --- a/Signal/src/environment/VersionMigrations.m +++ b/Signal/src/environment/VersionMigrations.m @@ -115,23 +115,7 @@ return [thisVersionString compare:thatVersionString options:NSNumericSearch] == NSOrderedAscending; } -#pragma mark Upgrading to 2.1 - Needs to register VOIP token + Removing video cache folder - -+ (void)nonBlockingPushRegistration { - void (^failedBlock)(NSError *) = ^(NSError *error) { - DDLogError(@"Failed to register VOIP push token: %@", error.debugDescription); - }; - [[PushManager sharedManager] requestPushTokenWithSuccess:^(NSString *pushToken, NSString *voipToken) { - [[TSAccountManager sharedInstance] - registerForPushNotificationsWithPushToken:pushToken - voipToken:voipToken - success:^{ - DDLogWarn(@"Registered for VOIP Push."); - } - failure:failedBlock]; - } - failure:failedBlock]; -} +#pragma mark Upgrading to 2.1 - Removing video cache folder + (void)clearVideoCache { NSArray *paths = NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES); @@ -162,13 +146,16 @@ TSUpdateAttributesRequest *request = [[TSUpdateAttributesRequest alloc] initWithUpdatedAttributesWithVoice]; [[TSNetworkManager sharedManager] makeRequest:request success:^(NSURLSessionDataTask *task, id responseObject) { - success = YES; - dispatch_semaphore_signal(sema); + success = YES; + dispatch_semaphore_signal(sema); } failure:^(NSURLSessionDataTask *task, NSError *error) { - success = NO; - DDLogError(@"Updating attributess failed with error: %@", error.description); - dispatch_semaphore_signal(sema); + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError(@"error_update_attributes_request_failed", error); + } + success = NO; + DDLogError(@"Updating attributess failed with error: %@", error.description); + dispatch_semaphore_signal(sema); }]; diff --git a/SignalServiceKit/src/Account/TSAccountManager.m b/SignalServiceKit/src/Account/TSAccountManager.m index 284a615eb..6ce48ff9a 100644 --- a/SignalServiceKit/src/Account/TSAccountManager.m +++ b/SignalServiceKit/src/Account/TSAccountManager.m @@ -159,6 +159,9 @@ NSString *const kNSNotificationName_LocalNumberDidChange = @"kNSNotificationName failure:failureHandler remainingRetries:remainingRetries - 1]; } else { + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError(@"accounts_error_register_push_tokens_failed", error); + } failureHandler(error); } }]; @@ -180,7 +183,7 @@ NSString *const kNSNotificationName_LocalNumberDidChange = @"kNSNotificationName // we make our verification code request. TSAccountManager *manager = [self sharedInstance]; manager.phoneNumberAwaitingVerification = phoneNumber; - + [[TSNetworkManager sharedManager] makeRequest:[[TSRequestVerificationCodeRequest alloc] initWithPhoneNumber:phoneNumber @@ -193,6 +196,9 @@ NSString *const kNSNotificationName_LocalNumberDidChange = @"kNSNotificationName successBlock(); } failure:^(NSURLSessionDataTask *task, NSError *error) { + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError(@"accounts_error_verification_code_request_failed", error); + } DDLogError(@"%@ Failed to request verification code request with error:%@", self.tag, error); failureBlock(error); }]; @@ -263,6 +269,9 @@ NSString *const kNSNotificationName_LocalNumberDidChange = @"kNSNotificationName } } failure:^(NSURLSessionDataTask *task, NSError *error) { + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError(@"accounts_error_verify_account_request_failed", error); + } DDLogWarn(@"%@ Error verifying code: %@", self.tag, error.debugDescription); switch (error.code) { case 403: { @@ -316,6 +325,9 @@ NSString *const kNSNotificationName_LocalNumberDidChange = @"kNSNotificationName userInfo:nil]; } failure:^(NSURLSessionDataTask *task, NSError *error) { + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError(@"accounts_error_unregister_account_request_failed", error); + } DDLogError(@"%@ Failed to unregister with error: %@", self.tag, error); failureBlock(error); }]; diff --git a/SignalServiceKit/src/Account/TSPreKeyManager.m b/SignalServiceKit/src/Account/TSPreKeyManager.m index a7b8a1102..4e9097f6d 100644 --- a/SignalServiceKit/src/Account/TSPreKeyManager.m +++ b/SignalServiceKit/src/Account/TSPreKeyManager.m @@ -181,10 +181,12 @@ static const NSTimeInterval kSignedPreKeyUpdateFailureMaxFailureDuration = 10 * [TSPreKeyManager clearPreKeyUpdateFailureCount]; } failure:^(NSURLSessionDataTask *task, NSError *error) { - if (modeCopy == RefreshPreKeysMode_SignedAndOneTime) { - OWSProdErrorWNSError(@"error_prekeys_update_failed_signed_and_onetime", error); - } else { - OWSProdErrorWNSError(@"error_prekeys_update_failed_just_signed", error); + if (!IsNSErrorNetworkFailure(error)) { + if (modeCopy == RefreshPreKeysMode_SignedAndOneTime) { + OWSProdErrorWNSError(@"error_prekeys_update_failed_signed_and_onetime", error); + } else { + OWSProdErrorWNSError(@"error_prekeys_update_failed_just_signed", error); + } } // Mark the prekeys as _NOT_ checked on failure. @@ -302,6 +304,9 @@ static const NSTimeInterval kSignedPreKeyUpdateFailureMaxFailureDuration = 10 * } } failure:^(NSURLSessionDataTask *task, NSError *error) { + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError(@"error_prekeys_current_signed_prekey_request_failed", error); + } DDLogWarn(@"%@ Could not retrieve current signed key from the service.", self.tag); // Mark the prekeys as _NOT_ checked on failure. @@ -310,6 +315,9 @@ static const NSTimeInterval kSignedPreKeyUpdateFailureMaxFailureDuration = 10 * } } failure:^(NSURLSessionDataTask *task, NSError *error) { + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError(@"error_prekeys_available_prekeys_request_failed", error); + } DDLogError(@"%@ Failed to retrieve the number of available prekeys.", self.tag); // Mark the prekeys as _NOT_ checked on failure. diff --git a/SignalServiceKit/src/Contacts/ContactsUpdater.m b/SignalServiceKit/src/Contacts/ContactsUpdater.m index cf1be99e5..5026c19e5 100644 --- a/SignalServiceKit/src/Contacts/ContactsUpdater.m +++ b/SignalServiceKit/src/Contacts/ContactsUpdater.m @@ -166,45 +166,49 @@ NS_ASSUME_NONNULL_BEGIN TSRequest *request = [[TSContactsIntersectionRequest alloc] initWithHashesArray:hashes]; [[TSNetworkManager sharedManager] makeRequest:request success:^(NSURLSessionDataTask *tsTask, id responseDict) { - NSMutableDictionary *attributesForIdentifier = [NSMutableDictionary dictionary]; - NSArray *contactsArray = [(NSDictionary *)responseDict objectForKey:@"contacts"]; - - // Map attributes to phone numbers - if (contactsArray) { - for (NSDictionary *dict in contactsArray) { - NSString *hash = [dict objectForKey:@"token"]; - NSString *identifier = [phoneNumbersByHashes objectForKey:hash]; - - if (!identifier) { - DDLogWarn(@"%@ An interesecting hash wasn't found in the mapping.", self.tag); - break; - } - - [attributesForIdentifier setObject:dict forKey:identifier]; - } - } - - // Insert or update contact attributes - [[TSStorageManager sharedManager].dbReadWriteConnection - readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { - for (NSString *identifier in attributesForIdentifier) { - SignalRecipient *recipient = - [SignalRecipient recipientWithTextSecureIdentifier:identifier withTransaction:transaction]; - if (!recipient) { - recipient = [[SignalRecipient alloc] initWithTextSecureIdentifier:identifier relay:nil]; - } - - NSDictionary *attributes = [attributesForIdentifier objectForKey:identifier]; - - recipient.relay = attributes[@"relay"]; - - [recipient saveWithTransaction:transaction]; - } - }]; - - success([NSSet setWithArray:attributesForIdentifier.allKeys]); + NSMutableDictionary *attributesForIdentifier = [NSMutableDictionary dictionary]; + NSArray *contactsArray = [(NSDictionary *)responseDict objectForKey:@"contacts"]; + + // Map attributes to phone numbers + if (contactsArray) { + for (NSDictionary *dict in contactsArray) { + NSString *hash = [dict objectForKey:@"token"]; + NSString *identifier = [phoneNumbersByHashes objectForKey:hash]; + + if (!identifier) { + DDLogWarn(@"%@ An interesecting hash wasn't found in the mapping.", self.tag); + break; + } + + [attributesForIdentifier setObject:dict forKey:identifier]; + } + } + + // Insert or update contact attributes + [[TSStorageManager sharedManager].dbReadWriteConnection + readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) { + for (NSString *identifier in attributesForIdentifier) { + SignalRecipient *recipient = [SignalRecipient recipientWithTextSecureIdentifier:identifier + withTransaction:transaction]; + if (!recipient) { + recipient = [[SignalRecipient alloc] initWithTextSecureIdentifier:identifier relay:nil]; + } + + NSDictionary *attributes = [attributesForIdentifier objectForKey:identifier]; + + recipient.relay = attributes[@"relay"]; + + [recipient saveWithTransaction:transaction]; + } + }]; + + success([NSSet setWithArray:attributesForIdentifier.allKeys]); } failure:^(NSURLSessionDataTask *task, NSError *error) { + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError(@"contacts_error_contacts_intersection_failed", error); + } + NSHTTPURLResponse *response = (NSHTTPURLResponse *)task.response; if (response.statusCode == 413) { failure(OWSErrorWithCodeDescription( diff --git a/SignalServiceKit/src/Messages/Attachments/OWSAttachmentsProcessor.m b/SignalServiceKit/src/Messages/Attachments/OWSAttachmentsProcessor.m index fabf2bc62..2f0be843d 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSAttachmentsProcessor.m +++ b/SignalServiceKit/src/Messages/Attachments/OWSAttachmentsProcessor.m @@ -143,67 +143,68 @@ static const CGFloat kAttachmentDownloadProgressTheta = 0.001f; TSAttachmentRequest *attachmentRequest = [[TSAttachmentRequest alloc] initWithId:attachment.serverId relay:attachment.relay]; [self.networkManager makeRequest:attachmentRequest - success:^(NSURLSessionDataTask *task, id responseObject) { - if (![responseObject isKindOfClass:[NSDictionary class]]) { - DDLogError(@"%@ Failed retrieval of attachment. Response had unexpected format.", - self.tag); - NSError *error = OWSErrorMakeUnableToProcessServerResponseError(); - return markAndHandleFailure(error); - } - NSString *location = [(NSDictionary *)responseObject objectForKey:@"location"]; - if (!location) { - DDLogError( - @"%@ Failed retrieval of attachment. Response had no location.", self.tag); - NSError *error = OWSErrorMakeUnableToProcessServerResponseError(); - return markAndHandleFailure(error); - } - - dispatch_async([OWSDispatch attachmentsQueue], ^{ - [self downloadFromLocation:location - pointer:attachment - success:^(NSData *_Nonnull encryptedData) { - [self decryptAttachmentData:encryptedData - pointer:attachment - success:markAndHandleSuccess - failure:markAndHandleFailure]; - } - failure:^(NSURLSessionDataTask *_Nullable task, NSError *_Nonnull error) { - if (attachment.serverId < 100) { - // This looks like the symptom of the "frequent 404 - // downloading attachments with low server ids". - NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)task.response; - NSInteger statusCode = [httpResponse statusCode]; - DDLogError(@"%@ %d Failure with suspicious attachment id: %llu, %@", - self.tag, - (int)statusCode, - (unsigned long long)attachment.serverId, - error); - [DDLog flushLog]; - OWSAssert(0); - } - if (markAndHandleFailure) { - markAndHandleFailure(error); - } - }]; - }); - } - failure:^(NSURLSessionDataTask *task, NSError *error) { - DDLogError(@"Failed retrieval of attachment with error: %@", error); - if (attachment.serverId < 100) { - // This _shouldn't_ be the symptom of the "frequent 404 - // downloading attachments with low server ids". - NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)task.response; - NSInteger statusCode = [httpResponse statusCode]; - DDLogError(@"%@ %d Failure with suspicious attachment id: %llu, %@", - self.tag, - (int)statusCode, - (unsigned long long)attachment.serverId, - error); - [DDLog flushLog]; - OWSAssert(0); - } - return markAndHandleFailure(error); - }]; + success:^(NSURLSessionDataTask *task, id responseObject) { + if (![responseObject isKindOfClass:[NSDictionary class]]) { + DDLogError(@"%@ Failed retrieval of attachment. Response had unexpected format.", self.tag); + NSError *error = OWSErrorMakeUnableToProcessServerResponseError(); + return markAndHandleFailure(error); + } + NSString *location = [(NSDictionary *)responseObject objectForKey:@"location"]; + if (!location) { + DDLogError(@"%@ Failed retrieval of attachment. Response had no location.", self.tag); + NSError *error = OWSErrorMakeUnableToProcessServerResponseError(); + return markAndHandleFailure(error); + } + + dispatch_async([OWSDispatch attachmentsQueue], ^{ + [self downloadFromLocation:location + pointer:attachment + success:^(NSData *_Nonnull encryptedData) { + [self decryptAttachmentData:encryptedData + pointer:attachment + success:markAndHandleSuccess + failure:markAndHandleFailure]; + } + failure:^(NSURLSessionDataTask *_Nullable task, NSError *_Nonnull error) { + if (attachment.serverId < 100) { + // This looks like the symptom of the "frequent 404 + // downloading attachments with low server ids". + NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)task.response; + NSInteger statusCode = [httpResponse statusCode]; + DDLogError(@"%@ %d Failure with suspicious attachment id: %llu, %@", + self.tag, + (int)statusCode, + (unsigned long long)attachment.serverId, + error); + [DDLog flushLog]; + OWSAssert(0); + } + if (markAndHandleFailure) { + markAndHandleFailure(error); + } + }]; + }); + } + failure:^(NSURLSessionDataTask *task, NSError *error) { + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError(@"error_attachment_request_failed", error); + } + DDLogError(@"Failed retrieval of attachment with error: %@", error); + if (attachment.serverId < 100) { + // This _shouldn't_ be the symptom of the "frequent 404 + // downloading attachments with low server ids". + NSHTTPURLResponse *httpResponse = (NSHTTPURLResponse *)task.response; + NSInteger statusCode = [httpResponse statusCode]; + DDLogError(@"%@ %d Failure with suspicious attachment id: %llu, %@", + self.tag, + (int)statusCode, + (unsigned long long)attachment.serverId, + error); + [DDLog flushLog]; + OWSAssert(0); + } + return markAndHandleFailure(error); + }]; } - (void)decryptAttachmentData:(NSData *)cipherText diff --git a/SignalServiceKit/src/Messages/OWSMessageSender.m b/SignalServiceKit/src/Messages/OWSMessageSender.m index 3376de51d..65866243a 100644 --- a/SignalServiceKit/src/Messages/OWSMessageSender.m +++ b/SignalServiceKit/src/Messages/OWSMessageSender.m @@ -1216,6 +1216,9 @@ NSString *const OWSMessageSenderRateLimitedException = @"RateLimitedException"; dispatch_semaphore_signal(sema); } failure:^(NSURLSessionDataTask *task, NSError *error) { + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError(@"message_sender_error_recipient_prekey_request_failed", error); + } DDLogError(@"Server replied to PreKeyBundle request with error: %@", error); NSHTTPURLResponse *response = (NSHTTPURLResponse *)task.response; if (response.statusCode == 404) { diff --git a/SignalServiceKit/src/Network/API/OWSDeviceProvisioningCodeService.m b/SignalServiceKit/src/Network/API/OWSDeviceProvisioningCodeService.m index 1b55f9690..e9198be64 100644 --- a/SignalServiceKit/src/Network/API/OWSDeviceProvisioningCodeService.m +++ b/SignalServiceKit/src/Network/API/OWSDeviceProvisioningCodeService.m @@ -1,4 +1,6 @@ -// Copyright © 2016 Open Whisper Systems. All rights reserved. +// +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// #import "OWSDeviceProvisioningCodeService.h" #import "OWSDeviceProvisioningCodeRequest.h" @@ -48,6 +50,9 @@ NSString *const OWSDeviceProvisioningCodeServiceProvisioningCodeKey = @"verifica } } failure:^(NSURLSessionDataTask *task, NSError *error) { + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError(@"error_provisioning_code_request_failed", error); + } DDLogVerbose(@"ProvisioningCode request failed with error: %@", error); failureCallback(error); }]; diff --git a/SignalServiceKit/src/Network/API/OWSDeviceProvisioningService.m b/SignalServiceKit/src/Network/API/OWSDeviceProvisioningService.m index b2b561c1f..fa7c26b7a 100644 --- a/SignalServiceKit/src/Network/API/OWSDeviceProvisioningService.m +++ b/SignalServiceKit/src/Network/API/OWSDeviceProvisioningService.m @@ -1,4 +1,6 @@ -// Copyright © 2016 Open Whisper Systems. All rights reserved. +// +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// #import "OWSDeviceProvisioningService.h" #import "OWSDeviceProvisioningRequest.h" @@ -45,6 +47,9 @@ NS_ASSUME_NONNULL_BEGIN successCallback(); } failure:^(NSURLSessionDataTask *task, NSError *error) { + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError(@"error_provisioning_request_failed", error); + } DDLogVerbose(@"Provisioning request failed with error: %@", error); failureCallback(error); }]; diff --git a/SignalServiceKit/src/Network/API/OWSDevicesService.m b/SignalServiceKit/src/Network/API/OWSDevicesService.m index f01f42695..e8e83b1df 100644 --- a/SignalServiceKit/src/Network/API/OWSDevicesService.m +++ b/SignalServiceKit/src/Network/API/OWSDevicesService.m @@ -1,4 +1,6 @@ -// Copyright © 2016 Open Whisper Systems. All rights reserved. +// +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// #import "OWSDevicesService.h" #import "OWSDeleteDeviceRequest.h" @@ -30,6 +32,9 @@ NS_ASSUME_NONNULL_BEGIN } } failure:^(NSURLSessionDataTask *task, NSError *error) { + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError(@"error_get_devices_failed", error); + } DDLogVerbose(@"Get devices request failed with error: %@", error); failureCallback(error); }]; @@ -47,6 +52,9 @@ NS_ASSUME_NONNULL_BEGIN successCallback(); } failure:^(NSURLSessionDataTask *task, NSError *error) { + if (!IsNSErrorNetworkFailure(error)) { + OWSProdErrorWNSError(@"error_unlink_device_failed", error); + } DDLogVerbose(@"Get devices request failed with error: %@", error); failureCallback(error); }]; diff --git a/SignalServiceKit/src/Network/API/TSNetworkManager.h b/SignalServiceKit/src/Network/API/TSNetworkManager.h index aca526709..9c33ac40e 100644 --- a/SignalServiceKit/src/Network/API/TSNetworkManager.h +++ b/SignalServiceKit/src/Network/API/TSNetworkManager.h @@ -26,6 +26,8 @@ NS_ASSUME_NONNULL_BEGIN extern NSString *const TSNetworkManagerDomain; +BOOL IsNSErrorNetworkFailure(NSError *_Nullable error); + @interface TSNetworkManager : NSObject - (instancetype)init NS_UNAVAILABLE; diff --git a/SignalServiceKit/src/Network/API/TSNetworkManager.m b/SignalServiceKit/src/Network/API/TSNetworkManager.m index 1fd8e8232..3b18a7b7b 100644 --- a/SignalServiceKit/src/Network/API/TSNetworkManager.m +++ b/SignalServiceKit/src/Network/API/TSNetworkManager.m @@ -12,6 +12,11 @@ NSString *const TSNetworkManagerDomain = @"org.whispersystems.signal.networkManager"; +BOOL IsNSErrorNetworkFailure(NSError *_Nullable error) +{ + return ([error.domain isEqualToString:TSNetworkManagerDomain] && error.code == 0); +} + @interface TSNetworkManager () typedef void (^failureBlock)(NSURLSessionDataTask *task, NSError *error); From 2418baec151f671c121a533d725ecc2f1f3226c5 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Mon, 24 Jul 2017 16:13:22 -0400 Subject: [PATCH 6/6] Respond to CR. // FREEBIE --- Podfile.lock | 2 +- SignalServiceKit/src/Messages/TSMessagesManager.m | 2 +- SignalServiceKit/src/Util/OWSAnalytics.h | 2 ++ SignalServiceKit/tests/Util/OWSAnalyticsTests.m | 6 +----- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/Podfile.lock b/Podfile.lock index af8dda1fe..153b82414 100644 --- a/Podfile.lock +++ b/Podfile.lock @@ -125,7 +125,7 @@ EXTERNAL SOURCES: :branch: signal-master :git: https://github.com/WhisperSystems/JSQMessagesViewController.git SignalServiceKit: - :path: "." + :path: . SocketRocket: :git: https://github.com/facebook/SocketRocket.git diff --git a/SignalServiceKit/src/Messages/TSMessagesManager.m b/SignalServiceKit/src/Messages/TSMessagesManager.m index 745d69021..d7fc2819d 100644 --- a/SignalServiceKit/src/Messages/TSMessagesManager.m +++ b/SignalServiceKit/src/Messages/TSMessagesManager.m @@ -408,7 +408,7 @@ NS_ASSUME_NONNULL_BEGIN if (encryptedData.length > kMaxEncryptedDataLength) { OWSProdErrorWParams(@"message_manager_error_oversize_message", ^{ return (@{ - @"message_size" : @(encryptedData.length), + @"message_size" : @([OWSAnalytics orderOfMagnitudeOf:(long)encryptedData.length]), }); }); completion(nil); diff --git a/SignalServiceKit/src/Util/OWSAnalytics.h b/SignalServiceKit/src/Util/OWSAnalytics.h index b3abfa4df..10714eeaa 100755 --- a/SignalServiceKit/src/Util/OWSAnalytics.h +++ b/SignalServiceKit/src/Util/OWSAnalytics.h @@ -48,6 +48,8 @@ typedef NS_ENUM(NSUInteger, OWSAnalyticsSeverity) { + (void)appLaunchDidComplete; ++ (long)orderOfMagnitudeOf:(long)value; + @end typedef NSDictionary *_Nonnull (^OWSProdAssertParametersBlock)(); diff --git a/SignalServiceKit/tests/Util/OWSAnalyticsTests.m b/SignalServiceKit/tests/Util/OWSAnalyticsTests.m index 8a13136e1..c4201fa01 100644 --- a/SignalServiceKit/tests/Util/OWSAnalyticsTests.m +++ b/SignalServiceKit/tests/Util/OWSAnalyticsTests.m @@ -12,11 +12,7 @@ NS_ASSUME_NONNULL_BEGIN @end -@interface OWSAnalytics (Test) - -+ (long)orderOfMagnitudeOf:(long)value; - -@end +#pragma mark - @implementation OWSAnalyticsTests