fix: record thread information for unhandled JS errors

bugsnag · Jul 29, 2020 · f13a538 · f13a538
1 parent b115a29
commit f13a538
Show file tree

Hide file tree

Showing 8 changed files with 40 additions and 29 deletions.
diff --git a/Bugsnag/Client/BugsnagClient.m b/Bugsnag/Client/BugsnagClient.m
@@ -975,7 +975,7 @@ - (void)notify:(NSException *)exception
      * 4. -[BSG_KSCrash captureThreads:depth:]
      */
     int depth = (int)(BSGNotifierStackFrameCount);
-    NSArray *threads = [[BSG_KSCrash sharedInstance] captureThreads:exception depth:depth];
+    NSArray *threads = [[BSG_KSCrash sharedInstance] captureThreads:exception depth:depth unhandled:false];
     NSArray *errors = @[[self generateError:exception threads:threads]];
 
     BugsnagMetadata *metadata = [self.metadata deepCopy];
@@ -1564,14 +1564,16 @@ - (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];
+    NSArray<BugsnagThread *> *threads = [[BSG_KSCrash sharedInstance] captureThreads:exc
+                                                                               depth:depth
+                                                                           unhandled:unhandled];
     return [BugsnagThread serializeThreads:threads];
 }
 

diff --git a/Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.h b/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
+                                   unhandled:(BOOL)unhandled;
 
 /**
  * Collects information about the application's foreground state (duration in foreground/background)

diff --git a/Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.m b/Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrash.m
@@ -282,7 +282,9 @@ - (void)sendAllReports {
             }];
 }
 
-- (NSArray<BugsnagThread *> *)captureThreads:(NSException *)exc depth:(int)depth {
+- (NSArray<BugsnagThread *> *)captureThreads:(NSException *)exc
+                                       depth:(int)depth
+                                   unhandled:(BOOL)unhandled {
     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, unhandled);
     free(callstack);
     NSDictionary *json = BSGDeserializeJson(trace);
     free(trace);

diff --git a/Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.c b/Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.c
@@ -217,7 +217,7 @@ void bsg_kscrash_setThreadTracingEnabled(int threadTracingEnabled) {
     crashContext()->crash.threadTracingEnabled = threadTracingEnabled;
 }
 
-char *bsg_kscrash_captureThreadTrace(int discardDepth, int frameCount, uintptr_t *callstack) {
+char *bsg_kscrash_captureThreadTrace(int discardDepth, int frameCount, uintptr_t *callstack, const bool unhandled) {
     BSG_KSCrash_Context *context = crashContext();
 
     // populate context with pre-recorded stacktrace/thread info
@@ -236,8 +236,8 @@ char *bsg_kscrash_captureThreadTrace(int discardDepth, int frameCount, uintptr_t
     if (context->crash.threadTracingEnabled == 0) {
         bsg_kscrashsentry_suspend_threads_user();
     }
-    
-    char *trace = bsg_kscrw_i_captureThreadTrace(context);
+
+    char *trace = bsg_kscrw_i_captureThreadTrace(context, unhandled);
 
     if (context->crash.threadTracingEnabled == 0) {
         bsg_kscrashsentry_resume_threads_user(false);

diff --git a/Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.h b/Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashC.h
@@ -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 unhandled);
 
 #ifdef __cplusplus
 }

diff --git a/Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReport.c b/Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReport.c
@@ -627,8 +627,9 @@ 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, 
-                                const BSG_KSCrashReportWriter *writer);
+void bsg_kscrw_i_writeTraceInfo(const BSG_KSCrash_Context *crashContext,
+                                const BSG_KSCrashReportWriter *writer,
+                                const bool unhandled);
 
 bool bsg_kscrw_i_exceedsBufferLen(const size_t length);
 
@@ -1034,11 +1035,13 @@ void bsg_kscrw_i_writeThread(const BSG_KSCrashReportWriter *const writer,
  * @param writeNotableAddresses whether notable addresses should be written
  * so additional information about the error can be extracted
  * only the main thread's stacktrace is serialized.
+ * @param recordAllThreads whether all threads should be recorded, or just the current one
  */
 void bsg_kscrw_i_writeAllThreads(const BSG_KSCrashReportWriter *const writer,
                                  const char *const key,
                                  const BSG_KSCrash_SentryContext *const crash,
-                                 bool writeNotableAddresses) {
+                                 bool writeNotableAddresses,
+                                 bool recordAllThreads) {
     const task_t thisTask = mach_task_self();
     thread_act_array_t threads;
     mach_msg_type_number_t numThreads;
@@ -1048,10 +1051,6 @@ 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);
@@ -1650,7 +1649,8 @@ void bsg_kscrashreport_writeKSCrashFields(BSG_KSCrash_Context *crashContext, BSG
         bsg_kscrw_i_addJSONElement(writer, BSG_KSCrashField_User,
                 crashContext->config.userInfoJSON);
     }
