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

[PLAT-4646] Implement discardClasses configuration option #938

Merged
merged 3 commits into from
Dec 15, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Bugsnag/BugsnagErrorReportSink.m
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,13 @@ - (void)sendStoredReports:(NSDictionary <NSString *, NSDictionary *> *)ksCrashRe
NSDictionary *report = ksCrashReports[fileKey];
BugsnagEvent *event = [[BugsnagEvent alloc] initWithKSReport:report];
event.redactedKeys = configuration.redactedKeys;

NSString *errorClass = event.errors.firstObject.errorClass;
if ([configuration shouldDiscardErrorClass:errorClass]) {
bsg_log_info(@"Discarding event because errorClass \"%@\" matched configuration.discardClasses", errorClass);
[self finishActiveRequest:fileKey completed:YES error:nil block:block];
continue;
}

if ([event shouldBeSent] && [self runOnSendBlocks:configuration event:event]) {
storedEvents[fileKey] = event;
Expand Down
6 changes: 6 additions & 0 deletions Bugsnag/Client/BugsnagClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -964,6 +964,12 @@ - (void)notify:(NSException *)exception
- (void)notifyInternal:(BugsnagEvent *_Nonnull)event
block:(BugsnagOnErrorBlock)block
{
NSString *errorClass = event.errors.firstObject.errorClass;
if ([self.configuration shouldDiscardErrorClass:errorClass]) {
bsg_log_info(@"Discarding event because errorClass \"%@\" matched configuration.discardClasses", errorClass);
return;
}

// enhance device information with additional metadata
NSDictionary *deviceFields = [self.state getMetadataFromSection:BSGKeyDeviceState];

Expand Down
2 changes: 2 additions & 0 deletions Bugsnag/Configuration/BugsnagConfiguration+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ NS_ASSUME_NONNULL_BEGIN

- (void)deletePersistedUserData;

- (BOOL)shouldDiscardErrorClass:(NSString *)errorClass;

- (BOOL)shouldRecordBreadcrumbType:(BSGBreadcrumbType)breadcrumbType;

/// Throws an NSInvalidArgumentException if the API key is empty or missing.
Expand Down
16 changes: 16 additions & 0 deletions Bugsnag/Configuration/BugsnagConfiguration.m
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ - (nonnull id)copyWithZone:(nullable NSZone *)zone {
[copy setEnabledBreadcrumbTypes:self.enabledBreadcrumbTypes];
[copy setEnabledErrorTypes:self.enabledErrorTypes];
[copy setEnabledReleaseStages:self.enabledReleaseStages];
copy.discardClasses = self.discardClasses;
[copy setRedactedKeys:self.redactedKeys];
[copy setMaxBreadcrumbs:self.maxBreadcrumbs];
copy->_metadata = [[BugsnagMetadata alloc] initWithDictionary:[[self.metadata toDictionary] mutableCopy]];
Expand Down Expand Up @@ -435,6 +436,21 @@ - (void)setMaxBreadcrumbs:(NSUInteger)maxBreadcrumbs {
}
}

- (BOOL)shouldDiscardErrorClass:(NSString *)errorClass {
for (id obj in self.discardClasses) {
if ([obj isKindOfClass:[NSString class]]) {
if ([obj isEqualToString:errorClass]) {
return YES;
}
} else if ([obj isKindOfClass:[NSRegularExpression class]]) {
if ([obj firstMatchInString:errorClass options:0 range:NSMakeRange(0, errorClass.length)]) {
return YES;
}
}
}
return NO;
}

/**
* Specific types of breadcrumb should be recorded if either enabledBreadcrumbTypes
* is None, or contains the type.
Expand Down
2 changes: 1 addition & 1 deletion Bugsnag/Payload/BugsnagEvent+Private.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ NS_ASSUME_NONNULL_BEGIN
/// Property overrides.
@property (readonly, copy) NSDictionary *overrides;

@property NSSet<NSString *> *redactedKeys;
@property NSSet<id> *redactedKeys;

/// The release stage of the application
@property (readwrite, copy, nullable) NSString *releaseStage;
Expand Down
14 changes: 14 additions & 0 deletions Bugsnag/include/Bugsnag/BugsnagConfiguration.h
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,20 @@ typedef BOOL (^BugsnagOnSessionBlock)(BugsnagSession *_Nonnull session);
*/
@property(readwrite, retain, nullable) NSSet<id> *redactedKeys;

/**
* A set of strings and / or NSRegularExpression objects that determine which errors should
* be discarded based on their `errorClass`.
*
* Comparisons are case sensitive.
*
* OnError / OnSendError blocks will not be called for discarded errors.
*
* Some examples of errorClass are: Objective-C exception names like "NSRangeException",
* signal names like "SIGABRT", mach exception names like "EXC_BREAKPOINT", and Swift
* error names like "Fatal error".
*/
@property(readwrite, copy, nullable) NSSet<id> *discardClasses;
nickdowell marked this conversation as resolved.
Show resolved Hide resolved

/**
* A general summary of what was occuring in the application
*/
Expand Down
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
Changelog
=========

## TBD

### Enhancements

* Errors may now be discarded based on their `errorClass` using the new `discardClasses` configuration option.
[#938](https://github.com/bugsnag/bugsnag-cocoa/pull/938)

## 6.4.0 (2020-12-08)

### Enhancements
Expand Down
70 changes: 70 additions & 0 deletions Tests/BugsnagConfigurationTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -850,4 +850,74 @@ - (void)testMetadataMutability {
XCTAssertTrue([metadata2 isKindOfClass:[NSMutableDictionary class]]);
}

- (void)testDiscardClasses {
XCTAssertNil([[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1].discardClasses, @"discardClasses should be nil be default");

NSArray<NSString *> *errorClasses = @[@"EXC_BAD_ACCESS",
@"EXC_BAD_INSTRUCTION",
@"EXC_BREAKPOINT",
@"Exception",
@"Fatal error",
@"NSError",
@"NSGenericException",
@"NSInternalInconsistencyException",
@"NSMallocException",
@"NSRangeException",
@"SIGABRT",
@"UIViewControllerHierarchyInconsistency",
@"std::__1::system_error"];

__block NSArray *discarded, *kept;

void (^ applyDiscardClasses)(NSSet *) = ^(NSSet *discardClasses){
BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1];
configuration.discardClasses = discardClasses;
NSPredicate *shouldDiscard = [NSPredicate predicateWithBlock:^BOOL(id evaluatedObject, NSDictionary *bindings) {
return [configuration shouldDiscardErrorClass:evaluatedObject];
}];
discarded = [errorClasses filteredArrayUsingPredicate:shouldDiscard];
kept = [errorClasses filteredArrayUsingPredicate:[NSCompoundPredicate notPredicateWithSubpredicate:shouldDiscard]];
};

applyDiscardClasses(nil);
XCTAssertEqualObjects(discarded, @[]);
XCTAssertEqualObjects(kept, errorClasses);

applyDiscardClasses([NSSet setWithObjects:@"nserror", nil]);
XCTAssertEqualObjects(discarded, @[]);
XCTAssertEqualObjects(kept, errorClasses);

applyDiscardClasses([NSSet setWithObjects:@"EXC_BAD_ACCESS", @"NSError", nil]);
XCTAssertEqualObjects(discarded, (@[@"EXC_BAD_ACCESS", @"NSError"]));
XCTAssertEqualObjects(kept, (@[@"EXC_BAD_INSTRUCTION",
@"EXC_BREAKPOINT",
@"Exception",
@"Fatal error",
@"NSGenericException",
@"NSInternalInconsistencyException",
@"NSMallocException",
@"NSRangeException",
@"SIGABRT",
@"UIViewControllerHierarchyInconsistency",
@"std::__1::system_error"]));

applyDiscardClasses([NSSet setWithObjects:@"Exception", @"NSError",
[NSRegularExpression regularExpressionWithPattern:@"std::__1::.*" options:0 error:nil], nil]);
XCTAssertEqualObjects(discarded, (@[@"Exception", @"NSError", @"std::__1::system_error"]));
XCTAssertEqualObjects(kept, (@[@"EXC_BAD_ACCESS",
@"EXC_BAD_INSTRUCTION",
@"EXC_BREAKPOINT",
@"Fatal error",
@"NSGenericException",
@"NSInternalInconsistencyException",
@"NSMallocException",
@"NSRangeException",
@"SIGABRT",
@"UIViewControllerHierarchyInconsistency"]));

applyDiscardClasses([NSSet setWithObjects:[NSRegularExpression regularExpressionWithPattern:@".*" options:0 error:nil], nil]);
XCTAssertEqualObjects(discarded, errorClasses);
XCTAssertEqualObjects(kept, (@[]));
}

@end
21 changes: 21 additions & 0 deletions features/discard_classes.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
Feature: Configuration discardClasses option

Background:
Given I clear all persistent data

Scenario: Discard handled exception via regular expression
When I run "DiscardClassesHandledExceptionRegexScenario"
And I wait to receive a request
And the exception "errorClass" equals "NotDiscarded"

Scenario: Discard unhandled exception
When I run "DiscardClassesUnhandledExceptionScenario" and relaunch the app
And I configure Bugsnag for "DiscardClassesUnhandledExceptionScenario"
And I wait to receive a request
And the exception "errorClass" equals "NotDiscarded"

Scenario: Discard unhandled crash
When I run "DiscardClassesUnhandledCrashScenario" and relaunch the app
And I configure Bugsnag for "DiscardClassesUnhandledCrashScenario"
And I wait to receive a request
And the exception "errorClass" equals "NotDiscarded"
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
00432CC4240912A100826D05 /* EnabledErrorTypesCxxScenario.mm in Sources */ = {isa = PBXBuildFile; fileRef = 00432CC2240912A000826D05 /* EnabledErrorTypesCxxScenario.mm */; };
00507A64242BFE5600EF1B87 /* EnabledBreadcrumbTypesIsNilScenario.swift in Sources */ = {isa = PBXBuildFile; fileRef = 00507A63242BFE5600EF1B87 /* EnabledBreadcrumbTypesIsNilScenario.swift */; };
00CEB60D24080C690004793D /* EnabledErrorTypesScenario.m in Sources */ = {isa = PBXBuildFile; fileRef = 00CEB60C24080C690004793D /* EnabledErrorTypesScenario.m */; };
0163BFA72583B3CF008DC28B /* DiscardClassesScenarios.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0163BFA62583B3CF008DC28B /* DiscardClassesScenarios.swift */; };
6526A0D4248A83350002E2C9 /* LoadConfigFromFileAutoScenario.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6526A0D3248A83350002E2C9 /* LoadConfigFromFileAutoScenario.swift */; };
8A14F0F62282D4AE00337B05 /* (null) in Sources */ = {isa = PBXBuildFile; };
8A32DB8222424E3000EDD92F /* NSExceptionShiftScenario.m in Sources */ = {isa = PBXBuildFile; fileRef = 8A32DB8122424E3000EDD92F /* NSExceptionShiftScenario.m */; };
Expand Down Expand Up @@ -159,6 +160,7 @@
00A98315240DBB7A0016A57E /* out_of_memory.feature */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; name = out_of_memory.feature; path = ../../../out_of_memory.feature; sourceTree = "<group>"; };
00CEB60B24080C690004793D /* EnabledErrorTypesScenario.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = EnabledErrorTypesScenario.h; sourceTree = "<group>"; };
00CEB60C24080C690004793D /* EnabledErrorTypesScenario.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = EnabledErrorTypesScenario.m; sourceTree = "<group>"; };
0163BFA62583B3CF008DC28B /* DiscardClassesScenarios.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = DiscardClassesScenarios.swift; sourceTree = "<group>"; };
4994F05E0421A0B037DD2CC5 /* Pods_iOSTestApp.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_iOSTestApp.framework; sourceTree = BUILT_PRODUCTS_DIR; };
6526A0D3248A83350002E2C9 /* LoadConfigFromFileAutoScenario.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = LoadConfigFromFileAutoScenario.swift; sourceTree = "<group>"; };
8A32DB8022424E3000EDD92F /* NSExceptionShiftScenario.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = NSExceptionShiftScenario.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -549,6 +551,7 @@
F49695A3243EF7B600105DA9 /* OOMs */,
F49695AE2445476700105DA9 /* Plugin */,
0037410E2473CF2300BE41AA /* AppAndDeviceAttributesScenario.swift */,
0163BFA62583B3CF008DC28B /* DiscardClassesScenarios.swift */,
8AB1081823301FE600672818 /* ReleaseStageScenarios.swift */,
F4295ABA693D273D52AA9F6B /* Scenario.h */,
F42954E8B66F3FB7F5333CF7 /* Scenario.m */,
Expand Down Expand Up @@ -877,6 +880,7 @@
8AB65FCC22DC77CB001200AB /* LoadConfigFromFileScenario.swift in Sources */,
E7B79CD4247FD6760039FB88 /* ManualContextOnErrorScenario.swift in Sources */,
E7767F13221C21E30006648C /* ResumedSessionScenario.swift in Sources */,
0163BFA72583B3CF008DC28B /* DiscardClassesScenarios.swift in Sources */,
E700EE69247D73F8008CFFB6 /* UnhandledMachExceptionScenario.m in Sources */,
E75040B12478214F005D33BD /* MetadataRedactionRegexScenario.swift in Sources */,
E700EE5B247D3224008CFFB6 /* OriginalErrorNSExceptionScenario.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
/* Begin PBXBuildFile section */
01452354254AFD7C00D436AA /* Bugsnag.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 01452353254AFD7C00D436AA /* Bugsnag.framework */; };
01452355254AFD7C00D436AA /* Bugsnag.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 01452353254AFD7C00D436AA /* Bugsnag.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
0163BF9B2583AF2A008DC28B /* DiscardClassesScenarios.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0163BF9A2583AF2A008DC28B /* DiscardClassesScenarios.swift */; };
0176C0AE254AE81B0066E0F3 /* main.m in Sources */ = {isa = PBXBuildFile; fileRef = 0176C0AD254AE81B0066E0F3 /* main.m */; };
0176C0B1254AE81B0066E0F3 /* AppDelegate.m in Sources */ = {isa = PBXBuildFile; fileRef = 0176C0B0254AE81B0066E0F3 /* AppDelegate.m */; };
0176C0B3254AE81B0066E0F3 /* Images.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 0176C0B2254AE81B0066E0F3 /* Images.xcassets */; };
Expand Down Expand Up @@ -146,6 +147,7 @@
/* Begin PBXFileReference section */
01452353254AFD7C00D436AA /* Bugsnag.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; path = Bugsnag.framework; sourceTree = BUILT_PRODUCTS_DIR; };
01452360254AFEA700D436AA /* macOSTestApp-Bridging-Header.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "macOSTestApp-Bridging-Header.h"; sourceTree = "<group>"; };
0163BF9A2583AF2A008DC28B /* DiscardClassesScenarios.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DiscardClassesScenarios.swift; sourceTree = "<group>"; };
0176C0A8254AE81B0066E0F3 /* macOSTestApp.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = macOSTestApp.app; sourceTree = BUILT_PRODUCTS_DIR; };
0176C0AC254AE81B0066E0F3 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; usesTabs = 1; };
0176C0AD254AE81B0066E0F3 /* main.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = main.m; sourceTree = "<group>"; };
Expand Down Expand Up @@ -380,6 +382,7 @@
01F47C8F254B1B2F00B184AD /* CxxExceptionScenario.mm */,
01F47C43254B1B2D00B184AD /* DisabledSessionTrackingScenario.h */,
01F47C88254B1B2F00B184AD /* DisabledSessionTrackingScenario.m */,
0163BF9A2583AF2A008DC28B /* DiscardClassesScenarios.swift */,
01F47CA8254B1B3000B184AD /* DiscardedBreadcrumbTypeScenario.swift */,
01F47CB6254B1B3000B184AD /* DiscardSessionScenario.swift */,
01F47C3D254B1B2D00B184AD /* EnabledBreadcrumbTypesIsNilScenario.swift */,
Expand All @@ -402,6 +405,8 @@
01F47C56254B1B2E00B184AD /* ManualSessionWithUserScenario.m */,
01F47C33254B1B2D00B184AD /* ManyConcurrentNotifyScenario.h */,
01F47C6E254B1B2E00B184AD /* ManyConcurrentNotifyScenario.m */,
CBB7878D2578FB3F0071BDE4 /* MarkUnhandledHandledScenario.h */,
CBB7878C2578FB3F0071BDE4 /* MarkUnhandledHandledScenario.m */,
01F47CBD254B1B3000B184AD /* MetadataMergeScenario.swift */,
01F47C29254B1B2C00B184AD /* MetadataRedactionDefaultScenario.swift */,
01F47C80254B1B2F00B184AD /* MetadataRedactionNestedScenario.swift */,
Expand Down Expand Up @@ -489,8 +494,6 @@
01F47C90254B1B2F00B184AD /* UnhandledInternalNotifyScenario.swift */,
01F47C38254B1B2D00B184AD /* UnhandledMachExceptionScenario.h */,
01F47C4C254B1B2D00B184AD /* UnhandledMachExceptionScenario.m */,
CBB7878D2578FB3F0071BDE4 /* MarkUnhandledHandledScenario.h */,
CBB7878C2578FB3F0071BDE4 /* MarkUnhandledHandledScenario.m */,
01F47CB7254B1B3000B184AD /* UserDefaultInfoScenario.swift */,
01F47CB3254B1B3000B184AD /* UserDisabledScenario.swift */,
01F47C75254B1B2E00B184AD /* UserEmailScenario.swift */,
Expand Down Expand Up @@ -686,6 +689,7 @@
017FBFB8254B09C300809042 /* MainWindowController.m in Sources */,
01F47CF0254B1B3100B184AD /* OnSendOverwriteScenario.swift in Sources */,
01F47D1B254B1B3100B184AD /* SIGSEGVScenario.m in Sources */,
0163BF9B2583AF2A008DC28B /* DiscardClassesScenarios.swift in Sources */,
01F47D09254B1B3100B184AD /* BreadcrumbCallbackOverrideScenario.swift in Sources */,
0176C0B1254AE81B0066E0F3 /* AppDelegate.m in Sources */,
01F47CD7254B1B3100B184AD /* AutoDetectFalseHandledScenario.swift in Sources */,
Expand Down
77 changes: 77 additions & 0 deletions features/fixtures/shared/scenarios/DiscardClassesScenarios.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
//
// DiscardClassesScenarios.swift
// macOSTestApp
//
// Created by Nick Dowell on 11/12/2020.
// Copyright © 2020 Bugsnag Inc. All rights reserved.
//

