Merge pull request #1573 from bugsnag/releases/5.18.0

v5.18.0
bugsnag · Jan 5, 2022 · 2fd4ec6 · 2fd4ec6
2 parents 9807e53 + 0f7d809
commit 2fd4ec6
Show file tree

Hide file tree

Showing 29 changed files with 395 additions and 425 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,26 @@
 # Changelog
 
+## 5.18.0 (2022-01-05)
+
+### Enhancements
+
+* Improve the memory use and performance overhead when handling the delivery response status codes
+  [#1558](https://github.com/bugsnag/bugsnag-android/pull/1558)
+
+* Harden ndk layer through use of const keyword
+  [#1566](https://github.com/bugsnag/bugsnag-android/pull/1566)
+
+### Bug fixes
+
+* Delete persisted NDK events earlier in delivery process
+  [#1562](https://github.com/bugsnag/bugsnag-android/pull/1562)
+
+* Add null checks for strlen()
+  [#1563](https://github.com/bugsnag/bugsnag-android/pull/1563)
+
+* Catch IOException when logging response status code
+  [#1567](https://github.com/bugsnag/bugsnag-android/pull/1567)
+
 ## 5.17.0 (2021-12-08)
 
 ### Enhancements

diff --git a/Gemfile b/Gemfile
@@ -4,7 +4,7 @@ source "https://rubygems.org"
 #gem 'bugsnag-maze-runner', path: '../maze-runner'
 
 # Or a specific release:
-gem 'bugsnag-maze-runner', git: 'https://github.com/bugsnag/maze-runner', tag: 'v6.3.0'
+gem 'bugsnag-maze-runner', git: 'https://github.com/bugsnag/maze-runner', tag: 'v6.8.0'
 
 # Or follow master:
 #gem 'bugsnag-maze-runner', git: 'https://github.com/bugsnag/maze-runner'

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -1,20 +1,20 @@
 GIT
   remote: https://github.com/bugsnag/maze-runner
-  revision: c9d5240f9ccbc5a3440e8aa72330f52cbdc6a082
-  tag: v6.3.0
+  revision: fe12189f83aad154f54221ee0fcd41b483d3c0d1
+  tag: v6.8.0
   specs:
-    bugsnag-maze-runner (6.3.0)
+    bugsnag-maze-runner (6.8.0)
       appium_lib (~> 11.2.0)
+      bugsnag (~> 6.24)
       cucumber (~> 7.1)
       cucumber-expressions (~> 6.0.0)
       curb (~> 0.9.6)
-      minitest (~> 5.0)
       optimist (~> 3.0.1)
       os (~> 1.0.0)
       rake (~> 12.3.3)
       rubyzip (~> 2.3.2)
       selenium-webdriver (~> 3.11)
-      test-unit (~> 3.3.0)
+      test-unit (~> 3.5.2)
       webrick (~> 1.7.0)
 
 GEM
@@ -27,8 +27,11 @@ GEM
     appium_lib_core (4.7.1)
       faye-websocket (~> 0.11.0)
       selenium-webdriver (~> 3.14, >= 3.14.1)
+    bugsnag (6.24.1)
+      concurrent-ruby (~> 1.0)
     builder (3.2.4)
     childprocess (3.0.0)
+    concurrent-ruby (1.1.9)
     cucumber (7.1.0)
       builder (~> 3.2, >= 3.2.4)
       cucumber-core (~> 10.1, >= 10.1.0)
@@ -46,7 +49,7 @@ GEM
       cucumber-gherkin (~> 22.0, >= 22.0.0)
       cucumber-messages (~> 17.1, >= 17.1.1)
       cucumber-tag-expressions (~> 4.0, >= 4.0.2)
-    cucumber-create-meta (6.0.2)
+    cucumber-create-meta (6.0.4)
       cucumber-messages (~> 17.1, >= 17.1.1)
       sys-uname (~> 1.2, >= 1.2.2)
     cucumber-cucumber-expressions (14.0.0)
@@ -68,17 +71,16 @@ GEM
       eventmachine (>= 0.12.0)
       websocket-driver (>= 0.5.1)
     ffi (1.15.4)
-    license_finder (6.14.1)
+    license_finder (6.15.0)
       bundler
       rubyzip (>= 1, < 3)
       thor (~> 1.0.1)
       tomlrb (>= 1.3, < 2.1)
       with_env (= 1.1.0)
       xml-simple (~> 1.1.5)
-    mime-types (3.3.1)
+    mime-types (3.4.1)
       mime-types-data (~> 3.2015)
-    mime-types-data (3.2021.0901)
-    minitest (5.14.4)
+    mime-types-data (3.2021.1115)
     multi_test (0.1.2)
     nokogiri (1.12.5-x86_64-darwin)
       racc (~> 1.4)
@@ -94,7 +96,7 @@ GEM
       rubyzip (>= 1.2.2)
     sys-uname (1.2.2)
       ffi (~> 1.1)
-    test-unit (3.3.9)
+    test-unit (3.5.3)
       power_assert
     thor (1.0.1)
     tomlrb (1.3.0)
@@ -114,4 +116,4 @@ DEPENDENCIES
   license_finder (~> 6.13)
 
 BUNDLED WITH
-   2.2.20
+   2.2.33
diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/DefaultDelivery.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/DefaultDelivery.kt
@@ -105,32 +105,38 @@ internal class DefaultDelivery(
     }
 
     private fun logRequestInfo(code: Int, conn: HttpURLConnection, status: DeliveryStatus) {
-        logger.i(
-            "Request completed with code $code, " +
-                "message: ${conn.responseMessage}, " +
-                "headers: ${conn.headerFields}"
-        )
-
-        conn.inputStream.bufferedReader().use {
-            logger.d("Received request response: ${it.readText()}")
+        runCatching {
+            logger.i(
+                "Request completed with code $code, " +
+                    "message: ${conn.responseMessage}, " +
+                    "headers: ${conn.headerFields}"
+            )
+        }
+        runCatching {
+            conn.inputStream.bufferedReader().use {
+                logger.d("Received request response: ${it.readText()}")
+            }
         }
 
-        if (status != DeliveryStatus.DELIVERED) {
-            conn.errorStream.bufferedReader().use {
-                logger.w("Request error details: ${it.readText()}")
+        runCatching {
+            if (status != DeliveryStatus.DELIVERED) {
+                conn.errorStream.bufferedReader().use {
+                    logger.w("Request error details: ${it.readText()}")
+                }
             }
         }
     }
 
     internal fun getDeliveryStatus(responseCode: Int): DeliveryStatus {
-        val unrecoverableCodes = IntRange(HTTP_BAD_REQUEST, 499).filter {
-            it != HTTP_CLIENT_TIMEOUT && it != 429
-        }
-
-        return when (responseCode) {
-            in HTTP_OK..299 -> DeliveryStatus.DELIVERED
-            in unrecoverableCodes -> DeliveryStatus.FAILURE
+        return when {
+            responseCode in HTTP_OK..299 -> DeliveryStatus.DELIVERED
+            isUnrecoverableStatusCode(responseCode) -> DeliveryStatus.FAILURE
             else -> DeliveryStatus.UNDELIVERED
         }
     }
+
+    private fun isUnrecoverableStatusCode(responseCode: Int) =
+        responseCode in HTTP_BAD_REQUEST..499 && // 400-499 are considered unrecoverable
+            responseCode != HTTP_CLIENT_TIMEOUT && // except for 408
+            responseCode != 429 // and 429
 }
diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Notifier.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/Notifier.kt
@@ -7,7 +7,7 @@ import java.io.IOException
  */
 class Notifier @JvmOverloads constructor(
     var name: String = "Android Bugsnag Notifier",
-    var version: String = "5.17.0",
+    var version: String = "5.18.0",
     var url: String = "https://bugsnag.com"
 ) : JsonStream.Streamable {
 

diff --git a/...com/bugsnag/android/BugsnagTestUtils.java → ...com/bugsnag/android/BugsnagTestUtils.java b/...com/bugsnag/android/BugsnagTestUtils.java → ...com/bugsnag/android/BugsnagTestUtils.java
@@ -3,7 +3,7 @@
 import com.bugsnag.android.internal.ImmutableConfig;
 import com.bugsnag.android.internal.ImmutableConfigKt;
 
-import org.jetbrains.annotations.NotNull;
+import androidx.annotation.NonNull;
 
 import java.io.File;
 import java.io.IOException;
@@ -39,6 +39,19 @@ static ImmutableConfig generateImmutableConfig() {
         return convert(generateConfiguration());
     }
 
+    static ImmutableConfig generateImmutableConfig(Configuration config) {
+        config.setDelivery(generateDelivery());
+        config.setLogger(NoopLogger.INSTANCE);
+        config.setProjectPackages(Collections.singleton("com.example.foo"));
+        try {
+            File dir = Files.createTempDirectory("test").toFile();
+            config.setPersistenceDirectory(dir);
+        } catch (IOException ignored) {
+            // ignore IO exception
+        }
+        return ImmutableConfigKt.convertToImmutableConfig(config);
+    }
+
     static EventPayload generateEventPayload(ImmutableConfig config) {
         return new EventPayload(config.getApiKey(), generateEvent(), new Notifier(), config);
     }
@@ -87,17 +100,17 @@ static DeviceWithState generateDeviceWithState() {
 
     public static Delivery generateDelivery() {
         return new Delivery() {
-            @NotNull
+            @NonNull
             @Override
-            public DeliveryStatus deliver(@NotNull EventPayload payload,
-                                          @NotNull DeliveryParams deliveryParams) {
+            public DeliveryStatus deliver(@NonNull EventPayload payload,
+                                          @NonNull DeliveryParams deliveryParams) {
                 return DeliveryStatus.DELIVERED;
             }
 
-            @NotNull
+            @NonNull
             @Override
-            public DeliveryStatus deliver(@NotNull Session payload,
-                                          @NotNull DeliveryParams deliveryParams) {
+            public DeliveryStatus deliver(@NonNull Session payload,
+                                          @NonNull DeliveryParams deliveryParams) {
                 return DeliveryStatus.DELIVERED;
             }
         };
@@ -111,4 +124,8 @@ public static AppWithState generateAppWithState() {
     public static App generateApp() {
         return new App(generateImmutableConfig(), null, null, null, null, null);
     }
+
+    static MetadataState generateMetadataState() {
+        return new MetadataState();
+    }
 }
diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/DeliveryTest.kt
@@ -14,5 +14,10 @@ class DeliveryTest {
         assertEquals(DeliveryStatus.UNDELIVERED, delivery.getDeliveryStatus(408))
         assertEquals(DeliveryStatus.UNDELIVERED, delivery.getDeliveryStatus(429))
         assertEquals(DeliveryStatus.FAILURE, delivery.getDeliveryStatus(400))
+        assertEquals(DeliveryStatus.FAILURE, delivery.getDeliveryStatus(401))
+        assertEquals(DeliveryStatus.FAILURE, delivery.getDeliveryStatus(498))
+        assertEquals(DeliveryStatus.FAILURE, delivery.getDeliveryStatus(499))
+        assertEquals(DeliveryStatus.UNDELIVERED, delivery.getDeliveryStatus(408))
+        assertEquals(DeliveryStatus.UNDELIVERED, delivery.getDeliveryStatus(429))
     }
 }
diff --git a/bugsnag-plugin-android-anr/src/test/java/com/bugsnag/android/BugsnagTestUtils.java b/bugsnag-plugin-android-anr/src/test/java/com/bugsnag/android/BugsnagTestUtils.java
diff --git a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag.c b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag.c
@@ -89,7 +89,7 @@ void bsg_populate_notify_stacktrace(JNIEnv *env, bugsnag_stackframe *stacktrace,
 
     // populate method
     jstring method = NULL;
-    if (strlen(frame.method) == 0) {
+    if (bsg_strlen(frame.method) == 0) {
       char frame_address[32];
       snprintf(frame_address, sizeof(frame_address), "0x%lx",
                (unsigned long)frame.frame_address);
@@ -258,7 +258,7 @@ void bugsnag_set_user_env(JNIEnv *env, const char *id, const char *email,
   bsg_safe_delete_local_ref(env, interface_class);
 }
 
-jfieldID bsg_parse_jcrumb_type(JNIEnv *env, bugsnag_breadcrumb_type type,
+jfieldID bsg_parse_jcrumb_type(JNIEnv *env, const bugsnag_breadcrumb_type type,
                                jclass type_class) {
   const char *type_sig = "Lcom/bugsnag/android/BreadcrumbType;";
   if (type == BSG_CRUMB_USER) {
@@ -282,7 +282,7 @@ jfieldID bsg_parse_jcrumb_type(JNIEnv *env, bugsnag_breadcrumb_type type,
 }
 
 void bugsnag_leave_breadcrumb_env(JNIEnv *env, const char *message,
-                                  bugsnag_breadcrumb_type type) {
+                                  const bugsnag_breadcrumb_type type) {
   jclass interface_class = NULL;
   jmethodID leave_breadcrumb_method = NULL;
   jclass type_class = NULL;