-    bsg_kscrw_i_writeTraceInfo(crashContext, writer);
+    bool unhandled = crashContext->crash.crashType != BSG_KSCrashTypeUserReported;
+    bsg_kscrw_i_writeTraceInfo(crashContext, writer, unhandled);
 }
 
 void bsg_kscrashreport_logCrash(const BSG_KSCrash_Context *const crashContext) {
@@ -1686,33 +1686,37 @@ int bsg_kscrw_i_collectJsonData(const char *const data, const size_t length, voi
     return BSG_KSJSON_OK;
 }
 
-char *bsg_kscrw_i_captureThreadTrace(const BSG_KSCrash_Context *crashContext) {
+char *bsg_kscrw_i_captureThreadTrace(const BSG_KSCrash_Context *crashContext, const bool unhandled) {
     BSG_KSJSONEncodeContext jsonContext;
     BSG_KSCrashReportWriter concreteWriter;
     BSG_KSCrashReportWriter *writer = &concreteWriter;
     bsg_kscrw_i_prepareReportWriter(writer, &jsonContext);
     BSG_ThreadDataBuffer userData = { NULL, 0 };
     bsg_ksjsonbeginEncode(bsg_getJsonContext(writer), false, bsg_kscrw_i_collectJsonData, &userData);
     writer->beginObject(writer, BSG_KSCrashField_Report);
-    bsg_kscrw_i_writeTraceInfo(crashContext, writer);
+    bsg_kscrw_i_writeTraceInfo(crashContext, writer, unhandled);
     writer->endContainer(writer);
     bsg_ksjsonendEncode(bsg_getJsonContext(writer));
     return userData.data;
 }
 
 void bsg_kscrw_i_writeTraceInfo(const BSG_KSCrash_Context *crashContext,
-                                const BSG_KSCrashReportWriter *writer) {
-    bool unhandledCrash = crashContext->crash.crashType != BSG_KSCrashTypeUserReported;
+                                const BSG_KSCrashReportWriter *writer,
+                                const bool unhandled) {
+    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 || crashContext->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,
-                crashContext->config.introspectionRules.enabled);
-        bsg_kscrw_i_writeError(writer, BSG_KSCrashField_Error,&crashContext->crash);
+        bool recordAllThreads = crash->threadTracingEnabled == 0 // Always
+                || (crash->threadTracingEnabled == 1 // Unhandled Only
+                && unhandled);
+        bsg_kscrw_i_writeAllThreads(writer, BSG_KSCrashField_Threads, crash,
+                crashContext->config.introspectionRules.enabled, recordAllThreads);
+        bsg_kscrw_i_writeError(writer, BSG_KSCrashField_Error,crash);
     }
     writer->endContainer(writer);
 }
diff --git a/Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReport.h b/Bugsnag/KSCrash/Source/KSCrash/Recording/BSG_KSCrashReport.h
@@ -69,10 +69,11 @@ void bsg_kscrashreport_logCrash(const BSG_KSCrash_Context *const crashContext);
  *
  * @param crashContext Contextual information about the crash and environment.
  *                     The caller must fill this out before passing it in.
+ * @param unhandled whether the crash was considered unhandled or not
  *
  * @return the thread trace encoded as a JSON string
  */
-char *bsg_kscrw_i_captureThreadTrace(const BSG_KSCrash_Context *crashContext);
+char *bsg_kscrw_i_captureThreadTrace(const BSG_KSCrash_Context *crashContext, const bool unhandled);
 
 #ifdef __cplusplus
 }

diff --git a/Tests/BugsnagClientMirrorTest.m b/Tests/BugsnagClientMirrorTest.m
@@ -95,7 +95,7 @@ - (void)setUp {
             @"context @16@0:8",
             @"collectAppWithState @16@0:8",
             @"collectBreadcrumbs @16@0:8",
-            @"collectThreads @16@0:8",
+            @"collectThreads: @20@0:8B16",
             @"collectDeviceWithState @16@0:8",
             @"extraRuntimeInfo @16@0:8",
             @"setExtraRuntimeInfo: v24@0:8@16",