Attempt to fix the "frequent attachment download errors with low server ids".

// FREEBIE

src/Devices/OWSReadReceiptsProcessor.m

@@ -1,4 +1,6 @@
-//  Copyright © 2016 Open Whisper Systems. All rights reserved.
+//
+//  Copyright (c) 2017 Open Whisper Systems. All rights reserved.
+//
 
 #import "OWSReadReceiptsProcessor.h"
 #import "OWSDisappearingMessagesJob.h"
@@ -91,7 +93,7 @@ NSString *const OWSReadReceiptsProcessorMarkedMessageAsReadNotification =
                               object:message];
 
         } else {
-            DDLogDebug(@"%@ Received read receipt for an unkown message. Saving it for later.", self.tag);
+            DDLogDebug(@"%@ Received read receipt for an unknown message. Saving it for later.", self.tag);
             [readReceipt save];
         }
     }

src/Messages/Attachments/OWSAttachmentsProcessor.m

@@ -119,6 +119,9 @@ NS_ASSUME_NONNULL_BEGIN
         }
     };
 
+    if (attachment.serverId < 100) {
+        DDLogError(@"%@ Suspicious attachment id: %llu", self.tag, (unsigned long long)attachment.serverId);
+    }
     TSAttachmentRequest *attachmentRequest = [[TSAttachmentRequest alloc] initWithId:attachment.serverId relay:attachment.relay];
 
     [self.networkManager makeRequest:attachmentRequest
@@ -139,18 +142,48 @@ NS_ASSUME_NONNULL_BEGIN
 
                                  dispatch_async([OWSDispatch attachmentsQueue], ^{
                                      [self downloadFromLocation:location
-                                                        pointer:attachment
-                                                        success:^(NSData *_Nonnull encryptedData) {
-                                                            [self decryptAttachmentData:encryptedData
-                                                                                pointer:attachment
-                                                                                success:markAndHandleSuccess
-                                                                                failure:markAndHandleFailure];
-                                                        }
-                                                        failure:markAndHandleFailure];
+                                         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);
                              }];
 }
@@ -184,7 +217,7 @@ NS_ASSUME_NONNULL_BEGIN
 - (void)downloadFromLocation:(NSString *)location
                      pointer:(TSAttachmentPointer *)pointer
                      success:(void (^)(NSData *encryptedData))successHandler
-                     failure:(void (^)(NSError *error))failureHandler
+                     failure:(void (^)(NSURLSessionDataTask *_Nullable task, NSError *_Nonnull error))failureHandler
 {
     AFHTTPSessionManager *manager = [AFHTTPSessionManager manager];
     manager.requestSerializer     = [AFHTTPRequestSerializer serializer];
@@ -200,13 +233,13 @@ NS_ASSUME_NONNULL_BEGIN
              if (![responseObject isKindOfClass:[NSData class]]) {
                  DDLogError(@"%@ Failed retrieval of attachment. Response had unexpected format.", self.tag);
                  NSError *error = OWSErrorMakeUnableToProcessServerResponseError();
-                 return failureHandler(error);
+                 return failureHandler(task, error);
              }
              successHandler((NSData *)responseObject);
          }
          failure:^(NSURLSessionDataTask *_Nullable task, NSError *_Nonnull error) {
              DDLogError(@"Failed to retrieve attachment with error: %@", error.description);
-             return failureHandler(error);
+             return failureHandler(task, error);
          }];
 }
 

src/Messages/Attachments/TSAttachment.h

@@ -6,20 +6,42 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+@class TSAttachmentPointer;
+
 @interface TSAttachment : TSYapDatabaseObject {
 
 @protected
     NSString *_contentType;
 }
 
+// TSAttachment is a base class for TSAttachmentPointer (a yet-to-be-downloaded
+// incoming attachment) and TSAttachmentStream (an outgoing or already-downloaded
+// incoming attachment).
+//
+// The attachmentSchemaVersion and serverId properties only apply to
+// TSAttachmentPointer, which can be distinguished by the isDownloaded
+// property.
 @property (atomic, readwrite) UInt64 serverId;
 @property (atomic, readwrite) NSData *encryptionKey;
 @property (nonatomic, readonly) NSString *contentType;
