Merge pull request #281 from bugsnag/state-serialisation-fix

Handled/unhandled error state serialisation fix

Source/BSG_KSCrashReportWriter.h

@@ -215,6 +215,9 @@ typedef struct BSG_KSCrashReportWriter {
 typedef void (*BSG_KSReportWriteCallback)(
     const BSG_KSCrashReportWriter *writer);
 
+typedef void (*BSGReportCallback)(
+        const BSG_KSCrashReportWriter *writer, int type);
+
 #ifdef __cplusplus
 }
 #endif

Source/BugsnagConfiguration.h

@@ -132,6 +132,7 @@ BugsnagBreadcrumbs *breadcrumbs;
  */
 @property void (*_Nullable onCrashHandler)
     (const BSG_KSCrashReportWriter *_Nonnull writer);
+
 /**
  *  YES if uncaught exceptions should be reported automatically
  */

Source/BugsnagCrashReport.m

@@ -235,10 +235,6 @@ - (instancetype)initWithKSReport:(NSDictionary *)report {
         }
         _binaryImages = report[@"binary_images"];
         _breadcrumbs = BSGParseBreadcrumbs(report);
-        _severity = BSGParseSeverity(
-            [report valueForKeyPath:@"user.state.crash.severity"]);
-        _depth = [[report valueForKeyPath:@"user.state.crash.depth"]
-            unsignedIntegerValue];
         _dsymUUID = [report valueForKeyPath:@"system.app_uuid"];
         _deviceAppHash = [report valueForKeyPath:@"system.device_app_hash"];
         _metaData =
