diff --git a/CHANGELOG.md b/CHANGELOG.md index ee1b20bd2..f5429f248 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ Bugsnag Notifiers on other platforms. ## Enhancements +* Make all callbacks return boolean values + [#534](https://github.com/bugsnag/bugsnag-cocoa/pull/534) + * Add `originalError` property to `BugsnagEvent` [#541](https://github.com/bugsnag/bugsnag-cocoa/pull/541) diff --git a/Source/BugsnagConfiguration.h b/Source/BugsnagConfiguration.h index a2f93dc8c..4c1e350d9 100644 --- a/Source/BugsnagConfiguration.h +++ b/Source/BugsnagConfiguration.h @@ -59,7 +59,7 @@ typedef void (^BugsnagOnErrorBlock)(BugsnagEvent *_Nonnull event); * * @return YES if the event should be sent */ -typedef bool (^BugsnagOnSendBlock)(BugsnagEvent *_Nonnull event); +typedef BOOL (^BugsnagOnSendBlock)(BugsnagEvent *_Nonnull event); /** * A configuration block for modifying a captured breadcrumb @@ -73,7 +73,7 @@ typedef BOOL (^BugsnagOnBreadcrumbBlock)(BugsnagBreadcrumb *_Nonnull breadcrumb) * * @param sessionPayload The session about to be delivered */ -typedef void(^BugsnagOnSessionBlock)(NSMutableDictionary *_Nonnull sessionPayload); +typedef BOOL (^BugsnagOnSessionBlock)(NSMutableDictionary *_Nonnull sessionPayload); typedef NS_OPTIONS(NSUInteger, BSGErrorType) { BSGErrorTypesNone NS_SWIFT_NAME(None) = 0, diff --git a/Source/BugsnagSessionTracker.m b/Source/BugsnagSessionTracker.m index 5a4e5c3b3..8b7b81171 100644 --- a/Source/BugsnagSessionTracker.m +++ b/Source/BugsnagSessionTracker.m @@ -32,6 +32,7 @@ - (void)resume; @interface BugsnagConfiguration () @property(readonly, retain, nullable) NSURL *sessionURL; +@property(nonatomic, readwrite, strong) NSMutableArray *onSessionBlocks; @end @interface BugsnagSessionTracker () @@ -116,11 +117,20 @@ - (void)startNewSessionWithAutoCaptureValue:(BOOL)isAutoCaptured { return; } + // TODO JL: refactor to pass as structured data into callback + BugsnagSessionTrackingPayload *sessions = [[BugsnagSessionTrackingPayload alloc] initWithSessions:@[] + config:self.config]; + NSMutableDictionary *sessionsJson = [sessions toJson]; + for (BugsnagOnSessionBlock onSessionBlock in self.config.onSessionBlocks) { + if (!onSessionBlock(sessionsJson)) { + return; + } + } + self.currentSession = [[BugsnagSession alloc] initWithId:[[NSUUID UUID] UUIDString] startDate:[NSDate date] user:self.config.user autoCaptured:isAutoCaptured]; - [self.sessionStore write:self.currentSession]; if (self.callback) { diff --git a/Source/BugsnagSessionTrackingApiClient.m b/Source/BugsnagSessionTrackingApiClient.m index 65cde9232..5a169d5b8 100644 --- a/Source/BugsnagSessionTrackingApiClient.m +++ b/Source/BugsnagSessionTrackingApiClient.m @@ -49,10 +49,6 @@ - (void)deliverSessionsInStore:(BugsnagSessionFileStore *)store { NSUInteger sessionCount = payload.sessions.count; NSMutableDictionary *data = [payload toJson]; - for (BugsnagOnSessionBlock cb in self.config.onSessionBlocks) { - cb(data); - } - if (sessionCount > 0) { NSDictionary *HTTPHeaders = @{ @"Bugsnag-Payload-Version": @"1.0", diff --git a/Tests/BugsnagBaseUnitTest.m b/Tests/BugsnagBaseUnitTest.m index 3929776f2..ec16b5f26 100644 --- a/Tests/BugsnagBaseUnitTest.m +++ b/Tests/BugsnagBaseUnitTest.m @@ -44,7 +44,7 @@ -(void)setUpBugsnagWillCallNotify:(bool)willNotify [configuration setPersistUser:willPersistUser]; if (willNotify) { - [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }]; + [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }]; } [Bugsnag startBugsnagWithConfiguration:configuration]; } diff --git a/Tests/BugsnagBreadcrumbsTest.m b/Tests/BugsnagBreadcrumbsTest.m index 25d4d0e67..bac738840 100644 --- a/Tests/BugsnagBreadcrumbsTest.m +++ b/Tests/BugsnagBreadcrumbsTest.m @@ -302,7 +302,7 @@ - (void)testBreadcrumbFromDict { - (void)testCallbackFreeConstructors2 { // Prevent sending events BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; - [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }]; + [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }]; [Bugsnag startBugsnagWithConfiguration:configuration]; NSDictionary *md1 = @{ @"x" : @"y"}; @@ -369,7 +369,7 @@ - (void)testCallbackFreeConstructors2 { - (void)testCallbackFreeConstructors3 { // Prevent sending events BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; - [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }]; + [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }]; [Bugsnag startBugsnagWithConfiguration:configuration]; [Bugsnag leaveBreadcrumbWithMessage:@"message1"]; diff --git a/Tests/BugsnagClientTests.m b/Tests/BugsnagClientTests.m index 9efe106f8..4185c2363 100644 --- a/Tests/BugsnagClientTests.m +++ b/Tests/BugsnagClientTests.m @@ -47,7 +47,7 @@ @implementation BugsnagClientTests -(void)setUpBugsnagWillCallNotify:(bool)willNotify { BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; if (willNotify) { - [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }]; + [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }]; } [Bugsnag startBugsnagWithConfiguration:configuration]; } diff --git a/Tests/BugsnagConfigurationTests.m b/Tests/BugsnagConfigurationTests.m index e7c133220..af86814a9 100644 --- a/Tests/BugsnagConfigurationTests.m +++ b/Tests/BugsnagConfigurationTests.m @@ -108,9 +108,10 @@ - (void)testAddOnSessionBlock { BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; [config setEndpointsForNotify:@"http://notreal.bugsnag.com" sessions:@"http://notreal.bugsnag.com"]; XCTAssertEqual([[config onSessionBlocks] count], 0); - BugsnagOnSessionBlock sessionBlock = ^(NSMutableDictionary * _Nonnull sessionPayload) { + BugsnagOnSessionBlock sessionBlock = ^BOOL(NSMutableDictionary * _Nonnull sessionPayload) { // We expect the session block to be called [expectation fulfill]; + return true; }; [config addOnSessionBlock:sessionBlock]; XCTAssertEqual([[config onSessionBlocks] count], 1); @@ -132,8 +133,9 @@ - (void)testRemoveOnSessionBlock { BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; [config setEndpointsForNotify:@"http://notreal.bugsnag.com" sessions:@"http://notreal.bugsnag.com"]; XCTAssertEqual([[config onSessionBlocks] count], 0); - BugsnagOnSessionBlock sessionBlock = ^(NSMutableDictionary * _Nonnull sessionPayload) { + BugsnagOnSessionBlock sessionBlock = ^BOOL(NSMutableDictionary * _Nonnull sessionPayload) { [calledExpectation fulfill]; + return true; }; // It's there (and from other tests we know it gets called) and then it's not there @@ -166,7 +168,7 @@ - (void)testAddOnSessionBlockThenRemove { [config setEndpointsForNotify:@"http://notreal.bugsnag.com" sessions:@"http://notreal.bugsnag.com"]; XCTAssertEqual([[config onSessionBlocks] count], 0); - BugsnagOnSessionBlock sessionBlock = ^(NSMutableDictionary * _Nonnull sessionPayload) { + BugsnagOnSessionBlock sessionBlock = ^BOOL(NSMutableDictionary * _Nonnull sessionPayload) { switch (called) { case 0: [expectation1 fulfill]; @@ -183,6 +185,7 @@ - (void)testAddOnSessionBlockThenRemove { [expectation4 fulfill]; break; } + return true; }; [config addOnSessionBlock:sessionBlock]; @@ -219,8 +222,8 @@ - (void)testAddOnSessionBlockThenRemove { - (void)testRemoveNonexistentOnSessionBlocks { BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; XCTAssertEqual([[config onSessionBlocks] count], 0); - BugsnagOnSessionBlock sessionBlock1 = ^(NSMutableDictionary * _Nonnull sessionPayload) {}; - BugsnagOnSessionBlock sessionBlock2 = ^(NSMutableDictionary * _Nonnull sessionPayload) {}; + BugsnagOnSessionBlock sessionBlock1 = ^BOOL(NSMutableDictionary * _Nonnull sessionPayload) { return true; }; + BugsnagOnSessionBlock sessionBlock2 = ^BOOL(NSMutableDictionary * _Nonnull sessionPayload) { return true; }; [config addOnSessionBlock:sessionBlock1]; XCTAssertEqual([[config onSessionBlocks] count], 1); @@ -731,7 +734,7 @@ - (void) testRemoveOnSendBlock { BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; XCTAssertEqual([[configuration onSendBlocks] count], 0); - BugsnagOnSendBlock block = ^bool(BugsnagEvent * _Nonnull event) { return false; }; + BugsnagOnSendBlock block = ^BOOL(BugsnagEvent * _Nonnull event) { return false; }; [configuration addOnSendBlock:block]; [Bugsnag startBugsnagWithConfiguration:configuration]; @@ -750,8 +753,8 @@ - (void) testClearOnSendBlock { BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; XCTAssertEqual([[configuration onSendBlocks] count], 0); - BugsnagOnSendBlock block1 = ^bool(BugsnagEvent * _Nonnull event) { return false; }; - BugsnagOnSendBlock block2 = ^bool(BugsnagEvent * _Nonnull event) { return false; }; + BugsnagOnSendBlock block1 = ^BOOL(BugsnagEvent * _Nonnull event) { return false; }; + BugsnagOnSendBlock block2 = ^BOOL(BugsnagEvent * _Nonnull event) { return false; }; // Add more than one [configuration addOnSendBlock:block1]; @@ -772,8 +775,8 @@ - (void)testNSCopying { [config setContext:@"context1"]; [config setAppType:@"The most amazing app, a brilliant app, the app to end all apps"]; [config setPersistUser:YES]; - BugsnagOnSendBlock onSendBlock1 = ^bool(BugsnagEvent * _Nonnull event) { return true; }; - BugsnagOnSendBlock onSendBlock2 = ^bool(BugsnagEvent * _Nonnull event) { return true; }; + BugsnagOnSendBlock onSendBlock1 = ^BOOL(BugsnagEvent * _Nonnull event) { return true; }; + BugsnagOnSendBlock onSendBlock2 = ^BOOL(BugsnagEvent * _Nonnull event) { return true; }; NSArray *sendBlocks = @[ onSendBlock1, onSendBlock2 ]; [config setOnSendBlocks:[sendBlocks mutableCopy]]; // Mutable arg required diff --git a/Tests/BugsnagSessionTrackerTest.m b/Tests/BugsnagSessionTrackerTest.m index 8f543de29..348fed919 100644 --- a/Tests/BugsnagSessionTrackerTest.m +++ b/Tests/BugsnagSessionTrackerTest.m @@ -24,6 +24,10 @@ @interface BugsnagConfiguration () - (void)deletePersistedUserData; @end +@interface BugsnagSessionTracker () +@property (strong, readwrite) BugsnagSession *currentSession; +@end + @interface BugsnagSessionTrackerTest : XCTestCase @property BugsnagConfiguration *configuration; @property BugsnagSessionTracker *sessionTracker; @@ -139,4 +143,31 @@ - (void)testIncrementCounts { XCTAssertEqual(1, session.unhandledCount); } +- (void)testOnSendBlockFalse { + self.configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; + [self.configuration addOnSessionBlock:^BOOL(NSMutableDictionary *sessionPayload) { + return NO; + }]; + self.sessionTracker = [[BugsnagSessionTracker alloc] initWithConfig:self.configuration + postRecordCallback:nil]; + [self.sessionTracker startNewSession]; + XCTAssertNil(self.sessionTracker.currentSession); +} + +- (void)testOnSendBlockTrue { + __block XCTestExpectation *expectation = [self expectationWithDescription:@"Session block is invoked"]; + + self.configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; + [self.configuration addOnSessionBlock:^BOOL(NSMutableDictionary *sessionPayload) { + [expectation fulfill]; + return YES; + }]; + self.sessionTracker = [[BugsnagSessionTracker alloc] initWithConfig:self.configuration + postRecordCallback:nil]; + [self.sessionTracker startNewSession]; + [self waitForExpectations:@[expectation] timeout:2]; + XCTAssertNotNil(self.sessionTracker.currentSession); +} + + @end diff --git a/Tests/BugsnagTests.m b/Tests/BugsnagTests.m index ce4adc422..a8811f8ec 100644 --- a/Tests/BugsnagTests.m +++ b/Tests/BugsnagTests.m @@ -44,7 +44,7 @@ @implementation BugsnagTests -(void)setUpBugsnagWillCallNotify:(bool)willNotify { BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; if (willNotify) { - [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }]; + [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }]; } [Bugsnag startBugsnagWithConfiguration:configuration]; } @@ -147,7 +147,7 @@ - (void)testGetMetadata { */ -(void)testBugsnagPauseSession { BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; - [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }]; + [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }]; [Bugsnag startBugsnagWithConfiguration:configuration]; @@ -164,7 +164,7 @@ - (void)testMutableContext { BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; [configuration setContext:@"firstContext"]; - [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }]; + [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }]; [Bugsnag startBugsnagWithConfiguration:configuration]; @@ -247,9 +247,9 @@ - (void)testRemoveOnSessionBlock { BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; // non-sending bugsnag - [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }]; + [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }]; - BugsnagOnSessionBlock sessionBlock = ^(NSMutableDictionary * _Nonnull sessionPayload) { + BugsnagOnSessionBlock sessionBlock = ^BOOL(NSMutableDictionary * _Nonnull sessionPayload) { switch (called) { case 0: [expectation1 fulfill]; @@ -258,6 +258,7 @@ - (void)testRemoveOnSessionBlock { [expectation2 fulfill]; break; } + return true; }; [configuration addOnSessionBlock:sessionBlock]; @@ -287,9 +288,9 @@ - (void)testAddOnSessionBlock { configuration.autoTrackSessions = NO; // non-sending bugsnag - [configuration addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { return false; }]; + [configuration addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { return false; }]; - BugsnagOnSessionBlock sessionBlock = ^(NSMutableDictionary * _Nonnull sessionPayload) { + BugsnagOnSessionBlock sessionBlock = ^BOOL(NSMutableDictionary * _Nonnull sessionPayload) { switch (called) { case 0: [expectation1 fulfill]; @@ -298,6 +299,7 @@ - (void)testAddOnSessionBlock { [expectation2 fulfill]; break; } + return true; }; // NOTE: Due to test conditions the state of the Bugsnag/client class is indeterminate. @@ -348,7 +350,7 @@ - (void) testOnSendBlocks { expectation6.inverted = YES; // Two blocks that will get called (or not) when we notify() - BugsnagOnSendBlock block1 = ^bool(BugsnagEvent * _Nonnull event) + BugsnagOnSendBlock block1 = ^BOOL(BugsnagEvent * _Nonnull event) { switch (called) { case 0: @@ -372,7 +374,7 @@ - (void) testOnSendBlocks { return false; }; - BugsnagOnSendBlock block2 = ^bool(BugsnagEvent * _Nonnull event) + BugsnagOnSendBlock block2 = ^BOOL(BugsnagEvent * _Nonnull event) { switch (called) { case 0: diff --git a/features/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/OOMScenario.m b/features/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/OOMScenario.m index b759f9edc..312bafabe 100644 --- a/features/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/OOMScenario.m +++ b/features/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/OOMScenario.m @@ -6,7 +6,7 @@ @implementation OOMScenario - (void)startBugsnag { self.config.autoTrackSessions = NO; self.config.releaseStage = @"alpha"; - [self.config addOnSendBlock:^bool(BugsnagEvent * _Nonnull event) { + [self.config addOnSendBlock:^BOOL(BugsnagEvent * _Nonnull event) { [event addMetadata:@{ @"shape": @"line" } toSection:@"extra"]; return YES; }];