From 3644d327397963c638aa9c0dde4d04267b940ca2 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Mon, 31 Oct 2022 21:49:18 +0100 Subject: [PATCH 01/28] WIP --- Sentry.xcodeproj/project.pbxproj | 8 ++ Sources/Sentry/SentryCrashScopeObserver.m | 5 - Sources/Sentry/SentryFileManager.m | 48 +++++--- .../Sentry/SentryOutOfMemoryScopeObserver.m | 111 ++++++++++++++++++ .../SentryOutOfMemoryTrackingIntegration.m | 11 ++ Sources/Sentry/include/SentryFileManager.h | 3 + .../include/SentryOutOfMemoryScopeObserver.h | 16 +++ 7 files changed, 180 insertions(+), 22 deletions(-) create mode 100644 Sources/Sentry/SentryOutOfMemoryScopeObserver.m create mode 100644 Sources/Sentry/include/SentryOutOfMemoryScopeObserver.h diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index a2c2a4b6321..cf29470d061 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -51,6 +51,8 @@ 0A5370A128A3EC2400B2DCDE /* SentryViewHierarchyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A5370A028A3EC2400B2DCDE /* SentryViewHierarchyTests.swift */; }; 0A56DA5F28ABA01B00C400D5 /* SentryTransactionContext+Private.h in Headers */ = {isa = PBXBuildFile; fileRef = 0A56DA5E28ABA01B00C400D5 /* SentryTransactionContext+Private.h */; }; 0A6EEADD28A657970076B469 /* UIViewRecursiveDescriptionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A6EEADC28A657970076B469 /* UIViewRecursiveDescriptionTests.swift */; }; + 0A80E433291017C300095219 /* SentryOutOfMemoryScopeObserver.m in Sources */ = {isa = PBXBuildFile; fileRef = 0A80E432291017C300095219 /* SentryOutOfMemoryScopeObserver.m */; }; + 0A80E435291017D500095219 /* SentryOutOfMemoryScopeObserver.h in Headers */ = {isa = PBXBuildFile; fileRef = 0A80E434291017D500095219 /* SentryOutOfMemoryScopeObserver.h */; }; 0A8F0A392886CC70000B15F6 /* SentryPermissionsObserver.h in Headers */ = {isa = PBXBuildFile; fileRef = 0AABE2EE288592750057ED69 /* SentryPermissionsObserver.h */; }; 0A94158228F6C4C2006A5DD1 /* SentryAppStateManagerTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A94158128F6C4C2006A5DD1 /* SentryAppStateManagerTests.swift */; }; 0A9415BA28F96CAC006A5DD1 /* TestSentryReachability.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A9415B928F96CAC006A5DD1 /* TestSentryReachability.swift */; }; @@ -771,6 +773,8 @@ 0A5370A028A3EC2400B2DCDE /* SentryViewHierarchyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryViewHierarchyTests.swift; sourceTree = ""; }; 0A56DA5E28ABA01B00C400D5 /* SentryTransactionContext+Private.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = "SentryTransactionContext+Private.h"; path = "include/SentryTransactionContext+Private.h"; sourceTree = ""; }; 0A6EEADC28A657970076B469 /* UIViewRecursiveDescriptionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UIViewRecursiveDescriptionTests.swift; sourceTree = ""; }; + 0A80E432291017C300095219 /* SentryOutOfMemoryScopeObserver.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryOutOfMemoryScopeObserver.m; sourceTree = ""; }; + 0A80E434291017D500095219 /* SentryOutOfMemoryScopeObserver.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryOutOfMemoryScopeObserver.h; path = include/SentryOutOfMemoryScopeObserver.h; sourceTree = ""; }; 0A94158128F6C4C2006A5DD1 /* SentryAppStateManagerTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryAppStateManagerTests.swift; sourceTree = ""; }; 0A9415B928F96CAC006A5DD1 /* TestSentryReachability.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestSentryReachability.swift; sourceTree = ""; }; 0A9BF4E128A114940068D266 /* SentryViewHierarchyIntegration.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryViewHierarchyIntegration.m; sourceTree = ""; }; @@ -2475,6 +2479,8 @@ 7B6C5F8626034395007F7DFF /* SentryOutOfMemoryLogic.m */, 7B98D7CA25FB64EC00C5A389 /* SentryOutOfMemoryTrackingIntegration.h */, 7B98D7CE25FB650F00C5A389 /* SentryOutOfMemoryTrackingIntegration.m */, + 0A80E434291017D500095219 /* SentryOutOfMemoryScopeObserver.h */, + 0A80E432291017C300095219 /* SentryOutOfMemoryScopeObserver.m */, ); name = OutOfMemory; sourceTree = ""; @@ -3161,6 +3167,7 @@ 7B42C48027E08F33009B58C2 /* SentryDependencyContainer.h in Headers */, 6334314120AD9AE40077E581 /* SentryMechanism.h in Headers */, 03F84D2827DD414C008FE43F /* SentryCPU.h in Headers */, + 0A80E435291017D500095219 /* SentryOutOfMemoryScopeObserver.h in Headers */, 7B610D642512399600B0B5D9 /* SentryHub+Private.h in Headers */, D8F6A24B2885515C00320515 /* SentryPredicateDescriptor.h in Headers */, 639FCF9C1EBC7F9500778193 /* SentryThread.h in Headers */, @@ -3380,6 +3387,7 @@ 84A8891D28DBD28900C51DFD /* SentryDevice.mm in Sources */, 7B56D73324616D9500B842DA /* SentryConcurrentRateLimitsDictionary.m in Sources */, 8ECC674825C23A20000E2BF6 /* SentryTransaction.m in Sources */, + 0A80E433291017C300095219 /* SentryOutOfMemoryScopeObserver.m in Sources */, 7BECF42826145CD900D9826E /* SentryMechanismMeta.m in Sources */, 8E7C982F2693D56000E6336C /* SentryTraceHeader.m in Sources */, 63FE715F20DA4C1100CDBAE8 /* SentryCrashID.c in Sources */, diff --git a/Sources/Sentry/SentryCrashScopeObserver.m b/Sources/Sentry/SentryCrashScopeObserver.m index 2c609667794..e4a2808dc1e 100644 --- a/Sources/Sentry/SentryCrashScopeObserver.m +++ b/Sources/Sentry/SentryCrashScopeObserver.m @@ -9,11 +9,6 @@ #import #import -@interface -SentryCrashScopeObserver () - -@end - @implementation SentryCrashScopeObserver - (instancetype)initWithMaxBreadcrumbs:(NSInteger)maxBreadcrumbs diff --git a/Sources/Sentry/SentryFileManager.m b/Sources/Sentry/SentryFileManager.m index a82eaa779de..c298b30fd27 100644 --- a/Sources/Sentry/SentryFileManager.m +++ b/Sources/Sentry/SentryFileManager.m @@ -28,6 +28,8 @@ @property (nonatomic, copy) NSString *lastInForegroundFilePath; @property (nonatomic, copy) NSString *previousAppStateFilePath; @property (nonatomic, copy) NSString *appStateFilePath; +@property (nonatomic, copy) NSString *previousBreadcrumbsFilePath; +@property (nonatomic, copy) NSString *breadcrumbsFilePath; @property (nonatomic, copy) NSString *timezoneOffsetFilePath; @property (nonatomic, assign) NSUInteger currentFileCounter; @property (nonatomic, assign) NSUInteger maxEnvelopes; @@ -83,6 +85,10 @@ - (nullable instancetype)initWithOptions:(SentryOptions *)options self.previousAppStateFilePath = [self.sentryPath stringByAppendingPathComponent:@"previous.app.state"]; self.appStateFilePath = [self.sentryPath stringByAppendingPathComponent:@"app.state"]; + self.previousBreadcrumbsFilePath = + [self.sentryPath stringByAppendingPathComponent:@"previous.breadcrumbs.state"]; + self.breadcrumbsFilePath = + [self.sentryPath stringByAppendingPathComponent:@"breadcrumbs.state"]; self.timezoneOffsetFilePath = [self.sentryPath stringByAppendingPathComponent:@"timezone.offset"]; @@ -240,7 +246,9 @@ - (BOOL)removeFileAtPath:(NSString *)path NSError *error = nil; @synchronized(self) { [fileManager removeItemAtPath:path error:&error]; - if (nil != error) { + + // We don't want to log an error if the file doesn't exist. + if (nil != error && error.code != NSFileNoSuchFileError) { SENTRY_LOG_ERROR(@"Couldn't delete file %@: %@", path, error); return NO; } @@ -455,25 +463,31 @@ - (void)storeAppState:(SentryAppState *)appState - (void)moveAppStateToPreviousAppState { @synchronized(self.appStateFilePath) { - NSFileManager *fileManager = [NSFileManager defaultManager]; + [self moveState:self.appStateFilePath toPreviousState:self.previousAppStateFilePath]; + } +} - // We first need to remove the old previous app state file, - // or we can't move the current app state file to it. - [self removeFileAtPath:self.previousAppStateFilePath]; +- (void)moveBreadcrumbsToPreviousBreadcrumbs +{ + @synchronized(self.breadcrumbsFilePath) { + [self moveState:self.breadcrumbsFilePath toPreviousState:self.previousBreadcrumbsFilePath]; + } +} - NSError *error = nil; - [fileManager moveItemAtPath:self.appStateFilePath - toPath:self.previousAppStateFilePath - error:&error]; +- (void)moveState:(NSString *)stateFilePath toPreviousState:(NSString *)previousStateFilePath +{ + NSFileManager *fileManager = [NSFileManager defaultManager]; - // We don't want to log an error if the file doesn't exist. - if (nil != error && error.code != NSFileNoSuchFileError) { - [SentryLog - logWithMessage:[NSString - stringWithFormat: - @"Failed to move app state to previous app state: %@", error] - andLevel:kSentryLevelError]; - } + // We first need to remove the old previous state file, + // or we can't move the current state file to it. + [self removeFileAtPath:previousStateFilePath]; + + NSError *error = nil; + [fileManager moveItemAtPath:stateFilePath toPath:previousStateFilePath error:&error]; + + // We don't want to log an error if the file doesn't exist. + if (nil != error && error.code != NSFileNoSuchFileError) { + SENTRY_LOG_ERROR(@"Failed to move %@ to previous state file: %@", stateFilePath, error); } } diff --git a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m new file mode 100644 index 00000000000..c762eb9fb96 --- /dev/null +++ b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m @@ -0,0 +1,111 @@ +#import "SentryOutOfMemoryScopeObserver.h" +#import +#import +#import + +@interface +SentryOutOfMemoryScopeObserver () + +@property (strong, nonatomic) NSFileHandle *fileHandle; + +@end + +@implementation SentryOutOfMemoryScopeObserver + +- (instancetype)initWithMaxBreadcrumbs:(NSInteger)maxBreadcrumbs + fileManager:(SentryFileManager *)fileManager +{ + if (self = [super init]) { + if (![[NSFileManager defaultManager] fileExistsAtPath:fileManager.breadcrumbsFilePath]) { + [[NSFileManager defaultManager] createFileAtPath:fileManager.breadcrumbsFilePath + contents:nil + attributes:nil]; + } + + self.fileHandle = [NSFileHandle fileHandleForWritingAtPath:fileManager.breadcrumbsFilePath]; + + if (!self.fileHandle) { + SENTRY_LOG_ERROR(@"Couldn't open file handle for %@", fileManager.breadcrumbsFilePath); + } + } + + return self; +} + +- (void)dealloc +{ + [self.fileHandle closeFile]; +} + +- (void)store:(NSData *)data +{ + [self.fileHandle seekToEndOfFile]; + [self.fileHandle writeData:[@"\n" dataUsingEncoding:NSASCIIStringEncoding]]; + [self.fileHandle writeData:data]; +} + +- (void)addBreadcrumb:(nonnull SentryBreadcrumb *)crumb +{ + NSError *error; + NSData *jsonData = [NSJSONSerialization dataWithJSONObject:[crumb serialize] + options:0 + error:&error]; + + if (error) { + SENTRY_LOG_ERROR(@"Error serializing breadcrumb: %@", error); + } else { + [self store:jsonData]; + } +} + +- (void)clear +{ + SENTRY_LOG_DEBUG(@"clear"); +} + +- (void)clearBreadcrumbs +{ + SENTRY_LOG_DEBUG(@"clearBreadcrumbs"); +} + +- (void)setContext:(nullable NSDictionary *)context +{ + // Left blank on purpose +} + +- (void)setDist:(nullable NSString *)dist +{ + // Left blank on purpose +} + +- (void)setEnvironment:(nullable NSString *)environment +{ + // Left blank on purpose +} + +- (void)setExtras:(nullable NSDictionary *)extras +{ + // Left blank on purpose +} + +- (void)setFingerprint:(nullable NSArray *)fingerprint +{ + // Left blank on purpose +} + +- (void)setLevel:(enum SentryLevel)level +{ + // Left blank on purpose +} + +- (void)setTags:(nullable NSDictionary *)tags +{ + // Left blank on purpose +} + +- (void)setUser:(nullable SentryUser *)user +{ + // Left blank on purpose +} + +@end diff --git a/Sources/Sentry/SentryOutOfMemoryTrackingIntegration.m b/Sources/Sentry/SentryOutOfMemoryTrackingIntegration.m index d83de13656d..ced712fa336 100644 --- a/Sources/Sentry/SentryOutOfMemoryTrackingIntegration.m +++ b/Sources/Sentry/SentryOutOfMemoryTrackingIntegration.m @@ -1,4 +1,5 @@ #import "SentryDefines.h" +#import "SentryScope+Private.h" #import #import #import @@ -8,6 +9,7 @@ #import #import #import +#import #import #import #import @@ -73,6 +75,15 @@ - (BOOL)installWithOptions:(SentryOptions *)options self.appStateManager = appStateManager; + [fileManager moveBreadcrumbsToPreviousBreadcrumbs]; + + SentryOutOfMemoryScopeObserver *scopeObserver = [[SentryOutOfMemoryScopeObserver alloc] + initWithMaxBreadcrumbs:options.maxBreadcrumbs + fileManager:[[[SentrySDK currentHub] getClient] fileManager]]; + + [SentrySDK.currentHub configureScope:^( + SentryScope *_Nonnull outerScope) { [outerScope addObserver:scopeObserver]; }]; + return YES; } diff --git a/Sources/Sentry/include/SentryFileManager.h b/Sources/Sentry/include/SentryFileManager.h index 44548397295..0eefed2c659 100644 --- a/Sources/Sentry/include/SentryFileManager.h +++ b/Sources/Sentry/include/SentryFileManager.h @@ -15,6 +15,7 @@ NS_SWIFT_NAME(SentryFileManager) SENTRY_NO_INIT @property (nonatomic, readonly) NSString *sentryPath; +@property (nonatomic, readonly) NSString *breadcrumbsFilePath; - (nullable instancetype)initWithOptions:(SentryOptions *)options andCurrentDateProvider:(id)currentDateProvider @@ -70,6 +71,8 @@ SENTRY_NO_INIT - (SentryAppState *_Nullable)readPreviousAppState; - (void)deleteAppState; +- (void)moveBreadcrumbsToPreviousBreadcrumbs; + - (NSNumber *_Nullable)readTimezoneOffset; - (void)storeTimezoneOffset:(NSInteger)offset; - (void)deleteTimezoneOffset; diff --git a/Sources/Sentry/include/SentryOutOfMemoryScopeObserver.h b/Sources/Sentry/include/SentryOutOfMemoryScopeObserver.h new file mode 100644 index 00000000000..4daae1e7da9 --- /dev/null +++ b/Sources/Sentry/include/SentryOutOfMemoryScopeObserver.h @@ -0,0 +1,16 @@ +#import "SentryDefines.h" +#import "SentryScopeObserver.h" + +@class SentryFileManager; + +NS_ASSUME_NONNULL_BEGIN + +@interface SentryOutOfMemoryScopeObserver : NSObject +SENTRY_NO_INIT + +- (instancetype)initWithMaxBreadcrumbs:(NSInteger)maxBreadcrumbs + fileManager:(SentryFileManager *)fileManager; + +@end + +NS_ASSUME_NONNULL_END From 05ab4cfc2b8a5f5359375dddf38c4450c6859268 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Tue, 1 Nov 2022 14:22:55 +0100 Subject: [PATCH 02/28] WIP2 --- Sources/Sentry/SentryEvent.m | 5 ++ Sources/Sentry/SentryFileManager.m | 3 +- .../Sentry/SentryOutOfMemoryScopeObserver.m | 51 ++++++++++++++----- Sources/Sentry/SentryOutOfMemoryTracker.m | 44 +++++++++++++++- .../SentryOutOfMemoryTrackingIntegration.m | 3 +- Sources/Sentry/SentrySDK.m | 1 + Sources/Sentry/include/SentryEvent+Private.h | 1 + Sources/Sentry/include/SentryFileManager.h | 1 + 8 files changed, 91 insertions(+), 18 deletions(-) diff --git a/Sources/Sentry/SentryEvent.m b/Sources/Sentry/SentryEvent.m index 243f9405c51..eb25083f538 100644 --- a/Sources/Sentry/SentryEvent.m +++ b/Sources/Sentry/SentryEvent.m @@ -5,6 +5,7 @@ #import "SentryClient.h" #import "SentryCurrentDate.h" #import "SentryDebugMeta.h" +#import "SentryEvent+Private.h" #import "SentryException.h" #import "SentryId.h" #import "SentryLevelMapper.h" @@ -139,6 +140,10 @@ - (void)addSimpleProperties:(NSMutableDictionary *)serializedData [serializedData setValue:[self serializeBreadcrumbs] forKey:@"breadcrumbs"]; + if (self.serializedBreadcrumbs.count > 0) { + [serializedData setValue:self.serializedBreadcrumbs forKey:@"breadcrumbs"]; + } + [serializedData setValue:[self.context sentry_sanitize] forKey:@"contexts"]; if (nil != self.message) { diff --git a/Sources/Sentry/SentryFileManager.m b/Sources/Sentry/SentryFileManager.m index c298b30fd27..ba97b18ac36 100644 --- a/Sources/Sentry/SentryFileManager.m +++ b/Sources/Sentry/SentryFileManager.m @@ -508,8 +508,7 @@ - (SentryAppState *_Nullable)readPreviousAppState - (SentryAppState *_Nullable)readAppStateFrom:(NSString *)path { NSFileManager *fileManager = [NSFileManager defaultManager]; - NSData *currentData = nil; - currentData = [fileManager contentsAtPath:path]; + NSData *currentData = [fileManager contentsAtPath:path]; if (nil == currentData) { return nil; } diff --git a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m index c762eb9fb96..d500cababea 100644 --- a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m +++ b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m @@ -6,6 +6,7 @@ @interface SentryOutOfMemoryScopeObserver () +@property (strong, nonatomic) SentryFileManager *fileManager; @property (strong, nonatomic) NSFileHandle *fileHandle; @end @@ -16,17 +17,8 @@ - (instancetype)initWithMaxBreadcrumbs:(NSInteger)maxBreadcrumbs fileManager:(SentryFileManager *)fileManager { if (self = [super init]) { - if (![[NSFileManager defaultManager] fileExistsAtPath:fileManager.breadcrumbsFilePath]) { - [[NSFileManager defaultManager] createFileAtPath:fileManager.breadcrumbsFilePath - contents:nil - attributes:nil]; - } - - self.fileHandle = [NSFileHandle fileHandleForWritingAtPath:fileManager.breadcrumbsFilePath]; - - if (!self.fileHandle) { - SENTRY_LOG_ERROR(@"Couldn't open file handle for %@", fileManager.breadcrumbsFilePath); - } + self.fileManager = fileManager; + [self createFile]; } return self; @@ -37,6 +29,36 @@ - (void)dealloc [self.fileHandle closeFile]; } +// PRAGMA MARK: - Helper methods + +- (void)deleteFile +{ + if ([[NSFileManager defaultManager] fileExistsAtPath:self.fileManager.breadcrumbsFilePath]) { + NSError *error; + [[NSFileManager defaultManager] removeItemAtPath:self.fileManager.breadcrumbsFilePath + error:&error]; + if (error) { + SENTRY_LOG_ERROR(@"Couldn't delete file %@", self.fileManager.breadcrumbsFilePath); + } + } +} + +- (void)createFile +{ + if (![[NSFileManager defaultManager] fileExistsAtPath:self.fileManager.breadcrumbsFilePath]) { + [[NSFileManager defaultManager] createFileAtPath:self.fileManager.breadcrumbsFilePath + contents:nil + attributes:nil]; + } + + self.fileHandle = + [NSFileHandle fileHandleForWritingAtPath:self.fileManager.breadcrumbsFilePath]; + + if (!self.fileHandle) { + SENTRY_LOG_ERROR(@"Couldn't open file handle for %@", self.fileManager.breadcrumbsFilePath); + } +} + - (void)store:(NSData *)data { [self.fileHandle seekToEndOfFile]; @@ -44,6 +66,8 @@ - (void)store:(NSData *)data [self.fileHandle writeData:data]; } +// PRAGMA MARK: - SentryScopeObserver + - (void)addBreadcrumb:(nonnull SentryBreadcrumb *)crumb { NSError *error; @@ -60,12 +84,13 @@ - (void)addBreadcrumb:(nonnull SentryBreadcrumb *)crumb - (void)clear { - SENTRY_LOG_DEBUG(@"clear"); + [self clearBreadcrumbs]; } - (void)clearBreadcrumbs { - SENTRY_LOG_DEBUG(@"clearBreadcrumbs"); + [self deleteFile]; + [self createFile]; } - (void)setContext:(nullable NSDictionary *)context diff --git a/Sources/Sentry/SentryOutOfMemoryTracker.m b/Sources/Sentry/SentryOutOfMemoryTracker.m index 65b9b372774..c153282993b 100644 --- a/Sources/Sentry/SentryOutOfMemoryTracker.m +++ b/Sources/Sentry/SentryOutOfMemoryTracker.m @@ -1,10 +1,10 @@ +#import "SentryEvent+Private.h" #import "SentryFileManager.h" #import #import #import #import #import -#import #import #import #import @@ -48,17 +48,59 @@ - (instancetype)initWithOptions:(SentryOptions *)options return self; } +- (NSArray *)loadBreadcrumbs +{ + if ([[NSFileManager defaultManager] + fileExistsAtPath:self.fileManager.previousBreadcrumbsFilePath]) { + NSMutableArray *breadcrumbs = [NSMutableArray array]; + + FILE *file = fopen([self.fileManager.previousBreadcrumbsFilePath UTF8String], "r"); + + char buffer[4096]; + while (fgets(buffer, 4096, file) != NULL) { + NSString *result = [[NSString stringWithUTF8String:buffer] + stringByTrimmingCharactersInSet:[NSCharacterSet newlineCharacterSet]]; + if (result.length == 0) { + continue; + } + + NSData *line = [result dataUsingEncoding:NSUTF8StringEncoding]; + + NSError *error; + NSDictionary *dict = [NSJSONSerialization JSONObjectWithData:line + options:0 + error:&error]; + + if (error) { + SENTRY_LOG_ERROR(@"Error deserializing breadcrumb: %@", error); + } else { + [breadcrumbs addObject:dict]; + } + } + + return breadcrumbs; + } + + SENTRY_LOG_ERROR(@"File not found: %@", self.fileManager.previousBreadcrumbsFilePath); + return [[NSArray alloc] init]; +} + - (void)start { #if SENTRY_HAS_UIKIT [self.appStateManager start]; [self.dispatchQueue dispatchAsyncWithBlock:^{ + [self loadBreadcrumbs]; + if ([self.outOfMemoryLogic isOOM]) { SentryEvent *event = [[SentryEvent alloc] initWithLevel:kSentryLevelFatal]; // Set to empty list so no breadcrumbs of the current scope are added event.breadcrumbs = @[]; + // Load the previous breascrumbs from disk, which are already serialized + event.serializedBreadcrumbs = [self loadBreadcrumbs]; + SentryException *exception = [[SentryException alloc] initWithValue:SentryOutOfMemoryExceptionValue type:SentryOutOfMemoryExceptionType]; diff --git a/Sources/Sentry/SentryOutOfMemoryTrackingIntegration.m b/Sources/Sentry/SentryOutOfMemoryTrackingIntegration.m index ced712fa336..e8854f26232 100644 --- a/Sources/Sentry/SentryOutOfMemoryTrackingIntegration.m +++ b/Sources/Sentry/SentryOutOfMemoryTrackingIntegration.m @@ -67,6 +67,7 @@ - (BOOL)installWithOptions:(SentryOptions *)options appStateManager:appStateManager dispatchQueueWrapper:dispatchQueueWrapper fileManager:fileManager]; + [self.tracker start]; self.anrTracker = @@ -75,8 +76,6 @@ - (BOOL)installWithOptions:(SentryOptions *)options self.appStateManager = appStateManager; - [fileManager moveBreadcrumbsToPreviousBreadcrumbs]; - SentryOutOfMemoryScopeObserver *scopeObserver = [[SentryOutOfMemoryScopeObserver alloc] initWithMaxBreadcrumbs:options.maxBreadcrumbs fileManager:[[[SentrySDK currentHub] getClient] fileManager]]; diff --git a/Sources/Sentry/SentrySDK.m b/Sources/Sentry/SentrySDK.m index ad9b925b709..e08b87dac1a 100644 --- a/Sources/Sentry/SentrySDK.m +++ b/Sources/Sentry/SentrySDK.m @@ -149,6 +149,7 @@ + (void)startWithOptionsObject:(SentryOptions *)options SentryClient *newClient = [[SentryClient alloc] initWithOptions:options]; [newClient.fileManager moveAppStateToPreviousAppState]; + [newClient.fileManager moveBreadcrumbsToPreviousBreadcrumbs]; // The Hub needs to be initialized with a client so that closing a session // can happen. diff --git a/Sources/Sentry/include/SentryEvent+Private.h b/Sources/Sentry/include/SentryEvent+Private.h index 734f1e5aeb8..f750acc84ef 100644 --- a/Sources/Sentry/include/SentryEvent+Private.h +++ b/Sources/Sentry/include/SentryEvent+Private.h @@ -8,5 +8,6 @@ SentryEvent (Private) * This indicates whether this event is a result of a crash. */ @property (nonatomic) BOOL isCrashEvent; +@property (nonatomic, strong) NSArray *serializedBreadcrumbs; @end diff --git a/Sources/Sentry/include/SentryFileManager.h b/Sources/Sentry/include/SentryFileManager.h index 0eefed2c659..2b3c964856d 100644 --- a/Sources/Sentry/include/SentryFileManager.h +++ b/Sources/Sentry/include/SentryFileManager.h @@ -16,6 +16,7 @@ SENTRY_NO_INIT @property (nonatomic, readonly) NSString *sentryPath; @property (nonatomic, readonly) NSString *breadcrumbsFilePath; +@property (nonatomic, readonly) NSString *previousBreadcrumbsFilePath; - (nullable instancetype)initWithOptions:(SentryOptions *)options andCurrentDateProvider:(id)currentDateProvider From a8f6ff495d77f2e016929af7b30590f917bdbbbd Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Tue, 1 Nov 2022 14:26:46 +0100 Subject: [PATCH 03/28] Don't call it twice --- Sources/Sentry/SentryOutOfMemoryTracker.m | 2 -- 1 file changed, 2 deletions(-) diff --git a/Sources/Sentry/SentryOutOfMemoryTracker.m b/Sources/Sentry/SentryOutOfMemoryTracker.m index c153282993b..1446d756fa4 100644 --- a/Sources/Sentry/SentryOutOfMemoryTracker.m +++ b/Sources/Sentry/SentryOutOfMemoryTracker.m @@ -91,8 +91,6 @@ - (void)start [self.appStateManager start]; [self.dispatchQueue dispatchAsyncWithBlock:^{ - [self loadBreadcrumbs]; - if ([self.outOfMemoryLogic isOOM]) { SentryEvent *event = [[SentryEvent alloc] initWithLevel:kSentryLevelFatal]; // Set to empty list so no breadcrumbs of the current scope are added From cd5a66f1370179e2b5c481c412a0a18dfd631a07 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Tue, 1 Nov 2022 14:33:31 +0100 Subject: [PATCH 04/28] Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c774f2edc56..646e2ba6620 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Profile concurrent transactions (#2227) - Disable bitcode for Carthage distribution (#2341) +- Store breadcrumbs to disk for OOM events (#2347) ### Fixes From 932433fe3218241d346bc31e12e2a305ab3a1f28 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Wed, 2 Nov 2022 10:53:03 +0100 Subject: [PATCH 05/28] Close file before deleting it --- Sources/Sentry/SentryOutOfMemoryScopeObserver.m | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m index d500cababea..82876d8d3c7 100644 --- a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m +++ b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m @@ -33,6 +33,8 @@ - (void)dealloc - (void)deleteFile { + [self.fileHandle closeFile]; + if ([[NSFileManager defaultManager] fileExistsAtPath:self.fileManager.breadcrumbsFilePath]) { NSError *error; [[NSFileManager defaultManager] removeItemAtPath:self.fileManager.breadcrumbsFilePath From c79db7b071203d3b1e730686564e3fc4178c1078 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Wed, 2 Nov 2022 12:53:14 +0100 Subject: [PATCH 06/28] Fix test crashes --- Sources/Sentry/SentryEvent.m | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Sentry/SentryEvent.m b/Sources/Sentry/SentryEvent.m index eb25083f538..03d3f19c372 100644 --- a/Sources/Sentry/SentryEvent.m +++ b/Sources/Sentry/SentryEvent.m @@ -21,6 +21,7 @@ SentryEvent () @property (nonatomic) BOOL isCrashEvent; +@property (nonatomic, strong) NSArray *serializedBreadcrumbs; @end From 1fad3e567043aee5957da4f025990e5086c8e1c5 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Wed, 2 Nov 2022 13:51:41 +0100 Subject: [PATCH 07/28] Fix removeFile behavior --- Sources/Sentry/SentryFileManager.m | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Sources/Sentry/SentryFileManager.m b/Sources/Sentry/SentryFileManager.m index ba97b18ac36..815ad41f3de 100644 --- a/Sources/Sentry/SentryFileManager.m +++ b/Sources/Sentry/SentryFileManager.m @@ -247,9 +247,11 @@ - (BOOL)removeFileAtPath:(NSString *)path @synchronized(self) { [fileManager removeItemAtPath:path error:&error]; - // We don't want to log an error if the file doesn't exist. - if (nil != error && error.code != NSFileNoSuchFileError) { - SENTRY_LOG_ERROR(@"Couldn't delete file %@: %@", path, error); + if (nil != error) { + // We don't want to log an error if the file doesn't exist. + if (error.code != NSFileNoSuchFileError) { + SENTRY_LOG_ERROR(@"Couldn't delete file %@: %@", path, error); + } return NO; } } From 52e596452e78ba3c5766f1f6e7a2547d50b44daf Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Wed, 2 Nov 2022 14:59:54 +0100 Subject: [PATCH 08/28] addBreadcrumb -> addSerializedBreadcrumb --- Sources/Sentry/SentryCrashScopeObserver.m | 5 ++--- Sources/Sentry/SentryOutOfMemoryScopeObserver.m | 6 ++---- Sources/Sentry/SentryScope.m | 2 +- Sources/Sentry/include/SentryScopeObserver.h | 2 +- .../SentryCrashScopeObserverTests.swift | 16 ++++++++-------- Tests/SentryTests/SentryScopeSwiftTests.swift | 13 ++++++++----- 6 files changed, 22 insertions(+), 22 deletions(-) diff --git a/Sources/Sentry/SentryCrashScopeObserver.m b/Sources/Sentry/SentryCrashScopeObserver.m index e4a2808dc1e..5395c93d438 100644 --- a/Sources/Sentry/SentryCrashScopeObserver.m +++ b/Sources/Sentry/SentryCrashScopeObserver.m @@ -85,10 +85,9 @@ - (void)setLevel:(enum SentryLevel)level sentrycrash_scopesync_setLevel([json bytes]); } -- (void)addBreadcrumb:(SentryBreadcrumb *)crumb +- (void)addSerializedBreadcrumb:(NSDictionary *)crumb { - NSDictionary *serialized = [crumb serialize]; - NSData *json = [self toJSONEncodedCString:serialized]; + NSData *json = [self toJSONEncodedCString:crumb]; if (json == nil) { return; } diff --git a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m index 82876d8d3c7..f99ed79c870 100644 --- a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m +++ b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m @@ -70,12 +70,10 @@ - (void)store:(NSData *)data // PRAGMA MARK: - SentryScopeObserver -- (void)addBreadcrumb:(nonnull SentryBreadcrumb *)crumb +- (void)addSerializedBreadcrumb:(NSDictionary *)crumb { NSError *error; - NSData *jsonData = [NSJSONSerialization dataWithJSONObject:[crumb serialize] - options:0 - error:&error]; + NSData *jsonData = [NSJSONSerialization dataWithJSONObject:crumb options:0 error:&error]; if (error) { SENTRY_LOG_ERROR(@"Error serializing breadcrumb: %@", error); diff --git a/Sources/Sentry/SentryScope.m b/Sources/Sentry/SentryScope.m index 48f3b4fe41a..067174e1a62 100644 --- a/Sources/Sentry/SentryScope.m +++ b/Sources/Sentry/SentryScope.m @@ -133,7 +133,7 @@ - (void)addBreadcrumb:(SentryBreadcrumb *)crumb } for (id observer in self.observers) { - [observer addBreadcrumb:crumb]; + [observer addSerializedBreadcrumb:[crumb serialize]]; } } } diff --git a/Sources/Sentry/include/SentryScopeObserver.h b/Sources/Sentry/include/SentryScopeObserver.h index 32aba7aff1a..a0fada9c772 100644 --- a/Sources/Sentry/include/SentryScopeObserver.h +++ b/Sources/Sentry/include/SentryScopeObserver.h @@ -25,7 +25,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)setLevel:(enum SentryLevel)level; -- (void)addBreadcrumb:(SentryBreadcrumb *)crumb; +- (void)addSerializedBreadcrumb:(NSDictionary *)crumb; - (void)clearBreadcrumbs; diff --git a/Tests/SentryTests/Integrations/SentryCrash/SentryCrashScopeObserverTests.swift b/Tests/SentryTests/Integrations/SentryCrash/SentryCrashScopeObserverTests.swift index a2ba1621253..79442d79a44 100644 --- a/Tests/SentryTests/Integrations/SentryCrash/SentryCrashScopeObserverTests.swift +++ b/Tests/SentryTests/Integrations/SentryCrash/SentryCrashScopeObserverTests.swift @@ -204,7 +204,7 @@ class SentryCrashScopeObserverTests: XCTestCase { func testAddCrumb() { let sut = fixture.sut let crumb = TestData.crumb - sut.add(crumb) + sut.addSerializedBreadcrumb(crumb.serialize()) assertOneCrumbSetToScope(crumb: crumb) } @@ -216,14 +216,14 @@ class SentryCrashScopeObserverTests: XCTestCase { func testCallConfigureCrumbTwice() { let sut = fixture.sut let crumb = TestData.crumb - sut.add(crumb) + sut.addSerializedBreadcrumb(crumb.serialize()) sentrycrash_scopesync_configureBreadcrumbs(fixture.maxBreadcrumbs) let scope = sentrycrash_scopesync_getScope() XCTAssertEqual(0, scope?.pointee.currentCrumb) - sut.add(crumb) + sut.addSerializedBreadcrumb(crumb.serialize()) assertOneCrumbSetToScope(crumb: crumb) } @@ -234,7 +234,7 @@ class SentryCrashScopeObserverTests: XCTestCase { for i in 0...fixture.maxBreadcrumbs { let crumb = TestData.crumb crumb.message = "\(i)" - sut.add(crumb) + sut.addSerializedBreadcrumb(crumb.serialize()) crumbs.append(crumb) } crumbs.removeFirst() @@ -273,11 +273,11 @@ class SentryCrashScopeObserverTests: XCTestCase { sut.setExtras(fixture.extras) sut.setFingerprint(fixture.fingerprint) sut.setLevel(SentryLevel.fatal) - sut.add(TestData.crumb) - + sut.addSerializedBreadcrumb(TestData.crumb.serialize()) + sut.clear() - - assertEmptyScope() + + assertEmptyScope() } func testEmptyScope() { diff --git a/Tests/SentryTests/SentryScopeSwiftTests.swift b/Tests/SentryTests/SentryScopeSwiftTests.swift index ee36b86426c..0fd895dd21f 100644 --- a/Tests/SentryTests/SentryScopeSwiftTests.swift +++ b/Tests/SentryTests/SentryScopeSwiftTests.swift @@ -455,7 +455,10 @@ class SentryScopeSwiftTests: XCTestCase { sut.add(crumb) sut.add(crumb) - XCTAssertEqual([crumb, crumb], observer.crumbs) + XCTAssertEqual( + [crumb.serialize() as! [String: AnyHashable], crumb.serialize() as! [String: AnyHashable]], + observer.crumbs + ) } func testScopeObserver_clearBreadcrumb() { @@ -516,11 +519,11 @@ class SentryScopeSwiftTests: XCTestCase { self.level = level } - var crumbs: [Breadcrumb] = [] - func add(_ crumb: Breadcrumb) { - crumbs.append(crumb) + var crumbs: [[String: AnyHashable]] = [] + func addSerializedBreadcrumb(_ crumb: [String: Any]) { + crumbs.append(crumb as! [String: AnyHashable]) } - + var clearBreadcrumbInvocations = 0 func clearBreadcrumbs() { clearBreadcrumbInvocations += 1 From b2f268d3a4bc3963b6a4969fe0e0920495d34d65 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Wed, 2 Nov 2022 16:29:35 +0100 Subject: [PATCH 09/28] Write to two breadcrumb files --- Sources/Sentry/SentryFileManager.m | 25 +++++--- .../Sentry/SentryOutOfMemoryScopeObserver.m | 59 ++++++++++++------- Sources/Sentry/SentryOutOfMemoryTracker.m | 42 ++++++++----- Sources/Sentry/include/SentryFileManager.h | 6 +- 4 files changed, 85 insertions(+), 47 deletions(-) diff --git a/Sources/Sentry/SentryFileManager.m b/Sources/Sentry/SentryFileManager.m index 815ad41f3de..9f707d826ff 100644 --- a/Sources/Sentry/SentryFileManager.m +++ b/Sources/Sentry/SentryFileManager.m @@ -28,8 +28,10 @@ @property (nonatomic, copy) NSString *lastInForegroundFilePath; @property (nonatomic, copy) NSString *previousAppStateFilePath; @property (nonatomic, copy) NSString *appStateFilePath; -@property (nonatomic, copy) NSString *previousBreadcrumbsFilePath; -@property (nonatomic, copy) NSString *breadcrumbsFilePath; +@property (nonatomic, copy) NSString *previousBreadcrumbsFilePathOne; +@property (nonatomic, copy) NSString *previousBreadcrumbsFilePathTwo; +@property (nonatomic, copy) NSString *breadcrumbsFilePathOne; +@property (nonatomic, copy) NSString *breadcrumbsFilePathTwo; @property (nonatomic, copy) NSString *timezoneOffsetFilePath; @property (nonatomic, assign) NSUInteger currentFileCounter; @property (nonatomic, assign) NSUInteger maxEnvelopes; @@ -85,10 +87,14 @@ - (nullable instancetype)initWithOptions:(SentryOptions *)options self.previousAppStateFilePath = [self.sentryPath stringByAppendingPathComponent:@"previous.app.state"]; self.appStateFilePath = [self.sentryPath stringByAppendingPathComponent:@"app.state"]; - self.previousBreadcrumbsFilePath = - [self.sentryPath stringByAppendingPathComponent:@"previous.breadcrumbs.state"]; - self.breadcrumbsFilePath = - [self.sentryPath stringByAppendingPathComponent:@"breadcrumbs.state"]; + self.previousBreadcrumbsFilePathOne = + [self.sentryPath stringByAppendingPathComponent:@"previous.breadcrumbs.1.state"]; + self.previousBreadcrumbsFilePathTwo = + [self.sentryPath stringByAppendingPathComponent:@"previous.breadcrumbs.2.state"]; + self.breadcrumbsFilePathOne = + [self.sentryPath stringByAppendingPathComponent:@"breadcrumbs.1.state"]; + self.breadcrumbsFilePathTwo = + [self.sentryPath stringByAppendingPathComponent:@"breadcrumbs.2.state"]; self.timezoneOffsetFilePath = [self.sentryPath stringByAppendingPathComponent:@"timezone.offset"]; @@ -471,8 +477,11 @@ - (void)moveAppStateToPreviousAppState - (void)moveBreadcrumbsToPreviousBreadcrumbs { - @synchronized(self.breadcrumbsFilePath) { - [self moveState:self.breadcrumbsFilePath toPreviousState:self.previousBreadcrumbsFilePath]; + @synchronized(self.breadcrumbsFilePathOne) { + [self moveState:self.breadcrumbsFilePathOne + toPreviousState:self.previousBreadcrumbsFilePathOne]; + [self moveState:self.breadcrumbsFilePathTwo + toPreviousState:self.previousBreadcrumbsFilePathTwo]; } } diff --git a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m index f99ed79c870..a9eb7e1bfaf 100644 --- a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m +++ b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m @@ -8,6 +8,9 @@ @property (strong, nonatomic) SentryFileManager *fileManager; @property (strong, nonatomic) NSFileHandle *fileHandle; +@property (nonatomic) NSInteger maxBreadcrumbs; +@property (nonatomic) NSInteger breadcrumbCounter; +@property (strong, nonatomic) NSString *activeFilePath; @end @@ -17,8 +20,11 @@ - (instancetype)initWithMaxBreadcrumbs:(NSInteger)maxBreadcrumbs fileManager:(SentryFileManager *)fileManager { if (self = [super init]) { + self.maxBreadcrumbs = maxBreadcrumbs; self.fileManager = fileManager; - [self createFile]; + self.breadcrumbCounter = 0; + + [self switchFileHandle]; } return self; @@ -31,41 +37,52 @@ - (void)dealloc // PRAGMA MARK: - Helper methods -- (void)deleteFile +- (void)deleteFiles { [self.fileHandle closeFile]; + self.fileHandle = nil; + self.activeFilePath = nil; - if ([[NSFileManager defaultManager] fileExistsAtPath:self.fileManager.breadcrumbsFilePath]) { - NSError *error; - [[NSFileManager defaultManager] removeItemAtPath:self.fileManager.breadcrumbsFilePath - error:&error]; - if (error) { - SENTRY_LOG_ERROR(@"Couldn't delete file %@", self.fileManager.breadcrumbsFilePath); - } - } + [self.fileManager removeFileAtPath:self.fileManager.breadcrumbsFilePathOne]; + [self.fileManager removeFileAtPath:self.fileManager.breadcrumbsFilePathTwo]; } -- (void)createFile +- (void)switchFileHandle { - if (![[NSFileManager defaultManager] fileExistsAtPath:self.fileManager.breadcrumbsFilePath]) { - [[NSFileManager defaultManager] createFileAtPath:self.fileManager.breadcrumbsFilePath - contents:nil - attributes:nil]; + if ([self.activeFilePath isEqualToString:self.fileManager.breadcrumbsFilePathOne]) { + self.activeFilePath = self.fileManager.breadcrumbsFilePathTwo; + } else { + self.activeFilePath = self.fileManager.breadcrumbsFilePathOne; } - self.fileHandle = - [NSFileHandle fileHandleForWritingAtPath:self.fileManager.breadcrumbsFilePath]; + // Close the current filehandle (if any) + [self.fileHandle closeFile]; + + // Create a fresh file for the new active path + [self.fileManager removeFileAtPath:self.activeFilePath]; + [[NSFileManager defaultManager] createFileAtPath:self.activeFilePath + contents:nil + attributes:nil]; + + // Open the file for writing + self.fileHandle = [NSFileHandle fileHandleForWritingAtPath:self.activeFilePath]; if (!self.fileHandle) { - SENTRY_LOG_ERROR(@"Couldn't open file handle for %@", self.fileManager.breadcrumbsFilePath); + SENTRY_LOG_ERROR(@"Couldn't open file handle for %@", self.activeFilePath); } } - (void)store:(NSData *)data { [self.fileHandle seekToEndOfFile]; - [self.fileHandle writeData:[@"\n" dataUsingEncoding:NSASCIIStringEncoding]]; [self.fileHandle writeData:data]; + [self.fileHandle writeData:[@"\n" dataUsingEncoding:NSASCIIStringEncoding]]; + + self.breadcrumbCounter += 1; + + if (self.breadcrumbCounter >= self.maxBreadcrumbs) { + [self switchFileHandle]; + } } // PRAGMA MARK: - SentryScopeObserver @@ -89,8 +106,8 @@ - (void)clear - (void)clearBreadcrumbs { - [self deleteFile]; - [self createFile]; + [self deleteFiles]; + [self switchFileHandle]; } - (void)setContext:(nullable NSDictionary *)context diff --git a/Sources/Sentry/SentryOutOfMemoryTracker.m b/Sources/Sentry/SentryOutOfMemoryTracker.m index 1446d756fa4..a4f3adac80a 100644 --- a/Sources/Sentry/SentryOutOfMemoryTracker.m +++ b/Sources/Sentry/SentryOutOfMemoryTracker.m @@ -50,24 +50,37 @@ - (instancetype)initWithOptions:(SentryOptions *)options - (NSArray *)loadBreadcrumbs { + NSMutableString *combinedFilesContents = [[NSMutableString alloc] init]; + if ([[NSFileManager defaultManager] - fileExistsAtPath:self.fileManager.previousBreadcrumbsFilePath]) { - NSMutableArray *breadcrumbs = [NSMutableArray array]; + fileExistsAtPath:self.fileManager.previousBreadcrumbsFilePathOne]) { + NSString *fileContents = + [NSString stringWithContentsOfFile:self.fileManager.previousBreadcrumbsFilePathOne + encoding:NSUTF8StringEncoding + error:nil]; + [combinedFilesContents appendString:fileContents]; + } - FILE *file = fopen([self.fileManager.previousBreadcrumbsFilePath UTF8String], "r"); + if ([[NSFileManager defaultManager] + fileExistsAtPath:self.fileManager.previousBreadcrumbsFilePathTwo]) { + NSString *fileContents = + [NSString stringWithContentsOfFile:self.fileManager.previousBreadcrumbsFilePathTwo + encoding:NSUTF8StringEncoding + error:nil]; + [combinedFilesContents appendString:fileContents]; + } - char buffer[4096]; - while (fgets(buffer, 4096, file) != NULL) { - NSString *result = [[NSString stringWithUTF8String:buffer] - stringByTrimmingCharactersInSet:[NSCharacterSet newlineCharacterSet]]; - if (result.length == 0) { - continue; - } + NSMutableArray *breadcrumbs = [NSMutableArray array]; + + if (combinedFilesContents.length > 0) { + NSArray *lines = [combinedFilesContents + componentsSeparatedByCharactersInSet:[NSCharacterSet newlineCharacterSet]]; - NSData *line = [result dataUsingEncoding:NSUTF8StringEncoding]; + for (NSString *line in lines) { + NSData *data = [line dataUsingEncoding:NSUTF8StringEncoding]; NSError *error; - NSDictionary *dict = [NSJSONSerialization JSONObjectWithData:line + NSDictionary *dict = [NSJSONSerialization JSONObjectWithData:data options:0 error:&error]; @@ -77,12 +90,9 @@ - (NSArray *)loadBreadcrumbs [breadcrumbs addObject:dict]; } } - - return breadcrumbs; } - SENTRY_LOG_ERROR(@"File not found: %@", self.fileManager.previousBreadcrumbsFilePath); - return [[NSArray alloc] init]; + return breadcrumbs; } - (void)start diff --git a/Sources/Sentry/include/SentryFileManager.h b/Sources/Sentry/include/SentryFileManager.h index 2b3c964856d..236ba7da270 100644 --- a/Sources/Sentry/include/SentryFileManager.h +++ b/Sources/Sentry/include/SentryFileManager.h @@ -15,8 +15,10 @@ NS_SWIFT_NAME(SentryFileManager) SENTRY_NO_INIT @property (nonatomic, readonly) NSString *sentryPath; -@property (nonatomic, readonly) NSString *breadcrumbsFilePath; -@property (nonatomic, readonly) NSString *previousBreadcrumbsFilePath; +@property (nonatomic, readonly) NSString *breadcrumbsFilePathOne; +@property (nonatomic, readonly) NSString *breadcrumbsFilePathTwo; +@property (nonatomic, readonly) NSString *previousBreadcrumbsFilePathOne; +@property (nonatomic, readonly) NSString *previousBreadcrumbsFilePathTwo; - (nullable instancetype)initWithOptions:(SentryOptions *)options andCurrentDateProvider:(id)currentDateProvider From 62a0eea315d469703dacf09c7d227755dabb9c24 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Wed, 2 Nov 2022 16:36:35 +0100 Subject: [PATCH 10/28] Fix bug --- Sources/Sentry/SentryOutOfMemoryScopeObserver.m | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m index a9eb7e1bfaf..841019b4ab1 100644 --- a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m +++ b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m @@ -82,6 +82,7 @@ - (void)store:(NSData *)data if (self.breadcrumbCounter >= self.maxBreadcrumbs) { [self switchFileHandle]; + self.breadcrumbCounter = 0; } } From 30d7ab24c7077c8100e35546231b11dc17ffd113 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Fri, 4 Nov 2022 12:58:28 +0100 Subject: [PATCH 11/28] Added comprehensive tests for SentryOutOfMemoryScopeObserver --- Sentry.xcodeproj/project.pbxproj | 4 + .../SentryOutOfMemoryScopeObserverTests.swift | 131 ++++++++++++++++++ .../SentryTests/SentryTests-Bridging-Header.h | 1 + 3 files changed, 136 insertions(+) create mode 100644 Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryScopeObserverTests.swift diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index cf29470d061..903203153ff 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -40,6 +40,7 @@ 0A1B497328E597DD00D7BFA3 /* TestLogOutput.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A1B497228E597DD00D7BFA3 /* TestLogOutput.swift */; }; 0A1C3592287D7107007D01E3 /* SentryMetaTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A1C3591287D7107007D01E3 /* SentryMetaTests.swift */; }; 0A2690B72885C2E000E4432D /* TestSentryPermissionsObserver.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0AABE2EF2885C2120057ED69 /* TestSentryPermissionsObserver.swift */; }; + 0A2D7BBA29152CBF008727AF /* SentryOutOfMemoryScopeObserverTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A2D7BB929152CBF008727AF /* SentryOutOfMemoryScopeObserverTests.swift */; }; 0A2D8D5B289815C0008720F6 /* SentryBaseIntegration.m in Sources */ = {isa = PBXBuildFile; fileRef = 0A2D8D5A289815C0008720F6 /* SentryBaseIntegration.m */; }; 0A2D8D5D289815EB008720F6 /* SentryBaseIntegration.h in Headers */ = {isa = PBXBuildFile; fileRef = 0A2D8D5C289815EB008720F6 /* SentryBaseIntegration.h */; }; 0A2D8D8728992260008720F6 /* SentryBaseIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0A2D8D8628992260008720F6 /* SentryBaseIntegrationTests.swift */; }; @@ -762,6 +763,7 @@ 03F9D37B2819A65C00602916 /* SentryProfilerTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = SentryProfilerTests.mm; sourceTree = ""; }; 0A1B497228E597DD00D7BFA3 /* TestLogOutput.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TestLogOutput.swift; sourceTree = ""; }; 0A1C3591287D7107007D01E3 /* SentryMetaTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryMetaTests.swift; sourceTree = ""; }; + 0A2D7BB929152CBF008727AF /* SentryOutOfMemoryScopeObserverTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryOutOfMemoryScopeObserverTests.swift; sourceTree = ""; }; 0A2D8D5A289815C0008720F6 /* SentryBaseIntegration.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryBaseIntegration.m; sourceTree = ""; }; 0A2D8D5C289815EB008720F6 /* SentryBaseIntegration.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryBaseIntegration.h; path = include/SentryBaseIntegration.h; sourceTree = ""; }; 0A2D8D8628992260008720F6 /* SentryBaseIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryBaseIntegrationTests.swift; sourceTree = ""; }; @@ -2601,6 +2603,7 @@ 7B98D7DF25FB73B900C5A389 /* SentryOutOfMemoryTrackerTests.swift */, 7BFE7A0927A1B6B000D2B66E /* SentryOutOfMemoryIntegrationTests.swift */, 7BFA69F327E07FF700233199 /* SentryOutOfMemoryLogicTests.swift */, + 0A2D7BB929152CBF008727AF /* SentryOutOfMemoryScopeObserverTests.swift */, ); path = OutOfMemory; sourceTree = ""; @@ -3662,6 +3665,7 @@ 7B30B68226527C55006B2752 /* TestDisplayLinkWrapper.swift in Sources */, 7BB6550D253EEB3900887E87 /* SentryUserFeedbackTests.swift in Sources */, 7BBD18B7245180FF00427C76 /* SentryDsnTests.m in Sources */, + 0A2D7BBA29152CBF008727AF /* SentryOutOfMemoryScopeObserverTests.swift in Sources */, 7BD4BD4B27EB2DC20071F4FF /* SentryDiscardedEventTests.swift in Sources */, 63FE721A20DA66EC00CDBAE8 /* SentryCrashSysCtl_Tests.m in Sources */, 7B88F30424BC8E6500ADF90A /* SentrySerializationTests.swift in Sources */, diff --git a/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryScopeObserverTests.swift b/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryScopeObserverTests.swift new file mode 100644 index 00000000000..b700cb45b9e --- /dev/null +++ b/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryScopeObserverTests.swift @@ -0,0 +1,131 @@ +import XCTest + +class SentryOutOfMemoryScopeObserverTests: XCTestCase { + private class Fixture { + let breadcrumb: Breadcrumb + let options: Options + let fileManager: SentryFileManager + let currentDate = TestCurrentDateProvider() + + init() { + breadcrumb = Breadcrumb() + breadcrumb.level = SentryLevel.info + breadcrumb.timestamp = Date(timeIntervalSince1970: 10) + breadcrumb.category = "category" + breadcrumb.type = "user" + breadcrumb.message = "Click something" + + options = Options() + fileManager = try! SentryFileManager(options: options, andCurrentDateProvider: currentDate) + } + + func getSut() -> SentryOutOfMemoryScopeObserver { + return getSut(fileManager: self.fileManager) + } + + func getSut(fileManager: SentryFileManager) -> SentryOutOfMemoryScopeObserver { + return SentryOutOfMemoryScopeObserver(maxBreadcrumbs: 10, fileManager: fileManager) + } + } + + private var fixture: Fixture! + private var sut: SentryOutOfMemoryScopeObserver! + + override func setUp() { + super.setUp() + + fixture = Fixture() + sut = fixture.getSut() + } + + override func tearDown() { + super.tearDown() + fixture.fileManager.deleteAllFolders() + } + + // Test that we're storing the serialized breadcrumb in a proper JSON string + func testStoreBreadcrumb() throws { + let breadcrumb = fixture.breadcrumb.serialize() as! [String: String] + + sut.addSerializedBreadcrumb(breadcrumb) + + let fileOneContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathOne) + let firstLine = String(fileOneContents.split(separator: "\n").first!) + let dict = try JSONSerialization.jsonObject(with: firstLine.data(using: .utf8)!) as! [String: String] + + XCTAssertEqual(dict, breadcrumb) + } + + func testStoreInMultipleFiles() throws { + let breadcrumb = fixture.breadcrumb.serialize() + + var count = 0 + while count < 9 { + sut.addSerializedBreadcrumb(breadcrumb) + count += 1 + } + + var fileOneContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathOne) + var fileOnelines = fileOneContents.split(separator: "\n") + XCTAssertEqual(fileOnelines.count, 9) + + XCTAssertFalse(FileManager.default.fileExists(atPath: fixture.fileManager.breadcrumbsFilePathTwo)) + + // Now store one more, which means it'll change over to the second file (which should be empty) + sut.addSerializedBreadcrumb(breadcrumb) + + fileOneContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathOne) + fileOnelines = fileOneContents.split(separator: "\n") + XCTAssertEqual(fileOnelines.count, 10) + + var fileTwoContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathTwo) + XCTAssertEqual(fileTwoContents, "") + + // Next one will be stored in the second file + sut.addSerializedBreadcrumb(breadcrumb) + + fileTwoContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathTwo) + var fileTwolines = fileTwoContents.split(separator: "\n") + + XCTAssertEqual(fileOnelines.count, 10) + XCTAssertEqual(fileTwolines.count, 1) + + // Store 10 more + count = 0 + while count < 10 { + sut.addSerializedBreadcrumb(breadcrumb) + count += 1 + } + + fileOneContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathOne) + fileOnelines = fileOneContents.split(separator: "\n") + XCTAssertEqual(fileOnelines.count, 1) + + fileTwoContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathTwo) + fileTwolines = fileTwoContents.split(separator: "\n") + XCTAssertEqual(fileTwolines.count, 10) + } + + func testClearBreadcrumbs() throws { + let breadcrumb = fixture.breadcrumb.serialize() + + var count = 0 + while count < 15 { + sut.addSerializedBreadcrumb(breadcrumb) + count += 1 + } + + var fileOneContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathOne) + XCTAssertEqual(fileOneContents.count, 1_200) + + let fileTwoContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathTwo) + XCTAssertEqual(fileTwoContents.count, 600) + + sut.clearBreadcrumbs() + + fileOneContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathOne) + XCTAssertEqual(fileOneContents.count, 0) + + XCTAssertFalse(FileManager.default.fileExists(atPath: fixture.fileManager.breadcrumbsFilePathTwo)) + } +} diff --git a/Tests/SentryTests/SentryTests-Bridging-Header.h b/Tests/SentryTests/SentryTests-Bridging-Header.h index 57a91cc6017..f6ec640f1ea 100644 --- a/Tests/SentryTests/SentryTests-Bridging-Header.h +++ b/Tests/SentryTests/SentryTests-Bridging-Header.h @@ -110,6 +110,7 @@ #import "SentryObjCRuntimeWrapper.h" #import "SentryOptions+Private.h" #import "SentryOutOfMemoryLogic.h" +#import "SentryOutOfMemoryScopeObserver.h" #import "SentryOutOfMemoryTracker.h" #import "SentryOutOfMemoryTrackingIntegration.h" #import "SentryPerformanceTracker.h" From 5e041ef0fc713f0a2d6e4f705d5f4f5d6dd7d1f0 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Fri, 4 Nov 2022 14:24:51 +0100 Subject: [PATCH 12/28] Add testAppOOM_WithBreadcrumbs --- .../SentryOutOfMemoryTrackerTests.swift | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift b/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift index fa72aeacfaf..ffc15067203 100644 --- a/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift +++ b/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift @@ -195,7 +195,7 @@ class SentryOutOfMemoryTrackerTests: NotificationCenterTestCase { sut.start() assertOOMEventSent() } - + func testANR_NoOOM() { sut.start() goToForeground() @@ -207,7 +207,27 @@ class SentryOutOfMemoryTrackerTests: NotificationCenterTestCase { sut.start() assertNoOOMSent() } - + + func testAppOOM_WithBreadcrumbs() { + let breadcrumb = Breadcrumb() + breadcrumb.level = SentryLevel.info + breadcrumb.timestamp = Date(timeIntervalSince1970: 10) + breadcrumb.category = "category" + breadcrumb.type = "user" + breadcrumb.message = "Click something" + + let sentryOutOfMemoryScopeObserver = SentryOutOfMemoryScopeObserver(maxBreadcrumbs: 10, fileManager: fixture.fileManager) + sentryOutOfMemoryScopeObserver.addSerializedBreadcrumb(breadcrumb.serialize()) + + sut.start() + goToForeground() + + fixture.fileManager.moveAppStateToPreviousAppState() + fixture.fileManager.moveBreadcrumbsToPreviousBreadcrumbs() + sut.start() + assertOOMEventSent(expectedBreadcrumbs: 1) + } + func testAppOOM_WithOnlyHybridSdkDidBecomeActive() { sut.start() hybridSdkDidBecomeActive() @@ -281,12 +301,13 @@ class SentryOutOfMemoryTrackerTests: NotificationCenterTestCase { } } - private func assertOOMEventSent() { + private func assertOOMEventSent(expectedBreadcrumbs: Int = 0) { XCTAssertEqual(1, fixture.client.captureCrashEventInvocations.count) let crashEvent = fixture.client.captureCrashEventInvocations.first?.event XCTAssertEqual(SentryLevel.fatal, crashEvent?.level) - XCTAssertEqual([], crashEvent?.breadcrumbs) + XCTAssertEqual(crashEvent?.breadcrumbs?.count, 0) + XCTAssertEqual(crashEvent?.serializedBreadcrumbs?.count, expectedBreadcrumbs) XCTAssertEqual(1, crashEvent?.exceptions?.count) @@ -298,7 +319,7 @@ class SentryOutOfMemoryTrackerTests: NotificationCenterTestCase { XCTAssertEqual(false, exception?.mechanism?.handled) XCTAssertEqual("out_of_memory", exception?.mechanism?.type) } - + private func assertNoOOMSent() { XCTAssertEqual(0, fixture.client.captureCrashEventInvocations.count) } From 87dee9320454b1287f3401c6630bd713576463d1 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Fri, 4 Nov 2022 14:27:02 +0100 Subject: [PATCH 13/28] Remove unnecessary import --- Sources/Sentry/SentryEvent.m | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/Sentry/SentryEvent.m b/Sources/Sentry/SentryEvent.m index 03d3f19c372..aba09c380d3 100644 --- a/Sources/Sentry/SentryEvent.m +++ b/Sources/Sentry/SentryEvent.m @@ -5,7 +5,6 @@ #import "SentryClient.h" #import "SentryCurrentDate.h" #import "SentryDebugMeta.h" -#import "SentryEvent+Private.h" #import "SentryException.h" #import "SentryId.h" #import "SentryLevelMapper.h" From c1b407dda03883b5288d60d2aaa074861b8f9bab Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Fri, 4 Nov 2022 14:29:00 +0100 Subject: [PATCH 14/28] Reset counter when needed --- Sources/Sentry/SentryOutOfMemoryScopeObserver.m | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m index 841019b4ab1..713202fc9f5 100644 --- a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m +++ b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m @@ -42,6 +42,7 @@ - (void)deleteFiles [self.fileHandle closeFile]; self.fileHandle = nil; self.activeFilePath = nil; + self.breadcrumbCounter = 0; [self.fileManager removeFileAtPath:self.fileManager.breadcrumbsFilePathOne]; [self.fileManager removeFileAtPath:self.fileManager.breadcrumbsFilePathTwo]; From 6b259bb538899b9c63d35d20c8e26304f71515cf Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Mon, 7 Nov 2022 09:47:00 +0100 Subject: [PATCH 15/28] Move changelog --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fc28a749802..1dbcbcf3bff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Store breadcrumbs to disk for OOM events (#2347) + ### Fixes - Fix issue with invalid profiles uploading (#2358 and #2359) @@ -13,7 +17,6 @@ - Profile concurrent transactions (#2227) - HTTP Client errors (#2308) - Disable bitcode for Carthage distribution (#2341) -- Store breadcrumbs to disk for OOM events (#2347) ### Fixes From 69dd0379842c5205382c18546693463975844335 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Mon, 7 Nov 2022 08:48:19 +0000 Subject: [PATCH 16/28] Format code --- Sources/SentryCrash/Recording/SentryCrash.m | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Sources/SentryCrash/Recording/SentryCrash.m b/Sources/SentryCrash/Recording/SentryCrash.m index 7fb39b38a80..c4e10cc9649 100644 --- a/Sources/SentryCrash/Recording/SentryCrash.m +++ b/Sources/SentryCrash/Recording/SentryCrash.m @@ -355,10 +355,7 @@ - (void)deleteReportWithID:(NSNumber *)reportID // ============================================================================ #define SYNTHESIZE_CRASH_STATE_PROPERTY(TYPE, NAME) \ - -(TYPE)NAME \ - { \ - return sentrycrashstate_currentState()->NAME; \ - } + -(TYPE)NAME { return sentrycrashstate_currentState()->NAME; } SYNTHESIZE_CRASH_STATE_PROPERTY(NSTimeInterval, activeDurationSinceLastCrash) SYNTHESIZE_CRASH_STATE_PROPERTY(NSTimeInterval, backgroundDurationSinceLastCrash) From 537f1ea48369e37e1315f8144ab68522e12ad871 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Mon, 7 Nov 2022 10:40:24 +0100 Subject: [PATCH 17/28] Merge the 2 breadcrumb arrays --- Sources/Sentry/SentryEvent.m | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/Sources/Sentry/SentryEvent.m b/Sources/Sentry/SentryEvent.m index 1422df67414..a048d377dd3 100644 --- a/Sources/Sentry/SentryEvent.m +++ b/Sources/Sentry/SentryEvent.m @@ -139,11 +139,11 @@ - (void)addSimpleProperties:(NSMutableDictionary *)serializedData [serializedData setValue:[self.stacktrace serialize] forKey:@"stacktrace"]; - [serializedData setValue:[self serializeBreadcrumbs] forKey:@"breadcrumbs"]; - + NSMutableArray *breadcrumbs = [self serializeBreadcrumbs]; if (self.serializedBreadcrumbs.count > 0) { - [serializedData setValue:self.serializedBreadcrumbs forKey:@"breadcrumbs"]; + [breadcrumbs addObjectsFromArray:self.serializedBreadcrumbs]; } + [serializedData setValue:breadcrumbs forKey:@"breadcrumbs"]; [serializedData setValue:[self.context sentry_sanitize] forKey:@"contexts"]; @@ -169,15 +169,12 @@ - (void)addSimpleProperties:(NSMutableDictionary *)serializedData } } -- (NSArray *_Nullable)serializeBreadcrumbs +- (NSMutableArray *)serializeBreadcrumbs { NSMutableArray *crumbs = [NSMutableArray new]; for (SentryBreadcrumb *crumb in self.breadcrumbs) { [crumbs addObject:[crumb serialize]]; } - if (crumbs.count <= 0) { - return nil; - } return crumbs; } From 8b27f889ce390876ac8bd5cbd527bff5e1dea771 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Mon, 7 Nov 2022 11:01:04 +0100 Subject: [PATCH 18/28] readPreviousBreadcrumbs is now in the file manager --- Sources/Sentry/SentryFileManager.m | 45 ++++++++++++++++++++ Sources/Sentry/SentryOutOfMemoryTracker.m | 49 +--------------------- Sources/Sentry/include/SentryFileManager.h | 1 + 3 files changed, 47 insertions(+), 48 deletions(-) diff --git a/Sources/Sentry/SentryFileManager.m b/Sources/Sentry/SentryFileManager.m index 9f707d826ff..1f62ff9774e 100644 --- a/Sources/Sentry/SentryFileManager.m +++ b/Sources/Sentry/SentryFileManager.m @@ -502,6 +502,51 @@ - (void)moveState:(NSString *)stateFilePath toPreviousState:(NSString *)previous } } +- (NSArray *)readPreviousBreadcrumbs +{ + NSMutableString *combinedFilesContents = [[NSMutableString alloc] init]; + + if ([[NSFileManager defaultManager] fileExistsAtPath:self.previousBreadcrumbsFilePathOne]) { + NSString *fileContents = + [NSString stringWithContentsOfFile:self.previousBreadcrumbsFilePathOne + encoding:NSUTF8StringEncoding + error:nil]; + [combinedFilesContents appendString:fileContents]; + } + + if ([[NSFileManager defaultManager] fileExistsAtPath:self.previousBreadcrumbsFilePathTwo]) { + NSString *fileContents = + [NSString stringWithContentsOfFile:self.previousBreadcrumbsFilePathTwo + encoding:NSUTF8StringEncoding + error:nil]; + [combinedFilesContents appendString:fileContents]; + } + + NSMutableArray *breadcrumbs = [NSMutableArray array]; + + if (combinedFilesContents.length > 0) { + NSArray *lines = [combinedFilesContents + componentsSeparatedByCharactersInSet:[NSCharacterSet newlineCharacterSet]]; + + for (NSString *line in lines) { + NSData *data = [line dataUsingEncoding:NSUTF8StringEncoding]; + + NSError *error; + NSDictionary *dict = [NSJSONSerialization JSONObjectWithData:data + options:0 + error:&error]; + + if (error) { + SENTRY_LOG_ERROR(@"Error deserializing breadcrumb: %@", error); + } else { + [breadcrumbs addObject:dict]; + } + } + } + + return breadcrumbs; +} + - (SentryAppState *_Nullable)readAppState { @synchronized(self.appStateFilePath) { diff --git a/Sources/Sentry/SentryOutOfMemoryTracker.m b/Sources/Sentry/SentryOutOfMemoryTracker.m index a4f3adac80a..a2bc7b2cbe6 100644 --- a/Sources/Sentry/SentryOutOfMemoryTracker.m +++ b/Sources/Sentry/SentryOutOfMemoryTracker.m @@ -48,53 +48,6 @@ - (instancetype)initWithOptions:(SentryOptions *)options return self; } -- (NSArray *)loadBreadcrumbs -{ - NSMutableString *combinedFilesContents = [[NSMutableString alloc] init]; - - if ([[NSFileManager defaultManager] - fileExistsAtPath:self.fileManager.previousBreadcrumbsFilePathOne]) { - NSString *fileContents = - [NSString stringWithContentsOfFile:self.fileManager.previousBreadcrumbsFilePathOne - encoding:NSUTF8StringEncoding - error:nil]; - [combinedFilesContents appendString:fileContents]; - } - - if ([[NSFileManager defaultManager] - fileExistsAtPath:self.fileManager.previousBreadcrumbsFilePathTwo]) { - NSString *fileContents = - [NSString stringWithContentsOfFile:self.fileManager.previousBreadcrumbsFilePathTwo - encoding:NSUTF8StringEncoding - error:nil]; - [combinedFilesContents appendString:fileContents]; - } - - NSMutableArray *breadcrumbs = [NSMutableArray array]; - - if (combinedFilesContents.length > 0) { - NSArray *lines = [combinedFilesContents - componentsSeparatedByCharactersInSet:[NSCharacterSet newlineCharacterSet]]; - - for (NSString *line in lines) { - NSData *data = [line dataUsingEncoding:NSUTF8StringEncoding]; - - NSError *error; - NSDictionary *dict = [NSJSONSerialization JSONObjectWithData:data - options:0 - error:&error]; - - if (error) { - SENTRY_LOG_ERROR(@"Error deserializing breadcrumb: %@", error); - } else { - [breadcrumbs addObject:dict]; - } - } - } - - return breadcrumbs; -} - - (void)start { #if SENTRY_HAS_UIKIT @@ -107,7 +60,7 @@ - (void)start event.breadcrumbs = @[]; // Load the previous breascrumbs from disk, which are already serialized - event.serializedBreadcrumbs = [self loadBreadcrumbs]; + event.serializedBreadcrumbs = [self.fileManager readPreviousBreadcrumbs]; SentryException *exception = [[SentryException alloc] initWithValue:SentryOutOfMemoryExceptionValue diff --git a/Sources/Sentry/include/SentryFileManager.h b/Sources/Sentry/include/SentryFileManager.h index 236ba7da270..e53edf6c2c1 100644 --- a/Sources/Sentry/include/SentryFileManager.h +++ b/Sources/Sentry/include/SentryFileManager.h @@ -75,6 +75,7 @@ SENTRY_NO_INIT - (void)deleteAppState; - (void)moveBreadcrumbsToPreviousBreadcrumbs; +- (NSArray *)readPreviousBreadcrumbs; - (NSNumber *_Nullable)readTimezoneOffset; - (void)storeTimezoneOffset:(NSInteger)offset; From 1a128a7faf3de30ee448b0351fbf902195c63cc4 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Mon, 7 Nov 2022 11:20:21 +0100 Subject: [PATCH 19/28] testReadPreviousBreadcrumbs --- .../Helper/SentryFileManagerTests.swift | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/Tests/SentryTests/Helper/SentryFileManagerTests.swift b/Tests/SentryTests/Helper/SentryFileManagerTests.swift index 1b1c81e1d0d..50a6a0be97e 100644 --- a/Tests/SentryTests/Helper/SentryFileManagerTests.swift +++ b/Tests/SentryTests/Helper/SentryFileManagerTests.swift @@ -578,6 +578,29 @@ class SentryFileManagerTests: XCTestCase { XCTAssertNotNil(sut.readTimezoneOffset()) } + func testReadPreviousBreadcrumbs() { + let observer = SentryOutOfMemoryScopeObserver(maxBreadcrumbs: 2, fileManager: sut) + + let breadcrumb = Breadcrumb() + breadcrumb.level = SentryLevel.info + breadcrumb.timestamp = Date(timeIntervalSince1970: 10) + breadcrumb.category = "category" + breadcrumb.type = "user" + breadcrumb.message = "Click something" + + let serializedBreadcrumb = breadcrumb.serialize() + + var count = 0 + while count < 3 { + observer.addSerializedBreadcrumb(serializedBreadcrumb) + count += 1 + } + + sut.moveBreadcrumbsToPreviousBreadcrumbs() + let result = sut.readPreviousBreadcrumbs() + XCTAssertEqual(result.count, 3) + } + func testReadGarbageTimezoneOffset() throws { try "garbage".write(to: URL(fileURLWithPath: sut.timezoneOffsetFilePath), atomically: true, encoding: .utf8) XCTAssertNil(sut.readTimezoneOffset()) From b89a2a21a2578db42d7d4b454984ca008190b9c4 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Mon, 7 Nov 2022 18:16:33 +0100 Subject: [PATCH 20/28] Fix tests --- Sources/Sentry/SentryEvent.m | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Sources/Sentry/SentryEvent.m b/Sources/Sentry/SentryEvent.m index a048d377dd3..dc7d0c3d113 100644 --- a/Sources/Sentry/SentryEvent.m +++ b/Sources/Sentry/SentryEvent.m @@ -143,7 +143,9 @@ - (void)addSimpleProperties:(NSMutableDictionary *)serializedData if (self.serializedBreadcrumbs.count > 0) { [breadcrumbs addObjectsFromArray:self.serializedBreadcrumbs]; } - [serializedData setValue:breadcrumbs forKey:@"breadcrumbs"]; + if (breadcrumbs.count > 0) { + [serializedData setValue:breadcrumbs forKey:@"breadcrumbs"]; + } [serializedData setValue:[self.context sentry_sanitize] forKey:@"contexts"]; From e82b8d091e176c16c7f9dda6ed5af737dc9d38a7 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Tue, 8 Nov 2022 16:45:10 +0100 Subject: [PATCH 21/28] First bit of feedback --- Sources/Sentry/SentryEvent.m | 7 ++++ .../Sentry/SentryOutOfMemoryScopeObserver.m | 40 ------------------- Sources/Sentry/include/SentryScopeObserver.h | 2 + 3 files changed, 9 insertions(+), 40 deletions(-) diff --git a/Sources/Sentry/SentryEvent.m b/Sources/Sentry/SentryEvent.m index dc7d0c3d113..23154046e25 100644 --- a/Sources/Sentry/SentryEvent.m +++ b/Sources/Sentry/SentryEvent.m @@ -21,6 +21,13 @@ SentryEvent () @property (nonatomic) BOOL isCrashEvent; + +// We're storing serialized breadcrumbs to disk in JSON, and when we're reading them back (in +// the case of OOM), we end up with the serialized breadcrumbs again. Instead of turning those +// dictionaries into proper SentryBreadcrumb instances which then need to be serialized again in +// SentryEvent, we use this serializedBreadcrumbs property to set the pre-serialized +// breadcrumbs. It saves a LOT of work - especially turning an NSDictionary into a SentryBreadcrumb +// is silly when we're just going to do the opposite right after. @property (nonatomic, strong) NSArray *serializedBreadcrumbs; @end diff --git a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m index 713202fc9f5..313ae4fb4aa 100644 --- a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m +++ b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m @@ -112,44 +112,4 @@ - (void)clearBreadcrumbs [self switchFileHandle]; } -- (void)setContext:(nullable NSDictionary *)context -{ - // Left blank on purpose -} - -- (void)setDist:(nullable NSString *)dist -{ - // Left blank on purpose -} - -- (void)setEnvironment:(nullable NSString *)environment -{ - // Left blank on purpose -} - -- (void)setExtras:(nullable NSDictionary *)extras -{ - // Left blank on purpose -} - -- (void)setFingerprint:(nullable NSArray *)fingerprint -{ - // Left blank on purpose -} - -- (void)setLevel:(enum SentryLevel)level -{ - // Left blank on purpose -} - -- (void)setTags:(nullable NSDictionary *)tags -{ - // Left blank on purpose -} - -- (void)setUser:(nullable SentryUser *)user -{ - // Left blank on purpose -} - @end diff --git a/Sources/Sentry/include/SentryScopeObserver.h b/Sources/Sentry/include/SentryScopeObserver.h index a0fada9c772..2340b8628fb 100644 --- a/Sources/Sentry/include/SentryScopeObserver.h +++ b/Sources/Sentry/include/SentryScopeObserver.h @@ -9,6 +9,8 @@ NS_ASSUME_NONNULL_BEGIN */ @protocol SentryScopeObserver +@optional + - (void)setUser:(nullable SentryUser *)user; - (void)setTags:(nullable NSDictionary *)tags; From 4796dfbd45110917fc66be22561912c7d5423d27 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Wed, 9 Nov 2022 14:19:29 +0100 Subject: [PATCH 22/28] Undo optional protocol methods --- .../Sentry/SentryOutOfMemoryScopeObserver.m | 40 +++++++++++++++++++ Sources/Sentry/include/SentryScopeObserver.h | 2 - 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m index 313ae4fb4aa..713202fc9f5 100644 --- a/Sources/Sentry/SentryOutOfMemoryScopeObserver.m +++ b/Sources/Sentry/SentryOutOfMemoryScopeObserver.m @@ -112,4 +112,44 @@ - (void)clearBreadcrumbs [self switchFileHandle]; } +- (void)setContext:(nullable NSDictionary *)context +{ + // Left blank on purpose +} + +- (void)setDist:(nullable NSString *)dist +{ + // Left blank on purpose +} + +- (void)setEnvironment:(nullable NSString *)environment +{ + // Left blank on purpose +} + +- (void)setExtras:(nullable NSDictionary *)extras +{ + // Left blank on purpose +} + +- (void)setFingerprint:(nullable NSArray *)fingerprint +{ + // Left blank on purpose +} + +- (void)setLevel:(enum SentryLevel)level +{ + // Left blank on purpose +} + +- (void)setTags:(nullable NSDictionary *)tags +{ + // Left blank on purpose +} + +- (void)setUser:(nullable SentryUser *)user +{ + // Left blank on purpose +} + @end diff --git a/Sources/Sentry/include/SentryScopeObserver.h b/Sources/Sentry/include/SentryScopeObserver.h index 2340b8628fb..a0fada9c772 100644 --- a/Sources/Sentry/include/SentryScopeObserver.h +++ b/Sources/Sentry/include/SentryScopeObserver.h @@ -9,8 +9,6 @@ NS_ASSUME_NONNULL_BEGIN */ @protocol SentryScopeObserver -@optional - - (void)setUser:(nullable SentryUser *)user; - (void)setTags:(nullable NSDictionary *)tags; From 384c77db5111c5b5f671dc0d1aeecd15d63fe1b4 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Wed, 9 Nov 2022 14:36:55 +0100 Subject: [PATCH 23/28] Use TestData.crumb --- .../OutOfMemory/SentryOutOfMemoryTrackerTests.swift | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift b/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift index ffc15067203..d75d41c0255 100644 --- a/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift +++ b/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift @@ -209,12 +209,7 @@ class SentryOutOfMemoryTrackerTests: NotificationCenterTestCase { } func testAppOOM_WithBreadcrumbs() { - let breadcrumb = Breadcrumb() - breadcrumb.level = SentryLevel.info - breadcrumb.timestamp = Date(timeIntervalSince1970: 10) - breadcrumb.category = "category" - breadcrumb.type = "user" - breadcrumb.message = "Click something" + let breadcrumb = TestData.crumb let sentryOutOfMemoryScopeObserver = SentryOutOfMemoryScopeObserver(maxBreadcrumbs: 10, fileManager: fixture.fileManager) sentryOutOfMemoryScopeObserver.addSerializedBreadcrumb(breadcrumb.serialize()) From 105b13fdc30b20bde752c1904a778b9d0fd97126 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Wed, 9 Nov 2022 15:22:44 +0100 Subject: [PATCH 24/28] Feedback --- .../Helper/SentryFileManagerTests.swift | 4 +-- .../SentryOutOfMemoryScopeObserverTests.swift | 34 ++++++++----------- develop-docs/README.md | 9 ++++- 3 files changed, 23 insertions(+), 24 deletions(-) diff --git a/Tests/SentryTests/Helper/SentryFileManagerTests.swift b/Tests/SentryTests/Helper/SentryFileManagerTests.swift index 866322bc8c2..b76601d7f2f 100644 --- a/Tests/SentryTests/Helper/SentryFileManagerTests.swift +++ b/Tests/SentryTests/Helper/SentryFileManagerTests.swift @@ -589,10 +589,8 @@ class SentryFileManagerTests: XCTestCase { let serializedBreadcrumb = breadcrumb.serialize() - var count = 0 - while count < 3 { + for _ in 0..<3 { observer.addSerializedBreadcrumb(serializedBreadcrumb) - count += 1 } sut.moveBreadcrumbsToPreviousBreadcrumbs() diff --git a/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryScopeObserverTests.swift b/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryScopeObserverTests.swift index b700cb45b9e..d0beec4a2d0 100644 --- a/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryScopeObserverTests.swift +++ b/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryScopeObserverTests.swift @@ -59,15 +59,13 @@ class SentryOutOfMemoryScopeObserverTests: XCTestCase { func testStoreInMultipleFiles() throws { let breadcrumb = fixture.breadcrumb.serialize() - var count = 0 - while count < 9 { + for _ in 0..<9 { sut.addSerializedBreadcrumb(breadcrumb) - count += 1 } var fileOneContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathOne) - var fileOnelines = fileOneContents.split(separator: "\n") - XCTAssertEqual(fileOnelines.count, 9) + var fileOneLines = fileOneContents.split(separator: "\n") + XCTAssertEqual(fileOneLines.count, 9) XCTAssertFalse(FileManager.default.fileExists(atPath: fixture.fileManager.breadcrumbsFilePathTwo)) @@ -75,8 +73,8 @@ class SentryOutOfMemoryScopeObserverTests: XCTestCase { sut.addSerializedBreadcrumb(breadcrumb) fileOneContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathOne) - fileOnelines = fileOneContents.split(separator: "\n") - XCTAssertEqual(fileOnelines.count, 10) + fileOneLines = fileOneContents.split(separator: "\n") + XCTAssertEqual(fileOneLines.count, 10) var fileTwoContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathTwo) XCTAssertEqual(fileTwoContents, "") @@ -85,34 +83,30 @@ class SentryOutOfMemoryScopeObserverTests: XCTestCase { sut.addSerializedBreadcrumb(breadcrumb) fileTwoContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathTwo) - var fileTwolines = fileTwoContents.split(separator: "\n") + var fileTwoLines = fileTwoContents.split(separator: "\n") - XCTAssertEqual(fileOnelines.count, 10) - XCTAssertEqual(fileTwolines.count, 1) + XCTAssertEqual(fileOneLines.count, 10) + XCTAssertEqual(fileTwoLines.count, 1) // Store 10 more - count = 0 - while count < 10 { + for _ in 0..<10 { sut.addSerializedBreadcrumb(breadcrumb) - count += 1 } fileOneContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathOne) - fileOnelines = fileOneContents.split(separator: "\n") - XCTAssertEqual(fileOnelines.count, 1) + fileOneLines = fileOneContents.split(separator: "\n") + XCTAssertEqual(fileOneLines.count, 1) fileTwoContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathTwo) - fileTwolines = fileTwoContents.split(separator: "\n") - XCTAssertEqual(fileTwolines.count, 10) + fileTwoLines = fileTwoContents.split(separator: "\n") + XCTAssertEqual(fileTwoLines.count, 10) } func testClearBreadcrumbs() throws { let breadcrumb = fixture.breadcrumb.serialize() - var count = 0 - while count < 15 { + for _ in 0..<15 { sut.addSerializedBreadcrumb(breadcrumb) - count += 1 } var fileOneContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathOne) diff --git a/develop-docs/README.md b/develop-docs/README.md index d3ddb94d7f8..be4e42bb524 100644 --- a/develop-docs/README.md +++ b/develop-docs/README.md @@ -127,7 +127,7 @@ Date: October 21st 2022 Contributors: @philipphofmann GH actions will remove the macOS-10.15 image, which contains an iOS 12 simulator on 12/1/22; see https://github.com/actions/runner-images/issues/5583. -Neither the[ macOS-11](https://github.com/actions/runner-images/blob/main/images/macos/macos-11-Readme.md#installed-sdks) nor the +Neither the [macOS-11](https://github.com/actions/runner-images/blob/main/images/macos/macos-11-Readme.md#installed-sdks) nor the [macOS-12](https://github.com/actions/runner-images/blob/main/images/macos/macos-12-Readme.md#installed-sdks) image contains an iOS 12 simulator. GH [concluded](https://github.com/actions/runner-images/issues/551#issuecomment-788822538) to not add more pre-installed simulators. SauceLabs doesn't support running unit tests and adding another cloud solution as Firebase TestLab would increase the complexity of CI. Not running the unit tests on @@ -135,3 +135,10 @@ iOS 12 opens a risk of introducing bugs, which has already happened in the past, the iOS 12 simulator a try. Related to [GH-2218](https://github.com/getsentry/sentry-cocoa/issues/2218) + +### Writing breadcrumbs to disk in the main thread + +Date November 15, 2022 +Contributors: @kevinrenskers, @brustolin and @philipphofmann + +For the benefit of OOM crashes we write breadcrumbs to disk, see https://github.com/getsentry/sentry-cocoa/pull/2347. We have decided to do this in the main thread because we want to make sure we're not missing out of any breadcrumbs. It's especially the last breadcrumb(s) that are important to figure out what is causing an OOM. And since we're only appending to an open file stream, the overhead is extremely small. From 746b37ed9f1c165322616e30da02399928a42b0d Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Wed, 9 Nov 2022 16:02:28 +0100 Subject: [PATCH 25/28] Add testMovesBreadcrumbsToPreviousBreadcrumbs --- .../Helper/SentryFileManagerTests.swift | 9 +-------- Tests/SentryTests/SentrySDKTests.swift | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/Tests/SentryTests/Helper/SentryFileManagerTests.swift b/Tests/SentryTests/Helper/SentryFileManagerTests.swift index b76601d7f2f..123715e426d 100644 --- a/Tests/SentryTests/Helper/SentryFileManagerTests.swift +++ b/Tests/SentryTests/Helper/SentryFileManagerTests.swift @@ -580,14 +580,7 @@ class SentryFileManagerTests: XCTestCase { func testReadPreviousBreadcrumbs() { let observer = SentryOutOfMemoryScopeObserver(maxBreadcrumbs: 2, fileManager: sut) - let breadcrumb = Breadcrumb() - breadcrumb.level = SentryLevel.info - breadcrumb.timestamp = Date(timeIntervalSince1970: 10) - breadcrumb.category = "category" - breadcrumb.type = "user" - breadcrumb.message = "Click something" - - let serializedBreadcrumb = breadcrumb.serialize() + let serializedBreadcrumb = TestData.crumb.serialize() for _ in 0..<3 { observer.addSerializedBreadcrumb(serializedBreadcrumb) diff --git a/Tests/SentryTests/SentrySDKTests.swift b/Tests/SentryTests/SentrySDKTests.swift index 47fdc84948a..13d9ffe079a 100644 --- a/Tests/SentryTests/SentrySDKTests.swift +++ b/Tests/SentryTests/SentrySDKTests.swift @@ -536,6 +536,24 @@ class SentrySDKTests: XCTestCase { let timestamp = self.fixture.currentDate.date().addingTimeInterval(TimeInterval(amount)) XCTAssertEqual(timestamp, SentrySDK.getAppStartMeasurement()?.appStartTimestamp) } + + func testMovesBreadcrumbsToPreviousBreadcrumbs() throws { + let options = Options() + options.dsn = SentrySDKTests.dsnAsString + + let filemanager = try SentryFileManager(options: options, andCurrentDateProvider: TestCurrentDateProvider()) + let observer = SentryOutOfMemoryScopeObserver(maxBreadcrumbs: 10, fileManager: filemanager) + let serializedBreadcrumb = TestData.crumb.serialize() + + for _ in 0..<3 { + observer.addSerializedBreadcrumb(serializedBreadcrumb) + } + + SentrySDK.start(options: options) + + let result = filemanager.readPreviousBreadcrumbs() + XCTAssertEqual(result.count, 3) + } private func givenSdkWithHub() { SentrySDK.setCurrentHub(fixture.hub) From 5b37f06147eea7b82d78183895133c6a5a1bed61 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Thu, 10 Nov 2022 19:12:43 +0100 Subject: [PATCH 26/28] Update develop-docs/README.md Co-authored-by: Philipp Hofmann --- develop-docs/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/develop-docs/README.md b/develop-docs/README.md index be4e42bb524..9a5110c50f9 100644 --- a/develop-docs/README.md +++ b/develop-docs/README.md @@ -141,4 +141,4 @@ Related to [GH-2218](https://github.com/getsentry/sentry-cocoa/issues/2218) Date November 15, 2022 Contributors: @kevinrenskers, @brustolin and @philipphofmann -For the benefit of OOM crashes we write breadcrumbs to disk, see https://github.com/getsentry/sentry-cocoa/pull/2347. We have decided to do this in the main thread because we want to make sure we're not missing out of any breadcrumbs. It's especially the last breadcrumb(s) that are important to figure out what is causing an OOM. And since we're only appending to an open file stream, the overhead is extremely small. +For the benefit of OOM crashes, we write breadcrumbs to disk; see https://github.com/getsentry/sentry-cocoa/pull/2347. We have decided to do this in the main thread to ensure we're not missing out on any breadcrumbs. It's mainly the last breadcrumb(s) that are important to figure out what is causing an OOM. And since we're only appending to an open file stream, the overhead is acceptable compared to the benefit of having accurate breadcrumbs. From bfb5a45153260a9823311ebcb7c133ebaad63591 Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Thu, 10 Nov 2022 19:20:17 +0100 Subject: [PATCH 27/28] Use TestData.crumb --- .../SentryOutOfMemoryScopeObserverTests.swift | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryScopeObserverTests.swift b/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryScopeObserverTests.swift index d0beec4a2d0..c26e10ad1a4 100644 --- a/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryScopeObserverTests.swift +++ b/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryScopeObserverTests.swift @@ -8,12 +8,8 @@ class SentryOutOfMemoryScopeObserverTests: XCTestCase { let currentDate = TestCurrentDateProvider() init() { - breadcrumb = Breadcrumb() - breadcrumb.level = SentryLevel.info - breadcrumb.timestamp = Date(timeIntervalSince1970: 10) - breadcrumb.category = "category" - breadcrumb.type = "user" - breadcrumb.message = "Click something" + breadcrumb = TestData.crumb + breadcrumb.data = nil options = Options() fileManager = try! SentryFileManager(options: options, andCurrentDateProvider: currentDate) @@ -110,10 +106,10 @@ class SentryOutOfMemoryScopeObserverTests: XCTestCase { } var fileOneContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathOne) - XCTAssertEqual(fileOneContents.count, 1_200) + XCTAssertEqual(fileOneContents.count, 1_210) let fileTwoContents = try String(contentsOfFile: fixture.fileManager.breadcrumbsFilePathTwo) - XCTAssertEqual(fileTwoContents.count, 600) + XCTAssertEqual(fileTwoContents.count, 605) sut.clearBreadcrumbs() From 15aa3ea7912b8b7a1933ecbc28039794e429cf4f Mon Sep 17 00:00:00 2001 From: Kevin Renskers Date: Fri, 11 Nov 2022 11:16:11 +0100 Subject: [PATCH 28/28] Sort and limit the breadcrumbs, added tests --- Sources/Sentry/SentryFileManager.m | 23 ++++++++++----- Sources/Sentry/SentryOutOfMemoryTracker.m | 6 ++++ .../Helper/SentryFileManagerTests.swift | 28 +++++++++++++++++-- .../SentryOutOfMemoryTrackerTests.swift | 10 +++++-- 4 files changed, 55 insertions(+), 12 deletions(-) diff --git a/Sources/Sentry/SentryFileManager.m b/Sources/Sentry/SentryFileManager.m index 1f62ff9774e..3f699bc8879 100644 --- a/Sources/Sentry/SentryFileManager.m +++ b/Sources/Sentry/SentryFileManager.m @@ -504,14 +504,17 @@ - (void)moveState:(NSString *)stateFilePath toPreviousState:(NSString *)previous - (NSArray *)readPreviousBreadcrumbs { - NSMutableString *combinedFilesContents = [[NSMutableString alloc] init]; + NSArray *fileOneLines = @[]; + NSArray *fileTwoLines = @[]; + NSArray *combinedLines = @[]; if ([[NSFileManager defaultManager] fileExistsAtPath:self.previousBreadcrumbsFilePathOne]) { NSString *fileContents = [NSString stringWithContentsOfFile:self.previousBreadcrumbsFilePathOne encoding:NSUTF8StringEncoding error:nil]; - [combinedFilesContents appendString:fileContents]; + fileOneLines = [fileContents + componentsSeparatedByCharactersInSet:[NSCharacterSet newlineCharacterSet]]; } if ([[NSFileManager defaultManager] fileExistsAtPath:self.previousBreadcrumbsFilePathTwo]) { @@ -519,16 +522,22 @@ - (NSArray *)readPreviousBreadcrumbs [NSString stringWithContentsOfFile:self.previousBreadcrumbsFilePathTwo encoding:NSUTF8StringEncoding error:nil]; - [combinedFilesContents appendString:fileContents]; + fileTwoLines = [fileContents + componentsSeparatedByCharactersInSet:[NSCharacterSet newlineCharacterSet]]; } NSMutableArray *breadcrumbs = [NSMutableArray array]; - if (combinedFilesContents.length > 0) { - NSArray *lines = [combinedFilesContents - componentsSeparatedByCharactersInSet:[NSCharacterSet newlineCharacterSet]]; + if (fileOneLines.count > 0 || fileTwoLines.count > 0) { + if (fileOneLines.count > fileTwoLines.count) { + // If file one has more lines than file two, then file one contains the older crumbs, + // and thus needs to come first. + combinedLines = [fileOneLines arrayByAddingObjectsFromArray:fileTwoLines]; + } else { + combinedLines = [fileTwoLines arrayByAddingObjectsFromArray:fileOneLines]; + } - for (NSString *line in lines) { + for (NSString *line in combinedLines) { NSData *data = [line dataUsingEncoding:NSUTF8StringEncoding]; NSError *error; diff --git a/Sources/Sentry/SentryOutOfMemoryTracker.m b/Sources/Sentry/SentryOutOfMemoryTracker.m index a2bc7b2cbe6..5aa953956d9 100644 --- a/Sources/Sentry/SentryOutOfMemoryTracker.m +++ b/Sources/Sentry/SentryOutOfMemoryTracker.m @@ -61,6 +61,12 @@ - (void)start // Load the previous breascrumbs from disk, which are already serialized event.serializedBreadcrumbs = [self.fileManager readPreviousBreadcrumbs]; + if (event.serializedBreadcrumbs.count > self.options.maxBreadcrumbs) { + event.serializedBreadcrumbs = [event.serializedBreadcrumbs + subarrayWithRange:NSMakeRange(event.serializedBreadcrumbs.count + - self.options.maxBreadcrumbs, + self.options.maxBreadcrumbs)]; + } SentryException *exception = [[SentryException alloc] initWithValue:SentryOutOfMemoryExceptionValue diff --git a/Tests/SentryTests/Helper/SentryFileManagerTests.swift b/Tests/SentryTests/Helper/SentryFileManagerTests.swift index 123715e426d..083333a284b 100644 --- a/Tests/SentryTests/Helper/SentryFileManagerTests.swift +++ b/Tests/SentryTests/Helper/SentryFileManagerTests.swift @@ -580,15 +580,39 @@ class SentryFileManagerTests: XCTestCase { func testReadPreviousBreadcrumbs() { let observer = SentryOutOfMemoryScopeObserver(maxBreadcrumbs: 2, fileManager: sut) - let serializedBreadcrumb = TestData.crumb.serialize() + for count in 0..<3 { + let crumb = TestData.crumb + crumb.message = "\(count)" + let serializedBreadcrumb = crumb.serialize() - for _ in 0..<3 { observer.addSerializedBreadcrumb(serializedBreadcrumb) } sut.moveBreadcrumbsToPreviousBreadcrumbs() let result = sut.readPreviousBreadcrumbs() XCTAssertEqual(result.count, 3) + XCTAssertEqual((result[0] as! NSDictionary)["message"] as! String, "0") + XCTAssertEqual((result[1] as! NSDictionary)["message"] as! String, "1") + XCTAssertEqual((result[2] as! NSDictionary)["message"] as! String, "2") + } + + func testReadPreviousBreadcrumbsCorrectOrderWhenFileTwoHasMoreCrumbs() { + let observer = SentryOutOfMemoryScopeObserver(maxBreadcrumbs: 2, fileManager: sut) + + for count in 0..<5 { + let crumb = TestData.crumb + crumb.message = "\(count)" + let serializedBreadcrumb = crumb.serialize() + + observer.addSerializedBreadcrumb(serializedBreadcrumb) + } + + sut.moveBreadcrumbsToPreviousBreadcrumbs() + let result = sut.readPreviousBreadcrumbs() + XCTAssertEqual(result.count, 3) + XCTAssertEqual((result[0] as! NSDictionary)["message"] as! String, "2") + XCTAssertEqual((result[1] as! NSDictionary)["message"] as! String, "3") + XCTAssertEqual((result[2] as! NSDictionary)["message"] as! String, "4") } func testReadGarbageTimezoneOffset() throws { diff --git a/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift b/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift index d75d41c0255..686ae66da3a 100644 --- a/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift +++ b/Tests/SentryTests/Integrations/OutOfMemory/SentryOutOfMemoryTrackerTests.swift @@ -18,6 +18,7 @@ class SentryOutOfMemoryTrackerTests: NotificationCenterTestCase { init() { options = Options() + options.maxBreadcrumbs = 2 options.dsn = SentryOutOfMemoryTrackerTests.dsnAsString options.releaseName = TestData.appState.releaseName @@ -211,8 +212,11 @@ class SentryOutOfMemoryTrackerTests: NotificationCenterTestCase { func testAppOOM_WithBreadcrumbs() { let breadcrumb = TestData.crumb - let sentryOutOfMemoryScopeObserver = SentryOutOfMemoryScopeObserver(maxBreadcrumbs: 10, fileManager: fixture.fileManager) - sentryOutOfMemoryScopeObserver.addSerializedBreadcrumb(breadcrumb.serialize()) + let sentryOutOfMemoryScopeObserver = SentryOutOfMemoryScopeObserver(maxBreadcrumbs: Int(fixture.options.maxBreadcrumbs), fileManager: fixture.fileManager) + + for _ in 0..<3 { + sentryOutOfMemoryScopeObserver.addSerializedBreadcrumb(breadcrumb.serialize()) + } sut.start() goToForeground() @@ -220,7 +224,7 @@ class SentryOutOfMemoryTrackerTests: NotificationCenterTestCase { fixture.fileManager.moveAppStateToPreviousAppState() fixture.fileManager.moveBreadcrumbsToPreviousBreadcrumbs() sut.start() - assertOOMEventSent(expectedBreadcrumbs: 1) + assertOOMEventSent(expectedBreadcrumbs: 2) } func testAppOOM_WithOnlyHybridSdkDidBecomeActive() {