@@ -259,6 +255,10 @@ - (instancetype)initWithKSReport:(NSDictionary *)report {
         if (recordedState) {
             _handledState =
                 [[BugsnagHandledState alloc] initWithDictionary:recordedState];
+
+            // only makes sense to use serialised value for handled exceptions
+            _depth = [[report valueForKeyPath:@"user.state.crash.depth"]
+                    unsignedIntegerValue];
         } else { // the event was unhandled.
             BOOL isSignal = [BSGKeySignal isEqualToString:_errorType];
             SeverityReasonType severityReason =
@@ -267,6 +267,7 @@ - (instancetype)initWithKSReport:(NSDictionary *)report {
                 handledStateWithSeverityReason:severityReason
                                       severity:BSGSeverityError
                                      attrValue:_errorClass];
+            _depth = 0;
         }
         _severity = _handledState.currentSeverity;
 

Source/BugsnagCrashSentry.h

@@ -16,7 +16,7 @@
 
 - (void)install:(BugsnagConfiguration *)config
       apiClient:(BugsnagErrorReportApiClient *)apiClient
-        onCrash:(BSG_KSReportWriteCallback)onCrash;
+        onCrash:(BSGReportCallback)onCrash;
 
 - (void)reportUserException:(NSString *)reportName
                      reason:(NSString *)reportMessage;

Source/BugsnagCrashSentry.m

@@ -19,7 +19,7 @@ @implementation BugsnagCrashSentry
 
 - (void)install:(BugsnagConfiguration *)config
       apiClient:(BugsnagErrorReportApiClient *)apiClient
-        onCrash:(BSG_KSReportWriteCallback)onCrash {
+        onCrash:(BSGReportCallback)onCrash {
 
     BugsnagSink *sink = [[BugsnagSink alloc] initWithApiClient:apiClient];
     [BSG_KSCrash sharedInstance].sink = sink;

Source/BugsnagNotifier.m

@@ -34,6 +34,7 @@
 #import "BugsnagSessionTracker.h"
 #import "BugsnagSessionTrackingApiClient.h"
 #import "BSG_RFC3339DateTool.h"
+#import "BSG_KSCrashType.h"
 
 #if TARGET_IPHONE_SIMULATOR || TARGET_OS_IPHONE
 #import <UIKit/UIKit.h>
@@ -81,40 +82,41 @@
  *
  *  @param writer report writer which will receive updated metadata
  */
-void BSSerializeDataCrashHandler(const BSG_KSCrashReportWriter *writer) {
-    if (bsg_g_bugsnag_data.configJSON) {
-        writer->addJSONElement(writer, "config", bsg_g_bugsnag_data.configJSON);
-    }
-    if (bsg_g_bugsnag_data.metaDataJSON) {
-        writer->addJSONElement(writer, "metaData",
-                               bsg_g_bugsnag_data.metaDataJSON);
-    }
+void BSSerializeDataCrashHandler(const BSG_KSCrashReportWriter *writer, int type) {
+    BOOL userReported = BSG_KSCrashTypeUserReported == type;
 
     if (hasRecordedSessions) { // a session is available
         // persist session info
         writer->addStringElement(writer, "id", (const char *) sessionId);
         writer->addStringElement(writer, "startedAt", (const char *) sessionStartDate);
         writer->addUIntegerElement(writer, "handledCount", handledCount);
 
-        if (!bsg_g_bugsnag_data.handledState) {
+        if (!userReported) {
             writer->addUIntegerElement(writer, "unhandledCount", 1);
         } else {
             writer->addUIntegerElement(writer, "unhandledCount", 0);
         }
     }
-
-    if (bsg_g_bugsnag_data.handledState) {
-        writer->addJSONElement(writer, "handledState",
-                               bsg_g_bugsnag_data.handledState);
+    if (bsg_g_bugsnag_data.configJSON) {
+        writer->addJSONElement(writer, "config", bsg_g_bugsnag_data.configJSON);
     }
-
     if (bsg_g_bugsnag_data.stateJSON) {
         writer->addJSONElement(writer, "state", bsg_g_bugsnag_data.stateJSON);
     }
-    if (bsg_g_bugsnag_data.userOverridesJSON) {
-        writer->addJSONElement(writer, "overrides",
-                               bsg_g_bugsnag_data.userOverridesJSON);
+    if (bsg_g_bugsnag_data.metaDataJSON) {
+        writer->addJSONElement(writer, "metaData", bsg_g_bugsnag_data.metaDataJSON);
     }
+
+    // write additional user-supplied metadata
+    if (userReported) {
+        if (bsg_g_bugsnag_data.handledState) {
+            writer->addJSONElement(writer, "handledState", bsg_g_bugsnag_data.handledState);
+        }
+        if (bsg_g_bugsnag_data.userOverridesJSON) {
+            writer->addJSONElement(writer, "overrides", bsg_g_bugsnag_data.userOverridesJSON);
+        }
+    }
+
     if (bsg_g_bugsnag_data.onCrash) {
         bsg_g_bugsnag_data.onCrash(writer);
     }

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

@@ -73,7 +73,7 @@ @interface BSG_KSCrash ()
 @property(nonatomic, readwrite, retain) NSString *logFilePath;
 @property(nonatomic, readwrite, retain)
     BSG_KSCrashReportStore *crashReportStore;
-@property(nonatomic, readwrite, assign) BSG_KSReportWriteCallback onCrash;
+@property(nonatomic, readwrite, assign) BSGReportCallback onCrash;
 @property(nonatomic, readwrite, assign) bool printTraceToStdout;
 @property(nonatomic, readwrite, assign) int maxStoredReports;
 
@@ -203,7 +203,7 @@ - (void)setPrintTraceToStdout:(bool)printTraceToStdout {
     bsg_kscrash_setPrintTraceToStdout(printTraceToStdout);
 }
 
-- (void)setOnCrash:(BSG_KSReportWriteCallback)onCrash {
+- (void)setOnCrash:(BSGReportCallback)onCrash {
     _onCrash = onCrash;
     bsg_kscrash_setCrashNotifyCallback(onCrash);
 }

Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashAdvanced.h

@@ -112,7 +112,7 @@ typedef enum {
  * Note: If you use an installation, it will automatically set this property.
  *       Do not modify it in such a case.
  */
-@property(nonatomic, readwrite, assign) BSG_KSReportWriteCallback onCrash;
+@property(nonatomic, readwrite, assign) BSGReportCallback onCrash;
 
 /** Path where the log of BSG_KSCrash's activities will be written.
  * If nil, log entries will be printed to the console.

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

@@ -236,7 +236,7 @@ void bsg_kscrash_setDoNotIntrospectClasses(const char **doNotIntrospectClasses,
 }
 
 void bsg_kscrash_setCrashNotifyCallback(
-    const BSG_KSReportWriteCallback onCrashNotify) {
+    const BSGReportCallback onCrashNotify) {
     BSG_KSLOG_TRACE("Set onCrashNotify to %p", onCrashNotify);
     crashContext()->config.onCrashNotify = onCrashNotify;
 }

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

@@ -166,7 +166,7 @@ void bsg_kscrash_setDoNotIntrospectClasses(const char **doNotIntrospectClasses,
  * Default: NULL
  */
 void bsg_kscrash_setCrashNotifyCallback(
-    const BSG_KSReportWriteCallback onCrashNotify);
+    const BSGReportCallback onCrashNotify);
 
 /** Report a custom, user defined exception.
  * This can be useful when dealing with scripting languages.

Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashContext.h

@@ -88,7 +88,7 @@ typedef struct {
     /** Callback allowing the application the opportunity to add extra data to
      * the report file. Application MUST NOT call async-unsafe methods!
      */
-    BSG_KSReportWriteCallback onCrashNotify;
+    BSGReportCallback onCrashNotify;
 } BSG_KSCrash_Configuration;
 
 /** Contextual data used by the crash report writer.

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

@@ -40,6 +40,7 @@
 
 //#define BSG_kSLogger_LocalLevel TRACE
 #include "BSG_KSLogger.h"
+#include "BSG_KSCrashContext.h"
 
 #ifdef __arm64__
 #include <sys/_types/_ucontext64.h>
@@ -1963,7 +1964,8 @@ void bsg_kscrw_i_updateStackOverflowStatus(
 
 void bsg_kscrw_i_callUserCrashHandler(BSG_KSCrash_Context *const crashContext,
                                       BSG_KSCrashReportWriter *writer) {
-    crashContext->config.onCrashNotify(writer);
+    BSG_KSCrashType type = crashContext->crash.crashType;
+    crashContext->config.onCrashNotify(writer, type);
 }
 
 // ============================================================================

Tests/BugsnagCrashReportTests.m

@@ -449,4 +449,68 @@ - (void)testEnhancedErrorMessageIgnoresUnknownAssertionTypes {
                           payload[@"exceptions"][0][@"message"]);
 }
 
+- (void)testEmptyReport {
+    BugsnagCrashReport *report = [[BugsnagCrashReport alloc] initWithKSReport:@{}];
+    XCTAssertNotNil(report);
+}
+
+- (void)testUnhandledReportDepth {
+    // unhandled reports should calculate their own depth
+    NSDictionary *dict = @{@"user.state.crash.depth": @2};
+    BugsnagCrashReport *report = [[BugsnagCrashReport alloc] initWithKSReport:dict];
+    XCTAssertEqual(report.depth, 0);
+}
+
+- (void)testHandledReportDepth {
+    // handled reports should use the serialised depth
+    BugsnagHandledState *state = [BugsnagHandledState handledStateWithSeverityReason:HandledException];
+    NSDictionary *dict = @{@"user.state.crash.depth": @2, @"user.handledState": [state toJson]};
+    BugsnagCrashReport *report = [[BugsnagCrashReport alloc] initWithKSReport:dict];
+    XCTAssertEqual(report.depth, 2);
+}
+
+- (void)testUnhandledReportSeverity {
+    // unhandled reports should calculate their own severity
+    NSDictionary *dict = @{@"user.state.crash.severity": @"info"};
+    BugsnagCrashReport *report = [[BugsnagCrashReport alloc] initWithKSReport:dict];
+    XCTAssertEqual(report.severity, BSGSeverityError);
+}
+
+- (void)testHandledReportSeverity {
+    // handled reports should use the serialised depth
+    BugsnagHandledState *state = [BugsnagHandledState handledStateWithSeverityReason:HandledException];
+    NSDictionary *dict = @{@"user.state.crash.severity": @"info", @"user.handledState": [state toJson]};
+    BugsnagCrashReport *report = [[BugsnagCrashReport alloc] initWithKSReport:dict];
+    XCTAssertEqual(report.severity, BSGSeverityWarning);
+}
+
+- (void)testHandledReportMetaData {
+    BugsnagHandledState *state = [BugsnagHandledState handledStateWithSeverityReason:HandledException];
+    BugsnagMetaData *metaData = [BugsnagMetaData new];
+    [metaData addAttribute:@"Foo" withValue:@"Bar" toTabWithName:@"Custom"];
+    NSDictionary *dict = @{@"user.handledState": [state toJson], @"user.metaData": [metaData toDictionary]};
+
+    BugsnagCrashReport *report = [[BugsnagCrashReport alloc] initWithKSReport:dict];
+    XCTAssertNotNil(report.metaData);
+    XCTAssertEqual(report.metaData.count, 1);
+    XCTAssertEqualObjects(report.metaData[@"Custom"][@"Foo"], @"Bar");
+}
+
+- (void)testUnhandledReportMetaData {
+    BugsnagMetaData *metaData = [BugsnagMetaData new];
+    [metaData addAttribute:@"Foo" withValue:@"Bar" toTabWithName:@"Custom"];
+    NSDictionary *dict = @{@"user.metaData": [metaData toDictionary]};
+
+    BugsnagCrashReport *report = [[BugsnagCrashReport alloc] initWithKSReport:dict];
+    XCTAssertNotNil(report.metaData);
+    XCTAssertEqual(report.metaData.count, 1);
+    XCTAssertEqualObjects(report.metaData[@"Custom"][@"Foo"], @"Bar");
+}
+
+- (void)testNoReportMetaData {
+    BugsnagCrashReport *report = [[BugsnagCrashReport alloc] initWithKSReport:@{}];
+    XCTAssertNotNil(report.metaData);
+    XCTAssertEqual(report.metaData.count, 0);
+}
+
 @end