Make discard classes apply to native crashes as well

bugsnag · Jun 23, 2022 · c7e1868 · c7e1868
1 parent 79e4cb2
commit c7e1868
Show file tree

Hide file tree

Showing 13 changed files with 107 additions and 12 deletions.
diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/NativeInterface.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/NativeInterface.java
@@ -300,6 +300,18 @@ public static void registerSession(long startedAt, @Nullable String sessionId,
                 unhandledCount, handledCount);
     }
 
+    /**
+     * Ask if an error class is on the configurable discard list.
+     * This is used by the native layer to decide whether to pass an event to
+     * deliverReport() or not.
+     *
+     * @param name The error class to ask about.
+     */
+    @SuppressWarnings("unused")
+    public static boolean isDiscardErrorClass(@NonNull String name) {
+        return getClient().getConfig().getDiscardClasses().contains(name);
+    }
+
     /**
      * Deliver a report, serialized as an event JSON payload.
      *

diff --git a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c
@@ -226,6 +226,7 @@ Java_com_bugsnag_android_ndk_NativeBridge_deliverReportAtPath(
   jbyteArray jstage = NULL;
   char *payload = NULL;
   jstring japi_key = NULL;
+  jstring errorClass = NULL;
 
   if (!bsg_jni_cache->initialized) {
     BUGSNAG_LOG("deliverReportAtPath failed: JNI cache not initialized.");
@@ -247,6 +248,13 @@ Java_com_bugsnag_android_ndk_NativeBridge_deliverReportAtPath(
     goto exit;
   }
 
+  errorClass = bsg_safe_new_string_utf(env, event->error.errorClass);
+  if (bsg_safe_call_static_boolean_method(
+          env, bsg_jni_cache->NativeInterface,
+          bsg_jni_cache->NativeInterface_isDiscardErrorClass, errorClass)) {
+    goto exit;
+  }
+
   payload = bsg_serialize_event_to_json_string(event);
   if (payload == NULL) {
     BUGSNAG_LOG("Failed to serialize event as JSON: %s", event_path);
@@ -276,6 +284,7 @@ Java_com_bugsnag_android_ndk_NativeBridge_deliverReportAtPath(
   }
 
 exit:
+  bsg_safe_delete_local_ref(env, errorClass);
   bsg_safe_release_string_utf_chars(env, _report_path, event_path);
   if (event != NULL) {
     bsg_safe_release_byte_array_elements(env, jstage,

diff --git a/bugsnag-plugin-android-ndk/src/main/jni/jni_cache.c b/bugsnag-plugin-android-ndk/src/main/jni/jni_cache.c
@@ -145,6 +145,8 @@ bool bsg_jni_cache_init(JNIEnv *env) {
   CACHE_STATIC_METHOD(
       NativeInterface, NativeInterface_notify, "notify",
       "([B[BLcom/bugsnag/android/Severity;[Ljava/lang/StackTraceElement;)V");
+  CACHE_STATIC_METHOD(NativeInterface, NativeInterface_isDiscardErrorClass,
+                      "isDiscardErrorClass", "(Ljava/lang/String;)Z");
   CACHE_STATIC_METHOD(NativeInterface, NativeInterface_deliverReport,
                       "deliverReport", "([B[BLjava/lang/String;Z)V");
   CACHE_STATIC_METHOD(NativeInterface, NativeInterface_leaveBreadcrumb,

diff --git a/bugsnag-plugin-android-ndk/src/main/jni/jni_cache.h b/bugsnag-plugin-android-ndk/src/main/jni/jni_cache.h
@@ -51,6 +51,7 @@ typedef struct {
   jmethodID NativeInterface_getContext;
   jmethodID NativeInterface_notify;
   jmethodID NativeInterface_leaveBreadcrumb;
+  jmethodID NativeInterface_isDiscardErrorClass;
   jmethodID NativeInterface_deliverReport;
 
   jclass StackTraceElement;

diff --git a/bugsnag-plugin-android-ndk/src/main/jni/safejni.c b/bugsnag-plugin-android-ndk/src/main/jni/safejni.c
@@ -244,6 +244,21 @@ jobject bsg_safe_call_static_object_method(JNIEnv *env, jclass clz,
   return obj;
 }
 
+jboolean bsg_safe_call_static_boolean_method(JNIEnv *env, jclass clz,
+                                             jmethodID method, ...) {
+  if (env == NULL || clz == NULL || method == NULL) {
+    return false;
+  }
+  va_list args;
+  va_start(args, method);
+  jboolean result = (*env)->CallStaticBooleanMethodV(env, clz, method, args);
+  va_end(args);
+  if (bsg_check_and_clear_exc(env)) {
+    return false;
+  }
+  return result;
+}
+
 void bsg_safe_delete_local_ref(JNIEnv *env, jobject obj) {
   if (env == NULL || obj == NULL) {
     return;

diff --git a/bugsnag-plugin-android-ndk/src/main/jni/safejni.h b/bugsnag-plugin-android-ndk/src/main/jni/safejni.h
@@ -155,6 +155,14 @@ void bsg_safe_call_static_void_method(JNIEnv *env, jclass clz, jmethodID method,
 jobject bsg_safe_call_static_object_method(JNIEnv *env, jclass clz,
                                            jmethodID method, ...);
 
+/**
+ * A safe wrapper for the JNI's CallStaticBooleanMethod. This method checks if
+ * an exception is pending and if so clears it so that execution can continue.
+ * On failure or exception, this function return false.
+ */
+jboolean bsg_safe_call_static_boolean_method(JNIEnv *env, jclass clz,
+                                             jmethodID method, ...);
+
 /**
  * A safe wrapper for the JNI's DeleteLocalRef. This method checks if the env
  * is NULL and no-ops if so.

diff --git a/features/fixtures/mazerunner/app/detekt-baseline.xml b/features/fixtures/mazerunner/app/detekt-baseline.xml
@@ -1,6 +1,6 @@
-<?xml version="1.0" ?>
+<?xml version='1.0' encoding='UTF-8'?>
 <SmellBaseline>
-  <ManuallySuppressedIssues></ManuallySuppressedIssues>
+  <ManuallySuppressedIssues/>
   <CurrentIssues>
     <ID>MagicNumber:MainActivity.kt$MainActivity$1000</ID>
   </CurrentIssues>

diff --git a/features/fixtures/mazerunner/cxx-scenarios-bugsnag/detekt-baseline.xml b/features/fixtures/mazerunner/cxx-scenarios-bugsnag/detekt-baseline.xml
@@ -1,6 +1,6 @@
-<?xml version="1.0" ?>
+<?xml version='1.0' encoding='UTF-8'?>
 <SmellBaseline>
-  <ManuallySuppressedIssues></ManuallySuppressedIssues>
+  <ManuallySuppressedIssues/>
   <CurrentIssues>
     <ID>MagicNumber:CXXExceptionSmokeScenario.kt$CXXExceptionSmokeScenario$500</ID>
     <ID>MagicNumber:CXXExceptionSmokeScenario.kt$CXXExceptionSmokeScenario$999</ID>

diff --git a/features/fixtures/mazerunner/cxx-scenarios/detekt-baseline.xml b/features/fixtures/mazerunner/cxx-scenarios/detekt-baseline.xml
@@ -1,8 +1,9 @@
-<?xml version="1.0" ?>
+<?xml version='1.0' encoding='UTF-8'?>
 <SmellBaseline>
-  <ManuallySuppressedIssues></ManuallySuppressedIssues>
+  <ManuallySuppressedIssues/>
   <CurrentIssues>
     <ID>MagicNumber:CXXDelayedCrashScenario.kt$CXXDelayedCrashScenario$405</ID>
+    <ID>MagicNumber:CXXIgnoredSigabrtScenario.kt$CXXIgnoredSigabrtScenario$2726</ID>
     <ID>MagicNumber:CXXSessionInfoCrashScenario.kt$CXXSessionInfoCrashScenario$3</ID>
     <ID>MagicNumber:CXXSessionInfoCrashScenario.kt$CXXSessionInfoCrashScenario$3837</ID>
     <ID>MagicNumber:CXXThrowSomethingOutsideReleaseStagesScenario.kt$CXXThrowSomethingOutsideReleaseStagesScenario$23</ID>

diff --git a/features/fixtures/mazerunner/cxx-scenarios/src/main/cpp/cxx-scenarios.cpp b/features/fixtures/mazerunner/cxx-scenarios/src/main/cpp/cxx-scenarios.cpp
@@ -125,6 +125,18 @@ Java_com_bugsnag_android_mazerunner_scenarios_CXXSigabrtScenario_crash(JNIEnv *e
   return value / x / 8;
 }
 
+JNIEXPORT jint JNICALL
+Java_com_bugsnag_android_mazerunner_scenarios_CXXIgnoredSigabrtScenario_crash(JNIEnv *env,
+                                                                               jobject thiz,
+                                                                               jint value) {
+  int x = 38;
+  if (value > 0) {
+    raise(SIGABRT);
+  }
+  printf("Yeah, Steve, I remember. You said Wolf 359 was an inside job.\n");
+  return value / x / 8;
+}
+
 JNIEXPORT int JNICALL
 Java_com_bugsnag_android_mazerunner_scenarios_CXXSigfpeScenario_crash(JNIEnv *env,
                                                                       jobject instance,

diff --git a/...arios/src/main/java/com/bugsnag/android/mazerunner/scenarios/CXXIgnoredSigabrtScenario.kt b/...arios/src/main/java/com/bugsnag/android/mazerunner/scenarios/CXXIgnoredSigabrtScenario.kt
@@ -0,0 +1,22 @@
+package com.bugsnag.android.mazerunner.scenarios
+
+import android.content.Context
+import com.bugsnag.android.Configuration
+
+internal class CXXIgnoredSigabrtScenario(
+    config: Configuration,
+    context: Context,
+    eventMetadata: String
+) : Scenario(config, context, eventMetadata) {
+
+    init {
+        config.discardClasses = setOf(eventMetadata)
+    }
+
+    external fun crash(value: Int): Int
+
+    override fun startScenario() {
+        super.startScenario()
+        crash(2726)
+    }
+}
diff --git a/features/fixtures/mazerunner/jvm-scenarios/detekt-baseline.xml b/features/fixtures/mazerunner/jvm-scenarios/detekt-baseline.xml
@@ -1,14 +1,13 @@
-<?xml version="1.0" ?>
+<?xml version='1.0' encoding='UTF-8'?>
 <SmellBaseline>
-  <ManuallySuppressedIssues></ManuallySuppressedIssues>
+  <ManuallySuppressedIssues/>
   <CurrentIssues>
     <ID>MagicNumber:AnrHelper.kt$1000</ID>
-    <ID>MagicNumber:AnrHelper.kt$&lt;no name provided&gt;$60000</ID>
+    <ID>MagicNumber:AnrHelper.kt$&lt;no name provided>$60000</ID>
     <ID>MagicNumber:AutoDetectAnrsFalseScenario.kt$AutoDetectAnrsFalseScenario$100000</ID>
     <ID>MagicNumber:AutoDetectAnrsTrueScenario.kt$AutoDetectAnrsTrueScenario$100000</ID>
     <ID>MagicNumber:BugsnagInitScenario.kt$BugsnagInitScenario$25</ID>
     <ID>MagicNumber:DiscardBigEventsScenario.kt$DiscardBigEventsScenario$1024</ID>
-    <ID>MagicNumber:DiscardOldEventsScenario.kt$DiscardOldEventsScenario$2000</ID>
     <ID>MagicNumber:DiscardOldEventsScenario.kt$DiscardOldEventsScenario$60</ID>
     <ID>MagicNumber:DiscardOldSessionScenario.kt$DiscardOldSessionScenario$100</ID>
     <ID>MagicNumber:DiscardOldSessionScenario.kt$DiscardOldSessionScenario$35</ID>
@@ -23,9 +22,9 @@
     <ID>MagicNumber:Scenario.kt$Scenario$100</ID>
     <ID>MagicNumber:Scenario.kt$Scenario$1000</ID>
     <ID>MagicNumber:StartupCrashFlushScenario.kt$StartupCrashFlushScenario$6000</ID>
-    <ID>MagicNumber:TestHarnessHooks.kt$&lt;no name provided&gt;$500</ID>
+    <ID>MagicNumber:TestHarnessHooks.kt$&lt;no name provided>$500</ID>
     <ID>MagicNumber:TrimmedStacktraceScenario.kt$TrimmedStacktraceScenario$100000</ID>
-    <ID>ThrowingExceptionsWithoutMessageOrCause:AnrHelper.kt$&lt;no name provided&gt;$IllegalStateException()</ID>
+    <ID>ThrowingExceptionsWithoutMessageOrCause:AnrHelper.kt$&lt;no name provided>$IllegalStateException()</ID>
     <ID>ThrowingExceptionsWithoutMessageOrCause:BugsnagInitScenario.kt$BugsnagInitScenario$RuntimeException()</ID>
     <ID>ThrowingExceptionsWithoutMessageOrCause:CustomPluginNotifierDescriptionScenario.kt$CustomPluginNotifierDescriptionScenario$RuntimeException()</ID>
     <ID>ThrowingExceptionsWithoutMessageOrCause:TestHarnessHooks.kt$RuntimeException()</ID>

diff --git a/features/full_tests/ignored_reports.feature b/features/full_tests/ignored_reports.feature
@@ -25,3 +25,17 @@ Feature: Reports are ignored
     When I run "CXXThrowSomethingOutsideReleaseStagesScenario" and relaunch the crashed app
     And I configure Bugsnag for "CXXThrowSomethingOutsideReleaseStagesScenario"
     Then Bugsnag confirms it has no errors to send
+
+  Scenario: SIGABRT ignored
+    When I run "CXXIgnoredSigabrtScenario" and relaunch the crashed app
+    And I configure Bugsnag for "CXXIgnoredSigabrtScenario"
+    And I configure the app to run in the "SIGABRT" state
+    And I should receive no errors
+
+  Scenario: SIGABRT not ignored
+    When I run "CXXIgnoredSigabrtScenario" and relaunch the crashed app
+    And I configure Bugsnag for "CXXIgnoredSigabrtScenario"
+    And I configure the app to run in the "SIGBUS" state
+    And I wait to receive an error
+    Then the error is valid for the error reporting API version "4.0" for the "Android Bugsnag Notifier" notifier
+    And the exception "errorClass" equals "SIGABRT"