[PLAT-4707] Record thread information for unhandled JS errors

Bugsnag/BugsnagCrashSentry.m

@@ -30,8 +30,11 @@ - (void)install:(BugsnagConfiguration *)config
     ksCrash.introspectMemory = YES;
     ksCrash.onCrash = onCrash;
     ksCrash.maxStoredReports = BSG_MAX_STORED_REPORTS;
-    ksCrash.threadTracingEnabled = (int) config.sendThreads;
-
+
+    // overridden elsewhere for handled errors, so we can assume that this only
+    // applies to unhandled errors
+    ksCrash.threadTracingEnabled = config.sendThreads != BSGThreadSendPolicyNever;
+
     // User reported events are *always* handled
     BSG_KSCrashType crashTypes = BSG_KSCrashTypeUserReported;
 

Bugsnag/Client/BugsnagClient.m

@@ -975,7 +975,11 @@ - (void)notify:(NSException *)exception
      * 4. -[BSG_KSCrash captureThreads:depth:]
      */
     int depth = (int)(BSGNotifierStackFrameCount);
-    NSArray *threads = [[BSG_KSCrash sharedInstance] captureThreads:exception depth:depth];
+
+    BOOL recordAllThreads = self.configuration.sendThreads == BSGThreadSendPolicyAlways;
+    NSArray *threads = [[BSG_KSCrash sharedInstance] captureThreads:exception
+                                                              depth:depth
+                                                   recordAllThreads:recordAllThreads];
     NSArray *errors = @[[self generateError:exception threads:threads]];
 
     BugsnagMetadata *metadata = [self.metadata deepCopy];
@@ -1564,14 +1568,19 @@ - (NSArray *)collectBreadcrumbs {
     return data;
 }
 
