From 40dcb5c4490cee0cd41df88adfbc211e91e2db72 Mon Sep 17 00:00:00 2001 From: fractalwrench <fractalwrench@gmail.com> Date: Mon, 20 Apr 2020 11:38:56 +0100 Subject: [PATCH] feat: add sendThreads configuration property --- CHANGELOG.md | 3 ++ Source/BugsnagConfiguration.h | 32 +++++++++++++++++++ Source/BugsnagConfiguration.m | 2 ++ Source/BugsnagCrashSentry.m | 14 ++++---- .../Source/KSCrash/Recording/BSG_KSCrash.h | 3 +- .../Source/KSCrash/Recording/BSG_KSCrash.m | 4 +-- .../Source/KSCrash/Recording/BSG_KSCrashC.c | 2 +- .../Source/KSCrash/Recording/BSG_KSCrashC.h | 2 +- .../KSCrash/Recording/BSG_KSCrashReport.c | 32 +++++++++++-------- .../Recording/Sentry/BSG_KSCrashSentry.c | 2 +- .../Recording/Sentry/BSG_KSCrashSentry.h | 2 +- Tests/BugsnagConfigurationTests.m | 11 ++++++- UPGRADING.md | 1 + .../Bugsnag Test App/AppDelegate.m | 3 +- 14 files changed, 84 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79e650c91..b5931fc4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ Bugsnag Notifiers on other platforms. ## Enhancements +* Add `sendThreads` property to `BugsnagConfiguration` + [#549](https://github.com/bugsnag/bugsnag-cocoa/pull/549) + * Add structured app/device fields to `BugsnagSession` [#546](https://github.com/bugsnag/bugsnag-cocoa/pull/546) diff --git a/Source/BugsnagConfiguration.h b/Source/BugsnagConfiguration.h index f7940d164..67ffe86f6 100644 --- a/Source/BugsnagConfiguration.h +++ b/Source/BugsnagConfiguration.h @@ -36,6 +36,28 @@ @class BugsnagUser; @class BugsnagEndpointConfiguration; +/** + * Controls whether Bugsnag should capture and serialize the state of all threads at the time + * of an error. + */ +typedef NS_ENUM(NSInteger, BSGThreadSendPolicy) { + + /** + * Threads should be captured for all events. + */ + BSGThreadSendPolicyAlways = 0, + + /** + * Threads should be captured for unhandled events only. + */ + BSGThreadSendPolicyUnhandledOnly = 1, + + /** + * Threads should never be captured. + */ + BSGThreadSendPolicyNever = 2 +}; + /** * BugsnagConfiguration error constants */ @@ -134,6 +156,16 @@ typedef NS_OPTIONS(NSUInteger, BSGErrorType) { */ @property(readwrite, strong, nonnull) NSURLSession *session; +/** + * Controls whether Bugsnag should capture and serialize the state of all threads at the time + * of an error. + * + * By default sendThreads is set to BSGThreadSendPolicyAlways. This can be set to + * BSGThreadSendPolicyNever to disable or BSGThreadSendPolicyUnhandledOnly + * to only do so for unhandled errors. + */ +@property BSGThreadSendPolicy sendThreads; + /** * Optional handler invoked when an error or crash occurs */ diff --git a/Source/BugsnagConfiguration.m b/Source/BugsnagConfiguration.m index dad40b6cc..6e3bad21d 100644 --- a/Source/BugsnagConfiguration.m +++ b/Source/BugsnagConfiguration.m @@ -134,6 +134,7 @@ - (nonnull id)copyWithZone:(nullable NSZone *)zone { [copy setPlugins:[self.plugins copy]]; [copy setReleaseStage:self.releaseStage]; [copy setSession:[self.session copy]]; + [copy setSendThreads:self.sendThreads]; [copy setUser:self.user.userId withEmail:self.user.emailAddress andName:self.user.name]; @@ -196,6 +197,7 @@ - (instancetype _Nonnull)initWithApiKey:(NSString *_Nonnull)apiKey _redactedKeys = @[@"password"]; _breadcrumbs = [BugsnagBreadcrumbs new]; _autoTrackSessions = YES; + _sendThreads = BSGThreadSendPolicyAlways; // Default to recording all error types _enabledErrorTypes = BSGErrorTypesCPP | BSGErrorTypesMach diff --git a/Source/BugsnagCrashSentry.m b/Source/BugsnagCrashSentry.m index fc2f9f435..b1c0073f8 100644 --- a/Source/BugsnagCrashSentry.m +++ b/Source/BugsnagCrashSentry.m @@ -24,12 +24,14 @@ - (void)install:(BugsnagConfiguration *)config onCrash:(BSGReportCallback)onCrash { BugsnagSink *sink = [[BugsnagSink alloc] initWithApiClient:apiClient]; - [BSG_KSCrash sharedInstance].sink = sink; - [BSG_KSCrash sharedInstance].introspectMemory = YES; - [BSG_KSCrash sharedInstance].deleteBehaviorAfterSendAll = + BSG_KSCrash *ksCrash = [BSG_KSCrash sharedInstance]; + ksCrash.sink = sink; + ksCrash.introspectMemory = YES; + ksCrash.deleteBehaviorAfterSendAll = BSG_KSCDeleteOnSuccess; - [BSG_KSCrash sharedInstance].onCrash = onCrash; - [BSG_KSCrash sharedInstance].maxStoredReports = BSG_MAX_STORED_REPORTS; + ksCrash.onCrash = onCrash; + ksCrash.maxStoredReports = BSG_MAX_STORED_REPORTS; + ksCrash.threadTracingEnabled = (int) config.sendThreads; // User reported events are *always* handled BSG_KSCrashType crashTypes = BSG_KSCrashTypeUserReported; @@ -44,7 +46,7 @@ - (void)install:(BugsnagConfiguration *)config bsg_kscrash_setHandlingCrashTypes(crashTypes); - if (![[BSG_KSCrash sharedInstance] install]) + if (![ksCrash install]) bsg_log_err(@"Failed to install crash handler. No exceptions will be reported!"); [sink.apiClient flushPendingData]; diff --git a/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.h b/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.h index e338a5d01..026ba9b5d 100644 --- a/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.h +++ b/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.h @@ -30,6 +30,7 @@ #import "BSG_KSCrashReportFilterCompletion.h" #import "BSG_KSCrashReportWriter.h" #import "BSG_KSCrashType.h" +#import "BugsnagConfiguration.h" typedef enum { BSG_KSCDeleteNever, @@ -155,7 +156,7 @@ typedef enum { /** * If YES, thread traces will be collected with each report. */ -@property(nonatomic, readwrite, assign) BOOL threadTracingEnabled; +@property(nonatomic, readwrite, assign) int threadTracingEnabled; /** * If YES, binary images will be collected for each report. diff --git a/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.m b/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.m index 886d74a1f..a59c8eefb 100644 --- a/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.m +++ b/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.m @@ -134,7 +134,7 @@ - (id)initWithReportFilesDirectory:(NSString *)reportFilesDirectory { self.suspendThreadsForUserReported = YES; self.reportWhenDebuggerIsAttached = NO; - self.threadTracingEnabled = YES; + self.threadTracingEnabled = BSGThreadSendPolicyAlways; self.writeBinaryImagesForUserReported = YES; } return self; @@ -191,7 +191,7 @@ - (void)setReportWhenDebuggerIsAttached:(BOOL)reportWhenDebuggerIsAttached { bsg_kscrash_setReportWhenDebuggerIsAttached(reportWhenDebuggerIsAttached); } -- (void)setThreadTracingEnabled:(BOOL)threadTracingEnabled { +- (void)setThreadTracingEnabled:(int)threadTracingEnabled { _threadTracingEnabled = threadTracingEnabled; bsg_kscrash_setThreadTracingEnabled(threadTracingEnabled); } diff --git a/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.c b/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.c index f26e5ae98..e6f1a0249 100644 --- a/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.c +++ b/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.c @@ -218,6 +218,6 @@ void bsg_kscrash_setReportWhenDebuggerIsAttached( reportWhenDebuggerIsAttached; } -void bsg_kscrash_setThreadTracingEnabled(bool threadTracingEnabled) { +void bsg_kscrash_setThreadTracingEnabled(int threadTracingEnabled) { crashContext()->crash.threadTracingEnabled = threadTracingEnabled; } diff --git a/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.h b/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.h index 3c6ab1a8b..cafff3c4a 100644 --- a/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.h +++ b/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.h @@ -171,7 +171,7 @@ void bsg_kscrash_setSuspendThreadsForUserReported( void bsg_kscrash_setReportWhenDebuggerIsAttached( bool reportWhenDebuggerIsAttached); -void bsg_kscrash_setThreadTracingEnabled(bool threadTracingEnabled); +void bsg_kscrash_setThreadTracingEnabled(int threadTracingEnabled); void bsg_kscrash_setWriteBinaryImagesForUserReported( bool writeBinaryImagesForUserReported); diff --git a/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReport.c b/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReport.c index 46af061dd..df3026466 100644 --- a/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReport.c +++ b/Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReport.c @@ -959,8 +959,13 @@ void bsg_kscrw_i_writeThread(const BSG_KSCrashReportWriter *const writer, const char *const key, const BSG_KSCrash_SentryContext *const crash, const thread_t thread, const int index, - const bool writeNotableAddresses) { + const bool writeNotableAddresses, + const bool recordAllThreads) { bool isCrashedThread = thread == crash->offendingThread; + if (!isCrashedThread && !recordAllThreads) { + return; + } + BSG_STRUCT_MCONTEXT_L machineContextBuffer; uintptr_t backtraceBuffer[BSG_kMaxBacktraceDepth]; int backtraceLength = sizeof(backtraceBuffer) / sizeof(*backtraceBuffer); @@ -1012,7 +1017,8 @@ void bsg_kscrw_i_writeThread(const BSG_KSCrashReportWriter *const writer, void bsg_kscrw_i_writeAllThreads(const BSG_KSCrashReportWriter *const writer, const char *const key, const BSG_KSCrash_SentryContext *const crash, - bool writeNotableAddresses) { + bool writeNotableAddresses, + const bool recordAllThreads) { const task_t thisTask = mach_task_self(); thread_act_array_t threads; mach_msg_type_number_t numThreads; @@ -1028,7 +1034,7 @@ void bsg_kscrw_i_writeAllThreads(const BSG_KSCrashReportWriter *const writer, { for (mach_msg_type_number_t i = 0; i < numThreads; i++) { bsg_kscrw_i_writeThread(writer, NULL, crash, threads[i], (int)i, - writeNotableAddresses); + writeNotableAddresses, recordAllThreads); } } writer->endContainer(writer); @@ -1539,7 +1545,7 @@ void bsg_kscrashreport_writeMinimalReport( writer, BSG_KSCrashField_CrashedThread, &crashContext->crash, crashContext->crash.offendingThread, bsg_kscrw_i_threadIndex(crashContext->crash.offendingThread), - false); + false, false); bsg_kscrw_i_writeError(writer, BSG_KSCrashField_Error, &crashContext->crash); } @@ -1610,16 +1616,14 @@ void bsg_kscrashreport_writeStandardReport( writer->beginObject(writer, BSG_KSCrashField_Crash); { - // Don't write the threads for user reported crashes to improve - // performance - if (crashContext->crash.threadTracingEnabled == true || - crashContext->crash.crashType != BSG_KSCrashTypeUserReported) { - bsg_kscrw_i_writeAllThreads( - writer, BSG_KSCrashField_Threads, &crashContext->crash, - crashContext->config.introspectionRules.enabled); - } - bsg_kscrw_i_writeError(writer, BSG_KSCrashField_Error, - &crashContext->crash); + // Conditionally write threads depending on user configuration + int sendPolicy = crashContext->crash.threadTracingEnabled; + bool unhandledCrash = crashContext->crash.crashType != BSG_KSCrashTypeUserReported; + bool recordAllThreads = sendPolicy == 0 || (unhandledCrash && sendPolicy == 1); + + bsg_kscrw_i_writeAllThreads(writer, BSG_KSCrashField_Threads, &crashContext->crash, + crashContext->config.introspectionRules.enabled, recordAllThreads); + bsg_kscrw_i_writeError(writer, BSG_KSCrashField_Error,&crashContext->crash); } writer->endContainer(writer); diff --git a/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry.c b/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry.c index 63deb9305..f183763c3 100644 --- a/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry.c +++ b/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry.c @@ -201,7 +201,7 @@ void bsg_kscrashsentry_resumeThreads(void) { void bsg_kscrashsentry_clearContext(BSG_KSCrash_SentryContext *context) { void (*onCrash)(void *) = context->onCrash; - bool threadTracingEnabled = context->threadTracingEnabled; + int threadTracingEnabled = context->threadTracingEnabled; bool reportWhenDebuggerIsAttached = context->reportWhenDebuggerIsAttached; bool suspendThreadsForUserReported = context->suspendThreadsForUserReported; bool writeBinaryImagesForUserReported = diff --git a/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry.h b/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry.h index add11c416..b0a9103c7 100644 --- a/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry.h +++ b/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry.h @@ -60,7 +60,7 @@ typedef struct BSG_KSCrash_SentryContext { bool reportWhenDebuggerIsAttached; /** If true, will trace threads and report binary images. */ - bool threadTracingEnabled; + int threadTracingEnabled; /** If true, will record binary images. */ bool writeBinaryImagesForUserReported; diff --git a/Tests/BugsnagConfigurationTests.m b/Tests/BugsnagConfigurationTests.m index c6ef0f4bb..dc2d21f47 100644 --- a/Tests/BugsnagConfigurationTests.m +++ b/Tests/BugsnagConfigurationTests.m @@ -786,6 +786,11 @@ - (void) testClearOnSendBlock { XCTAssertEqual([[configuration onSendBlocks] count], 2); } +- (void)testSendThreadsDefault { + BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; + XCTAssertEqual(BSGThreadSendPolicyAlways, config.sendThreads); +} + - (void)testNSCopying { BugsnagConfiguration *config = [[BugsnagConfiguration alloc] initWithApiKey:DUMMY_APIKEY_32CHAR_1]; @@ -796,6 +801,7 @@ - (void)testNSCopying { [config setContext:@"context1"]; [config setAppType:@"The most amazing app, a brilliant app, the app to end all apps"]; [config setPersistUser:YES]; + [config setSendThreads:BSGThreadSendPolicyUnhandledOnly]; BugsnagOnSendBlock onSendBlock1 = ^BOOL(BugsnagEvent * _Nonnull event) { return true; }; BugsnagOnSendBlock onSendBlock2 = ^BOOL(BugsnagEvent * _Nonnull event) { return true; }; @@ -810,7 +816,10 @@ - (void)testNSCopying { // Redacted keys XCTAssertEqualObjects(config.redactedKeys, clone.redactedKeys); - + + // sendThreads + XCTAssertEqual(config.sendThreads, clone.sendThreads); + // Object [clone setUser:@"Cthulu" withEmail:@"hp@lovecraft.com" andName:@"Howard"]; XCTAssertEqualObjects(config.user.userId, @"foo"); diff --git a/UPGRADING.md b/UPGRADING.md index d0bec21e5..11a165333 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -38,6 +38,7 @@ The exact error is available using the `BSGConfigurationErrorDomain` and + config.removeOnBreadcrumb(block:) + config.redactedKeys ++ config.sendThreads ``` #### Renames diff --git a/examples/objective-c-ios/Bugsnag Test App/AppDelegate.m b/examples/objective-c-ios/Bugsnag Test App/AppDelegate.m index d8d2803ed..a1fffffa7 100644 --- a/examples/objective-c-ios/Bugsnag Test App/AppDelegate.m +++ b/examples/objective-c-ios/Bugsnag Test App/AppDelegate.m @@ -13,8 +13,9 @@ @implementation AppDelegate - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions { - NSString *apiKey = @"<YOUR_APIKEY_HERE>"; + NSString *apiKey = @"5d1ec8bd39a74caa1267142706a7fb20"; BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:apiKey]; + configuration.sendThreads = BSGThreadSendPolicyAlways; [Bugsnag startBugsnagWithConfiguration:configuration]; return YES;