extension NSExceptionName {

/// An exception name that should not be discarded by the discardClasses values in these scenarios.
static let notDiscarded = NSExceptionName("NotDiscarded")
}

// MARK: -

class DiscardClassesHandledExceptionRegexScenario: Scenario {

override func startBugsnag() {
config.autoTrackSessions = false
config.discardClasses = try! [NSRegularExpression(pattern: #"NS\w+Exception"#)]
super.startBugsnag()
}

override func run() {
Bugsnag.notify(NSException(name: .genericException, reason: "This exception should be discarded")) { _ in
fatalError("OnError should not be called for discarded errors")
}
Bugsnag.notify(NSException(name: .notDiscarded, reason: "This exception should not be discarded"))
}
}

// MARK: -

class DiscardClassesUnhandledExceptionScenario: Scenario {

override func startBugsnag() {
config.autoTrackSessions = false
config.discardClasses = [NSExceptionName.rangeException.rawValue]
config.addOnSendError {
precondition(!$0.unhandled, "OnSendError should not be called for discarded errors (NSRangeException)")
return true
}
super.startBugsnag()

if Bugsnag.appDidCrashLastLaunch() {
Bugsnag.notify(NSException(name: .notDiscarded, reason: "This exception should not be discarded"))
}
}

override func run() {
NSArray().object(at: 0)
}
}

// MARK: -

class DiscardClassesUnhandledCrashScenario: Scenario {

override func startBugsnag() {
config.autoTrackSessions = false
config.discardClasses = ["EXC_BREAKPOINT"]
config.addOnSendError {
precondition(!$0.unhandled, "OnSendError should not be called for discarded errors (EXC_BREAKPOINT)")
return true
}
super.startBugsnag()

if Bugsnag.appDidCrashLastLaunch() {
Bugsnag.notify(NSException(name: .notDiscarded, reason: "This exception should not be discarded"))
}
}

override func run() {
triggerExcBreakpoint()
}
}
3 changes: 3 additions & 0 deletions features/fixtures/shared/scenarios/Scenario.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,8 @@ void markErrorHandledCallback(const BSG_KSCrashReportWriter * _Nonnull writer);

- (void)didEnterBackgroundNotification;

- (void)triggerExcBreakpoint;

@property (nonatomic, strong, nullable) NSString *eventMode;

@end
4 changes: 4 additions & 0 deletions features/fixtures/shared/scenarios/Scenario.m
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,8 @@ - (void)startBugsnag {
- (void)didEnterBackgroundNotification {
}

- (void)triggerExcBreakpoint {
__builtin_trap();
}

@end