From eb3594d643c831033e29abfeff72f8c510c4d089 Mon Sep 17 00:00:00 2001
From: Delisa <kattrali@users.noreply.github.com>
Date: Thu, 13 Sep 2018 16:05:15 -0700
Subject: [PATCH] fix: Record NSException details immediately from C++ handler
 (#313)

This avoids a case where if the NSException handler is overridden
without calling the previous handler, no report is recorded. In the case
that the handler is invoked twice for a given exception, it is only
recorded once by checking that the last recorded exception is the same.
---
 CHANGELOG.md                                  |  4 ++
 .../Sentry/BSG_KSCrashSentry_CPPException.mm  | 13 +++++-
 .../Sentry/BSG_KSCrashSentry_NSException.m    | 43 +++++++++++++++----
 3 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2d5420f99..043b6bd37 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,10 +5,14 @@ Changelog
 
 ### Bug Fixes
 
+* Ensure NSException is captured when handler is overridden
+  [#313](https://github.com/bugsnag/bugsnag-cocoa/pull/313)
+
 * Fix mach handler declaration and imports. This resolves an issue where signal
   codes were less specific than is possible.
   [#314](https://github.com/bugsnag/bugsnag-cocoa/pull/314)
 
+
 ## 5.16.3 (14 Aug 2018)
 
 ### Bug Fixes
diff --git a/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_CPPException.mm b/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_CPPException.mm
index 508478bbb..5da6bb3ce 100644
--- a/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_CPPException.mm
+++ b/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_CPPException.mm
@@ -44,6 +44,15 @@
 #define likely_if(x) if (__builtin_expect(x, 1))
 #define unlikely_if(x) if (__builtin_expect(x, 0))
 
+#ifdef __cplusplus
+extern "C" {
+#endif
+// Internal NSException recorder
+void bsg_recordException(NSException *exception);
+#ifdef __cplusplus
+}
+#endif
+
 // ============================================================================
 #pragma mark - Globals -
 // ============================================================================
@@ -114,9 +123,11 @@ static void CPPExceptionTerminate(void) {
     try {
         throw;
     } catch (NSException *exception) {
-        BSG_KSLOG_DEBUG(@"Detected NSException. Letting the current "
+        BSG_KSLOG_DEBUG(@"Detected NSException. Recording details "
+                        @"and letting the current "
                         @"NSException handler deal with it.");
         isNSException = true;
+        bsg_recordException(exception);
     } catch (std::exception &exc) {
         strncpy(descriptionBuff, exc.what(), sizeof(descriptionBuff));
     }
diff --git a/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_NSException.m b/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_NSException.m
index 346bfb3f9..b72084eb4 100644
--- a/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_NSException.m
+++ b/Source/KSCrash/Source/KSCrash/Recording/Sentry/BSG_KSCrashSentry_NSException.m
@@ -47,11 +47,21 @@
 /** Context to fill with crash information. */
 static BSG_KSCrash_SentryContext *bsg_g_context;
 
+static NSException *bsg_lastHandledException = NULL;
+
 // ============================================================================
 #pragma mark - Callbacks -
 // ============================================================================
 
+
 // Avoiding static methods due to linker issue.
+/**
+ Capture exception details and write a new report. If the exception was
+ recorded before, no new report will be generated.
+
+ @param exception The exception to process
+ */
+void bsg_recordException(NSException *exception);
 
 /** Our custom excepetion handler.
  * Fetch the stack trace from the exception and write a report.
@@ -74,6 +84,30 @@ void bsg_ksnsexc_i_handleException(NSException *exception) {
             bsg_kscrashsentry_uninstall(BSG_KSCrashTypeAll);
         }
 
+        bsg_recordException(exception);
+
+        BSG_KSLOG_DEBUG(
+            @"Crash handling complete. Restoring original handlers.");
+        bsg_kscrashsentry_uninstall(BSG_KSCrashTypeAll);
+
+        if (bsg_g_previousUncaughtExceptionHandler != NULL) {
+            BSG_KSLOG_DEBUG(@"Calling original exception handler.");
+            bsg_g_previousUncaughtExceptionHandler(exception);
+        }
+    }
+}
+
+void bsg_recordException(NSException *exception) {
+    if (bsg_g_installed) {
+        BOOL previouslyHandled = exception == bsg_lastHandledException;
+        if (previouslyHandled) {
+            BSG_KSLOG_DEBUG(@"Handled exception previously, "
+                            @"exiting exception recorder.");
+            return;
+        }
+        bsg_lastHandledException = exception;
+        BSG_KSLOG_DEBUG(@"Writing exception info into a new report");
+
         BSG_KSLOG_DEBUG(@"Suspending all threads.");
         bsg_kscrashsentry_suspendThreads();
 
@@ -95,15 +129,6 @@ void bsg_ksnsexc_i_handleException(NSException *exception) {
 
         BSG_KSLOG_DEBUG(@"Calling main crash handler.");
         bsg_g_context->onCrash();
-
-        BSG_KSLOG_DEBUG(
-            @"Crash handling complete. Restoring original handlers.");
-        bsg_kscrashsentry_uninstall(BSG_KSCrashTypeAll);
-
-        if (bsg_g_previousUncaughtExceptionHandler != NULL) {
-            BSG_KSLOG_DEBUG(@"Calling original exception handler.");
-            bsg_g_previousUncaughtExceptionHandler(exception);
-        }
     }
 }