Merge pull request #445 from bugsnag/anr-detection-ndk

Add unhandled_events field to native payload
bugsnag · Mar 21, 2019 · d3cfcb6 · d3cfcb6
2 parents fdef465 + 2e1c86c
commit d3cfcb6
Show file tree

Hide file tree

Showing 14 changed files with 203 additions and 22 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,9 @@
 * Add ANR detection to bugsnag-android
 [#442](https://github.com/bugsnag/bugsnag-android/pull/442)
 
+* Add unhandled_events field to native payload
+[#445](https://github.com/bugsnag/bugsnag-android/pull/445)
+
 ## 4.12.0 (2019-02-27)
 
 ### Enhancements

diff --git a/ndk/src/main/java/com/bugsnag/android/ndk/NativeBridge.java b/ndk/src/main/java/com/bugsnag/android/ndk/NativeBridge.java
@@ -48,13 +48,16 @@ public static native void addBreadcrumb(String name, String type, String timesta
 
     public static native void addHandledEvent();
 
+    public static native void addUnhandledEvent();
+
     public static native void clearBreadcrumbs();
 
     public static native void clearMetadataTab(String tab);
 
     public static native void removeMetadata(String tab, String key);
 
-    public static native void startedSession(String sessionID, String key, int handledCount);
+    public static native void startedSession(String sessionID, String key,
+                                             int handledCount, int unhandledCount);
 
     public static native void stoppedSession();
 
@@ -127,6 +130,9 @@ public void update(Observable observable, Object rawMessage) {
             case NOTIFY_HANDLED:
                 addHandledEvent();
                 break;
+            case NOTIFY_UNHANDLED:
+                addUnhandledEvent();
+                break;
             case REMOVE_METADATA:
                 handleRemoveMetadata(arg);
                 break;
@@ -318,14 +324,16 @@ private void handleStartSession(Object arg) {
         if (arg instanceof List) {
             @SuppressWarnings("unchecked")
             List<Object> metadata = (List<Object>)arg;
-            if (metadata.size() == 3) {
+            if (metadata.size() == 4) {
                 Object id = metadata.get(0);
                 Object startTime = metadata.get(1);
                 Object handledCount = metadata.get(2);
+                Object unhandledCount = metadata.get(3);
 
                 if (id instanceof String && startTime instanceof String
-                    && handledCount instanceof Integer) {
-                    startedSession((String)id, (String)startTime, (Integer) handledCount);
+                    && handledCount instanceof Integer && unhandledCount instanceof Integer) {
+                    startedSession((String)id, (String)startTime,
+                        (Integer) handledCount, (Integer) unhandledCount);
                     return;
                 }
             }

diff --git a/ndk/src/main/jni/bugsnag_ndk.c b/ndk/src/main/jni/bugsnag_ndk.c
@@ -129,15 +129,30 @@ Java_com_bugsnag_android_ndk_NativeBridge_addHandledEvent(JNIEnv *env,
   bsg_release_env_write_lock();
 }
 
+JNIEXPORT void JNICALL
+Java_com_bugsnag_android_ndk_NativeBridge_addUnhandledEvent(JNIEnv *env,
+                                                            jobject _this) {
+    if (bsg_global_env == NULL)
+        return;
+    bsg_request_env_write_lock();
+    bugsnag_report *report = &bsg_global_env->next_report;
+
+    if (bugsnag_report_has_session(report)) {
+        report->unhandled_events++;
+    }
+    bsg_release_env_write_lock();
+}
+
 JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_startedSession(
-    JNIEnv *env, jobject _this, jstring session_id_, jstring start_date_, jint handled_count) {
+    JNIEnv *env, jobject _this, jstring session_id_, jstring start_date_,
+    jint handled_count, jint unhandled_count) {
   if (bsg_global_env == NULL || session_id_ == NULL)
     return;
   char *session_id = (char *)(*env)->GetStringUTFChars(env, session_id_, 0);
   char *started_at = (char *)(*env)->GetStringUTFChars(env, start_date_, 0);
   bsg_request_env_write_lock();
   bugsnag_report_start_session(&bsg_global_env->next_report, session_id,
-                               started_at, handled_count);
+                               started_at, handled_count, unhandled_count);
 
   bsg_release_env_write_lock();
   (*env)->ReleaseStringUTFChars(env, session_id_, session_id);
@@ -154,6 +169,7 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_stoppedSession(
     memset(report->session_id, 0, strlen(report->session_id));
     memset(report->session_start, 0, strlen(report->session_start));
     report->handled_events = 0;
+    report->unhandled_events = 0;
     bsg_release_env_write_lock();
 }
 

diff --git a/ndk/src/main/jni/handlers/cpp_handler.cpp b/ndk/src/main/jni/handlers/cpp_handler.cpp
@@ -87,6 +87,7 @@ void bsg_handle_cpp_terminate() {
 
   bsg_global_env->handling_crash = true;
   bsg_populate_report_as(bsg_global_env);
+  bsg_global_env->next_report.unhandled_events++;
   bsg_global_env->next_report.exception.frame_count = bsg_unwind_stack(
       bsg_global_env->unwind_style,
       bsg_global_env->next_report.exception.stacktrace, NULL, NULL);

diff --git a/ndk/src/main/jni/handlers/signal_handler.c b/ndk/src/main/jni/handlers/signal_handler.c
@@ -174,6 +174,7 @@ void bsg_handle_signal(int signum, siginfo_t *info,
 
   bsg_global_env->handling_crash = true;
   bsg_populate_report_as(bsg_global_env);
+  bsg_global_env->next_report.unhandled_events++;
   bsg_global_env->next_report.exception.frame_count = bsg_unwind_stack(
       bsg_global_env->signal_unwind_style,
       bsg_global_env->next_report.exception.stacktrace, info, user_context);

diff --git a/ndk/src/main/jni/report.c b/ndk/src/main/jni/report.c
@@ -83,11 +83,12 @@ void bugsnag_report_remove_metadata_tab(bugsnag_report *report, char *section) {
 }
 
 void bugsnag_report_start_session(bugsnag_report *report, char *session_id,
-                                  char *started_at, int handled_count) {
+                                  char *started_at, int handled_count, int unhandled_count) {
   bsg_strncpy_safe(report->session_id, session_id, sizeof(report->session_id));
   bsg_strncpy_safe(report->session_start, started_at,
                    sizeof(report->session_start));
   report->handled_events = handled_count;
+  report->unhandled_events = unhandled_count;
 }
 
 void bugsnag_report_set_context(bugsnag_report *report, char *value) {

diff --git a/ndk/src/main/jni/report.h b/ndk/src/main/jni/report.h
@@ -31,7 +31,7 @@
 /**
  * Version of the bugsnag_report struct. Serialized to report header.
  */
-#define BUGSNAG_REPORT_VERSION 1
+#define BUGSNAG_REPORT_VERSION 2
 
 #define BUGSNAG_USER_INFO_LEN 64
 #ifdef __cplusplus
@@ -292,6 +292,7 @@ typedef struct {
   char session_id[33];
   char session_start[33];
   int handled_events;
+  int unhandled_events;
 } bugsnag_report;
 
 void bugsnag_report_add_metadata_double(bugsnag_report *report, char *section,
@@ -315,7 +316,7 @@ void bugsnag_report_set_user_email(bugsnag_report *report, char *value);
 void bugsnag_report_set_user_id(bugsnag_report *report, char *value);
 void bugsnag_report_set_user_name(bugsnag_report *report, char *value);
 void bugsnag_report_start_session(bugsnag_report *report, char *session_id,
-                                  char *started_at, int handled_count);
+                                  char *started_at, int handled_count, int unhandled_count);
 bool bugsnag_report_has_session(bugsnag_report *report);
 
 #ifdef __cplusplus

diff --git a/ndk/src/main/jni/utils/migrate.h b/ndk/src/main/jni/utils/migrate.h
@@ -0,0 +1,35 @@
+#ifndef BUGSNAG_MIGRATE_H
+#define BUGSNAG_MIGRATE_H
+
+#include "report.h"
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+typedef struct {
+    bsg_library notifier;
+    bsg_app_info app;
+    bsg_device_info device;
+    bsg_user user;
+    bsg_exception exception;
+    bugsnag_metadata metadata;
+
+    int crumb_count;
+    // Breadcrumbs are a ring; the first index moves as the
+    // structure is filled and replaced.
+    int crumb_first_index;
+    bugsnag_breadcrumb breadcrumbs[BUGSNAG_CRUMBS_MAX];
+
+    char context[64];
+    bsg_severity_t severity;
+
+    char session_id[33];
+    char session_start[33];
+    int handled_events;
+} bugsnag_report_v1;
+
+#ifdef __cplusplus
+}
+#endif
+#endif
diff --git a/ndk/src/main/jni/utils/serializer.c b/ndk/src/main/jni/utils/serializer.c
@@ -8,6 +8,7 @@
 #include <string.h>
 #include <time.h>
 #include <unistd.h>
+#include <utils/migrate.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -40,18 +41,68 @@ bugsnag_report *bsg_deserialize_report_from_file(char *filepath) {
   return bsg_report_read(fd);
 }
 
+bugsnag_report_v1 *bsg_report_v1_read(int fd) {
+    size_t report_size = sizeof(bugsnag_report_v1);
+    bugsnag_report_v1 *report = malloc(report_size);
+
+    ssize_t len = read(fd, report, report_size);
+    if (len != report_size) {
+        return NULL;
+    }
+    return report;
+}
+
+bugsnag_report *bsg_report_v2_read(int fd) {
+    size_t report_size = sizeof(bugsnag_report);
+    bugsnag_report *report = malloc(report_size);
+
+    ssize_t len = read(fd, report, report_size);
+    if (len != report_size) {
+        return NULL;
+    }
+    return report;
+}
+
 bugsnag_report *bsg_report_read(int fd) {
   bsg_report_header *header = bsg_report_header_read(fd);
   if (header == NULL) {
     return NULL;
   }
-  // FUTURE: use header info to check version/endianness & migrate
+
+  int report_version = header->version;
   free(header);
-  bugsnag_report *report = malloc(sizeof(bugsnag_report));
 
-  ssize_t len = read(fd, report, sizeof(bugsnag_report));
-  if (len != sizeof(bugsnag_report)) {
-    return NULL;
+  bugsnag_report *report = NULL;
+
+  if (report_version == 1) { // 'report->unhandled_events' was added in v2
+      bugsnag_report_v1 *report_v1 = bsg_report_v1_read(fd);
+
+      if (report_v1 != NULL) {
+          report = malloc(sizeof(bugsnag_report));
+
+          report->notifier = report_v1->notifier;
+          report->app = report_v1->app;
+          report->device = report_v1->device;
+          report->user = report_v1->user;
+          report->exception = report_v1->exception;
+          report->metadata = report_v1->metadata;
+          report->crumb_count = report_v1->crumb_count;
+          report->crumb_first_index = report_v1->crumb_first_index;
+
+          size_t breadcrumb_size = sizeof(bugsnag_breadcrumb) * BUGSNAG_CRUMBS_MAX;
+          memcpy(&report->breadcrumbs, report_v1->breadcrumbs, breadcrumb_size);
+
+          strcpy(report->context, report_v1->context);
+          report->severity = report_v1->severity;
+          strcpy(report->session_id, report_v1->session_id);
+          strcpy(report->session_start, report_v1->session_start);
+          report->handled_events = report_v1->handled_events;
+          report->unhandled_events = 1;
+
+          free(report_v1);
+      }
+  } else {
+      report = bsg_report_v2_read(fd);
   }
   return report;
 }
@@ -262,7 +313,7 @@ char *bsg_serialize_report_to_json_string(bugsnag_report *report) {
         json_object_dotset_string(event, "session.id", report->session_id);
         json_object_dotset_number(event, "session.events.handled",
                                   report->handled_events);
-        json_object_dotset_number(event, "session.events.unhandled", 1);
+        json_object_dotset_number(event, "session.events.unhandled", report->unhandled_events);
     }
 
     json_object_set_string(exception, "errorClass", report->exception.name);

diff --git a/ndk/src/test/cpp/test_utils_serialize.c b/ndk/src/test/cpp/test_utils_serialize.c
@@ -1,13 +1,13 @@
 #include <greatest/greatest.h>
 #include <utils/serializer.h>
 #include <stdlib.h>
+#include <utils/migrate.h>
 
 #define SERIALIZE_TEST_FILE "/data/data/com.bugsnag.android.ndk.test/cache/foo.crash"
 
 bugsnag_breadcrumb *init_breadcrumb(const char *name, const char *message, bsg_breadcrumb_t type);
 
-bugsnag_report *bsg_generate_report(void) {
-  bugsnag_report *report = calloc(1, sizeof(bugsnag_report));
+void generate_basic_report(bugsnag_report *report) {
   strcpy(report->context, "SomeActivity");
   strcpy(report->exception.name, "SIGBUS");
   strcpy(report->exception.message, "POSIX is serious about oncoming traffic");
@@ -43,8 +43,25 @@ bugsnag_report *bsg_generate_report(void) {
   bugsnag_breadcrumb *crumb2 = init_breadcrumb("enable blasters", "this is a drill.", BSG_CRUMB_USER);
   bugsnag_report_add_breadcrumb(report, crumb2);
 
+  report->handled_events = 1;
+  report->unhandled_events = 1;
+  strcpy(report->session_id, "f1ab");
+  strcpy(report->session_start, "2019-03-19T12:58:19+00:00");
+}
+
+bugsnag_report_v1 *bsg_generate_report_v1(void) {
+  bugsnag_report_v1 *report = calloc(1, sizeof(bugsnag_report_v1));
+  generate_basic_report(report);
   return report;
 }
+
+bugsnag_report *bsg_generate_report(void) {
+  bugsnag_report *report = calloc(1, sizeof(bugsnag_report));
+  generate_basic_report(report);
+  report->unhandled_events = 2;
+  return report;
+}
+
 TEST test_report_to_file(void) {
   bsg_environment *env = malloc(sizeof(bsg_environment));
   env->report_header.version = 7;
@@ -79,6 +96,29 @@ TEST test_file_to_report(void) {
   PASS();
 }
 
+TEST test_report_v1_migration(void) {
+  bsg_environment *env = malloc(sizeof(bsg_environment));
+  env->report_header.version = 1;
+  env->report_header.big_endian = 1;
+  strcpy(env->report_header.os_build, "macOS Sierra");
+  bugsnag_report_v1 *generated_report = bsg_generate_report_v1();
+  memcpy(&env->next_report, generated_report, sizeof(bugsnag_report_v1));
+  strcpy(env->next_report_path, SERIALIZE_TEST_FILE);
+  bsg_serialize_report_to_file(env);
+
+  bugsnag_report *report = bsg_deserialize_report_from_file(SERIALIZE_TEST_FILE);
+  ASSERT(report != NULL);
+  ASSERT(strcmp("f1ab", report->session_id) == 0);
+  ASSERT(strcmp("2019-03-19T12:58:19+00:00", report->session_start) == 0);
+  ASSERT_EQ(1, report->handled_events);
+  ASSERT_EQ(1, report->unhandled_events);
+
+  free(generated_report);
+  free(env);
+  free(report);
+  PASS();
+}
+
 // helper function
 JSON_Value *bsg_generate_json(void) {
   bugsnag_report *report = bsg_generate_report();
@@ -104,6 +144,16 @@ TEST test_report_app_info_to_json(void) {
   PASS();
 }
 
+TEST test_session_handled_counts(void) {
+  JSON_Value *root_value = bsg_generate_json();
+  JSON_Object *event = json_value_get_object(root_value);
+  ASSERT(strcmp("f1ab", json_object_dotget_string(event, "session.id")) == 0);
+  ASSERT(strcmp("2019-03-19T12:58:19+00:00", json_object_dotget_string(event, "session.startedAt")) == 0);
+  ASSERT_EQ(1, json_object_dotget_number(event, "session.events.handled"));
+  ASSERT_EQ(2, json_object_dotget_number(event, "session.events.unhandled"));
+  PASS();
+}
+
 TEST test_report_context_to_json(void) {
   JSON_Value *root_value = bsg_generate_json();
   JSON_Object *event = json_value_get_object(root_value);
@@ -192,6 +242,8 @@ TEST test_report_breadcrumbs_to_json(void) {
 SUITE(serialize_utils) {
   RUN_TEST(test_report_to_file);
   RUN_TEST(test_file_to_report);
+  RUN_TEST(test_report_v1_migration);
+  RUN_TEST(test_session_handled_counts);
   RUN_TEST(test_report_context_to_json);
   RUN_TEST(test_report_app_info_to_json);
   RUN_TEST(test_report_device_info_to_json);

diff --git a/sdk/src/androidTest/java/com/bugsnag/android/ObserverInterfaceTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ObserverInterfaceTest.java
@@ -153,10 +153,11 @@ public void testStartSessionSendsMessage() throws InterruptedException {
         client.startSession();
         List<Object> sessionInfo = (List<Object>)findMessageInQueue(
                 NativeInterface.MessageType.START_SESSION, List.class);
-        assertEquals(3, sessionInfo.size());
+        assertEquals(4, sessionInfo.size());
         assertTrue(sessionInfo.get(0) instanceof String);
         assertTrue(sessionInfo.get(1) instanceof String);
         assertTrue(sessionInfo.get(2) instanceof Integer);
+        assertTrue(sessionInfo.get(3) instanceof Integer);
     }
 
     @Test