From cf17380532a603dac68e975c0d1c70f979e7b026 Mon Sep 17 00:00:00 2001 From: Karl Stenerud Date: Fri, 11 Dec 2020 15:58:16 +0100 Subject: [PATCH] Added maxPersistedEvents config option so that users can control how many unsent events will be kept before deleting the oldest. --- Bugsnag/BugsnagCrashSentry.m | 4 +-- Bugsnag/Client/BugsnagClient.m | 1 - .../Configuration/BSGConfigurationBuilder.m | 17 +++++----- Bugsnag/Configuration/BugsnagConfiguration.m | 22 +++++++++++++ Bugsnag/Helpers/BugsnagKeys.h | 3 +- Bugsnag/Helpers/BugsnagKeys.m | 3 +- .../include/Bugsnag/BugsnagConfiguration.h | 8 +++++ Tests/BSGConfigurationBuilderTests.m | 4 +++ Tests/BugsnagClientTests.m | 4 +++ Tests/BugsnagConfigurationTests.m | 32 +++++++++++++++++++ Tests/ConfigurationApiValidationTest.m | 19 +++++++++++ 11 files changed, 103 insertions(+), 14 deletions(-) diff --git a/Bugsnag/BugsnagCrashSentry.m b/Bugsnag/BugsnagCrashSentry.m index 7e779faf3..7233bdb58 100644 --- a/Bugsnag/BugsnagCrashSentry.m +++ b/Bugsnag/BugsnagCrashSentry.m @@ -16,8 +16,6 @@ #import "Bugsnag.h" #import "BugsnagErrorTypes.h" -NSUInteger const BSG_MAX_STORED_REPORTS = 12; - @implementation BugsnagCrashSentry - (void)install:(BugsnagConfiguration *)config @@ -29,7 +27,7 @@ - (void)install:(BugsnagConfiguration *)config ksCrash.sink = sink; ksCrash.introspectMemory = YES; ksCrash.onCrash = onCrash; - ksCrash.maxStoredReports = BSG_MAX_STORED_REPORTS; + ksCrash.maxStoredReports = (int)config.maxPersistedEvents; // overridden elsewhere for handled errors, so we can assume that this only // applies to unhandled errors diff --git a/Bugsnag/Client/BugsnagClient.m b/Bugsnag/Client/BugsnagClient.m index 71ff25e80..07bb96c19 100644 --- a/Bugsnag/Client/BugsnagClient.m +++ b/Bugsnag/Client/BugsnagClient.m @@ -434,7 +434,6 @@ - (void)initializeNotificationNameMap { - (void)start { [self.configuration validate]; - [self.crashSentry install:self.configuration apiClient:self.errorReportApiClient onCrash:&BSSerializeDataCrashHandler]; diff --git a/Bugsnag/Configuration/BSGConfigurationBuilder.m b/Bugsnag/Configuration/BSGConfigurationBuilder.m index d704d34e9..f203d17bf 100644 --- a/Bugsnag/Configuration/BSGConfigurationBuilder.m +++ b/Bugsnag/Configuration/BSGConfigurationBuilder.m @@ -29,6 +29,7 @@ + (BugsnagConfiguration *)configurationFromOptions:(NSDictionary *)options { BSGKeyBundleVersion, BSGKeyEnabledReleaseStages, BSGKeyEndpoints, + BSGKeyMaxPersistedEvents, BSGKeyMaxBreadcrumbs, BSGKeyPersistUser, BSGKeyRedactedKeys, @@ -54,7 +55,8 @@ + (BugsnagConfiguration *)configurationFromOptions:(NSDictionary *)options { [self loadStringArray:config options:options key:BSGKeyRedactedKeys]; [self loadEndpoints:config options:options]; - [self loadMaxBreadcrumbs:config options:options]; + [self loadNumber:config options:options key:BSGKeyMaxBreadcrumbs]; + [self loadNumber:config options:options key:BSGKeyMaxPersistedEvents]; [self loadSendThreads:config options:options]; return config; } @@ -71,6 +73,12 @@ + (void)loadString:(BugsnagConfiguration *)config options:(NSDictionary *)option } } ++ (void)loadNumber:(BugsnagConfiguration *)config options:(NSDictionary *)options key:(NSString *)key { + if (options[key] && [options[key] isKindOfClass:[NSNumber class]]) { + [config setValue:options[key] forKey:key]; + } +} + + (void)loadStringArray:(BugsnagConfiguration *)config options:(NSDictionary *)options key:(NSString *)key { if (options[key] && [options[key] isKindOfClass:[NSArray class]]) { NSArray *val = options[key]; @@ -97,13 +105,6 @@ + (void)loadEndpoints:(BugsnagConfiguration *)config options:(NSDictionary *)opt } } -+ (void)loadMaxBreadcrumbs:(BugsnagConfiguration *)config options:(NSDictionary *)options { - if (options[BSGKeyMaxBreadcrumbs] && [options[BSGKeyMaxBreadcrumbs] isKindOfClass:[NSNumber class]]) { - NSNumber *num = options[BSGKeyMaxBreadcrumbs]; - config.maxBreadcrumbs = [num unsignedIntValue]; - } -} - + (void)loadSendThreads:(BugsnagConfiguration *)config options:(NSDictionary *)options { if (options[BSGKeySendThreads] && [options[BSGKeySendThreads] isKindOfClass:[NSString class]]) { NSString *sendThreads = [options[BSGKeySendThreads] lowercaseString]; diff --git a/Bugsnag/Configuration/BugsnagConfiguration.m b/Bugsnag/Configuration/BugsnagConfiguration.m index 1d9c04368..acff49840 100644 --- a/Bugsnag/Configuration/BugsnagConfiguration.m +++ b/Bugsnag/Configuration/BugsnagConfiguration.m @@ -83,6 +83,7 @@ - (nonnull id)copyWithZone:(nullable NSZone *)zone { [copy setEnabledErrorTypes:self.enabledErrorTypes]; [copy setEnabledReleaseStages:self.enabledReleaseStages]; [copy setRedactedKeys:self.redactedKeys]; + [copy setMaxPersistedEvents:self.maxPersistedEvents]; [copy setMaxBreadcrumbs:self.maxBreadcrumbs]; copy->_metadata = [[BugsnagMetadata alloc] initWithDictionary:[[self.metadata toDictionary] mutableCopy]]; [copy setEndpoints:self.endpoints]; @@ -159,6 +160,7 @@ - (instancetype)initWithApiKey:(NSString *)apiKey { _enabledReleaseStages = nil; _redactedKeys = [NSSet setWithArray:@[@"password"]]; _enabledBreadcrumbTypes = BSGEnabledBreadcrumbTypeAll; + _maxPersistedEvents = 12; _maxBreadcrumbs = 25; _autoTrackSessions = YES; _sendThreads = BSGThreadSendPolicyAlways; @@ -415,6 +417,26 @@ -(void)deletePersistedUserData { // MARK: - Properties: Getters and Setters // ----------------------------------------------------------------------------- +@synthesize maxPersistedEvents = _maxPersistedEvents; + +- (NSUInteger)maxPersistedEvents { + @synchronized (self) { + return _maxPersistedEvents; + } +} + +- (void)setMaxPersistedEvents:(NSUInteger)maxPersistedEvents { + @synchronized (self) { + if (maxPersistedEvents >= 1 && maxPersistedEvents <= 100) { + _maxPersistedEvents = maxPersistedEvents; + } else { + bsg_log_err(@"Invalid configuration value detected. Option maxPersistedEvents " + "should be an integer between 1-100. Supplied value is %lu", + (unsigned long) maxPersistedEvents); + } + } +} + @synthesize maxBreadcrumbs = _maxBreadcrumbs; - (NSUInteger)maxBreadcrumbs { diff --git a/Bugsnag/Helpers/BugsnagKeys.h b/Bugsnag/Helpers/BugsnagKeys.h index aeba03ee3..90fc4c835 100644 --- a/Bugsnag/Helpers/BugsnagKeys.h +++ b/Bugsnag/Helpers/BugsnagKeys.h @@ -42,8 +42,8 @@ extern NSString *const BSGKeyExceptionName; extern NSString *const BSGKeyExceptions; extern NSString *const BSGKeyExecutableName; extern NSString *const BSGKeyExtraRuntimeInfo; -extern NSString *const BSGKeyFrameAddrFormat; extern NSString *const BSGKeyFrameAddress; +extern NSString *const BSGKeyFrameAddrFormat; extern NSString *const BSGKeyGroupingHash; extern NSString *const BSGKeyHwMachine; extern NSString *const BSGKeyHwModel; @@ -63,6 +63,7 @@ extern NSString *const BSGKeyMachoLoadAddr; extern NSString *const BSGKeyMachoUUID; extern NSString *const BSGKeyMachoVMAddress; extern NSString *const BSGKeyMaxBreadcrumbs; +extern NSString *const BSGKeyMaxPersistedEvents; extern NSString *const BSGKeyMessage; extern NSString *const BSGKeyMetadata; extern NSString *const BSGKeyMethod; diff --git a/Bugsnag/Helpers/BugsnagKeys.m b/Bugsnag/Helpers/BugsnagKeys.m index 300a94fb3..060fd3f35 100644 --- a/Bugsnag/Helpers/BugsnagKeys.m +++ b/Bugsnag/Helpers/BugsnagKeys.m @@ -38,8 +38,8 @@ NSString *const BSGKeyExceptions = @"exceptions"; NSString *const BSGKeyExecutableName = @"CFBundleExecutable"; NSString *const BSGKeyExtraRuntimeInfo = @"extraRuntimeInfo"; -NSString *const BSGKeyFrameAddrFormat = @"0x%lx"; NSString *const BSGKeyFrameAddress = @"frameAddress"; +NSString *const BSGKeyFrameAddrFormat = @"0x%lx"; NSString *const BSGKeyGroupingHash = @"groupingHash"; NSString *const BSGKeyHwMachine = @"hw.machine"; NSString *const BSGKeyHwModel = @"hw.model"; @@ -59,6 +59,7 @@ NSString *const BSGKeyMachoUUID = @"machoUUID"; NSString *const BSGKeyMachoVMAddress = @"machoVMAddress"; NSString *const BSGKeyMaxBreadcrumbs = @"maxBreadcrumbs"; +NSString *const BSGKeyMaxPersistedEvents = @"maxPersistedEvents"; NSString *const BSGKeyMessage = @"message"; NSString *const BSGKeyMetadata = @"metaData"; NSString *const BSGKeyMethod = @"method"; diff --git a/Bugsnag/include/Bugsnag/BugsnagConfiguration.h b/Bugsnag/include/Bugsnag/BugsnagConfiguration.h index 844f4406e..e3989cc24 100644 --- a/Bugsnag/include/Bugsnag/BugsnagConfiguration.h +++ b/Bugsnag/include/Bugsnag/BugsnagConfiguration.h @@ -204,6 +204,14 @@ typedef BOOL (^BugsnagOnSessionBlock)(BugsnagSession *_Nonnull session); @property(retain, nullable) NSString *appType; +/** + * Sets the maximum number of events which will be stored. Once the threshold is reached, + * the oldest events will be deleted. + * + * By default, 12 events are stored: this can be amended up to a maximum of 100. + */ +@property (nonatomic) NSUInteger maxPersistedEvents; + /** * Sets the maximum number of breadcrumbs which will be stored. Once the threshold is reached, * the oldest breadcrumbs will be deleted. diff --git a/Tests/BSGConfigurationBuilderTests.m b/Tests/BSGConfigurationBuilderTests.m index 2ab535f7a..3ad82f82d 100644 --- a/Tests/BSGConfigurationBuilderTests.m +++ b/Tests/BSGConfigurationBuilderTests.m @@ -55,6 +55,7 @@ - (void)testDecodeDefaultValues { XCTAssertNil(config.appVersion); XCTAssertTrue(config.autoDetectErrors); XCTAssertTrue(config.autoTrackSessions); + XCTAssertEqual(12, config.maxPersistedEvents); XCTAssertEqual(25, config.maxBreadcrumbs); XCTAssertTrue(config.persistUser); XCTAssertEqualObjects(@[@"password"], [config.redactedKeys allObjects]); @@ -92,6 +93,7 @@ - (void)testDecodeFullConfig { @"sessions": @"https://sessions.example.co" }, @"enabledReleaseStages": @[@"beta2", @"prod"], + @"maxPersistedEvents": @29, @"maxBreadcrumbs": @27, @"persistUser": @NO, @"redactedKeys": @[@"foo"], @@ -105,6 +107,7 @@ - (void)testDecodeFullConfig { XCTAssertFalse(config.autoDetectErrors); XCTAssertFalse(config.autoTrackSessions); XCTAssertEqualObjects(@"7.22", config.bundleVersion); + XCTAssertEqual(29, config.maxPersistedEvents); XCTAssertEqual(27, config.maxBreadcrumbs); XCTAssertFalse(config.persistUser); XCTAssertEqualObjects(@[@"foo"], config.redactedKeys); @@ -138,6 +141,7 @@ - (void)testInvalidConfigOptions { @"endpoints": [NSNull null], @"enabledReleaseStages": @[@"beta2", @"prod"], @"enabledErrorTypes": @[@"ooms", @"signals"], + @"maxPersistedEvents": @29, @"maxBreadcrumbs": @27, @"persistUser": @"pomelo", @"redactedKeys": @[@77], diff --git a/Tests/BugsnagClientTests.m b/Tests/BugsnagClientTests.m index 7a958109d..b61e1639a 100644 --- a/Tests/BugsnagClientTests.m +++ b/Tests/BugsnagClientTests.m @@ -187,6 +187,7 @@ - (void)assertEqualConfiguration:(BugsnagConfiguration *)expected withActual:(Bu XCTAssertEqual(expected.enabledReleaseStages, actual.enabledReleaseStages); XCTAssertEqualObjects(expected.endpoints.notify, actual.endpoints.notify); XCTAssertEqualObjects(expected.endpoints.sessions, actual.endpoints.sessions); + XCTAssertEqual(expected.maxPersistedEvents, actual.maxPersistedEvents); XCTAssertEqual(expected.maxBreadcrumbs, actual.maxBreadcrumbs); XCTAssertEqual(expected.persistUser, actual.persistUser); XCTAssertEqual([expected.redactedKeys count], [actual.redactedKeys count]); @@ -209,11 +210,13 @@ - (void)testChangesToConfigurationAreIgnoredAfterCallingStart { // Modify some arbitrary properties config.persistUser = !config.persistUser; + config.maxPersistedEvents = config.maxPersistedEvents * 2; config.maxBreadcrumbs = config.maxBreadcrumbs * 2; config.appVersion = @"99.99.99"; // Ensure the changes haven't been reflected in our copy XCTAssertNotEqual(initialConfig.persistUser, config.persistUser); + XCTAssertNotEqual(initialConfig.maxPersistedEvents, config.maxPersistedEvents); XCTAssertNotEqual(initialConfig.maxBreadcrumbs, config.maxBreadcrumbs); XCTAssertNotEqualObjects(initialConfig.appVersion, config.appVersion); @@ -231,6 +234,7 @@ - (void)testStartingBugsnagTwiceLogsAWarningAndIgnoresNewConfiguration { BugsnagConfiguration *updatedConfig = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_2]; updatedConfig.persistUser = !initialConfig.persistUser; updatedConfig.maxBreadcrumbs = initialConfig.maxBreadcrumbs * 2; + updatedConfig.maxPersistedEvents = initialConfig.maxPersistedEvents * 2; updatedConfig.appVersion = @"99.99.99"; [Bugsnag startWithConfiguration:updatedConfig]; diff --git a/Tests/BugsnagConfigurationTests.m b/Tests/BugsnagConfigurationTests.m index e7a11bd9f..248745ef3 100644 --- a/Tests/BugsnagConfigurationTests.m +++ b/Tests/BugsnagConfigurationTests.m @@ -529,6 +529,38 @@ - (void)testSettingPersistUser { XCTAssertTrue(config.persistUser); } +// ============================================================================= +// MARK: - Max Persisted Events +// ============================================================================= + +- (void)testMaxPersistedEvents { + BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; + XCTAssertEqual(12, config.maxPersistedEvents); + + // alter to valid value + config.maxPersistedEvents = 10; + XCTAssertEqual(10, config.maxPersistedEvents); + + // alter to max value + config.maxPersistedEvents = 100; + XCTAssertEqual(100, config.maxPersistedEvents); + + // alter to min value + config.maxPersistedEvents = 1; + XCTAssertEqual(1, config.maxPersistedEvents); + + config.maxPersistedEvents = 0; + XCTAssertEqual(1, config.maxPersistedEvents); + + // alter to negative value + config.maxPersistedEvents = -1; + XCTAssertEqual(1, config.maxPersistedEvents); + + // alter to > max value + config.maxPersistedEvents = 500; + XCTAssertEqual(1, config.maxPersistedEvents); +} + // ============================================================================= // MARK: - Max Breadcrumb // ============================================================================= diff --git a/Tests/ConfigurationApiValidationTest.m b/Tests/ConfigurationApiValidationTest.m index a1a099264..4afa12d5f 100644 --- a/Tests/ConfigurationApiValidationTest.m +++ b/Tests/ConfigurationApiValidationTest.m @@ -128,6 +128,25 @@ - (void)testValidAppType { XCTAssertEqualObjects(@"cocoa", self.config.appType); } +- (void)testValidMaxPersistedEvents { + self.config.maxPersistedEvents = 1; + XCTAssertEqual(1, self.config.maxPersistedEvents); + self.config.maxPersistedEvents = 100; + XCTAssertEqual(100, self.config.maxPersistedEvents); + self.config.maxPersistedEvents = 40; + XCTAssertEqual(40, self.config.maxPersistedEvents); +} + +- (void)testInvalidMaxPersistedEvents { + self.config.maxPersistedEvents = 1; + self.config.maxPersistedEvents = 0; + XCTAssertEqual(1, self.config.maxPersistedEvents); + self.config.maxPersistedEvents = -1; + XCTAssertEqual(1, self.config.maxPersistedEvents); + self.config.maxPersistedEvents = 590; + XCTAssertEqual(1, self.config.maxPersistedEvents); +} + - (void)testValidMaxBreadcrumbs { self.config.maxBreadcrumbs = 0; XCTAssertEqual(0, self.config.maxBreadcrumbs);