Add logging, error checking and recovery around corrupt message mappings.

pull/1/head
Matthew Chen 7 years ago
parent 8f6151048c
commit 420f5f88ff

@ -455,29 +455,11 @@ typedef enum : NSUInteger {
[self ensureDynamicInteractions]; [self ensureDynamicInteractions];
[[OWSPrimaryStorage sharedManager] updateUIDatabaseConnectionToLatest]; [[OWSPrimaryStorage sharedManager] updateUIDatabaseConnectionToLatest];
if (thread.uniqueId.length > 0) { [self createNewMessageMappings];
self.messageMappings = [[YapDatabaseViewMappings alloc] initWithGroups:@[ thread.uniqueId ] if (![self reloadViewItems]) {
view:TSMessageDatabaseViewExtensionName]; OWSFail(@"%@ failed to reload view items in configureForThread.", self.logTag);
} else {
OWSFail(@"uniqueId unexpectedly empty for thread: %@", thread);
self.messageMappings =
[[YapDatabaseViewMappings alloc] initWithGroups:@[] view:TSMessageDatabaseViewExtensionName];
return;
} }
// Cells' appearance can depend on adjacent cells in both directions.
[self.messageMappings setCellDrawingDependencyOffsets:[NSSet setWithArray:@[
@(-1),
@(+1),
]]
forGroup:self.thread.uniqueId];
// We need to impose the range restrictions on the mappings immediately to avoid
// doing a great deal of unnecessary work and causing a perf hotspot.
[self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) {
[self.messageMappings updateWithTransaction:transaction];
}];
[self updateMessageMappingRangeOptions];
[self updateShouldObserveDBModifications]; [self updateShouldObserveDBModifications];
self.reloadTimer = [NSTimer weakScheduledTimerWithTimeInterval:1.f self.reloadTimer = [NSTimer weakScheduledTimerWithTimeInterval:1.f
@ -814,7 +796,9 @@ typedef enum : NSUInteger {
// Avoid layout corrupt issues and out-of-date message subtitles. // Avoid layout corrupt issues and out-of-date message subtitles.
self.lastReloadDate = [NSDate new]; self.lastReloadDate = [NSDate new];
self.collapseCutoffDate = [NSDate new]; self.collapseCutoffDate = [NSDate new];
[self reloadViewItems]; if (![self reloadViewItems]) {
OWSFail(@"%@ failed to reload view items in resetContentAndLayout.", self.logTag);
}
[self.collectionView.collectionViewLayout invalidateLayout]; [self.collectionView.collectionViewLayout invalidateLayout];
[self.collectionView reloadData]; [self.collectionView reloadData];
} }
@ -1771,7 +1755,6 @@ typedef enum : NSUInteger {
[self.messageMappings setRangeOptions:rangeOptions forGroup:self.thread.uniqueId]; [self.messageMappings setRangeOptions:rangeOptions forGroup:self.thread.uniqueId];
[self updateShowLoadMoreHeader]; [self updateShowLoadMoreHeader];
self.collapseCutoffDate = [NSDate new]; self.collapseCutoffDate = [NSDate new];
[self reloadViewItems];
} }
#pragma mark Bubble User Actions #pragma mark Bubble User Actions
@ -3241,7 +3224,7 @@ typedef enum : NSUInteger {
{ {
OWSAssertIsOnMainThread(); OWSAssertIsOnMainThread();
DDLogVerbose(@"%@ %s", self.logTag, __PRETTY_FUNCTION__); DDLogInfo(@"%@ %s", self.logTag, __PRETTY_FUNCTION__);
if (self.shouldObserveDBModifications) { if (self.shouldObserveDBModifications) {
// External database modifications can't be converted into incremental updates, // External database modifications can't be converted into incremental updates,
@ -3250,12 +3233,14 @@ typedef enum : NSUInteger {
// //
// We don't need to do this if we're not observing db modifications since we'll // We don't need to do this if we're not observing db modifications since we'll
// do it when we resume. // do it when we resume.
[self resetMappings]; [self hardResetMappings];
} }
} }
- (void)uiDatabaseWillUpdate:(NSNotification *)notification - (void)uiDatabaseWillUpdate:(NSNotification *)notification
{ {
DDLogInfo(@"%@ %s", self.logTag, __PRETTY_FUNCTION__);
// HACK to work around radar #28167779 // HACK to work around radar #28167779
// "UICollectionView performBatchUpdates can trigger a crash if the collection view is flagged for layout" // "UICollectionView performBatchUpdates can trigger a crash if the collection view is flagged for layout"
// more: https://github.com/PSPDFKit-labs/radar.apple.com/tree/master/28167779%20-%20CollectionViewBatchingIssue // more: https://github.com/PSPDFKit-labs/radar.apple.com/tree/master/28167779%20-%20CollectionViewBatchingIssue
@ -3265,14 +3250,17 @@ typedef enum : NSUInteger {
// We want to relayout our contents using the old message mappings and // We want to relayout our contents using the old message mappings and
// view items before they are updated. // view items before they are updated.
[self.collectionView layoutIfNeeded]; [self.collectionView layoutIfNeeded];
// ENDHACK to work around radar #28167779 // ENDHACK to work around radar #28167779)
} }
- (void)uiDatabaseDidUpdate:(NSNotification *)notification - (void)uiDatabaseDidUpdate:(NSNotification *)notification
{ {
OWSAssertIsOnMainThread(); OWSAssertIsOnMainThread();
DDLogInfo(@"%@ %s", self.logTag, __PRETTY_FUNCTION__);
if (!self.shouldObserveDBModifications) { if (!self.shouldObserveDBModifications) {
DDLogInfo(@"%@ %s ignoring", self.logTag, __PRETTY_FUNCTION__);
return; return;
} }
@ -3378,14 +3366,21 @@ typedef enum : NSUInteger {
// These errors seems to be very rare; they can only be reproduced // These errors seems to be very rare; they can only be reproduced
// using the more extreme actions in the debug UI. // using the more extreme actions in the debug UI.
OWSFail(@"%@ hasMalformedRowChange", self.logTag); OWSFail(@"%@ hasMalformedRowChange", self.logTag);
[self resetContentAndLayout]; [self hardResetMappings];
[self updateLastVisibleTimestamp]; [self updateLastVisibleTimestamp];
[self scrollToBottomAnimated:NO]; [self scrollToBottomAnimated:NO];
return; return;
} }
NSUInteger oldViewItemCount = self.viewItems.count; NSUInteger oldViewItemCount = self.viewItems.count;
[self reloadViewItems]; if (![self reloadViewItems]) {
// These errors are rare.
OWSFail(@"%@ could not reload view items; hard resetting message mappings.", self.logTag);
[self hardResetMappings];
[self updateLastVisibleTimestamp];
[self scrollToBottomAnimated:NO];
return;
}
BOOL wasAtBottom = [self isScrolledToBottom]; BOOL wasAtBottom = [self isScrolledToBottom];
// We want sending messages to feel snappy. So, if the only // We want sending messages to feel snappy. So, if the only
@ -4800,6 +4795,40 @@ typedef enum : NSUInteger {
self.previousLastTimestamp = nil; self.previousLastTimestamp = nil;
} }
- (void)createNewMessageMappings
{
if (self.thread.uniqueId.length > 0) {
self.messageMappings = [[YapDatabaseViewMappings alloc] initWithGroups:@[ self.thread.uniqueId ]
view:TSMessageDatabaseViewExtensionName];
} else {
OWSFail(@"uniqueId unexpectedly empty for thread: %@", self.thread);
self.messageMappings =
[[YapDatabaseViewMappings alloc] initWithGroups:@[] view:TSMessageDatabaseViewExtensionName];
}
// Cells' appearance can depend on adjacent cells in both directions.
[self.messageMappings setCellDrawingDependencyOffsets:[NSSet setWithArray:@[
@(-1),
@(+1),
]]
forGroup:self.thread.uniqueId];
// We need to impose the range restrictions on the mappings immediately to avoid
// doing a great deal of unnecessary work and causing a perf hotspot.
[self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) {
[self.messageMappings updateWithTransaction:transaction];
}];
[self updateMessageMappingRangeOptions];
}
- (void)hardResetMappings
{
DDLogInfo(@"%@ %s", self.logTag, __PRETTY_FUNCTION__);
[self createNewMessageMappings];
[self resetMappings];
}
- (void)resetMappings - (void)resetMappings
{ {
// If we're entering "active" mode (e.g. view is visible and app is in foreground), // If we're entering "active" mode (e.g. view is visible and app is in foreground),
@ -4814,7 +4843,6 @@ typedef enum : NSUInteger {
[self updateMessageMappingRangeOptions]; [self updateMessageMappingRangeOptions];
} }
self.collapseCutoffDate = [NSDate new]; self.collapseCutoffDate = [NSDate new];
[self reloadViewItems];
[self resetContentAndLayout]; [self resetContentAndLayout];
[self ensureDynamicInteractions]; [self ensureDynamicInteractions];
@ -4849,7 +4877,9 @@ typedef enum : NSUInteger {
// This is a key method. It builds or rebuilds the list of // This is a key method. It builds or rebuilds the list of
// cell view models. // cell view models.
- (void)reloadViewItems //
// Returns NO on error.
- (BOOL)reloadViewItems
{ {
NSMutableArray<ConversationViewItem *> *viewItems = [NSMutableArray new]; NSMutableArray<ConversationViewItem *> *viewItems = [NSMutableArray new];
NSMutableDictionary<NSString *, ConversationViewItem *> *viewItemCache = [NSMutableDictionary new]; NSMutableDictionary<NSString *, ConversationViewItem *> *viewItemCache = [NSMutableDictionary new];
@ -4857,6 +4887,7 @@ typedef enum : NSUInteger {
NSUInteger count = [self.messageMappings numberOfItemsInSection:0]; NSUInteger count = [self.messageMappings numberOfItemsInSection:0];
BOOL isGroupThread = self.isGroupConversation; BOOL isGroupThread = self.isGroupConversation;
__block BOOL hasError = NO;
[self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) { [self.uiDatabaseConnection readWithBlock:^(YapDatabaseReadTransaction *transaction) {
YapDatabaseViewTransaction *viewTransaction = [transaction ext:TSMessageDatabaseViewExtensionName]; YapDatabaseViewTransaction *viewTransaction = [transaction ext:TSMessageDatabaseViewExtensionName];
OWSAssert(viewTransaction); OWSAssert(viewTransaction);
@ -4869,6 +4900,7 @@ typedef enum : NSUInteger {
(unsigned long)row, (unsigned long)row,
(unsigned long)count); (unsigned long)count);
// TODO: Add analytics. // TODO: Add analytics.
hasError = YES;
continue; continue;
} }
if (!interaction.uniqueId) { if (!interaction.uniqueId) {
@ -4878,6 +4910,7 @@ typedef enum : NSUInteger {
(unsigned long)count, (unsigned long)count,
interaction); interaction);
// TODO: Add analytics. // TODO: Add analytics.
hasError = YES;
continue; continue;
} }
@ -5122,6 +5155,8 @@ typedef enum : NSUInteger {
self.viewItems = viewItems; self.viewItems = viewItems;
self.viewItemCache = viewItemCache; self.viewItemCache = viewItemCache;
return !hasError;
} }
// Whenever an interaction is modified, we need to reload it from the DB // Whenever an interaction is modified, we need to reload it from the DB

Loading…
Cancel
Save