From 2cf8a88a96ca1876c26644a504a345d5858f791e Mon Sep 17 00:00:00 2001 From: Alexander Cohen Date: Fri, 29 Nov 2024 12:41:53 -0500 Subject: [PATCH] Add Send Failure Callback API --- Bugsnag/BugsnagInternals.h | 2 + Bugsnag/Configuration/BugsnagConfiguration.m | 20 +++++ Bugsnag/Delivery/BSGEventUploadOperation.m | 22 +++++- Bugsnag/Helpers/BSGTelemetry.m | 1 + .../include/Bugsnag/BugsnagConfiguration.h | 35 +++++++++ CHANGELOG.md | 5 ++ .../BugsnagTests/BugsnagConfigurationTests.m | 77 ++++++++++++++++--- .../BugsnagSwiftConfigurationTests.swift | 23 ++++++ .../ConfigurationApiValidationTest.m | 8 ++ 9 files changed, 179 insertions(+), 14 deletions(-) diff --git a/Bugsnag/BugsnagInternals.h b/Bugsnag/BugsnagInternals.h index 76c00bea4..a79123f23 100644 --- a/Bugsnag/BugsnagInternals.h +++ b/Bugsnag/BugsnagInternals.h @@ -112,6 +112,8 @@ typedef void (^ BSGClientObserver)(BSGClientObserverEvent event, _Nullable id va @property (nonatomic) NSMutableArray *onSendBlocks; +@property (nonatomic) NSMutableArray *onFailureBlocks; + @property (nonatomic) NSMutableArray *onSessionBlocks; @end diff --git a/Bugsnag/Configuration/BugsnagConfiguration.m b/Bugsnag/Configuration/BugsnagConfiguration.m index edab5f2a5..97435fe63 100644 --- a/Bugsnag/Configuration/BugsnagConfiguration.m +++ b/Bugsnag/Configuration/BugsnagConfiguration.m @@ -119,6 +119,7 @@ - (nonnull id)copyWithZone:(nullable NSZone *)zone { // as creating a copy of the array would prevent this [copy setOnBreadcrumbBlocks:self.onBreadcrumbBlocks]; [copy setOnSendBlocks:self.onSendBlocks]; + [copy setOnFailureBlocks:self.onFailureBlocks]; [copy setOnSessionBlocks:self.onSessionBlocks]; [copy setTelemetry:self.telemetry]; return copy; @@ -174,6 +175,7 @@ - (instancetype)initWithApiKey:(NSString *)apiKey { _appHangThresholdMillis = BugsnagAppHangThresholdFatalOnly; #endif _onSendBlocks = [NSMutableArray new]; + _onFailureBlocks = [NSMutableArray new]; _onSessionBlocks = [NSMutableArray new]; _onBreadcrumbBlocks = [NSMutableArray new]; _plugins = [NSMutableSet new]; @@ -284,6 +286,24 @@ - (NSURLSession *)sessionOrDefault { return session ? session : getConfigDefaultURLSession(); } +// ============================================================================= +// MARK: - onSendFailure +// ============================================================================= + +- (BugsnagOnSendFailureRef)addOnSendFailureBlock:(BugsnagOnSendFailureBlock)block { + BugsnagOnSendFailureBlock callback = [block copy]; + [self.onFailureBlocks addObject:callback]; + return callback; +} + +- (void)removeOnSendFailure:(BugsnagOnSendFailureRef)callback { + if (![callback isKindOfClass:NSClassFromString(@"NSBlock")]) { + bsg_log_err(@"Invalid object type passed to %@", NSStringFromSelector(_cmd)); + return; + } + [self.onFailureBlocks removeObject:(id)callback]; +} + // ============================================================================= // MARK: - onSendBlock // ============================================================================= diff --git a/Bugsnag/Delivery/BSGEventUploadOperation.m b/Bugsnag/Delivery/BSGEventUploadOperation.m index 5d7763103..2486458f8 100644 --- a/Bugsnag/Delivery/BSGEventUploadOperation.m +++ b/Bugsnag/Delivery/BSGEventUploadOperation.m @@ -46,22 +46,36 @@ - (instancetype)initWithDelegate:(id)delegate { return self; } +- (void)_sendFailureForEvent:(BugsnagEvent *)event configuration:(BugsnagConfiguration *)configuration { + NSArray *failureBlocks = [configuration.onFailureBlocks copy]; + for (BugsnagOnSendFailureBlock block in failureBlocks) { + @try { + block(event); + } @catch (NSException *exception) { + bsg_log_err(@"Ignoring exception thrown by onFailure callback: %@", exception); + } + } +} + - (void)runWithDelegate:(id)delegate completionHandler:(nonnull void (^)(void))completionHandler { bsg_log_debug(@"Preparing event %@", self.name); + BugsnagConfiguration *configuration = delegate.configuration; + NSError *error = nil; BugsnagEvent *event = [self loadEventAndReturnError:&error]; if (!event) { bsg_log_err(@"Failed to load event %@ due to error %@", self.name, error); if (!(error.domain == NSCocoaErrorDomain && error.code == NSFileReadNoSuchFileError)) { [self deleteEvent]; + // If this happens, there will be no event, but at least we'll notify + // the client that an event wasn't sent. + [self _sendFailureForEvent:event configuration:configuration]; } completionHandler(); return; } - BugsnagConfiguration *configuration = delegate.configuration; - if (!configuration.shouldSendReports || ![event shouldBeSent]) { bsg_log_info(@"Discarding event %@ because releaseStage not in enabledReleaseStages", self.name); [self deleteEvent]; @@ -107,6 +121,7 @@ - (void)runWithDelegate:(id)delegate completion [NSString stringWithFormat:@"BSGEventUploadOperation -[runWithDelegate:completionHandler:] %@ %@", exception.name, exception.reason]]; [self deleteEvent]; + [self _sendFailureForEvent:event configuration:configuration]; completionHandler(); return; } @@ -135,6 +150,7 @@ - (void)runWithDelegate:(id)delegate completion if (!data) { bsg_log_debug(@"Encoding failed; will discard event %@", self.name); [self deleteEvent]; + [self _sendFailureForEvent:event configuration:configuration]; completionHandler(); return; } @@ -154,6 +170,7 @@ - (void)runWithDelegate:(id)delegate completion [NSString stringWithFormat:@"BSGEventUploadOperation -[runWithDelegate:completionHandler:] %@ %@", exception.name, exception.reason]]; [self deleteEvent]; + [self _sendFailureForEvent:event configuration:configuration]; completionHandler(); return; } @@ -174,6 +191,7 @@ - (void)runWithDelegate:(id)delegate completion case BSGDeliveryStatusUndeliverable: bsg_log_debug(@"Upload failed; will discard event %@", self.name); [self deleteEvent]; + [self _sendFailureForEvent:event configuration:configuration]; break; } diff --git a/Bugsnag/Helpers/BSGTelemetry.m b/Bugsnag/Helpers/BSGTelemetry.m index fbc61526a..2c0700312 100644 --- a/Bugsnag/Helpers/BSGTelemetry.m +++ b/Bugsnag/Helpers/BSGTelemetry.m @@ -116,6 +116,7 @@ static BOOL IsStaticallyLinked(void) { callbacks[@"onBreadcrumb"] = IntegerValue(configuration.onBreadcrumbBlocks.count, 0); callbacks[@"onCrashHandler"] = configuration.onCrashHandler ? @1 : nil; callbacks[@"onSendError"] = IntegerValue(configuration.onSendBlocks.count, 0); + callbacks[@"onFailure"] = IntegerValue(configuration.onFailureBlocks.count, 0); callbacks[@"onSession"] = IntegerValue(configuration.onSessionBlocks.count, 0); return @{ diff --git a/Bugsnag/include/Bugsnag/BugsnagConfiguration.h b/Bugsnag/include/Bugsnag/BugsnagConfiguration.h index 9b5bb38c5..68b8cc0d2 100644 --- a/Bugsnag/include/Bugsnag/BugsnagConfiguration.h +++ b/Bugsnag/include/Bugsnag/BugsnagConfiguration.h @@ -114,6 +114,18 @@ typedef BOOL (^BugsnagOnSendErrorBlock)(BugsnagEvent *_Nonnull event); */ typedef id BugsnagOnSendErrorRef; +/** + * A configuration block for being notified when an event fails to send. + * + * @param event The event report. NULL if the event cannot be loaded from disk. + */ +typedef void (^BugsnagOnSendFailureBlock)(BugsnagEvent *_Nullable event); + +/** + * An opaque object that identifies and allows the removal of a BugsnagOnSendFailureBlock. + */ +typedef id BugsnagOnSendFailureRef; + /** * A configuration block for modifying a captured breadcrumb * @@ -441,6 +453,29 @@ BUGSNAG_EXTERN BUGSNAG_DEPRECATED_WITH_REPLACEMENT("removeOnSession:") NS_SWIFT_NAME(removeOnSession(block:)); +// ============================================================================= +// MARK: - onSendFailure +// ============================================================================= + +/** + * Add a callback to be invoked when a report fails to be + * send to Bugsnag. + * + * @param block A block to be called on send failure. + * + * @returns An opaque reference to the callback which can be passed to `removeOnSendFailure:` + */ +- (BugsnagOnSendFailureRef)addOnSendFailureBlock:(BugsnagOnSendFailureBlock)block +NS_SWIFT_NAME(addOnSendFailure(block:)); + +/** + * Remove the callback that would be invoked on send failure. + * + * @param callback The opaque reference of the callback, returned by `addOnSendFailureBlock:` + */ +- (void)removeOnSendFailure:(BugsnagOnSendFailureRef)callback +NS_SWIFT_NAME(removeOnSendFailure(_:)); + // ============================================================================= // MARK: - onSend // ============================================================================= diff --git a/CHANGELOG.md b/CHANGELOG.md index 6a13acd3c..830257606 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ Changelog ========= +### Enhancements + +* Add callback for when an Error fails to send. + []() + ## 6.30.2 (2024-11-07) ### Bug Fixes diff --git a/Tests/BugsnagTests/BugsnagConfigurationTests.m b/Tests/BugsnagTests/BugsnagConfigurationTests.m index b21702bc9..9eca5ec34 100644 --- a/Tests/BugsnagTests/BugsnagConfigurationTests.m +++ b/Tests/BugsnagTests/BugsnagConfigurationTests.m @@ -803,25 +803,44 @@ -(void)testCrashTypeMapping { #endif /** - * Test that removeOnSendBlock() performs as expected. - * Note: We don't test that set blocks are executed since this is tested elsewhere - * (e.g. in BugsnagBreadcrumbsTest) + * Test that removeOnFailureBlock() performs as expected. */ -- (void) testRemoveOnSendBlock { +- (void) testRemoveOnFailureBlock { // Prevent sending events BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; XCTAssertEqual([[configuration onSendBlocks] count], 0); - - BugsnagOnSendErrorBlock block = ^BOOL(BugsnagEvent * _Nonnull event) { return false; }; - - BugsnagOnSendErrorRef callback = [configuration addOnSendErrorBlock:block]; + + BugsnagOnSendFailureBlock block = ^(BugsnagEvent * _Nonnull event) {}; + + BugsnagOnSendFailureRef callback = [configuration addOnSendFailureBlock:block]; BugsnagClient *client = [[BugsnagClient alloc] initWithConfiguration:configuration]; [client start]; + + XCTAssertEqual([[configuration onFailureBlocks] count], 1); + + [configuration removeOnSendFailure:callback]; + XCTAssertEqual([[configuration onFailureBlocks] count], 0); +} - XCTAssertEqual([[configuration onSendBlocks] count], 1); - - [configuration removeOnSendError:callback]; - XCTAssertEqual([[configuration onSendBlocks] count], 0); +/** + * Test that clearOnSendFailureBlock() performs as expected. + */ +- (void) testClearOnSendFailureBlock { + // Prevent sending events + BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; + XCTAssertEqual([[configuration onFailureBlocks] count], 0); + + BugsnagOnSendFailureBlock block1 = ^(BugsnagEvent * _Nonnull event) {}; + BugsnagOnSendFailureBlock block2 = ^(BugsnagEvent * _Nonnull event) {}; + + // Add more than one + [configuration addOnSendFailureBlock:block1]; + [configuration addOnSendFailureBlock:block2]; + + BugsnagClient *client = [[BugsnagClient alloc] initWithConfiguration:configuration]; + [client start]; + + XCTAssertEqual([[configuration onFailureBlocks] count], 2); } /** @@ -845,6 +864,28 @@ - (void) testClearOnSendBlock { XCTAssertEqual([[configuration onSendBlocks] count], 2); } +/** + * Test that removeOnSendBlock() performs as expected. + * Note: We don't test that set blocks are executed since this is tested elsewhere + * (e.g. in BugsnagBreadcrumbsTest) + */ +- (void) testRemoveOnSendBlock { + // Prevent sending events + BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; + XCTAssertEqual([[configuration onSendBlocks] count], 0); + + BugsnagOnSendErrorBlock block = ^BOOL(BugsnagEvent * _Nonnull event) { return false; }; + + BugsnagOnSendErrorRef callback = [configuration addOnSendErrorBlock:block]; + BugsnagClient *client = [[BugsnagClient alloc] initWithConfiguration:configuration]; + [client start]; + + XCTAssertEqual([[configuration onSendBlocks] count], 1); + + [configuration removeOnSendError:callback]; + XCTAssertEqual([[configuration onSendBlocks] count], 0); +} + - (void)testSendThreadsDefault { #if !TARGET_OS_WATCH BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; @@ -878,6 +919,12 @@ - (void)testNSCopying { NSArray *sendBlocks = @[ onSendBlock1, onSendBlock2 ]; [config setOnSendBlocks:[sendBlocks mutableCopy]]; // Mutable arg required + BugsnagOnSendFailureBlock onFailureBlock1 = ^(BugsnagEvent * _Nonnull event) {}; + BugsnagOnSendFailureBlock onFailureBlock2 = ^(BugsnagEvent * _Nonnull event) {}; + + NSArray *failureBlocks = @[ onFailureBlock1, onFailureBlock2 ]; + [config setOnFailureBlocks:[failureBlocks mutableCopy]]; // Mutable arg required + // Clone BugsnagConfiguration *clone = [config copy]; XCTAssertNotEqual(config, clone); @@ -919,6 +966,12 @@ - (void)testNSCopying { [clone setOnSendBlocks:[@[ onSendBlock2 ] mutableCopy]]; XCTAssertNotEqual(config.onSendBlocks[0], clone.onSendBlocks[0]); + // Array (of blocks) + XCTAssertEqual(config.onFailureBlocks, clone.onFailureBlocks); + XCTAssertEqual(config.onFailureBlocks[0], clone.onFailureBlocks[0]); + [clone setOnFailureBlocks:[@[ onFailureBlock2 ] mutableCopy]]; + XCTAssertNotEqual(config.onFailureBlocks[0], clone.onFailureBlocks[0]); + XCTAssertEqualObjects(clone.notifier.name, config.notifier.name); XCTAssertEqualObjects(clone.notifier.version, config.notifier.version); XCTAssertEqualObjects(clone.notifier.url, config.notifier.url); diff --git a/Tests/BugsnagTests/BugsnagSwiftConfigurationTests.swift b/Tests/BugsnagTests/BugsnagSwiftConfigurationTests.swift index fa648f29f..7c961cd6c 100644 --- a/Tests/BugsnagTests/BugsnagSwiftConfigurationTests.swift +++ b/Tests/BugsnagTests/BugsnagSwiftConfigurationTests.swift @@ -20,6 +20,29 @@ class BugsnagSwiftConfigurationTests: BSGTestCase { XCTAssertEqual(config.apiKey, DUMMY_APIKEY_16CHAR) } + func testRemoveOnFailureError() { + let config = BugsnagConfiguration(DUMMY_APIKEY_16CHAR) + let onFailureBlocks: NSMutableArray = config.value(forKey: "onFailureBlocks") as! NSMutableArray + XCTAssertEqual(onFailureBlocks.count, 0) + + let onFailureError = config.addOnSendFailure { _ in } + XCTAssertEqual(onFailureBlocks.count, 1) + + config.removeOnSendFailure(onFailureError) + XCTAssertEqual(onFailureBlocks.count, 0) + } + + func testRemoveInvalidOnFailureDoesNotCrash() { + let config = BugsnagConfiguration(DUMMY_APIKEY_16CHAR) + let onSendFailureBlock: (BugsnagEvent?) -> Void = { _ in } + config.addOnSendFailure(block: onSendFailureBlock) + + // This does not compile: + // config.removeOnSendFailure(onSendErrorBlock) + + config.removeOnSendFailure("" as NSString) + } + func testRemoveOnSendError() { let config = BugsnagConfiguration(DUMMY_APIKEY_16CHAR) let onSendBlocks: NSMutableArray = config.value(forKey: "onSendBlocks") as! NSMutableArray diff --git a/Tests/BugsnagTests/ConfigurationApiValidationTest.m b/Tests/BugsnagTests/ConfigurationApiValidationTest.m index 3cb8a1101..c46b50f52 100644 --- a/Tests/BugsnagTests/ConfigurationApiValidationTest.m +++ b/Tests/BugsnagTests/ConfigurationApiValidationTest.m @@ -225,6 +225,14 @@ - (void)testValidOnSessionBlock { XCTAssertEqual(0, [self.config.onSessionBlocks count]); } +- (void)testValidOnSendFailureBlock { + void (^block)(BugsnagEvent *) = ^(BugsnagEvent *event) {}; + BugsnagOnSendFailureRef callback = [self.config addOnSendFailureBlock:block]; + XCTAssertEqual(1, [self.config.onFailureBlocks count]); + [self.config removeOnSendFailure:callback]; + XCTAssertEqual(0, [self.config.onFailureBlocks count]); +} + - (void)testValidOnSendErrorBlock { BOOL (^block)(BugsnagEvent *) = ^BOOL(BugsnagEvent *event) { return NO;