From 8a1187fe69ba50d50e5cbd29dd88c9bea5b4796f Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 15 Mar 2019 17:20:49 +0000 Subject: [PATCH 1/7] feat: send unhandled count in notify_unhandled and start_session native message --- .../com/bugsnag/android/ndk/NativeBridge.java | 16 ++++++++++++---- .../bugsnag/android/ObserverInterfaceTest.java | 3 ++- .../main/java/com/bugsnag/android/Client.java | 12 +++++++++--- .../com/bugsnag/android/NativeInterface.java | 4 ++++ .../java/com/bugsnag/android/SessionTracker.java | 3 ++- 5 files changed, 29 insertions(+), 9 deletions(-) diff --git a/ndk/src/main/java/com/bugsnag/android/ndk/NativeBridge.java b/ndk/src/main/java/com/bugsnag/android/ndk/NativeBridge.java index 73d467c492..a55b089e89 100644 --- 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 metadata = (List)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/sdk/src/androidTest/java/com/bugsnag/android/ObserverInterfaceTest.java b/sdk/src/androidTest/java/com/bugsnag/android/ObserverInterfaceTest.java index ec4e52d33c..87b7fdb7bd 100644 --- 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 sessionInfo = (List)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 diff --git a/sdk/src/main/java/com/bugsnag/android/Client.java b/sdk/src/main/java/com/bugsnag/android/Client.java index 3cb95a79f1..137b4eac23 100644 --- a/sdk/src/main/java/com/bugsnag/android/Client.java +++ b/sdk/src/main/java/com/bugsnag/android/Client.java @@ -917,10 +917,16 @@ void notify(@NonNull Error error, callback.beforeNotify(report); } - if (!error.getHandledState().isUnhandled() && error.getSession() != null) { + if (error.getSession() != null) { setChanged(); - notifyObservers(new NativeInterface.Message( - NativeInterface.MessageType.NOTIFY_HANDLED, error.getExceptionName())); + + if (error.getHandledState().isUnhandled()) { + notifyObservers(new Message( + NativeInterface.MessageType.NOTIFY_UNHANDLED, null)); + } else { + notifyObservers(new Message( + NativeInterface.MessageType.NOTIFY_HANDLED, error.getExceptionName())); + } } switch (style) { diff --git a/sdk/src/main/java/com/bugsnag/android/NativeInterface.java b/sdk/src/main/java/com/bugsnag/android/NativeInterface.java index 3b6026caf3..f45b4d6d49 100644 --- a/sdk/src/main/java/com/bugsnag/android/NativeInterface.java +++ b/sdk/src/main/java/com/bugsnag/android/NativeInterface.java @@ -48,6 +48,10 @@ public enum MessageType { * Send a report for a handled Java exception */ NOTIFY_HANDLED, + /** + * Send a report for an unhandled error in the Java layer + */ + NOTIFY_UNHANDLED, /** * Remove a metadata value. The Message object should be a string array * containing [tab, key] diff --git a/sdk/src/main/java/com/bugsnag/android/SessionTracker.java b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java index ac53d52232..11c6145e59 100644 --- a/sdk/src/main/java/com/bugsnag/android/SessionTracker.java +++ b/sdk/src/main/java/com/bugsnag/android/SessionTracker.java @@ -116,7 +116,8 @@ private void notifySessionStartObserver(Session session) { String startedAt = DateUtils.toIso8601(session.getStartedAt()); notifyObservers(new NativeInterface.Message( NativeInterface.MessageType.START_SESSION, - Arrays.asList(session.getId(), startedAt, session.getHandledCount()))); + Arrays.asList(session.getId(), startedAt, + session.getHandledCount(), session.getUnhandledCount()))); } /** From 751500e8279aa5041f09e7f1815eb4257f3cbf1d Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Fri, 15 Mar 2019 17:22:15 +0000 Subject: [PATCH 2/7] feat: add unhandled_count to native report --- ndk/src/main/jni/bugsnag_ndk.c | 20 ++++++++++++++++++-- ndk/src/main/jni/report.c | 3 ++- ndk/src/main/jni/report.h | 3 ++- ndk/src/main/jni/utils/serializer.c | 2 +- 4 files changed, 23 insertions(+), 5 deletions(-) diff --git a/ndk/src/main/jni/bugsnag_ndk.c b/ndk/src/main/jni/bugsnag_ndk.c index ddd414aba4..498236fc3a 100644 --- 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/report.c b/ndk/src/main/jni/report.c index 97e49e37e9..5ab3e13f2e 100644 --- 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 index 1c126e7ef6..d4651a7fe6 100644 --- a/ndk/src/main/jni/report.h +++ b/ndk/src/main/jni/report.h @@ -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/serializer.c b/ndk/src/main/jni/utils/serializer.c index 3361b3671c..e8e1fb62f8 100644 --- a/ndk/src/main/jni/utils/serializer.c +++ b/ndk/src/main/jni/utils/serializer.c @@ -262,7 +262,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); From 3f42f07da303d1ac3fc7d508845fa9b8aa7e94a4 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 19 Mar 2019 06:21:18 -0700 Subject: [PATCH 3/7] refactor: create bugsnag_report_v1 for migrating reports --- ndk/src/main/jni/utils/migrate.h | 35 ++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 ndk/src/main/jni/utils/migrate.h diff --git a/ndk/src/main/jni/utils/migrate.h b/ndk/src/main/jni/utils/migrate.h new file mode 100644 index 0000000000..30d257abb3 --- /dev/null +++ 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 From 6197628720c5cdb8ed247543ae5dbacd20a9f24c Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 19 Mar 2019 06:24:47 -0700 Subject: [PATCH 4/7] test: add test for migration from bugsnag_report_v1 to bugsnag_report --- ndk/src/test/cpp/test_utils_serialize.c | 56 ++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/ndk/src/test/cpp/test_utils_serialize.c b/ndk/src/test/cpp/test_utils_serialize.c index 17baa6080d..7d107ef64b 100644 --- a/ndk/src/test/cpp/test_utils_serialize.c +++ b/ndk/src/test/cpp/test_utils_serialize.c @@ -1,13 +1,13 @@ #include #include #include +#include #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); From 5f4cd4c53f6aa840b459162c2adc2d58a839aa23 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 19 Mar 2019 07:43:42 -0700 Subject: [PATCH 5/7] feat: add migration between bugsnag_report_v1 and v2 --- ndk/src/main/jni/report.h | 2 +- ndk/src/main/jni/utils/serializer.c | 61 ++++++++++++++++++++++++++--- 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/ndk/src/main/jni/report.h b/ndk/src/main/jni/report.h index d4651a7fe6..fe8504eb23 100644 --- 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 diff --git a/ndk/src/main/jni/utils/serializer.c b/ndk/src/main/jni/utils/serializer.c index e8e1fb62f8..752690ceeb 100644 --- a/ndk/src/main/jni/utils/serializer.c +++ b/ndk/src/main/jni/utils/serializer.c @@ -8,6 +8,7 @@ #include #include #include +#include #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; } From 60a805dea2ecc307201039e18c35b04c58f983e4 Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 19 Mar 2019 08:02:41 -0700 Subject: [PATCH 6/7] docs: add changelog entry --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5efc07bca2..07b93655a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 4.X.X (TBD) + +### Enhancements + +* Add unhandled_events field to native payload +[#445](https://github.com/bugsnag/bugsnag-android/pull/445) + ## 4.12.0 (2019-02-27) ### Enhancements From 2e1c86c261e7ceeee45efeba3f6a942f4a30236a Mon Sep 17 00:00:00 2001 From: fractalwrench Date: Tue, 19 Mar 2019 12:59:08 -0700 Subject: [PATCH 7/7] refactor: increment unhandled_events count in handlers when receiving a fatal err --- ndk/src/main/jni/handlers/cpp_handler.cpp | 1 + ndk/src/main/jni/handlers/signal_handler.c | 1 + 2 files changed, 2 insertions(+) diff --git a/ndk/src/main/jni/handlers/cpp_handler.cpp b/ndk/src/main/jni/handlers/cpp_handler.cpp index 83372f6725..50e347cb77 100644 --- 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 index 12be8ea2b6..51095eb551 100644 --- 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);