-- (NSArray *)collectThreads {
+- (NSArray *)collectThreads:(BOOL)unhandled {
     // discard the following
     // 1. [BugsnagReactNative getPayloadInfo:resolve:reject:]
     // 2. [BugsnagClient collectThreads:]
-    // 3. [BSG_KSCrash captureThreads:]
+    // 3. [BSG_KSCrash captureThreads:depth:unhandled:]
     int depth = 3;
     NSException *exc = [NSException exceptionWithName:@"Bugsnag" reason:@"" userInfo:nil];
-    NSArray<BugsnagThread *> *threads = [[BSG_KSCrash sharedInstance] captureThreads:exc depth:depth];
+    BSGThreadSendPolicy sendThreads = self.configuration.sendThreads;
+    BOOL recordAllThreads = sendThreads == BSGThreadSendPolicyAlways
+            || (unhandled && sendThreads == BSGThreadSendPolicyUnhandledOnly);
+    NSArray<BugsnagThread *> *threads = [[BSG_KSCrash sharedInstance] captureThreads:exc
+                                                                               depth:depth
+                                                                    recordAllThreads:recordAllThreads];
     return [BugsnagThread serializeThreads:threads];
 }
 

Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.h

@@ -117,7 +117,9 @@
  * @param depth the number of frames to discard from the main thread's stacktrace
  * @return an array of BugsnagThread
  */
-- (NSArray<BugsnagThread *> *)captureThreads:(NSException *)exc depth:(int)depth;
+- (NSArray<BugsnagThread *> *)captureThreads:(NSException *)exc
+                                       depth:(int)depth
+                            recordAllThreads:(BOOL)recordAllThreads;
 
 /**
  * Collects information about the application's foreground state (duration in foreground/background)
@@ -132,9 +134,8 @@
 
 /**
 * The methodology used for tracing threads.
-* The value will be equal to an enum value from BSGThreadSendPolicy
  */
-@property(nonatomic, readwrite, assign) int threadTracingEnabled;
+@property(nonatomic, readwrite, assign) BOOL threadTracingEnabled;
 
 /**
  * If YES, binary images will be collected for each report.

Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.m

@@ -38,9 +38,10 @@
 #import "BSG_KSLogger.h"
 #import "BugsnagThread.h"
 #import "BSGSerialization.h"
-#import "BugsnagErrorReportSink.h"
+#import "Bugsnag.h"
 #import "BugsnagCollections.h"
 #import "BSG_KSCrashReportFields.h"
+#import "Private.h"
 
 #if BSG_HAS_UIKIT
 #import <UIKit/UIKit.h>
@@ -147,7 +148,6 @@ - (id)initWithReportFilesDirectory:(NSString *)reportFilesDirectory {
         self.maxStoredReports = 5;
 
         self.reportWhenDebuggerIsAttached = NO;
-        self.threadTracingEnabled = BSGThreadSendPolicyAlways;
         self.writeBinaryImagesForUserReported = YES;
     }
     return self;
@@ -199,7 +199,7 @@ - (void)setReportWhenDebuggerIsAttached:(BOOL)reportWhenDebuggerIsAttached {
     bsg_kscrash_setReportWhenDebuggerIsAttached(reportWhenDebuggerIsAttached);
 }
 
-- (void)setThreadTracingEnabled:(int)threadTracingEnabled {
+- (void)setThreadTracingEnabled:(BOOL)threadTracingEnabled {
     _threadTracingEnabled = threadTracingEnabled;
     bsg_kscrash_setThreadTracingEnabled(threadTracingEnabled);
 }
@@ -282,7 +282,9 @@ - (void)sendAllReports {
             }];
 }
 
-- (NSArray<BugsnagThread *> *)captureThreads:(NSException *)exc depth:(int)depth {
+- (NSArray<BugsnagThread *> *)captureThreads:(NSException *)exc
+                                       depth:(int)depth
+                            recordAllThreads:(BOOL)recordAllThreads {
     NSArray *addresses = [exc callStackReturnAddresses];
     int numFrames = (int) [addresses count];
     uintptr_t *callstack;
@@ -308,7 +310,7 @@ - (void)sendAllReports {
         }
     }
 
-    char *trace = bsg_kscrash_captureThreadTrace(depth, numFrames, callstack);
+    char *trace = bsg_kscrash_captureThreadTrace(depth, numFrames, callstack, recordAllThreads);
     free(callstack);
     NSDictionary *json = BSGDeserializeJson(trace);
     free(trace);

Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.c

@@ -213,12 +213,15 @@ void bsg_kscrash_setReportWhenDebuggerIsAttached(
         reportWhenDebuggerIsAttached;
 }
 
-void bsg_kscrash_setThreadTracingEnabled(int threadTracingEnabled) {
+void bsg_kscrash_setThreadTracingEnabled(bool threadTracingEnabled) {
     crashContext()->crash.threadTracingEnabled = threadTracingEnabled;
 }
 
-char *bsg_kscrash_captureThreadTrace(int discardDepth, int frameCount, uintptr_t *callstack) {
-    BSG_KSCrash_Context *context = crashContext();
+char *bsg_kscrash_captureThreadTrace(int discardDepth, int frameCount, uintptr_t *callstack, const bool recordAllThreads) {
+    BSG_KSCrash_Context *globalContext = crashContext();
+    BSG_KSCrash_Context localContext = {};
+    BSG_KSCrash_Context *context = &localContext;
+    memcpy(context, globalContext, sizeof(BSG_KSCrash_Context));
 
     // populate context with pre-recorded stacktrace/thread info
     // for KSCrash to serialize
@@ -227,19 +230,20 @@ char *bsg_kscrash_captureThreadTrace(int discardDepth, int frameCount, uintptr_t
     context->crash.userException.discardDepth = discardDepth;
     context->crash.offendingThread = bsg_ksmachthread_self();
     context->crash.crashType = BSG_KSCrashTypeUserReported;
+    context->crash.threadTracingEnabled = recordAllThreads;
 
     // No need to gather notable addresses for handled errors
     context->config.introspectionRules.enabled = false;
 
     // Only suspend threads if tracing is set to always
     // (to ensure trace is captured at the same point in time)
-    if (context->crash.threadTracingEnabled == 0) {
+    if (context->crash.threadTracingEnabled) {
         bsg_kscrashsentry_suspend_threads_user();
     }
-    
+
     char *trace = bsg_kscrw_i_captureThreadTrace(context);
 
-    if (context->crash.threadTracingEnabled == 0) {
+    if (context->crash.threadTracingEnabled) {
         bsg_kscrashsentry_resume_threads_user(false);
     }
     return trace;

Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.h

@@ -153,7 +153,7 @@ void bsg_kscrash_reportUserException(const char *name, const char *reason,
 void bsg_kscrash_setReportWhenDebuggerIsAttached(
     bool reportWhenDebuggerIsAttached);
 
-void bsg_kscrash_setThreadTracingEnabled(int threadTracingEnabled);
+void bsg_kscrash_setThreadTracingEnabled(bool threadTracingEnabled);
 
 void bsg_kscrash_setWriteBinaryImagesForUserReported(
     bool writeBinaryImagesForUserReported);
@@ -171,7 +171,7 @@ BSG_KSCrash_Context *crashContext(void);
  *
  * @return a trace of all the threads as a JSON string.
  */
-char *bsg_kscrash_captureThreadTrace(int discardDepth, int frameCount, uintptr_t *callstack);
+char *bsg_kscrash_captureThreadTrace(int discardDepth, int frameCount, uintptr_t *callstack, const bool recordAllThreads);
 
 #ifdef __cplusplus
 }

Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReport.c

@@ -627,7 +627,7 @@ void bsg_kscrw_i_writeMemoryContents(
     const BSG_KSCrashReportWriter *const writer, const char *const key,
     const uintptr_t address, int *limit);
 
