diff --git a/Bugsnag.xcodeproj/project.pbxproj b/Bugsnag.xcodeproj/project.pbxproj index a704ac71a..e5a8ef6c7 100644 --- a/Bugsnag.xcodeproj/project.pbxproj +++ b/Bugsnag.xcodeproj/project.pbxproj @@ -661,6 +661,9 @@ 01B14C56251CE55F00118748 /* report-react-native-promise-rejection.json in Resources */ = {isa = PBXBuildFile; fileRef = 01B14C55251CE55F00118748 /* report-react-native-promise-rejection.json */; }; 01B14C57251CE55F00118748 /* report-react-native-promise-rejection.json in Resources */ = {isa = PBXBuildFile; fileRef = 01B14C55251CE55F00118748 /* report-react-native-promise-rejection.json */; }; 01B14C58251CE55F00118748 /* report-react-native-promise-rejection.json in Resources */ = {isa = PBXBuildFile; fileRef = 01B14C55251CE55F00118748 /* report-react-native-promise-rejection.json */; }; + 01C17AE72542ED7F00C102C9 /* KSCrashReportWriterTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 01C17AE62542ED7F00C102C9 /* KSCrashReportWriterTests.m */; }; + 01C17AE82542ED7F00C102C9 /* KSCrashReportWriterTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 01C17AE62542ED7F00C102C9 /* KSCrashReportWriterTests.m */; }; + 01C17AE92542ED7F00C102C9 /* KSCrashReportWriterTests.m in Sources */ = {isa = PBXBuildFile; fileRef = 01C17AE62542ED7F00C102C9 /* KSCrashReportWriterTests.m */; }; 3A700A9424A63ABC0068CD1B /* BugsnagThread.h in Headers */ = {isa = PBXBuildFile; fileRef = 3A700A8024A63A8E0068CD1B /* BugsnagThread.h */; settings = {ATTRIBUTES = (Public, ); }; }; 3A700A9524A63AC50068CD1B /* BugsnagSession.h in Headers */ = {isa = PBXBuildFile; fileRef = 3A700A8124A63A8E0068CD1B /* BugsnagSession.h */; settings = {ATTRIBUTES = (Public, ); }; }; 3A700A9624A63AC60068CD1B /* BugsnagStackframe.h in Headers */ = {isa = PBXBuildFile; fileRef = 3A700A8224A63A8E0068CD1B /* BugsnagStackframe.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -1272,6 +1275,7 @@ 00E636C12487031D006CBF1A /* docker-compose.yml */ = {isa = PBXFileReference; lastKnownFileType = text.yaml; path = "docker-compose.yml"; sourceTree = SOURCE_ROOT; }; 00E636C324878FFC006CBF1A /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = ""; }; 01B14C55251CE55F00118748 /* report-react-native-promise-rejection.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = "report-react-native-promise-rejection.json"; sourceTree = ""; }; + 01C17AE62542ED7F00C102C9 /* KSCrashReportWriterTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = KSCrashReportWriterTests.m; sourceTree = ""; }; 3A700A8024A63A8E0068CD1B /* BugsnagThread.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = BugsnagThread.h; sourceTree = ""; }; 3A700A8124A63A8E0068CD1B /* BugsnagSession.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = BugsnagSession.h; sourceTree = ""; }; 3A700A8224A63A8E0068CD1B /* BugsnagStackframe.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = BugsnagStackframe.h; sourceTree = ""; }; @@ -1414,6 +1418,7 @@ 008966E92486D43700DC48C2 /* KSCrashIdentifierTests.m */, 008966DF2486D43700DC48C2 /* KSCrashReportConverter_Tests.m */, 008966DE2486D43700DC48C2 /* KSCrashReportStore_Tests.m */, + 01C17AE62542ED7F00C102C9 /* KSCrashReportWriterTests.m */, 008966DD2486D43700DC48C2 /* KSCrashSentry_NSException_Tests.m */, 008966E72486D43700DC48C2 /* KSCrashSentry_Signal_Tests.m */, 008966E62486D43700DC48C2 /* KSCrashSentry_Tests.m */, @@ -2545,6 +2550,7 @@ 008966FD2486D43700DC48C2 /* BugsnagOnBreadcrumbTest.m in Sources */, 008967812486D43700DC48C2 /* KSSystemInfo_Tests.m in Sources */, 0089671B2486D43700DC48C2 /* BugsnagSessionTest.m in Sources */, + 01C17AE72542ED7F00C102C9 /* KSCrashReportWriterTests.m in Sources */, 008967A82486D43700DC48C2 /* KSCrashIdentifierTests.m in Sources */, 008967572486D43700DC48C2 /* BugsnagClientMirrorTest.m in Sources */, 0089677B2486D43700DC48C2 /* RFC3339DateTool_Tests.m in Sources */, @@ -2746,6 +2752,7 @@ 008967762486D43700DC48C2 /* XCTestCase+KSCrash.m in Sources */, 008967312486D43700DC48C2 /* BugsnagStateEventTest.m in Sources */, 004E35362487AFF2007FBAE4 /* BugsnagHandledStateTest.m in Sources */, + 01C17AE82542ED7F00C102C9 /* KSCrashReportWriterTests.m in Sources */, 0089678B2486D43700DC48C2 /* KSCrashReportStore_Tests.m in Sources */, 00896A412486DBDD00DC48C2 /* BSGConfigurationBuilderTests.m in Sources */, 008967672486D43700DC48C2 /* BugsnagNotifierTest.m in Sources */, @@ -2905,6 +2912,7 @@ CBA2249D251E429C00B87416 /* TestSupport.m in Sources */, 004E35372487AFF2007FBAE4 /* BugsnagHandledStateTest.m in Sources */, 0089678C2486D43700DC48C2 /* KSCrashReportStore_Tests.m in Sources */, + 01C17AE92542ED7F00C102C9 /* KSCrashReportWriterTests.m in Sources */, 00896A422486DBDD00DC48C2 /* BSGConfigurationBuilderTests.m in Sources */, 008967682486D43700DC48C2 /* BugsnagNotifierTest.m in Sources */, 0089676E2486D43700DC48C2 /* BugsnagTestsDummyClass.m in Sources */, diff --git a/Bugsnag.xcodeproj/xcshareddata/xcbaselines/00AD1C7A24869B0E00A27979.xcbaseline/AAE3C352-6D5A-468D-8448-CC2EA52E7868.plist b/Bugsnag.xcodeproj/xcshareddata/xcbaselines/00AD1C7A24869B0E00A27979.xcbaseline/AAE3C352-6D5A-468D-8448-CC2EA52E7868.plist index 117d91285..e550e1e0d 100644 --- a/Bugsnag.xcodeproj/xcshareddata/xcbaselines/00AD1C7A24869B0E00A27979.xcbaseline/AAE3C352-6D5A-468D-8448-CC2EA52E7868.plist +++ b/Bugsnag.xcodeproj/xcshareddata/xcbaselines/00AD1C7A24869B0E00A27979.xcbaseline/AAE3C352-6D5A-468D-8448-CC2EA52E7868.plist @@ -11,7 +11,7 @@ com.apple.XCTPerformanceMetric_WallClockTime baselineAverage - 0.28 + 0.0488 baselineIntegrationDisplayName Local Baseline diff --git a/Bugsnag/Breadcrumbs/BugsnagBreadcrumbs.h b/Bugsnag/Breadcrumbs/BugsnagBreadcrumbs.h index 19edc32e5..11cafce37 100644 --- a/Bugsnag/Breadcrumbs/BugsnagBreadcrumbs.h +++ b/Bugsnag/Breadcrumbs/BugsnagBreadcrumbs.h @@ -8,47 +8,61 @@ #import -#import "BugsnagBreadcrumb.h" -#import "BugsnagConfiguration.h" +@class BugsnagBreadcrumb; +@class BugsnagConfiguration; +typedef struct BSG_KSCrashReportWriter BSG_KSCrashReportWriter; typedef void (^BSGBreadcrumbConfiguration)(BugsnagBreadcrumb *_Nonnull); +#pragma mark - + +NS_ASSUME_NONNULL_BEGIN + @interface BugsnagBreadcrumbs : NSObject -- (instancetype _Nonnull)initWithConfiguration:(BugsnagConfiguration *_Nonnull)config; +- (instancetype)initWithConfiguration:(BugsnagConfiguration *)config; + +/** + * The current breadcrumbs, loaded from disk. + */ +@property (readonly) NSArray *breadcrumbs; /** * Path where breadcrumbs are persisted on disk */ -@property (nonatomic, readonly, strong, nullable) NSString *cachePath; +@property (readonly) NSString *cachePath; /** * Store a new breadcrumb with a provided message. */ -- (void)addBreadcrumb:(NSString *_Nonnull)breadcrumbMessage; +- (void)addBreadcrumb:(NSString *)breadcrumbMessage; /** * Store a new breadcrumb configured via block. * * @param block configuration block */ -- (void)addBreadcrumbWithBlock: - (void (^_Nonnull)(BugsnagBreadcrumb *_Nonnull))block; +- (void)addBreadcrumbWithBlock:(BSGBreadcrumbConfiguration)block; /** - * Generates an array of dictionaries representing the current buffer of breadcrumbs. + * Returns the breadcrumb JSON dictionaries stored on disk. */ -- (NSArray *_Nonnull)arrayValue; +- (nullable NSArray *)cachedBreadcrumbs; /** - * Returns an array containing the current buffer of breadcrumbs. + * Removes breadcrumbs from disk. */ -- (NSArray *_Nonnull)getBreadcrumbs; +- (void)removeAllBreadcrumbs; + +@end + +NS_ASSUME_NONNULL_END + +#pragma mark - /** - * The types of breadcrumbs which will be automatically captured. - * By default, this is all types. + * Inserts the current breadcrumbs into a crash report. + * + * This function is async-signal-safe. */ -@property BSGEnabledBreadcrumbType enabledBreadcrumbTypes; - -@end +void BugsnagBreadcrumbsWriteCrashReport(const BSG_KSCrashReportWriter * _Nonnull writer); diff --git a/Bugsnag/Breadcrumbs/BugsnagBreadcrumbs.m b/Bugsnag/Breadcrumbs/BugsnagBreadcrumbs.m index db28f85d4..b5d0cfcdf 100644 --- a/Bugsnag/Breadcrumbs/BugsnagBreadcrumbs.m +++ b/Bugsnag/Breadcrumbs/BugsnagBreadcrumbs.m @@ -8,10 +8,12 @@ #import "BugsnagBreadcrumbs.h" -#import "BugsnagBreadcrumb.h" + +#import "BSGCachesDirectory.h" +#import "BSGJSONSerialization.h" +#import "BSG_KSCrashReportWriter.h" #import "BugsnagLogger.h" #import "Private.h" -#import "BSGJSONSerialization.h" @interface BugsnagConfiguration () @property(nonatomic) NSMutableArray *onBreadcrumbBlocks; @@ -23,68 +25,84 @@ + (instancetype _Nullable)breadcrumbWithBlock: + (instancetype _Nullable)breadcrumbFromDict:(NSDictionary *_Nonnull)dict; @end +/** + * Information that can be accessed in an async-safe manner from the crash handler. + */ +typedef struct { + char directoryPath[PATH_MAX]; + unsigned int firstFileNumber; + unsigned int nextFileNumber; +} BugsnagBreadcrumbsContext; + +static BugsnagBreadcrumbsContext g_context; + +#pragma mark - + @interface BugsnagBreadcrumbs () + @property BugsnagConfiguration *config; -@property(nonatomic, readwrite, strong) NSMutableArray *breadcrumbs; -@property(nonatomic, readonly, strong) dispatch_queue_t readWriteQueue; +@property unsigned int nextFileNumber; +@property unsigned int maxBreadcrumbs; + @end +#pragma mark - + @implementation BugsnagBreadcrumbs - (instancetype)initWithConfiguration:(BugsnagConfiguration *)config { - static NSString *const BSGBreadcrumbCacheFileName = @"bugsnag_breadcrumbs.json"; - if (self = [super init]) { - _config = config; - _breadcrumbs = [NSMutableArray arrayWithCapacity:config.maxBreadcrumbs]; - _readWriteQueue = dispatch_queue_create("com.bugsnag.BreadcrumbRead", - DISPATCH_QUEUE_SERIAL); - NSString *cacheDir = [NSSearchPathForDirectoriesInDomains( - NSCachesDirectory, NSUserDomainMask, YES) firstObject]; - if (cacheDir != nil) { - _cachePath = [cacheDir stringByAppendingPathComponent: - BSGBreadcrumbCacheFileName]; - } + if (!(self = [super init])) { + return nil; + } + + _config = config; + // Capture maxBreadcrumbs to protect against config being changed after initialization + _maxBreadcrumbs = (unsigned int)config.maxBreadcrumbs; + + NSError *error = nil; + NSString *cachesDir = [BSGCachesDirectory cachesDirectory]; + _cachePath = [[cachesDir stringByAppendingPathComponent:@"bugsnag"] stringByAppendingPathComponent:@"breadcrumbs"]; + if (![[NSFileManager defaultManager] createDirectoryAtPath:_cachePath withIntermediateDirectories:YES attributes:nil error:&error]) { + bsg_log_err(@"Unable to create breadcrumbs directory: %@", error); } + + [_cachePath getFileSystemRepresentation:g_context.directoryPath maxLength:sizeof(g_context.directoryPath)]; + return self; } +- (NSArray *)breadcrumbs { + return [self loadBreadcrumbsAsDictionaries:NO] ?: @[]; +} + - (void)addBreadcrumb:(NSString *)breadcrumbMessage { [self addBreadcrumbWithBlock:^(BugsnagBreadcrumb *_Nonnull crumb) { crumb.message = breadcrumbMessage; }]; } -- (void)addBreadcrumbWithBlock: - (void (^_Nonnull)(BugsnagBreadcrumb *_Nonnull))block { - if (self.config.maxBreadcrumbs == 0) { +- (void)addBreadcrumbWithBlock:(BSGBreadcrumbConfiguration)block { + if (self.maxBreadcrumbs == 0) { return; } BugsnagBreadcrumb *crumb = [BugsnagBreadcrumb breadcrumbWithBlock:block]; - if (crumb != nil && [self shouldSendBreadcrumb:crumb]) { - dispatch_barrier_sync(self.readWriteQueue, ^{ - if ((self.breadcrumbs.count > 0) && - (self.breadcrumbs.count == self.config.maxBreadcrumbs)) { - [self.breadcrumbs removeObjectAtIndex:0]; - } - [self.breadcrumbs addObject:crumb]; - // Serialize crumbs to disk inside barrier to avoid simultaneous - // access to the file - if (self.cachePath != nil) { - static NSString *const arrayKeyPath = @"objectValue"; - NSArray *items = [self.breadcrumbs valueForKeyPath:arrayKeyPath]; - if ([BSGJSONSerialization isValidJSONObject:items]) { - NSError *error = nil; - NSData *data = [BSGJSONSerialization dataWithJSONObject:items - options:0 - error:&error]; - [data writeToFile:self.cachePath atomically:NO]; - if (error != nil) { - bsg_log_err(@"Failed to write breadcrumbs to disk: %@", error); - } - } - } - }); + if (!crumb || ![self shouldSendBreadcrumb:crumb]) { + return; + } + NSData *data = [self dataForBreadcrumb:crumb]; + if (!data) { + return; } + unsigned int fileNumber; + @synchronized (self) { + fileNumber = self.nextFileNumber; + self.nextFileNumber = fileNumber + 1; + if (fileNumber + 1 > self.maxBreadcrumbs) { + g_context.firstFileNumber = fileNumber + 1 - self.maxBreadcrumbs; + } + g_context.nextFileNumber = fileNumber + 1; + } + [self writeBreadcrumbData:(NSData *)data toFileNumber:fileNumber]; } - (BOOL)shouldSendBreadcrumb:(BugsnagBreadcrumb *)crumb { @@ -100,49 +118,118 @@ - (BOOL)shouldSendBreadcrumb:(BugsnagBreadcrumb *)crumb { return YES; } -- (NSArray *)cachedBreadcrumbs { - __block NSArray *cache = nil; - dispatch_barrier_sync(self.readWriteQueue, ^{ - NSError *error = nil; - NSData *data = [NSData dataWithContentsOfFile:self.cachePath options:0 error:&error]; - if (error == nil) { - cache = [BSGJSONSerialization JSONObjectWithData:data options:0 error:&error]; - } - if (error != nil) { - bsg_log_err(@"Failed to read breadcrumbs from disk: %@", error); +- (void)removeAllBreadcrumbs { + @synchronized (self) { + self.nextFileNumber = 0; + g_context.firstFileNumber = 0; + g_context.nextFileNumber = 0; + } + [self deleteBreadcrumbFiles]; +} + +#pragma mark - File storage + +- (NSData *)dataForBreadcrumb:(BugsnagBreadcrumb *)breadcrumb { + id JSONObject = [breadcrumb objectValue]; + if (![BSGJSONSerialization isValidJSONObject:JSONObject]) { + bsg_log_err(@"Unable to serialize breadcrumb: Not a valid JSON object"); + return nil; + } + NSError *error = nil; + NSData *data = [BSGJSONSerialization dataWithJSONObject:JSONObject options:0 error:&error]; + if (!data) { + bsg_log_err(@"Unable to serialize breadcrumb: %@", error); + } + return data; +} + +- (NSString *)pathForFileNumber:(unsigned int)fileNumber { + return [self.cachePath stringByAppendingPathComponent:[NSString stringWithFormat:@"%u.json", fileNumber]]; +} + +- (void)writeBreadcrumbData:(NSData *)data toFileNumber:(unsigned int)fileNumber { + NSString *path = [self pathForFileNumber:fileNumber]; + + NSError *error = nil; + if (![data writeToFile:path options:NSDataWritingAtomic error:&error]) { + bsg_log_err(@"Unable to write breadcrumb: %@", error); + return; + } + + if (fileNumber >= self.maxBreadcrumbs) { + NSString *path = [self pathForFileNumber:fileNumber - self.maxBreadcrumbs]; + if (![[NSFileManager defaultManager] removeItemAtPath:path error:&error]) { + bsg_log_err(@"Unable to delete old breadcrumb: %@", error); } - }); - return [cache isKindOfClass:[NSArray class]] ? cache : nil; + } } -- (NSArray *)arrayValue { - __block NSMutableArray *contents; - dispatch_barrier_sync(self.readWriteQueue, ^{ - contents = [[NSMutableArray alloc] initWithCapacity:self.breadcrumbs.count]; - for (BugsnagBreadcrumb *crumb in self.breadcrumbs) { - NSDictionary *objectValue = [crumb objectValue]; - NSError *error = nil; - @try { - if (![BSGJSONSerialization isValidJSONObject:objectValue]) { - bsg_log_err(@"Unable to serialize breadcrumb: Not a valid " - @"JSON object"); - continue; - } - [contents addObject:objectValue]; - } @catch (NSException *exception) { - bsg_log_err(@"Unable to serialize breadcrumb: %@", error); - } +- (nullable NSArray *)cachedBreadcrumbs { + return [self loadBreadcrumbsAsDictionaries:YES]; +} + +- (nullable NSArray *)loadBreadcrumbsAsDictionaries:(BOOL)asDictionaries { + NSError *error = nil; + + NSArray *filenames = [[NSFileManager defaultManager] contentsOfDirectoryAtPath:_cachePath error:&error]; + if (!filenames) { + bsg_log_err(@"Unable to read breadcrumbs: %@", error); + return nil; + } + + NSMutableArray *breadcrumbs = [NSMutableArray array]; + + for (NSString *file in [filenames sortedArrayUsingSelector:@selector(compare:)]) { + NSString *path = [self.cachePath stringByAppendingPathComponent:file]; + NSData *data = [NSData dataWithContentsOfFile:path]; + if (!data) { + bsg_log_err(@"Unable to read breadcrumb from %@", path); + continue; + } + id JSONObject = [BSGJSONSerialization JSONObjectWithData:data options:0 error:&error]; + if (!JSONObject) { + bsg_log_err(@"Unable to parse breadcrumb: %@", error); + continue; } - }); - return contents; + BugsnagBreadcrumb *breadcrumb; + if (![JSONObject isKindOfClass:[NSDictionary class]] || + !(breadcrumb = [BugsnagBreadcrumb breadcrumbFromDict:JSONObject])) { + bsg_log_err(@"Unexpected breadcrumb payload in file %@", file); + continue; + } + [breadcrumbs addObject:asDictionaries ? JSONObject : breadcrumb]; + } + + return breadcrumbs; } -- (NSArray *)getBreadcrumbs { - __block NSArray *result = nil; - dispatch_barrier_sync(self.readWriteQueue, ^{ - result = [NSArray arrayWithArray:self.breadcrumbs]; - }); - return result; +- (void)deleteBreadcrumbFiles { + [[NSFileManager defaultManager] removeItemAtPath:self.cachePath error:NULL]; + + NSError *error = nil; + if (![[NSFileManager defaultManager] createDirectoryAtPath:self.cachePath withIntermediateDirectories:YES attributes:nil error:&error]) { + bsg_log_err(@"Unable to create breadcrumbs directory: %@", error); + } + + NSString *cachesDir = [BSGCachesDirectory cachesDirectory]; + NSString *oldBreadcrumbsPath = [cachesDir stringByAppendingPathComponent:@"bugsnag_breadcrumbs.json"]; + [[NSFileManager defaultManager] removeItemAtPath:oldBreadcrumbsPath error:NULL]; } @end + +#pragma mark - + +void BugsnagBreadcrumbsWriteCrashReport(const BSG_KSCrashReportWriter *writer) { + char path[PATH_MAX]; + writer->beginArray(writer, "breadcrumbs"); + for (unsigned int i = g_context.firstFileNumber; i < g_context.nextFileNumber; i++) { + int result = snprintf(path, sizeof(path), "%s/%u.json", g_context.directoryPath, i); + if (result < 0 || result >= sizeof(path)) { + bsg_log_err(@"Breadcrumb path is too long"); + continue; + } + writer->addJSONFileElement(writer, NULL, path); + } + writer->endContainer(writer); +} diff --git a/Bugsnag/Bugsnag.m b/Bugsnag/Bugsnag.m index c662fdc58..6d69c3d07 100644 --- a/Bugsnag/Bugsnag.m +++ b/Bugsnag/Bugsnag.m @@ -203,7 +203,7 @@ + (void)leaveBreadcrumbWithMessage:(NSString *_Nonnull)message + (NSArray *_Nonnull)breadcrumbs { if ([self bugsnagStarted]) { - return [self.client.breadcrumbs getBreadcrumbs]; + return self.client.breadcrumbs.breadcrumbs; } else { return @[]; } diff --git a/Bugsnag/Client/BugsnagClient.m b/Bugsnag/Client/BugsnagClient.m index d71386042..ddc81f8c3 100644 --- a/Bugsnag/Client/BugsnagClient.m +++ b/Bugsnag/Client/BugsnagClient.m @@ -27,6 +27,8 @@ #import "BugsnagPlatformConditional.h" #import "BugsnagClient.h" + +#import "BugsnagBreadcrumbs.h" #import "BugsnagClientInternal.h" #import "BSGConnectivity.h" #import "Bugsnag.h" @@ -84,8 +86,6 @@ // Contains notifier state, under "deviceState" and crash-specific // information under "crash". char *stateJSON; - // Path to the JSON formatted file containing breadcrumbs. - char *breadcrumbsPath; // Contains properties in the Bugsnag payload overridden by the user before // it was sent char *userOverridesJSON; @@ -161,9 +161,7 @@ void BSSerializeDataCrashHandler(const BSG_KSCrashReportWriter *writer, int type if (bsg_g_bugsnag_data.stateJSON) { writer->addJSONElement(writer, "state", bsg_g_bugsnag_data.stateJSON); } - if (bsg_g_bugsnag_data.breadcrumbsPath) { - writer->addJSONFileElement(writer, "breadcrumbs", bsg_g_bugsnag_data.breadcrumbsPath); - } + BugsnagBreadcrumbsWriteCrashReport(writer); if (bsg_g_bugsnag_data.metadataJSON) { // The API expects "metaData", capitalised as such. Elsewhere is is one word. writer->addJSONElement(writer, "metaData", bsg_g_bugsnag_data.metadataJSON); @@ -363,10 +361,6 @@ - (instancetype)initWithUserId:(NSString *)userId name:(NSString *)name emailAdd - (NSDictionary *)toJson; @end -@interface BugsnagBreadcrumbs () -@property(nonatomic, readwrite, strong) NSMutableArray *breadcrumbs; -@end - // ============================================================================= // MARK: - BugsnagClient // ============================================================================= @@ -416,9 +410,6 @@ - (id)initWithConfiguration:(BugsnagConfiguration *)initConfiguration { }]; self.breadcrumbs = [[BugsnagBreadcrumbs alloc] initWithConfiguration:self.configuration]; - if (self.breadcrumbs.cachePath != nil) { - bsg_g_bugsnag_data.breadcrumbsPath = strdup(self.breadcrumbs.cachePath.fileSystemRepresentation); - } // Start with a copy of the configuration metadata self.metadata = [[configuration metadata] deepCopy]; @@ -560,6 +551,7 @@ - (void)start { apiClient:self.errorReportApiClient onCrash:&BSSerializeDataCrashHandler]; [self computeDidCrashLastLaunch]; + [self.breadcrumbs removeAllBreadcrumbs]; [self setupConnectivityListener]; [self updateAutomaticBreadcrumbDetectionSettings]; @@ -1615,10 +1607,9 @@ - (NSDictionary *)collectDeviceWithState { } - (NSArray *)collectBreadcrumbs { - NSMutableArray *crumbs = self.breadcrumbs.breadcrumbs; NSMutableArray *data = [NSMutableArray new]; - for (BugsnagBreadcrumb *crumb in crumbs) { + for (BugsnagBreadcrumb *crumb in self.breadcrumbs.breadcrumbs) { NSMutableDictionary *crumbData = [[crumb objectValue] mutableCopy]; // JSON is serialized as 'name', we want as 'message' when passing to RN crumbData[@"message"] = crumbData[@"name"]; diff --git a/Bugsnag/Payload/BugsnagEvent.m b/Bugsnag/Payload/BugsnagEvent.m index d20358dc3..3a6008186 100644 --- a/Bugsnag/Payload/BugsnagEvent.m +++ b/Bugsnag/Payload/BugsnagEvent.m @@ -16,6 +16,7 @@ #import #import "BSGSerialization.h" #import "Bugsnag.h" +#import "BugsnagBreadcrumbs.h" #import "BugsnagCollections.h" #import "BugsnagHandledState.h" #import "BugsnagLogger.h" diff --git a/Bugsnag/Private.h b/Bugsnag/Private.h index 758f19afa..012345506 100644 --- a/Bugsnag/Private.h +++ b/Bugsnag/Private.h @@ -8,7 +8,6 @@ #import "BSG_RFC3339DateTool.h" #import "Bugsnag.h" #import "BugsnagBreadcrumb.h" -#import "BugsnagBreadcrumbs.h" #import "BugsnagKeys.h" #import "BugsnagLogger.h" #import "BugsnagMetadataInternal.h" @@ -36,16 +35,6 @@ #pragma mark - -@interface BugsnagBreadcrumbs () -/** - * Reads and return breadcrumb data currently stored on disk - */ -- (NSArray *_Nullable)cachedBreadcrumbs; - -@end - -#pragma mark - - @interface Bugsnag () /** Get the current Bugsnag configuration. diff --git a/CHANGELOG.md b/CHANGELOG.md index ca3ea936b..37d069ab2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Changelog * Reduced the CPU and memory impact of leaving breadcrumbs. [853](https://github.com/bugsnag/bugsnag-cocoa/pull/853) + [863](https://github.com/bugsnag/bugsnag-cocoa/pull/863) ## 6.2.2 (2020-10-21) diff --git a/Tests/BugsnagBreadcrumbsTest.m b/Tests/BugsnagBreadcrumbsTest.m index d596bc474..418a6142f 100644 --- a/Tests/BugsnagBreadcrumbsTest.m +++ b/Tests/BugsnagBreadcrumbsTest.m @@ -6,14 +6,34 @@ // // +#import "BSG_KSJSONCodec.h" #import "Bugsnag.h" #import "BugsnagClient.h" #import "BugsnagClientInternal.h" #import "BugsnagBreadcrumb.h" #import "BugsnagBreadcrumbs.h" #import "BugsnagTestConstants.h" + #import +// Defined in BSG_KSCrashReport.c +void bsg_kscrw_i_prepareReportWriter(BSG_KSCrashReportWriter *const writer, BSG_KSJSONEncodeContext *const context); + +static int addJSONData(const char *data, size_t length, NSMutableData *userData) { + [userData appendBytes:data length:length]; + return BSG_KSJSON_OK; +} + +static id JSONObject(void (^ block)(BSG_KSCrashReportWriter *writer)) { + NSMutableData *data = [NSMutableData data]; + BSG_KSJSONEncodeContext encodeContext; + BSG_KSCrashReportWriter reportWriter; + bsg_kscrw_i_prepareReportWriter(&reportWriter, &encodeContext); + bsg_ksjsonbeginEncode(&encodeContext, false, (BSG_KSJSONAddDataFunc)addJSONData, (__bridge void *)data); + block(&reportWriter); + return [NSJSONSerialization JSONObjectWithData:data options:0 error:NULL]; +} + @interface BugsnagBreadcrumbsTest : XCTestCase @property(nonatomic, strong) BugsnagBreadcrumbs *crumbs; @end @@ -24,9 +44,8 @@ + (instancetype _Nullable)breadcrumbWithBlock: + (instancetype _Nullable)breadcrumbFromDict:(NSDictionary *_Nonnull)dict; @end -@interface BugsnagBreadcrumbs () -@property(nonatomic, readwrite, strong) NSMutableArray *breadcrumbs; -@property(nonatomic, readonly, strong) dispatch_queue_t readWriteQueue; +@interface BugsnagBreadcrumbs (Testing) +- (NSArray *)arrayValue; @end @interface Bugsnag () @@ -38,9 +57,9 @@ - (void)start; @end void awaitBreadcrumbSync(BugsnagBreadcrumbs *crumbs) { - dispatch_barrier_sync(crumbs.readWriteQueue, ^{ - usleep(300000); - }); + // This used to wait for the queue to finish adding breadcrumb(s). Not + // required in current implementation but leaving this function in case + // it needs to be reintroduced. } BSGBreadcrumbType BSGBreadcrumbTypeFromString(NSString *value); @@ -51,6 +70,7 @@ - (void)setUp { [super setUp]; BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; BugsnagBreadcrumbs *crumbs = [[BugsnagBreadcrumbs alloc] initWithConfiguration:config]; + [crumbs removeAllBreadcrumbs]; [crumbs addBreadcrumb:@"Launch app"]; [crumbs addBreadcrumb:@"Tap button"]; [crumbs addBreadcrumb:@"Close tutorial"]; @@ -60,32 +80,25 @@ - (void)setUp { - (void)testDefaultCount { BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; BugsnagBreadcrumbs *crumbs = [[BugsnagBreadcrumbs alloc] initWithConfiguration:config]; + [crumbs removeAllBreadcrumbs]; XCTAssertTrue(crumbs.breadcrumbs.count == 0); } -- (void)testCachePath { - NSString *cachePath = [[NSSearchPathForDirectoriesInDomains( - NSCachesDirectory, NSUserDomainMask, YES) - firstObject] - stringByAppendingPathComponent:@"bugsnag_breadcrumbs.json"]; - BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; - BugsnagBreadcrumbs *crumbs = [[BugsnagBreadcrumbs alloc] initWithConfiguration:config]; - XCTAssertEqualObjects(crumbs.cachePath, cachePath); -} - - (void)testMaxBreadcrumbs { BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; config.maxBreadcrumbs = 3; self.crumbs = [[BugsnagBreadcrumbs alloc] initWithConfiguration:config]; + [self.crumbs removeAllBreadcrumbs]; [self.crumbs addBreadcrumb:@"Crumb 1"]; [self.crumbs addBreadcrumb:@"Crumb 2"]; [self.crumbs addBreadcrumb:@"Crumb 3"]; [self.crumbs addBreadcrumb:@"Clear notifications"]; awaitBreadcrumbSync(self.crumbs); - XCTAssertEqual(self.crumbs.breadcrumbs.count, 3); - XCTAssertEqualObjects(self.crumbs.breadcrumbs[0].message, @"Crumb 2"); - XCTAssertEqualObjects(self.crumbs.breadcrumbs[1].message, @"Crumb 3"); - XCTAssertEqualObjects(self.crumbs.breadcrumbs[2].message, @"Clear notifications"); + NSArray *breadcrumbs = self.crumbs.breadcrumbs; + XCTAssertEqual(breadcrumbs.count, 3); + XCTAssertEqualObjects(breadcrumbs[0].message, @"Crumb 2"); + XCTAssertEqualObjects(breadcrumbs[1].message, @"Crumb 3"); + XCTAssertEqualObjects(breadcrumbs[2].message, @"Clear notifications"); } - (void)testBreadcrumbsInvalidKey { @@ -108,6 +121,7 @@ - (void)testEmptyCapacity { BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; config.maxBreadcrumbs = 0; self.crumbs = [[BugsnagBreadcrumbs alloc] initWithConfiguration:config]; + [self.crumbs removeAllBreadcrumbs]; [self.crumbs addBreadcrumb:@"Clear notifications"]; XCTAssertEqual(self.crumbs.breadcrumbs.count, 0); } @@ -147,8 +161,7 @@ - (void)testStateType { } - (void)testPersistentCrumbManual { - NSData *crumbs = [NSData dataWithContentsOfFile:self.crumbs.cachePath]; - NSArray *value = [NSJSONSerialization JSONObjectWithData:crumbs options:0 error:nil]; + NSArray *value = [self.crumbs cachedBreadcrumbs]; XCTAssertEqual(value.count, 3); XCTAssertEqualObjects(value[0][@"type"], @"manual"); XCTAssertEqualObjects(value[0][@"name"], @"Launch app"); @@ -167,8 +180,7 @@ - (void)testPersistentCrumbCustom { crumb.metadata = @{ @"captain": @"Bob"}; crumb.type = BSGBreadcrumbTypeState; }]; - NSData *crumbs = [NSData dataWithContentsOfFile:self.crumbs.cachePath]; - NSArray *value = [NSJSONSerialization JSONObjectWithData:crumbs options:0 error:nil]; + NSArray *value = [self.crumbs cachedBreadcrumbs]; XCTAssertEqual(value.count, 4); XCTAssertEqualObjects(value[3][@"type"], @"state"); XCTAssertEqualObjects(value[3][@"name"], @"Initiate sequence"); @@ -226,7 +238,7 @@ - (void)testDefaultDiscardByType { - (void)testAlwaysAllowManual { BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; self.crumbs = [[BugsnagBreadcrumbs alloc] initWithConfiguration:config]; - self.crumbs.enabledBreadcrumbTypes = 0; + [self.crumbs removeAllBreadcrumbs]; [self.crumbs addBreadcrumb:@"this is a test"]; awaitBreadcrumbSync(self.crumbs); NSArray *value = [self.crumbs arrayValue]; @@ -242,7 +254,7 @@ - (void)testAlwaysAllowManual { - (void)testDiscardByTypeDoesNotApply { BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; self.crumbs = [[BugsnagBreadcrumbs alloc] initWithConfiguration:config]; - self.crumbs.enabledBreadcrumbTypes = BSGEnabledBreadcrumbTypeProcess; + [self.crumbs removeAllBreadcrumbs]; // Don't discard this [self.crumbs addBreadcrumbWithBlock:^(BugsnagBreadcrumb *_Nonnull crumb) { crumb.type = BSGBreadcrumbTypeState; @@ -401,14 +413,28 @@ - (void)testCallbackFreeConstructors3 { XCTAssertEqual([bc2[@"metaData"] count], 0); } -- (void)testGetBreadcrumbs { - NSArray *breadcrumbs = [self.crumbs getBreadcrumbs]; - XCTAssertEqual(breadcrumbs[0].message, @"Launch app"); - XCTAssertEqual(breadcrumbs[0].type, BSGBreadcrumbTypeManual); - XCTAssertEqual(breadcrumbs[1].message, @"Tap button"); - XCTAssertEqual(breadcrumbs[1].type, BSGBreadcrumbTypeManual); - XCTAssertEqual(breadcrumbs[2].message, @"Close tutorial"); - XCTAssertEqual(breadcrumbs[2].type, BSGBreadcrumbTypeManual); +- (void)testCrashReportWriter { + NSDictionary *object = JSONObject(^(BSG_KSCrashReportWriter *writer) { + writer->beginObject(writer, ""); + BugsnagBreadcrumbsWriteCrashReport(writer); + writer->endContainer(writer); + }); + + XCTAssertEqualObjects(object.allKeys, @[@"breadcrumbs"]); + NSArray *breadcrumbs = object[@"breadcrumbs"]; + XCTAssertEqual(breadcrumbs.count, 3); + + XCTAssertEqualObjects(breadcrumbs[0][@"type"], @"manual"); + XCTAssertEqualObjects(breadcrumbs[0][@"name"], @"Launch app"); + XCTAssertEqualObjects(breadcrumbs[0][@"metaData"], @{}); + + XCTAssertEqualObjects(breadcrumbs[1][@"type"], @"manual"); + XCTAssertEqualObjects(breadcrumbs[1][@"name"], @"Tap button"); + XCTAssertEqualObjects(breadcrumbs[1][@"metaData"], @{}); + + XCTAssertEqualObjects(breadcrumbs[2][@"type"], @"manual"); + XCTAssertEqualObjects(breadcrumbs[2][@"name"], @"Close tutorial"); + XCTAssertEqualObjects(breadcrumbs[2][@"metaData"], @{}); } - (void)testPerformance { @@ -436,3 +462,13 @@ - (void)testPerformance { } @end + +#pragma mark - + +@implementation BugsnagBreadcrumbs (Testing) + +- (NSArray *)arrayValue { + return [self.breadcrumbs valueForKey:@"objectValue"]; +} + +@end diff --git a/Tests/BugsnagClientTests.m b/Tests/BugsnagClientTests.m index d4f631505..fe4f23e5f 100644 --- a/Tests/BugsnagClientTests.m +++ b/Tests/BugsnagClientTests.m @@ -32,10 +32,6 @@ @interface BugsnagBreadcrumb () - (NSDictionary *)objectValue; @end -@interface BugsnagBreadcrumbs () -@property(nonatomic, readwrite, strong) NSMutableArray *breadcrumbs; -@end - @interface BugsnagConfiguration () @property(readwrite, retain, nullable) BugsnagMetadata *metadata; @end @@ -270,12 +266,11 @@ - (void)testLargeBreadcrumbSize { BugsnagClient *client = [[BugsnagClient alloc] initWithConfiguration:configuration]; [client start]; - NSMutableArray *breadcrumbs = client.breadcrumbs.breadcrumbs; - XCTAssertEqual(0, [breadcrumbs count]); + XCTAssertEqual(client.breadcrumbs.breadcrumbs.count, 0); // small breadcrumb can be left without issue [client leaveBreadcrumbWithMessage:@"Hello World"]; - XCTAssertEqual(1, [breadcrumbs count]); + XCTAssertEqual(client.breadcrumbs.breadcrumbs.count, 1); // large breadcrumb is also left without issue __block NSUInteger crumbSize = 0; @@ -293,7 +288,7 @@ - (void)testLargeBreadcrumbSize { metadata:largeMetadata andType:BSGBreadcrumbTypeManual]; XCTAssertTrue(crumbSize > 4096); // previous 4kb limit - XCTAssertEqual(2, [breadcrumbs count]); + XCTAssertEqual(client.breadcrumbs.breadcrumbs.count, 2); XCTAssertNotNil(crumb); XCTAssertEqualObjects(@"Hello World", crumb.message); XCTAssertEqualObjects(largeMetadata, crumb.metadata); @@ -305,8 +300,7 @@ - (void)testMetadataInvalidKey { BugsnagClient *client = [[BugsnagClient alloc] initWithConfiguration:configuration]; [client start]; - NSMutableArray *breadcrumbs = client.breadcrumbs.breadcrumbs; - XCTAssertEqual(0, [breadcrumbs count]); + XCTAssertEqual(client.breadcrumbs.breadcrumbs.count, 0); id badMetadata = @{ @"test": @"string key is fine", @@ -315,7 +309,7 @@ - (void)testMetadataInvalidKey { [client leaveBreadcrumbWithMessage:@"test msg" metadata:badMetadata andType:BSGBreadcrumbTypeUser]; - XCTAssertEqual(1, [breadcrumbs count]); + XCTAssertEqual(client.breadcrumbs.breadcrumbs.count, 0, @"A breadcrumb with invalid JSON payload should be rejected"); [client notifyError:[NSError errorWithDomain:@"test" code:0 userInfo:badMetadata]]; } diff --git a/Tests/BugsnagOnBreadcrumbTest.m b/Tests/BugsnagOnBreadcrumbTest.m index 9540955df..3e3c67545 100644 --- a/Tests/BugsnagOnBreadcrumbTest.m +++ b/Tests/BugsnagOnBreadcrumbTest.m @@ -12,6 +12,7 @@ #import "BugsnagConfiguration.h" #import "BugsnagTestConstants.h" #import "BugsnagBreadcrumbs.h" +#import "Private.h" @interface BugsnagClient () @property(nonatomic, readwrite, retain) BugsnagConfiguration *_Nullable configuration; @@ -28,10 +29,6 @@ @interface BugsnagConfiguration () @property NSMutableArray *onBreadcrumbBlocks; @end -@interface BugsnagBreadcrumbs () -@property(nonatomic, readwrite, strong) NSMutableArray *breadcrumbs; -@end - @interface BugsnagOnBreadcrumbTest : XCTestCase @end @@ -190,7 +187,7 @@ - (void)testAddOnBreadcrumbMutation { BugsnagClient *client = [[BugsnagClient alloc] initWithConfiguration:config]; [client start]; XCTAssertEqual([[config onBreadcrumbBlocks] count], 1); - NSDictionary *crumb = [[client.breadcrumbs arrayValue] firstObject]; + NSDictionary *crumb = [client.breadcrumbs.breadcrumbs.firstObject objectValue]; XCTAssertEqualObjects(@"Foo", crumb[@"name"]); } diff --git a/Tests/KSCrash/KSCrashReportWriterTests.m b/Tests/KSCrash/KSCrashReportWriterTests.m new file mode 100644 index 000000000..be85fb4b7 --- /dev/null +++ b/Tests/KSCrash/KSCrashReportWriterTests.m @@ -0,0 +1,88 @@ +// +// KSCrashReportWriterTests.m +// Bugsnag +// +// Created by Nick Dowell on 23/10/2020. +// Copyright © 2020 Bugsnag Inc. All rights reserved. +// + +#import + +#import "BSG_KSCrashReportWriter.h" +#import "BSG_KSFileUtils.h" +#import "BSG_KSJSONCodec.h" + +// Defined in BSG_KSCrashReport.c +void bsg_kscrw_i_prepareReportWriter(BSG_KSCrashReportWriter *const writer, BSG_KSJSONEncodeContext *const context); + +static int addJSONData(const char *data, size_t length, NSMutableData *userData) { + [userData appendBytes:data length:length]; + return BSG_KSJSON_OK; +} + +static id JSONObject(void (^ block)(BSG_KSCrashReportWriter *writer)) { + NSMutableData *data = [NSMutableData data]; + BSG_KSJSONEncodeContext encodeContext; + BSG_KSCrashReportWriter reportWriter; + bsg_kscrw_i_prepareReportWriter(&reportWriter, &encodeContext); + bsg_ksjsonbeginEncode(&encodeContext, false, (BSG_KSJSONAddDataFunc)addJSONData, (__bridge void *)data); + block(&reportWriter); + return [NSJSONSerialization JSONObjectWithData:data options:0 error:NULL]; +} + +#pragma mark - + +@interface KSCrashReportWriterTests : XCTestCase +@end + +#pragma mark - + +@implementation KSCrashReportWriterTests + +- (void)testSimpleObject { + id object = JSONObject(^(BSG_KSCrashReportWriter *writer) { + writer->beginObject(writer, NULL); + writer->addStringElement(writer, "foo", "bar"); + writer->endContainer(writer); + }); + XCTAssertEqualObjects(object, @{@"foo": @"bar"}); +} + +- (void)testArray { + id object = JSONObject(^(BSG_KSCrashReportWriter *writer) { + writer->beginArray(writer, NULL); + writer->addStringElement(writer, "foo", "bar"); + writer->endContainer(writer); + }); + XCTAssertEqualObjects(object, @[@"bar"]); +} + +- (void)testArrayInsideObject { + id object = JSONObject(^(BSG_KSCrashReportWriter *writer) { + writer->beginObject(writer, NULL); + writer->beginArray(writer, "items"); + writer->addStringElement(writer, NULL, "bar"); + writer->addStringElement(writer, NULL, "foo"); + writer->endContainer(writer); + writer->endContainer(writer); + }); + id expected = @{@"items": @[@"bar", @"foo"]}; + XCTAssertEqualObjects(object, expected); +} + +- (void)testFileElementsInsideArray { + NSString *temporaryFile = [NSTemporaryDirectory() stringByAppendingPathComponent:@"testFileElementsInsideArray.json"]; + [@"{\"foo\":\"bar\"}" writeToFile:temporaryFile atomically:NO encoding:NSUTF8StringEncoding error:NULL]; + id object = JSONObject(^(BSG_KSCrashReportWriter *writer) { + writer->beginArray(writer, NULL); + writer->addJSONFileElement(writer, NULL, temporaryFile.fileSystemRepresentation); + writer->addJSONFileElement(writer, NULL, "/invalid/files/should/be/ignored"); + writer->addJSONFileElement(writer, NULL, temporaryFile.fileSystemRepresentation); + writer->endContainer(writer); + }); + id expected = @[@{@"foo": @"bar"}, @{@"foo": @"bar"}]; + XCTAssertEqualObjects(object, expected); + [[NSFileManager defaultManager] removeItemAtPath:temporaryFile error:NULL]; +} + +@end