Merge pull request #555 from bugsnag/v6-on-error

Make BugsnagOnErrorBlock return BOOL rather than void
bugsnag · Apr 22, 2020 · 0781176 · 0781176
2 parents 724b26d + 2011265
commit 0781176
Show file tree

Hide file tree

Showing 12 changed files with 51 additions and 28 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,9 @@ Bugsnag Notifiers on other platforms.
 
 ## Enhancements
 
+* Make `BugsnagOnErrorBlock` return `BOOL` rather than `void`
+  [#555](https://github.com/bugsnag/bugsnag-cocoa/pull/555)
+
 * Make `BugsnagPlugin` take `BugsnagClient` as param
   [#558](https://github.com/bugsnag/bugsnag-cocoa/pull/558)
 

diff --git a/Source/Bugsnag.m b/Source/Bugsnag.m
@@ -106,43 +106,47 @@ + (BOOL)appDidCrashLastLaunch {
 + (void)notify:(NSException *)exception {
     if ([self bugsnagStarted]) {
         [self.client notify:exception
-                      block:^(BugsnagEvent *_Nonnull report) {
+                      block:^BOOL(BugsnagEvent *_Nonnull report) {
                           report.depth += 2;
+                          return true;
                       }];
     }
 }
 
 + (void)notify:(NSException *)exception block:(BugsnagOnErrorBlock)block {
     if ([self bugsnagStarted]) {
         [[self client] notify:exception
-                        block:^(BugsnagEvent *_Nonnull report) {
+                        block:^BOOL(BugsnagEvent *_Nonnull report) {
                             report.depth += 2;
 
                             if (block) {
-                                block(report);
+                                return block(report);
                             }
+                            return true;
                         }];
     }
 }
 
 + (void)notifyError:(NSError *)error {
     if ([self bugsnagStarted]) {
         [self.client notifyError:error
-                             block:^(BugsnagEvent *_Nonnull report) {
+                             block:^BOOL(BugsnagEvent *_Nonnull report) {
                                  report.depth += 2;
+                                 return true;
                              }];
     }
 }
 
 + (void)notifyError:(NSError *)error block:(BugsnagOnErrorBlock)block {
     if ([self bugsnagStarted]) {
         [[self client] notifyError:error
-                               block:^(BugsnagEvent *_Nonnull report) {
+                               block:^BOOL(BugsnagEvent *_Nonnull report) {
                                    report.depth += 2;
 
                                    if (block) {
-                                       block(report);
+                                       return block(report);
                                    }
+                                   return true;
                                }];
     }
 }

diff --git a/Source/BugsnagClient.m b/Source/BugsnagClient.m
@@ -771,7 +771,7 @@ - (void)notifyError:(NSError *_Nonnull)error {
 }
 
 - (void)notifyError:(NSError *)error
-              block:(void (^)(BugsnagEvent *))block
+              block:(BugsnagOnErrorBlock)block
 {
     BugsnagHandledState *state = [BugsnagHandledState handledStateWithSeverityReason:HandledError
                                                                             severity:BSGSeverityWarning
@@ -781,7 +781,7 @@ - (void)notifyError:(NSError *)error
                                                  userInfo:error.userInfo];
     [self notify:wrapper
     handledState:state
-           block:^(BugsnagEvent *_Nonnull event) {
+           block:^BOOL(BugsnagEvent *_Nonnull event) {
                 event.originalError = error;
                 [event addMetadata:@{
                                         @"code" : @(error.code),
@@ -794,8 +794,9 @@ - (void)notifyError:(NSError *)error
                }
 
                if (block) {
-                   block(event);
+                   return block(event);
                }
+               return true;
            }];
 }
 
@@ -804,7 +805,7 @@ - (void)notify:(NSException *_Nonnull)exception {
 }
 
 - (void)notify:(NSException *)exception 
-         block:(void (^)(BugsnagEvent *))block 
+         block:(BugsnagOnErrorBlock)block
 {
     BugsnagHandledState *state =
         [BugsnagHandledState handledStateWithSeverityReason:HandledException];
@@ -881,7 +882,7 @@ - (void)notifyOutOfMemoryEvent {
 
 - (void)notify:(NSException *)exception
   handledState:(BugsnagHandledState *_Nonnull)handledState
-         block:(void (^)(BugsnagEvent *))block
+         block:(BugsnagOnErrorBlock)block
 {
     NSString *exceptionName = exception.name ?: NSStringFromClass([exception class]);
     NSString *message = exception.reason;
@@ -899,8 +900,8 @@ - (void)notify:(NSException *)exception
                                                           session:self.sessionTracker.runningSession];
     event.originalError = exception;
 
-    if (block) {
-        block(event);
+    if (block != nil && !block(event)) { // skip notifying if callback false
+        return;
     }
 
     //    We discard 5 stack frames (including this one) by default,

diff --git a/Source/BugsnagConfiguration.h b/Source/BugsnagConfiguration.h
@@ -70,7 +70,7 @@ typedef NS_ENUM(NSInteger, BSGConfigurationErrorCode) {
  *
  *  @param event the error report to be modified
  */
-typedef void (^BugsnagOnErrorBlock)(BugsnagEvent *_Nonnull event);
+typedef BOOL (^BugsnagOnErrorBlock)(BugsnagEvent *_Nonnull event);
 
 /**
  *  A handler for modifying data before sending it to Bugsnag.

diff --git a/Tests/BugsnagClientTests.m b/Tests/BugsnagClientTests.m
@@ -68,14 +68,15 @@ - (void)testAutomaticNotifyBreadcrumbData {
     __block NSString *eventSeverity;
 
     // Check that the event is passed the apiKey
-    [Bugsnag notify:ex block:^(BugsnagEvent * _Nonnull event) {
+    [Bugsnag notify:ex block:^BOOL(BugsnagEvent * _Nonnull event) {
         XCTAssertEqualObjects(event.apiKey, DUMMY_APIKEY_32CHAR_1);
 
         // Grab the values that end up in the event for later comparison
         eventErrorClass = event.errors[0].errorClass;
         eventErrorMessage = event.errors[0].errorMessage;
         eventUnhandled = [event valueForKeyPath:@"handledState.unhandled"] ? YES : NO;
         eventSeverity = BSGFormatSeverity([event severity]);
+        return true;
     }];
 
     // Check that we can change it

diff --git a/Tests/BugsnagEventTests.m b/Tests/BugsnagEventTests.m
@@ -514,44 +514,49 @@ - (void)testApiKey {
     NSException *ex = [[NSException alloc] initWithName:@"myName" reason:@"myReason1" userInfo:nil];
 
     // Check that the event is passed the apiKey
-    [Bugsnag notify:ex block:^(BugsnagEvent * _Nonnull event) {
+    [Bugsnag notify:ex block:^BOOL(BugsnagEvent * _Nonnull event) {
         XCTAssertEqualObjects(event.apiKey, DUMMY_APIKEY_32CHAR_1);
+        return true;
     }];
 
     // Check that we can change it
-    [Bugsnag notify:ex block:^(BugsnagEvent * _Nonnull event) {
+    [Bugsnag notify:ex block:^BOOL(BugsnagEvent * _Nonnull event) {
         XCTAssertEqualObjects(event.apiKey, DUMMY_APIKEY_32CHAR_1);
         event.apiKey = DUMMY_APIKEY_32CHAR_2;
         XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_2);
         XCTAssertEqualObjects(Bugsnag.configuration.apiKey, DUMMY_APIKEY_32CHAR_1);
+        return true;
     }];
 
     // Check that the global configuration is unaffected
-    [Bugsnag notify:ex block:^(BugsnagEvent * _Nonnull event) {
+    [Bugsnag notify:ex block:^BOOL(BugsnagEvent * _Nonnull event) {
         XCTAssertEqualObjects(event.apiKey, DUMMY_APIKEY_32CHAR_1);
         event.apiKey = DUMMY_APIKEY_32CHAR_1;
         XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_1);
         XCTAssertEqualObjects(Bugsnag.configuration.apiKey, DUMMY_APIKEY_32CHAR_1);
         event.apiKey = DUMMY_APIKEY_32CHAR_3;
         XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_3);
+        return true;
     }];
 
     // Check that previous local and global values are not persisted erroneously
     Bugsnag.configuration.apiKey = DUMMY_APIKEY_32CHAR_4;
-    [Bugsnag notify:ex block:^(BugsnagEvent * _Nonnull event) {
+    [Bugsnag notify:ex block:^BOOL(BugsnagEvent * _Nonnull event) {
         XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_4);
         event.apiKey = DUMMY_APIKEY_32CHAR_1;
         XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_1);
         XCTAssertEqual(Bugsnag.configuration.apiKey, DUMMY_APIKEY_32CHAR_4);
         event.apiKey = DUMMY_APIKEY_32CHAR_2;
         XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_2);
+        return true;
     }];
 
     // Check that validation is performed and that invalid API keys can't be set
     Bugsnag.configuration.apiKey = DUMMY_APIKEY_32CHAR_1;
-    [Bugsnag notify:ex block:^(BugsnagEvent * _Nonnull event) {
+    [Bugsnag notify:ex block:^BOOL(BugsnagEvent * _Nonnull event) {
         event.apiKey = DUMMY_APIKEY_16CHAR;
         XCTAssertEqual(event.apiKey, DUMMY_APIKEY_32CHAR_1);
+        return true;
     }];
 }
 
@@ -592,13 +597,14 @@ - (void)testInvalidSectionData {
 
     NSException *ex = [[NSException alloc] initWithName:@"myName" reason:@"myReason1" userInfo:nil];
 
-    [Bugsnag notify:ex block:^(BugsnagEvent * _Nonnull event) {
+    [Bugsnag notify:ex block:^BOOL(BugsnagEvent * _Nonnull event) {
         NSDictionary *invalidDict = @{};
         NSDictionary *validDict = @{@"myKey" : @"myValue"};
         [event addMetadata:invalidDict toSection:@"mySection"];
         XCTAssertEqual([[event.metadata toDictionary] count], 0);
         [event addMetadata:validDict toSection:@"mySection"];
         XCTAssertEqual([[event.metadata toDictionary] count], 1);
+        return true;
     }];
 }
 
@@ -607,7 +613,7 @@ - (void)testInvalidKeyValueData {
 
     NSException *ex = [[NSException alloc] initWithName:@"myName" reason:@"myReason1" userInfo:nil];
 
-    [Bugsnag notify:ex block:^(BugsnagEvent * _Nonnull event) {
+    [Bugsnag notify:ex block:^BOOL(BugsnagEvent * _Nonnull event) {
         [event addMetadata:[NSNull null] withKey:@"myKey" toSection:@"mySection"];
 
         // Invalid value for a non-existant section doesn't cause the section to be created
@@ -626,6 +632,7 @@ - (void)testInvalidKeyValueData {
         [event addMetadata:@"realValue" withKey:@"myNewKey" toSection:@"mySection"];
         XCTAssertEqual([[event.metadata toDictionary] count], 1);
         XCTAssertNotNil([event.metadata getMetadataFromSection:@"mySection" withKey:@"myNewKey"]);
+        return true;
     }];
 }
 

diff --git a/Tests/BugsnagTests.m b/Tests/BugsnagTests.m
@@ -65,7 +65,7 @@ - (void)testBugsnagMetadataAddition {
     NSException *exception1 = [[NSException alloc] initWithName:@"exception1" reason:@"reason1" userInfo:nil];
     NSException *exception2 = [[NSException alloc] initWithName:@"exception2" reason:@"reason2" userInfo:nil];
 
-    [Bugsnag notify:exception1 block:^(BugsnagEvent * _Nonnull event) {
+    [Bugsnag notify:exception1 block:^BOOL(BugsnagEvent * _Nonnull event) {
         XCTAssertEqualObjects([event getMetadataFromSection:@"mySection1" withKey:@"aKey1"], @"aValue1");
         XCTAssertEqual(event.errors[0].errorClass, @"exception1");
         XCTAssertEqual(event.errors[0].errorMessage, @"reason1");
@@ -75,7 +75,7 @@ - (void)testBugsnagMetadataAddition {
         [Bugsnag addMetadata:@"aValue2" withKey:@"aKey2" toSection:@"mySection2"];
     }];
 
-    [Bugsnag notify:exception2 block:^(BugsnagEvent * _Nonnull event) {
+    [Bugsnag notify:exception2 block:^BOOL(BugsnagEvent * _Nonnull event) {
         XCTAssertEqualObjects([event getMetadataFromSection:@"mySection1" withKey:@"aKey1"], @"aValue1");
         XCTAssertEqualObjects([event getMetadataFromSection:@"mySection2" withKey:@"aKey2"], @"aValue2");
         XCTAssertEqual(event.errors[0].errorClass, @"exception2");
@@ -87,7 +87,7 @@ - (void)testBugsnagMetadataAddition {
     [Bugsnag addMetadata:nil withKey:@"aKey1" toSection:@"mySection1"];
     [Bugsnag addMetadata:nil withKey:@"aKey2" toSection:@"mySection2"];
 
-    [Bugsnag notify:exception1 block:^(BugsnagEvent * _Nonnull event) {
+    [Bugsnag notify:exception1 block:^BOOL(BugsnagEvent * _Nonnull event) {
         XCTAssertNil([event getMetadataFromSection:@"mySection1" withKey:@"aKey1"]);
         XCTAssertNil([event getMetadataFromSection:@"mySection2" withKey:@"aKey2"]);
     }];
@@ -96,7 +96,7 @@ - (void)testBugsnagMetadataAddition {
 
     // This goes to Client
     [Bugsnag addMetadata:@"aValue1" withKey:@"aKey1" toSection:@"mySection1"];
-    [Bugsnag notify:exception1 block:^(BugsnagEvent * _Nonnull event) {
+    [Bugsnag notify:exception1 block:^BOOL(BugsnagEvent * _Nonnull event) {
         // event should have a copy of Client metadata
 
         XCTAssertEqualObjects([[[Bugsnag client] metadata] getMetadataFromSection:@"mySection1" withKey:@"aKey1"],
@@ -171,7 +171,7 @@ - (void)testMutableContext {
     NSException *exception1 = [[NSException alloc] initWithName:@"exception1" reason:@"reason1" userInfo:nil];
 
     // Check that the context is set going in to the test and that we can change it
-    [Bugsnag notify:exception1 block:^(BugsnagEvent * _Nonnull event) {
+    [Bugsnag notify:exception1 block:^BOOL(BugsnagEvent * _Nonnull event) {
         XCTAssertEqual([[Bugsnag configuration] context], @"firstContext");
 
         // Change the global context
@@ -183,16 +183,18 @@ - (void)testMutableContext {
         XCTAssertEqual([event context], @"firstContext");
 
         [expectation1 fulfill];
+        return true;
     }];
 
     // Test that the context (changed inside the notify block) remains changed
     // And that the event picks up this value.
     [self waitForExpectationsWithTimeout:5.0 handler:^(NSError * _Nullable error) {
         XCTAssertEqual([[Bugsnag configuration] context], @"secondContext");
 
-        [Bugsnag notify:exception1 block:^(BugsnagEvent * _Nonnull report) {
+        [Bugsnag notify:exception1 block:^BOOL(BugsnagEvent * _Nonnull report) {
             XCTAssertEqual([[Bugsnag configuration] context], @"secondContext");
             XCTAssertEqual([report context], @"secondContext");
+            return true;
         }];
     }];
 }

diff --git a/...ures/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/HandledErrorOverrideScenario.swift b/...ures/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/HandledErrorOverrideScenario.swift
@@ -26,6 +26,7 @@ class HandledErrorOverrideScenario: Scenario {
             let depth: Int = report.value(forKey: "depth") as! Int
             report.setValue(depth + 2, forKey: "depth")
             report.addMetadata(["items": [400,200]], section: "account")
+            return true
         }
     }
 

diff --git a/...res/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/HandledInternalNotifyScenario.swift b/...res/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/HandledInternalNotifyScenario.swift
@@ -20,6 +20,7 @@ import Bugsnag
                 ["method":"yes()"]
             ]
             report.attachCustomStacktrace(frames, withType: "unreal")
+            return true
         }
     }
 }

diff --git a/features/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/ModifyBreadcrumbInNotify.swift b/features/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/ModifyBreadcrumbInNotify.swift
@@ -17,6 +17,7 @@ class ModifyBreadcrumbInNotify: Scenario {
                     crumb.message = "Cache locked"
                 }
             })
+            return true
         }
     }
 

diff --git a/...s/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/UnhandledInternalNotifyScenario.swift b/...s/fixtures/ios-swift-cocoapods/iOSTestApp/scenarios/UnhandledInternalNotifyScenario.swift
@@ -20,6 +20,7 @@ import Bugsnag
                 ["method":"is_done()"]
             ]
             report.attachCustomStacktrace(frames, withType: "fake")
+            return true
         }
     }
 }

diff --git a/iOS/BugsnagTests/Swift Tests/BugsnagSwiftTests.swift b/iOS/BugsnagTests/Swift Tests/BugsnagSwiftTests.swift
@@ -25,6 +25,7 @@ class BugsnagSwiftTests: XCTestCase {
             // Arbitrary test, replicating the ObjC one
             let value = event.getMetadata(section: "mySection1", key: "myKey1") as? String
             XCTAssertEqual(value, "myValue1")
+            return true
         }
     }