-void bsg_kscrw_i_writeTraceInfo(const BSG_KSCrash_Context *crashContext, 
+void bsg_kscrw_i_writeTraceInfo(const BSG_KSCrash_Context *crashContext,
                                 const BSG_KSCrashReportWriter *writer);
 
 bool bsg_kscrw_i_exceedsBufferLen(const size_t length);
@@ -1048,17 +1048,13 @@ void bsg_kscrw_i_writeAllThreads(const BSG_KSCrashReportWriter *const writer,
         BSG_KSLOG_ERROR("task_threads: %s", mach_error_string(kr));
         return;
     }
-
-    bool recordAllThreads = crash->threadTracingEnabled == 0 // Always
-                            || (crash->threadTracingEnabled == 1 // Unhandled Only
-                                && crash->crashType != BSG_KSCrashTypeUserReported);
 
     // Fetch info for all threads.
     writer->beginArray(writer, key);
     {
         for (mach_msg_type_number_t i = 0; i < numThreads; i++) {
             thread_t thread = threads[i];
-            if (recordAllThreads || thread == crash->offendingThread) {
+            if (crash->threadTracingEnabled || thread == crash->offendingThread) {
                 bsg_kscrw_i_writeThread(writer, NULL, crash, thread, (int) i, writeNotableAddresses);
             }
         }
@@ -1702,17 +1698,17 @@ char *bsg_kscrw_i_captureThreadTrace(const BSG_KSCrash_Context *crashContext) {
 
 void bsg_kscrw_i_writeTraceInfo(const BSG_KSCrash_Context *crashContext,
                                 const BSG_KSCrashReportWriter *writer) {
-    bool unhandledCrash = crashContext->crash.crashType != BSG_KSCrashTypeUserReported;
+    const BSG_KSCrash_SentryContext *crash = &crashContext->crash;
 
     // Don't write the binary images for user reported crashes to improve performance
-    if (crashContext->crash.writeBinaryImagesForUserReported == true || unhandledCrash) {
+    if (crash->writeBinaryImagesForUserReported == true || crash->crashType != BSG_KSCrashTypeUserReported) {
         bsg_kscrw_i_writeBinaryImages(writer, BSG_KSCrashField_BinaryImages);
     }
     writer->beginObject(writer, BSG_KSCrashField_Crash);
     {
-        bsg_kscrw_i_writeAllThreads(writer, BSG_KSCrashField_Threads, &crashContext->crash,
+        bsg_kscrw_i_writeAllThreads(writer, BSG_KSCrashField_Threads, crash,
                 crashContext->config.introspectionRules.enabled);
-        bsg_kscrw_i_writeError(writer, BSG_KSCrashField_Error,&crashContext->crash);
+        bsg_kscrw_i_writeError(writer, BSG_KSCrashField_Error,crash);
     }
     writer->endContainer(writer);
 }

Bugsnag/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry.c

@@ -202,7 +202,7 @@ void bsg_kscrashsentry_resumeThreads(void) {
 
 void bsg_kscrashsentry_clearContext(BSG_KSCrash_SentryContext *context) {
     void (*onCrash)(void *) = context->onCrash;
-    int threadTracingEnabled = context->threadTracingEnabled;
+    bool threadTracingEnabled = context->threadTracingEnabled;
     bool reportWhenDebuggerIsAttached = context->reportWhenDebuggerIsAttached;
     bool writeBinaryImagesForUserReported =
         context->writeBinaryImagesForUserReported;

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

@@ -58,9 +58,9 @@ typedef struct BSG_KSCrash_SentryContext {
 
     /**
      * The methodology used for tracing threads.
-     * The value will be equal to an enum value from BSGThreadSendPolicy
+     * If true, will capture traces for all running threads
      */
-    int threadTracingEnabled;
+    bool threadTracingEnabled;
 
     /** If true, will record binary images. */
     bool writeBinaryImagesForUserReported;

CHANGELOG.md

@@ -4,6 +4,10 @@ Changelog
 ## TBD
 
 ### Bug fixes
+
+* Record thread information for unhandled JS errors
+  [#766](https://github.com/bugsnag/bugsnag-cocoa/pull/766)
+
 * Respect bundle version set from config
   [#762](https://github.com/bugsnag/bugsnag-cocoa/pull/762)
 

Tests/BugsnagClientMirrorTest.m

@@ -95,7 +95,8 @@ - (void)setUp {
             @"context @16@0:8",
             @"collectAppWithState @16@0:8",
             @"collectBreadcrumbs @16@0:8",
-            @"collectThreads @16@0:8",
+            @"collectThreads: @20@0:8B16",
+            @"collectThreads: @20@0:8c16",
             @"collectDeviceWithState @16@0:8",
             @"extraRuntimeInfo @16@0:8",
             @"setExtraRuntimeInfo: v24@0:8@16",