Merge pull request #549 from bugsnag/v6-send-threads

Add sendThreads property to `BugsnagConfiguration`

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)
 

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
  */
@@ -135,6 +157,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
  */

Source/BugsnagConfiguration.m

@@ -135,6 +135,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];
@@ -198,6 +199,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

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,8 +46,9 @@ - (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];
 }

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.

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);
 }

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;
 }

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);

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);
 

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 =

Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry.h

@@ -59,8 +59,11 @@ typedef struct BSG_KSCrash_SentryContext {
     /** If true, will send reports even if debugger is attached. */
     bool reportWhenDebuggerIsAttached;
 
-    /** If true, will trace threads and report binary images. */
-    bool threadTracingEnabled;
+    /**
+     * The methodology used for tracing threads and report binary images.
+     * The value will be equal to an enum value from BSGThreadSendPolicy
+     */
+    int threadTracingEnabled;
 
     /** If true, will record binary images. */
     bool writeBinaryImagesForUserReported;

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:@"[email protected]" andName:@"Howard"];
     XCTAssertEqualObjects(config.user.userId, @"foo");

UPGRADING.md

@@ -38,6 +38,7 @@ The exact error is available using the `BSGConfigurationErrorDomain` and
 + config.removeOnBreadcrumb(block:)
 
 + config.redactedKeys
++ config.sendThreads
 ```
 
 #### Renames

examples/objective-c-ios/Bugsnag Test App/AppDelegate.m

@@ -15,6 +15,7 @@ - (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(
 
     NSString *apiKey = @"<YOUR_APIKEY_HERE>";
     BugsnagConfiguration *configuration = [[BugsnagConfiguration alloc] initWithApiKey:apiKey];
+    configuration.sendThreads = BSGThreadSendPolicyAlways;
     [Bugsnag startBugsnagWithConfiguration:configuration];
 
     return YES;