+// This property
+
+@property (atomic, readwrite) BOOL isDownloaded;
 
+// This constructor is used for new instances of TSAttachmentPointer,
+// i.e. undownloaded incoming attachments.
 - (instancetype)initWithServerId:(UInt64)serverId
                    encryptionKey:(NSData *)encryptionKey
                      contentType:(NSString *)contentType;
 
+// This constructor is used for new instances of TSAttachmentStream
+// that represent new, un-uploaded outgoing attachments.
+- (instancetype)initWithContentType:(NSString *)contentType;
+
+// This constructor is used for new instances of TSAttachmentStream
+// that represent downloaded incoming attachments.
+- (instancetype)initWithPointer:(TSAttachmentPointer *)pointer;
+
 - (void)upgradeFromAttachmentSchemaVersion:(NSUInteger)attachmentSchemaVersion;
 
 @end

src/Messages/Attachments/TSAttachment.m

@@ -4,6 +4,7 @@
 
 #import "TSAttachment.h"
 #import "MIMETypeUtil.h"
+#import "TSAttachmentPointer.h"
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -17,6 +18,8 @@ NSUInteger const TSAttachmentSchemaVersion = 3;
 
 @implementation TSAttachment
 
+// This constructor is used for new instances of TSAttachmentPointer,
+// i.e. undownloaded incoming attachments.
 - (instancetype)initWithServerId:(UInt64)serverId
                    encryptionKey:(NSData *)encryptionKey
                      contentType:(NSString *)contentType
@@ -34,6 +37,44 @@ NSUInteger const TSAttachmentSchemaVersion = 3;
     return self;
 }
 
