Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make BugsnagOnErrorBlock return BOOL rather than void #555

Merged
merged 2 commits into from Apr 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -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)

Expand Down
16 changes: 10 additions & 6 deletions Source/Bugsnag.m
Expand Up @@ -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;
}];
}
}
Expand Down
15 changes: 8 additions & 7 deletions Source/BugsnagClient.m
Expand Up @@ -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
Expand All @@ -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),
Expand All @@ -794,8 +794,9 @@ - (void)notifyError:(NSError *)error
}

if (block) {
block(event);
return block(event);
}
return true;
}];
}

Expand All @@ -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];
Expand Down Expand Up @@ -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;
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion Source/BugsnagConfiguration.h
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion Tests/BugsnagClientTests.m
Expand Up @@ -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
Expand Down
21 changes: 14 additions & 7 deletions Tests/BugsnagEventTests.m
Expand Up @@ -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;
}];
}

Expand Down Expand Up @@ -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;
}];
}

Expand All @@ -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
Expand All @@ -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;
}];
}

Expand Down
14 changes: 8 additions & 6 deletions Tests/BugsnagTests.m
Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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"]);
}];
Expand All @@ -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"],
Expand Down Expand Up @@ -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
Expand All @@ -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;
}];
}];
}
Expand Down
Expand Up @@ -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
}
}

Expand Down
Expand Up @@ -20,6 +20,7 @@ import Bugsnag
["method":"yes()"]
]
report.attachCustomStacktrace(frames, withType: "unreal")
return true
}
}
}
Expand Down
Expand Up @@ -17,6 +17,7 @@ class ModifyBreadcrumbInNotify: Scenario {
crumb.message = "Cache locked"
}
})
return true
}
}

Expand Down
Expand Up @@ -20,6 +20,7 @@ import Bugsnag
["method":"is_done()"]
]
report.attachCustomStacktrace(frames, withType: "fake")
return true
}
}
}
Expand Down
1 change: 1 addition & 0 deletions iOS/BugsnagTests/Swift Tests/BugsnagSwiftTests.swift
Expand Up @@ -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
}
}

Expand Down