Merge branch 'master' into pr/332

CHANGELOG.md

@@ -1,6 +1,20 @@
 Changelog
 =========
 
+## TBD
+
+### Bug fixes
+
+* Fix generating an incorrect stacktrace used when logging an exception to
+  Bugsnag from a location other than the original call site (for example, from a
+  logging function or across threads).  If an exception was raised/thrown, then
+  the resulting Bugsnag report from `notify()` will now use the `NSException`
+  instance's call stack addresses to construct the stacktrace, ignoring depth.
+  This fixes an issue in macOS exception reporting where `reportException` is
+  reporting the handler code stacktrace rather than the reported exception
+  stack.
+  [#334](https://github.com/bugsnag/bugsnag-cocoa/pull/334)
+
 ## 5.19.0 (2019-02-28)
 
 Note for Carthage users: this release updates the Xcode configuration to the settings recommended by Xcode 10.

Source/BugsnagCrashReport.h

@@ -181,7 +181,8 @@ __deprecated_msg("Use toJson: instead.");
  */
 @property(readonly, copy, nonnull) NSDictionary *overrides;
 /**
- *  Number of frames to discard at the top of the stacktrace
+ *  Number of frames to discard at the top of the generated stacktrace.
+ *  Stacktraces from raised exceptions are unaffected.
  */
 @property(readwrite) NSUInteger depth;
 /**

Source/BugsnagCrashSentry.h

@@ -20,6 +20,7 @@
 
 - (void)reportUserException:(NSString *)reportName
                      reason:(NSString *)reportMessage
+          originalException:(NSException *)ex
                handledState:(NSDictionary *)handledState
                    appState:(NSDictionary *)appState
           callbackOverrides:(NSDictionary *)overrides

Source/BugsnagCrashSentry.m

@@ -43,6 +43,7 @@ - (void)install:(BugsnagConfiguration *)config
 
 - (void)reportUserException:(NSString *)reportName
                      reason:(NSString *)reportMessage
+          originalException:(NSException *)ex
                handledState:(NSDictionary *)handledState
                    appState:(NSDictionary *)appState
           callbackOverrides:(NSDictionary *)overrides
@@ -52,6 +53,7 @@ - (void)reportUserException:(NSString *)reportName
 
     [[BSG_KSCrash sharedInstance] reportUserException:reportName
                                                reason:reportMessage
+                                    originalException:ex
                                          handledState:handledState
                                              appState:appState
                                     callbackOverrides:overrides

Source/BugsnagNotifier.h

@@ -29,7 +29,7 @@
 #import "BugsnagConfiguration.h"
 #import "BugsnagMetaData.h"
 
-@class BSGConnectivity;
+@class BSGConnectivity, BugsnagSessionTracker;
 
 @interface BugsnagNotifier : NSObject <BugsnagMetaDataDelegate>
 
@@ -38,6 +38,7 @@
 @property(nonatomic, readwrite, retain) BugsnagMetaData *_Nonnull state;
 @property(nonatomic, readwrite, retain) NSDictionary *_Nonnull details;
 @property(nonatomic, readwrite, retain) NSLock *_Nonnull metaDataLock;
+@property(nonatomic, readonly) BugsnagSessionTracker *_Nonnull sessionTracker;
 
 @property(nonatomic) BSGConnectivity *_Nonnull networkReachable;
 @property(readonly) BOOL started;

Source/BugsnagNotifier.m

@@ -179,7 +179,7 @@ void BSGWriteSessionCrashData(BugsnagSession *session) {
 @interface BugsnagNotifier ()
 @property(nonatomic) BugsnagCrashSentry *crashSentry;
 @property(nonatomic) BugsnagErrorReportApiClient *errorReportApiClient;
-@property(nonatomic) BugsnagSessionTracker *sessionTracker;
+@property(nonatomic, readwrite) BugsnagSessionTracker *sessionTracker;
 @end
 
 @implementation BugsnagNotifier
@@ -443,8 +443,10 @@ - (void)notifyError:(NSError *)error
         [BugsnagHandledState handledStateWithSeverityReason:HandledError
                                                    severity:BSGSeverityWarning
                                                   attrValue:error.domain];
-    [self notify:NSStringFromClass([error class])
-             message:error.localizedDescription
+    NSException *wrapper = [NSException exceptionWithName:NSStringFromClass([error class])
+                                                   reason:error.localizedDescription
+                                                 userInfo:error.userInfo];
+    [self notify:wrapper
         handledState:state
                block:^(BugsnagCrashReport *_Nonnull report) {
                  NSMutableDictionary *metadata = [report.metaData mutableCopy];
@@ -472,20 +474,14 @@ - (void)notifyException:(NSException *)exception
         handledStateWithSeverityReason:UserSpecifiedSeverity
                               severity:severity
                              attrValue:nil];
-    [self notify:exception.name ?: NSStringFromClass([exception class])
-             message:exception.reason
-        handledState:state
-               block:block];
+    [self notify:exception handledState:state block:block];
 }
 
 - (void)notifyException:(NSException *)exception
                   block:(void (^)(BugsnagCrashReport *))block {
     BugsnagHandledState *state =
         [BugsnagHandledState handledStateWithSeverityReason:HandledException];
-    [self notify:exception.name ?: NSStringFromClass([exception class])
-             message:exception.reason
-        handledState:state
-               block:block];
+    [self notify:exception handledState:state block:block];
 }
 
 - (void)internalClientNotify:(NSException *_Nonnull)exception
@@ -506,20 +502,14 @@ - (void)internalClientNotify:(NSException *_Nonnull)exception
                               severity:BSGParseSeverity(severity)
                              attrValue:logLevel];
 
-    [self notify:exception.name ?: NSStringFromClass([exception class])
-             message:exception.reason
-        handledState:state
-               block:^(BugsnagCrashReport *_Nonnull report) {
-                 if (block) {
-                     block(report);
-                 }
-               }];
+    [self notify:exception handledState:state block:block];
 }
 
-- (void)notify:(NSString *)exceptionName
-         message:(NSString *)message
+- (void)notify:(NSException *)exception
     handledState:(BugsnagHandledState *_Nonnull)handledState
            block:(void (^)(BugsnagCrashReport *))block {
+    NSString *exceptionName = exception.name ?: NSStringFromClass([exception class]);
+    NSString *message = exception.reason;
     [self.sessionTracker handleHandledErrorEvent];
 
     BugsnagCrashReport *report = [[BugsnagCrashReport alloc]
@@ -550,6 +540,7 @@ - (void)notify:(NSString *)exceptionName
 
     [self.crashSentry reportUserException:reportName
                                    reason:reportMessage
+                        originalException:exception
                              handledState:[handledState toJson]
                                  appState:[self.state toDictionary]
                         callbackOverrides:report.overrides

Source/BugsnagSession.h

@@ -19,6 +19,12 @@
 
 - (_Nonnull instancetype)initWithDictionary:(NSDictionary *_Nonnull)dict;
 
+- (_Nonnull instancetype)initWithId:(NSString *_Nonnull)sessionId
+                          startDate:(NSDate *_Nonnull)startDate
+                               user:(BugsnagUser *_Nullable)user
+                       handledCount:(NSUInteger)handledCount
+                     unhandledCount:(NSUInteger)unhandledCount;
+
 - (NSDictionary *_Nonnull)toJson;
 - (void)stop;
 - (void)resume;

Source/BugsnagSession.m

@@ -52,6 +52,21 @@ - (instancetype)initWithDictionary:(NSDictionary *_Nonnull)dict {
     return self;
 }
 
+- (_Nonnull instancetype)initWithId:(NSString *_Nonnull)sessionId
+                          startDate:(NSDate *_Nonnull)startDate
+                               user:(BugsnagUser *_Nullable)user
+                       handledCount:(NSUInteger)handledCount
+                     unhandledCount:(NSUInteger)unhandledCount {
+    if (self = [super init]) {
+        _sessionId = sessionId;
+        _startedAt = startDate;
+        _unhandledCount = unhandledCount;
+        _handledCount = handledCount;
+        _user = user;
+    }
+    return self;
+}
+
 - (NSDictionary *)toJson {
     NSMutableDictionary *dict = [NSMutableDictionary new];
     BSGDictInsertIfNotNil(dict, self.sessionId, kBugsnagSessionId);

Source/BugsnagSessionTracker.h

@@ -41,6 +41,17 @@ typedef void (^SessionTrackerCallback)(BugsnagSession *newSession);
  */
 - (void)startNewSessionIfAutoCaptureEnabled;
 
+/**
+ Update the details of the current session to account for externally reported
+ session information. Current session details are included in subsequent crash
+ reports.
+ */
+- (void)registerExistingSession:(NSString *)sessionId
+                      startedAt:(NSDate *)startedAt
+                           user:(BugsnagUser *)user
+                   handledCount:(NSUInteger)handledCount
+                 unhandledCount:(NSUInteger)unhandledCount;
+
 /**
  Handle the app foregrounding event. If more than 30s has elapsed since being
  sent to the background, records a new session if session auto-capture is

Source/BugsnagSessionTracker.m

@@ -111,6 +111,25 @@ - (void)startNewSessionWithAutoCaptureValue:(BOOL)isAutoCaptured {
     [self.apiClient deliverSessionsInStore:self.sessionStore];
 }
 
+- (void)registerExistingSession:(NSString *)sessionId
+                      startedAt:(NSDate *)startedAt
+                           user:(BugsnagUser *)user
+                   handledCount:(NSUInteger)handledCount
+                 unhandledCount:(NSUInteger)unhandledCount {
+    if (sessionId == nil || startedAt == nil) {
+        self.currentSession = nil;
+    } else {
+        self.currentSession = [[BugsnagSession alloc] initWithId:sessionId
+                                                       startDate:startedAt
+                                                            user:user
+                                                    handledCount:handledCount
+                                                  unhandledCount:unhandledCount];
+    }
+    if (self.callback) {
+        self.callback(self.currentSession);
+    }
+}
+
 #pragma mark - Handling events
 
 - (void)handleAppBackgroundEvent {

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

@@ -173,6 +173,7 @@ typedef enum {
  *
  * @param name The exception name (for namespacing exception types).
  * @param reason A description of why the exception occurred
+ * @param exception The exception which was thrown (if any)
  * @param handledState The severity, reason, and handled-ness of the report
  * @param appState breadcrumbs and other app environmental info
  * @param overrides Report fields overridden by callbacks, collated in the
@@ -185,6 +186,7 @@ typedef enum {
  */
 - (void)reportUserException:(NSString *)name
                      reason:(NSString *)reason
+          originalException:(NSException *)exception
                handledState:(NSDictionary *)handledState
                    appState:(NSDictionary *)appState
           callbackOverrides:(NSDictionary *)overrides

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

@@ -347,6 +347,7 @@ - (void)deleteAllReports {
 
 - (void)reportUserException:(NSString *)name
                      reason:(NSString *)reason
+          originalException:(NSException *)exception
                handledState:(NSDictionary *)handledState
                    appState:(NSDictionary *)appState
           callbackOverrides:(NSDictionary *)overrides
@@ -356,7 +357,17 @@ - (void)reportUserException:(NSString *)name
            terminateProgram:(BOOL)terminateProgram {
     const char *cName = [name cStringUsingEncoding:NSUTF8StringEncoding];
     const char *cReason = [reason cStringUsingEncoding:NSUTF8StringEncoding];
+    NSArray *addresses = [exception callStackReturnAddresses];
+    NSUInteger numFrames = [addresses count];
+    uintptr_t *callstack = malloc(numFrames * sizeof(*callstack));
+    for (NSUInteger i = 0; i < numFrames; i++) {
+        callstack[i] = [addresses[i] unsignedLongValue];
+    }
+    if (numFrames > 0) {
+        depth = 0; // reset depth if the stack does not need to be generated
+    }
     bsg_kscrash_reportUserException(cName, cReason,
+                                    callstack, numFrames,
                                     [handledState[@"currentSeverity"] UTF8String],
                                     [self encodeAsJSONString:handledState],
                                     [self encodeAsJSONString:overrides],

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

@@ -122,10 +122,11 @@ int bsg_create_filepath(char *base, char filepath[bsg_filepath_len], char severi
  */
 void bsg_kscrash_i_onCrash(char severity, char *errorClass) {
     BSG_KSLOG_DEBUG("Updating application state to note crash.");
-    bsg_kscrashstate_notifyAppCrash();
 
     BSG_KSCrash_Context *context = crashContext();
 
+    bsg_kscrashstate_notifyAppCrash(context->crash.crashType);
+
     if (context->config.printTraceToStdout) {
         bsg_kscrashreport_logCrash(context);
     }
@@ -285,6 +286,8 @@ void bsg_kscrash_setCrashNotifyCallback(
 }
 
 void bsg_kscrash_reportUserException(const char *name, const char *reason,
+                                     uintptr_t *stackAddresses,
+                                     unsigned long stackLength,
                                      const char *severity,
                                      const char *handledState,
                                      const char *overrides,
@@ -293,7 +296,11 @@ void bsg_kscrash_reportUserException(const char *name, const char *reason,
                                      const char *config,
                                      int discardDepth,
                                      bool terminateProgram) {
-    bsg_kscrashsentry_reportUserException(name, reason, severity, handledState, overrides,
+    bsg_kscrashsentry_reportUserException(name, reason,
+                                          stackAddresses,
+                                          stackLength,
+                                          severity,
+                                          handledState, overrides,
                                           metadata, appState, config, discardDepth,
                                           terminateProgram);
 }

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

@@ -176,6 +176,8 @@ void bsg_kscrash_setCrashNotifyCallback(
  *
  * @param name The exception name (for namespacing exception types).
  * @param reason A description of why the exception occurred.
+ * @param stackAddresses An array of addresses or NULL
+ * @param stackLength The number of addresses in stackAddresses
  * @param handledState The severity, reason, and handled-ness of the report
  * @param appState breadcrumbs and other app environmental info
  * @param overrides Report fields overridden by callbacks, collated in the
@@ -187,6 +189,8 @@ void bsg_kscrash_setCrashNotifyCallback(
  * Terminate the program instead.
  */
 void bsg_kscrash_reportUserException(const char *name, const char *reason,
+                                     uintptr_t *stackAddresses,
+                                     unsigned long stackLength,
                                      const char *severity,
                                      const char *handledState,
                                      const char *overrides,

Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashState.c

@@ -352,7 +352,7 @@ void bsg_kscrashstate_notifyAppTerminate(void) {
     bsg_kscrashstate_i_saveState(state, stateFilePath);
 }
 
-void bsg_kscrashstate_notifyAppCrash(void) {
+void bsg_kscrashstate_notifyAppCrash(BSG_KSCrashType type) {
     BSG_KSCrash_State *const state = bsg_g_state;
     const char *const stateFilePath = bsg_g_stateFilePath;
 
@@ -365,7 +365,7 @@ void bsg_kscrashstate_notifyAppCrash(void) {
         state->backgroundDurationSinceLaunch += duration;
         state->backgroundDurationSinceLastCrash += duration;
     }
-    state->crashedThisLaunch = true;
+    state->crashedThisLaunch |= type != BSG_KSCrashTypeUserReported;
     bsg_kscrashstate_i_saveState(state, stateFilePath);
 }
 

Source/KSCrash/Source/KSCrash/Recording/BSG_KSCrashState.h

@@ -37,6 +37,7 @@ extern "C" {
 
 #include <stdbool.h>
 #include <stdint.h>
+#include "BSG_KSCrashType.h"
 
 typedef struct {
     // Saved data
@@ -114,7 +115,7 @@ void bsg_kscrashstate_notifyAppTerminate(void);
 
 /** Notify the crash reporter that the application has crashed.
  */
-void bsg_kscrashstate_notifyAppCrash(void);
+void bsg_kscrashstate_notifyAppCrash(BSG_KSCrashType type);
 
 /** Read-only access into the current state.
  */