+// This constructor is used for new instances of TSAttachmentStream
+// that represent new, un-uploaded outgoing attachments.
+- (instancetype)initWithContentType:(NSString *)contentType
+{
+    self = [super init];
+    if (!self) {
+        return self;
+    }
+
+    _contentType = contentType;
+    _attachmentSchemaVersion = TSAttachmentSchemaVersion;
+
+    return self;
+}
+
+// This constructor is used for new instances of TSAttachmentStream
+// that represent downloaded incoming attachments.
+- (instancetype)initWithPointer:(TSAttachmentPointer *)pointer
+{
+    // Once saved, this AttachmentStream will replace the AttachmentPointer in the attachments collection.
+    self = [super initWithUniqueId:pointer.uniqueId];
+    if (!self) {
+        return self;
+    }
+
+    _serverId = pointer.serverId;
+    _encryptionKey = pointer.encryptionKey;
+    _contentType = pointer.contentType;
+    _attachmentSchemaVersion = TSAttachmentSchemaVersion;
+
+    return self;
+}
+
+- (BOOL)isDecimalNumberText:(NSString *)text
+{
+    return [text componentsSeparatedByCharactersInSet:[NSCharacterSet decimalDigitCharacterSet]].count == 1;
+}
+
 - (nullable instancetype)initWithCoder:(NSCoder *)coder
 {
     self = [super initWithCoder:coder];
@@ -51,11 +92,22 @@ NSUInteger const TSAttachmentSchemaVersion = 3;
 
 - (void)upgradeFromAttachmentSchemaVersion:(NSUInteger)attachmentSchemaVersion
 {
-    if (attachmentSchemaVersion < 2) {
+    // TSAttachment is a base class for TSAttachmentPointer (a yet-to-be-downloaded
+    // incoming attachment) and TSAttachmentStream (an outgoing or already-downloaded
+    // incoming attachment).
+    //
+    // The attachmentSchemaVersion and serverId properties only apply to
+    // TSAttachmentPointer, which can be distinguished by the isDownloaded
+    // property.
+    if (!_isDownloaded && _attachmentSchemaVersion < 2) {
         if (!_serverId) {
+            OWSAssert([self isDecimalNumberText:self.uniqueId]);
+            if (![self isDecimalNumberText:self.uniqueId]) {
+                DDLogError(@"%@ invalid legacy attachment uniqueId: %@.", self.tag, self.uniqueId);
+            }
             _serverId = [self.uniqueId integerValue];
             if (!_serverId) {
-                DDLogError(@"%@ failed to parse legacy uniqueId:%@ as integer.", self.tag, self.uniqueId);
+                DDLogError(@"%@ failed to parse legacy attachment uniqueId: %@.", self.tag, self.uniqueId);
             }
         }
     }

src/Messages/Attachments/TSAttachmentPointer.m

@@ -28,10 +28,6 @@ NS_ASSUME_NONNULL_BEGIN
     return self;
 }
 
-- (BOOL)isDownloaded {
-    return NO;
-}
-
 @end
 
 NS_ASSUME_NONNULL_END

src/Messages/Attachments/TSAttachmentStream.h

@@ -17,8 +17,6 @@ NS_ASSUME_NONNULL_BEGIN
 - (instancetype)initWithContentType:(NSString *)contentType NS_DESIGNATED_INITIALIZER;
 - (instancetype)initWithPointer:(TSAttachmentPointer *)pointer NS_DESIGNATED_INITIALIZER;
 
-@property (atomic, readwrite) BOOL isDownloaded;
-
 // Though now required, `digest` may be null for pre-existing records or from
 // messages received from other clients
 @property (nullable, nonatomic) NSData *digest;

src/Messages/Attachments/TSAttachmentStream.m

@@ -14,13 +14,12 @@ NS_ASSUME_NONNULL_BEGIN
 
 - (instancetype)initWithContentType:(NSString *)contentType
 {
-    self = [super init];
+    self = [super initWithContentType:contentType];
     if (!self) {
         return self;
     }
 
-    _contentType = contentType;
-    _isDownloaded = YES;
+    self.isDownloaded = YES;
     // TSAttachmentStream doesn't have any "incoming vs. outgoing"
     // state, but this constructor is used only for new outgoing
     // attachments which haven't been uploaded yet.
@@ -32,13 +31,13 @@ NS_ASSUME_NONNULL_BEGIN
 - (instancetype)initWithPointer:(TSAttachmentPointer *)pointer
 {
     // Once saved, this AttachmentStream will replace the AttachmentPointer in the attachments collection.
-    self = [super initWithUniqueId:pointer.uniqueId];
+    self = [super initWithPointer:pointer];
     if (!self) {
         return self;
     }
 
     _contentType = pointer.contentType;
-    _isDownloaded = YES;
+    self.isDownloaded = YES;
     // TSAttachmentStream doesn't have any "incoming vs. outgoing"
     // state, but this constructor is used only for new incoming
     // attachments which don't need to be uploaded.

src/Network/WebSockets/TSSocketManager.m

@@ -114,6 +114,8 @@ NSString *const SocketConnectingNotification = @"SocketConnectingNotification";
         }
     }
 
+    DDLogWarn(@"Creating new websocket");
+
     // If socket is not already open or connecting, connect now.
     self.status = kSocketStatusConnecting;
 }
@@ -229,6 +231,7 @@ NSString *const SocketConnectingNotification = @"SocketConnectingNotification";
 + (void)resignActivity {
     OWSAssert([NSThread isMainThread]);
 
+    DDLogWarn(@"resignActivity closing web socket");
     [[self sharedManager] closeWebSocket];
 }
 
@@ -253,8 +256,8 @@ NSString *const SocketConnectingNotification = @"SocketConnectingNotification";
 
 - (void)webSocket:(SRWebSocket *)webSocket didFailWithError:(NSError *)error {
     OWSAssert([NSThread isMainThread]);
-    
-    DDLogError(@"Error connecting to socket %@", error);
+
+    DDLogError(@"Websocket did fail with error: %@", error);
 
     [self closeWebSocket];
 
@@ -352,6 +355,8 @@ NSString *const SocketConnectingNotification = @"SocketConnectingNotification";
          wasClean:(BOOL)wasClean {
     OWSAssert([NSThread isMainThread]);
 
+    DDLogWarn(@"Websocket did close with code: %ld", (long)code);
+
     [self closeWebSocket];
 
     if (!wasClean && [self shouldKeepWebSocketAlive]) {
@@ -370,6 +375,7 @@ NSString *const SocketConnectingNotification = @"SocketConnectingNotification";
             [self scheduleRetry];
         }
     } else {
+        DDLogWarn(@"webSocketHeartBeat closing web socket");
         [self closeWebSocket];
     }
 }