Merge pull request #532 from bugsnag/v6-structured-thread

Create structured BugsnagThread class
bugsnag · Apr 9, 2020 · 35ffa96 · 35ffa96
2 parents b8229be + cbed144
commit 35ffa96
Show file tree

Hide file tree

Showing 12 changed files with 488 additions and 122 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,9 @@ Bugsnag Notifiers on other platforms.
 
 ## Enhancements
 
+* Create structured `BugsnagThread` class
+  [#532](https://github.com/bugsnag/bugsnag-cocoa/pull/532)
+
 * Convert `event.device` from `NSDictionary` to a structured class
   [#526](https://github.com/bugsnag/bugsnag-cocoa/pull/526)
 

diff --git a/OSX/Bugsnag.xcodeproj/project.pbxproj b/OSX/Bugsnag.xcodeproj/project.pbxproj
@@ -60,6 +60,8 @@
 		E7433AD31F4F64F400C082D1 /* libc++.tbd in Frameworks */ = {isa = PBXBuildFile; fileRef = 8A2C8FF31C6BC3AE00846019 /* libc++.tbd */; };
 		E7529F8E243C8EBF006B4932 /* RegisterErrorData.h in Headers */ = {isa = PBXBuildFile; fileRef = E7529F8C243C8EBF006B4932 /* RegisterErrorData.h */; };
 		E7529F8F243C8EBF006B4932 /* RegisterErrorData.m in Sources */ = {isa = PBXBuildFile; fileRef = E7529F8D243C8EBF006B4932 /* RegisterErrorData.m */; };
+		E7529FA0243CAE35006B4932 /* BugsnagThreadTest.m in Sources */ = {isa = PBXBuildFile; fileRef = E7529F9F243CAE34006B4932 /* BugsnagThreadTest.m */; };
+		E7529FA2243CAE3F006B4932 /* BugsnagThreadSerializationTest.m in Sources */ = {isa = PBXBuildFile; fileRef = E7529FA1243CAE3F006B4932 /* BugsnagThreadSerializationTest.m */; };
 		E762E9F91F73F7F300E82B43 /* BugsnagHandledStateTest.m in Sources */ = {isa = PBXBuildFile; fileRef = E762E9F71F73F7E900E82B43 /* BugsnagHandledStateTest.m */; };
 		E762E9FC1F73F80200E82B43 /* BugsnagHandledState.h in Headers */ = {isa = PBXBuildFile; fileRef = E762E9FA1F73F80200E82B43 /* BugsnagHandledState.h */; };
 		E762E9FD1F73F80200E82B43 /* BugsnagHandledState.m in Sources */ = {isa = PBXBuildFile; fileRef = E762E9FB1F73F80200E82B43 /* BugsnagHandledState.m */; };
@@ -279,6 +281,8 @@
 		E72AE1F8241A4E7500ED8972 /* BugsnagPluginClient.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = BugsnagPluginClient.h; path = ../Source/BugsnagPluginClient.h; sourceTree = "<group>"; };
 		E7529F8C243C8EBF006B4932 /* RegisterErrorData.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = RegisterErrorData.h; path = ../Source/RegisterErrorData.h; sourceTree = "<group>"; };
 		E7529F8D243C8EBF006B4932 /* RegisterErrorData.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = RegisterErrorData.m; path = ../Source/RegisterErrorData.m; sourceTree = "<group>"; };
+		E7529F9F243CAE34006B4932 /* BugsnagThreadTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = BugsnagThreadTest.m; sourceTree = "<group>"; };
+		E7529FA1243CAE3F006B4932 /* BugsnagThreadSerializationTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = BugsnagThreadSerializationTest.m; path = ../iOS/BugsnagTests/BugsnagThreadSerializationTest.m; sourceTree = "<group>"; };
 		E762E9F71F73F7E900E82B43 /* BugsnagHandledStateTest.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = BugsnagHandledStateTest.m; path = ../Tests/BugsnagHandledStateTest.m; sourceTree = SOURCE_ROOT; };
 		E762E9FA1F73F80200E82B43 /* BugsnagHandledState.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = BugsnagHandledState.h; path = ../Source/BugsnagHandledState.h; sourceTree = SOURCE_ROOT; };
 		E762E9FB1F73F80200E82B43 /* BugsnagHandledState.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = BugsnagHandledState.m; path = ../Source/BugsnagHandledState.m; sourceTree = SOURCE_ROOT; };
@@ -548,9 +552,11 @@
 				E791482E1FD82B0C003EFEBF /* BugsnagUserTest.m */,
 				E7D2E675243B8FB6005A3041 /* BugsnagStacktraceTest.m */,
 				E722105D243B6A0E0083CF15 /* BugsnagStackframeTest.m */,
-				E791482E1FD82B0C003EFEBF /* BugsnagUserTest.m */,
 				00F9393B23FD2D9B008C7073 /* BugsnagTestsDummyClass.h */,
 				00F9393A23FD2D9B008C7073 /* BugsnagTestsDummyClass.m */,
+				E7529FA1243CAE3F006B4932 /* BugsnagThreadSerializationTest.m */,
+				E7529F9F243CAE34006B4932 /* BugsnagThreadTest.m */,
+				E791482E1FD82B0C003EFEBF /* BugsnagUserTest.m */,
 				8A2C8FE41C6BC38200846019 /* report.json */,
 				8A2C8FB21C6BC1F700846019 /* TestsInfo.plist */,
 				E7CE78861FD94E40001D07E0 /* KSCrash */,
@@ -1135,6 +1141,7 @@
 				E790C42324324528006FFB26 /* BugsnagClientMirrorTest.m in Sources */,
 				E7CE78D01FD94E77001D07E0 /* RFC3339DateTool_Tests.m in Sources */,
 				E79148621FD82BB7003EFEBF /* BugsnagSessionTrackingPayloadTest.m in Sources */,
+				E7529FA2243CAE3F006B4932 /* BugsnagThreadSerializationTest.m in Sources */,
 				E79148631FD82BB7003EFEBF /* BugsnagUserTest.m in Sources */,
 				8A2C8FEA1C6BC38900846019 /* BugsnagBreadcrumbsTest.m in Sources */,
 				E7CE78BB1FD94E77001D07E0 /* KSCrashReportConverter_Tests.m in Sources */,
@@ -1152,6 +1159,7 @@
 				E762E9F91F73F7F300E82B43 /* BugsnagHandledStateTest.m in Sources */,
 				E7CE78C51FD94E77001D07E0 /* KSLogger_Tests.m in Sources */,
 				E7CE78C11FD94E77001D07E0 /* KSCrashState_Tests.m in Sources */,
+				E7529FA0243CAE35006B4932 /* BugsnagThreadTest.m in Sources */,
 				E7AB4B9E2423E184004F015A /* BugsnagOnBreadcrumbTest.m in Sources */,
 				E7CE78C31FD94E77001D07E0 /* KSFileUtils_Tests.m in Sources */,
 				E7CE78BC1FD94E77001D07E0 /* KSCrashReportStore_Tests.m in Sources */,

diff --git a/Source/BugsnagEvent.h b/Source/BugsnagEvent.h
@@ -16,6 +16,7 @@
 @class BugsnagAppWithState;
 @class BugsnagDeviceWithState;
 @class BugsnagMetadata;
+@class BugsnagThread;
 
 typedef NS_ENUM(NSUInteger, BSGSeverity) {
     BSGSeverityError,
@@ -132,5 +133,10 @@ initWithErrorName:(NSString *_Nonnull)name
  */
 @property(readonly) BOOL unhandled;
 
+/**
+ * Thread traces for the error that occurred, if collection was enabled.
+ */
+@property(readonly, nonnull) NSMutableArray<BugsnagThread *> *threads;
+
 @end
 
diff --git a/Source/BugsnagEvent.m b/Source/BugsnagEvent.m
@@ -26,6 +26,7 @@
 #import "BugsnagDeviceWithState.h"
 #import "BugsnagClient.h"
 #import "BugsnagStacktrace.h"
+#import "BugsnagThread.h"
 #import "RegisterErrorData.h"
 
 static NSString *const DEFAULT_EXCEPTION_TYPE = @"cocoa";
@@ -72,6 +73,25 @@ + (BugsnagStackframe *)frameFromDict:(NSDictionary *)dict
                           withImages:(NSArray *)binaryImages;
 @end
 
+@interface BugsnagThread ()
+@property BugsnagStacktrace *trace;
+- (NSDictionary *)toDictionary;
+
+- (instancetype)initWithThread:(NSDictionary *)thread
+                  binaryImages:(NSArray *)binaryImages;
+
++ (NSMutableArray<BugsnagThread *> *)threadsFromArray:(NSArray *)threads
+                                         binaryImages:(NSArray *)binaryImages
+                                                depth:(NSUInteger)depth
+                                            errorType:(NSString *)errorType;
+
++ (NSMutableArray *)serializeThreads:(NSArray<BugsnagThread *> *)threads;
+@end
+
+@interface BugsnagStacktrace ()
+- (NSArray *)toArray;
+@end
+
 // MARK: - KSCrashReport parsing
 
 NSString *_Nonnull BSGParseErrorClass(NSDictionary *error,
@@ -216,14 +236,6 @@ @interface BugsnagEvent ()
  *  A unique hash identifying this device for the application or vendor
  */
 @property(nonatomic, readonly, copy, nullable) NSString *deviceAppHash;
-/**
- *  Binary images used to identify application symbols
- */
-@property(nonatomic, readonly, copy, nullable) NSArray *binaryImages;
-/**
- *  Thread information captured at the time of the error
- */
-@property(nonatomic, readonly, copy, nullable) NSArray *threads;
 /**
  *  User-provided exception metadata
  */
@@ -300,6 +312,9 @@ - (instancetype)initWithKSReport:(NSDictionary *)report {
             _releaseStage = [report valueForKeyPath:@"user.state.oom.app.releaseStage"];
             _handledState = [BugsnagHandledState handledStateWithSeverityReason:LikelyOutOfMemory];
             _deviceAppHash = [report valueForKeyPath:@"user.state.oom.device.id"];
+
+            // no threads or metadata captured for OOMs
+            _threads = [NSMutableArray new];
             self.metadata = [BugsnagMetadata new];
 
             NSDictionary *sessionData = [report valueForKeyPath:@"user.state.oom.session"];
@@ -313,16 +328,17 @@ - (instancetype)initWithKSReport:(NSDictionary *)report {
         } else {
             _enabledReleaseStages = BSGLoadConfigValue(report, BSGKeyEnabledReleaseStages);
             _releaseStage = BSGParseReleaseStage(report);
-            _threads = [report valueForKeyPath:@"crash.threads"];
-            RegisterErrorData *data = [RegisterErrorData errorDataFromThreads:_threads];
+            NSArray *binaryImages = report[@"binary_images"];
+            NSArray *threadDict = [report valueForKeyPath:@"crash.threads"];
+
+            RegisterErrorData *data = [RegisterErrorData errorDataFromThreads:threadDict];
             if (data) {
                 _errorClass = data.errorClass ;
                 _errorMessage = data.errorMessage;
             } else {
                 _errorClass = BSGParseErrorClass(_error, _errorType);
                 _errorMessage = BSGParseErrorMessage(report, _error, _errorType);
             }
-            _binaryImages = report[@"binary_images"];
             _breadcrumbs = BSGParseBreadcrumbs(report);
             _deviceAppHash = [report valueForKeyPath:@"system.device_app_hash"];
 
@@ -370,6 +386,12 @@ - (instancetype)initWithKSReport:(NSDictionary *)report {
             if (report[@"user"][@"id"]) {
                 _session = [[BugsnagSession alloc] initWithDictionary:report[@"user"]];
             }
+
+            // generate threads last, relies on depth/errorType properties being calculated first
+            _threads = [BugsnagThread threadsFromArray:threadDict
+                                          binaryImages:binaryImages
+                                                 depth:self.depth
+                                             errorType:self.errorType];
         }
     }
     return self;
@@ -404,6 +426,7 @@ - (instancetype _Nonnull)initWithErrorName:(NSString *_Nonnull)name
         _handledState = handledState;
         _severity = handledState.currentSeverity;
         _session = session;
+        _threads = [NSMutableArray new];
     }
     return self;
 }
@@ -539,18 +562,23 @@ - (NSDictionary *)toJson {
 
     if (self.customException) {
         BSGDictSetSafeObject(event, @[ self.customException ], BSGKeyExceptions);
-        BSGDictSetSafeObject(event, [self serializeThreadsWithException:nil],
-                BSGKeyThreads);
     } else {
         NSMutableDictionary *exception = [NSMutableDictionary dictionary];
         BSGDictSetSafeObject(exception, [self errorClass], BSGKeyErrorClass);
         BSGDictInsertIfNotNil(exception, [self errorMessage], BSGKeyMessage);
         BSGDictInsertIfNotNil(exception, DEFAULT_EXCEPTION_TYPE, BSGKeyType);
         BSGDictSetSafeObject(event, @[ exception ], BSGKeyExceptions);
 
-        BSGDictSetSafeObject(
-            event, [self serializeThreadsWithException:exception], BSGKeyThreads);
+        // set the stacktrace for the exception from the threads
+        for (BugsnagThread *thread in self.threads) {
+            if (thread.errorReportingThread) {
+                BSGDictSetSafeObject(exception, [thread.trace toArray], BSGKeyStacktrace);
+            }
+        }
     }
+
+    BSGDictSetSafeObject(event, [BugsnagThread serializeThreads:self.threads], BSGKeyThreads);
+
     // Build Event
     BSGDictSetSafeObject(event, BSGFormatSeverity(self.severity), BSGKeySeverity);
     BSGDictSetSafeObject(event, [self serializeBreadcrumbs], BSGKeyBreadcrumbs);
@@ -614,59 +642,6 @@ - (NSDictionary *)generateSessionDict {
     return sessionJson;
 }
 
-// Build all stacktraces for threads and the error
-- (NSArray *)serializeThreadsWithException:(NSMutableDictionary *)exception {
-    NSMutableArray *bugsnagThreads = [NSMutableArray array];
-
-    for (NSDictionary *thread in self.threads) {
-        NSArray *backtrace = thread[@"backtrace"][@"contents"];
-        BOOL stackOverflow = [thread[@"stack"][@"overflow"] boolValue];
-        BOOL isReportingThread = [thread[@"crashed"] boolValue];
-
-        if (isReportingThread) {
-            NSUInteger seen = 0;
-            NSMutableArray *stacktrace = [NSMutableArray array];
-
-            for (NSDictionary *frame in backtrace) {
-                NSMutableDictionary *mutableFrame = [frame mutableCopy];
-                if (seen++ >= [self depth]) {
-                    // Mark the frame so we know where it came from
-                    if (seen == 1 && !stackOverflow) {
-                        BSGDictSetSafeObject(mutableFrame, @YES, BSGKeyIsPC);
-                    }
-                    if (seen == 2 && !stackOverflow &&
-                        [@[ BSGKeySignal, BSGKeyMach ]
-                            containsObject:[self errorType]]) {
-                        BSGDictSetSafeObject(mutableFrame, @YES, BSGKeyIsLR);
-                    }
-                    BSGArrayInsertIfNotNil(stacktrace, mutableFrame);
-                }
-            }
-            BugsnagStacktrace *trace = [[BugsnagStacktrace alloc] initWithTrace:stacktrace binaryImages:self.binaryImages];
-            BSGDictSetSafeObject(exception, [trace toArray], BSGKeyStacktrace);
-        }
-        [self serialiseThread:bugsnagThreads thread:thread backtrace:backtrace reportingThread:isReportingThread];
-    }
-    return bugsnagThreads;
-}
-
-- (void)serialiseThread:(NSMutableArray *)bugsnagThreads
-                 thread:(NSDictionary *)thread
-              backtrace:(NSArray *)backtrace
-          reportingThread:(BOOL)isReportingThread {
-    BugsnagStacktrace *stacktrace = [[BugsnagStacktrace alloc] initWithTrace:backtrace binaryImages:self.binaryImages];
-    NSMutableDictionary *threadDict = [NSMutableDictionary dictionary];
-    BSGDictSetSafeObject(threadDict, thread[@"index"], BSGKeyId);
-    BSGDictSetSafeObject(threadDict, [stacktrace toArray], BSGKeyStacktrace);
-    BSGDictSetSafeObject(threadDict, DEFAULT_EXCEPTION_TYPE, BSGKeyType);
-
-    if (isReportingThread) {
-        BSGDictSetSafeObject(threadDict, @YES, @"errorReportingThread");
-    }
-
-    BSGArrayAddSafeObject(bugsnagThreads, threadDict);
-}
-
 - (BOOL)unhandled {
     return self.handledState.unhandled;
 }

diff --git a/Source/BugsnagThread.h b/Source/BugsnagThread.h
@@ -8,13 +8,42 @@
 
 #import <Foundation/Foundation.h>
 
-NS_ASSUME_NONNULL_BEGIN
+typedef NS_OPTIONS(NSUInteger, BSGThreadType) {
+    BSGThreadTypeCocoa = 0,
+    BSGThreadTypeReactNativeJs = 1 << 1
+};
+
+@class BugsnagStackframe;
 
 /**
  * A representation of thread information recorded as part of a BugsnagEvent.
  */
 @interface BugsnagThread : NSObject
 
-@end
+/**
+ * A unique ID which identifies this thread
+ */
+@property(nullable) NSString *id;
 
-NS_ASSUME_NONNULL_END
+/**
+ * The name which identifies this thread
+ */
+@property(nullable) NSString *name;
+
+/**
+ * Whether this thread was the thread that triggered the captured error
+ */
+@property BOOL errorReportingThread;
+
+/**
+ * Sets a representation of this thread's stacktrace
+ */
+@property(readonly, nonnull) NSArray<BugsnagStackframe *> *stacktrace;
+
+/**
+ * Determines the type of thread based on the originating platform
+ * (intended for internal use only)
+ */
+@property BSGThreadType type;
+
+@end