Android P FileStore flush fix

CHANGELOG.md

@@ -1,5 +1,12 @@
 # Changelog
 
+## 4.4.1 (TBD)
+
+### Bug fixes
+
+* Ensure that unhandled error reports are always sent immediately on launch for Android P and in situations with no connectivity.
+[#319](https://github.com/bugsnag/bugsnag-android/pull/319)
+
 ## 4.4.0 (2018-05-17)
 
 ### Features

features/startup_crash.feature

@@ -0,0 +1,14 @@
+Feature: All errors are flushed if a startup crash is persisted
+
+Scenario: 1 startup crash and 1 regular crash persisted
+    When I configure the app to run in the "CrashOfflineWithDelay" state
+    And I run "StartupCrashFlushScenario" with the defaults
+    And I wait for 10 seconds
+
+    And I configure the app to run in the "CrashOfflineAtStartup" state
+    And I relaunch the app
+
+    And I configure the app to run in the "No crash" state
+    And I relaunch the app
+    And I wait for 5 seconds
+    Then I should receive 2 requests

features/steps/build_steps.rb

@@ -8,3 +8,12 @@
     And I start the "com.bugsnag.android.mazerunner" Android app using the "com.bugsnag.android.mazerunner.MainActivity" activity
   }
 end
+
+When("I relaunch the app") do
+  step('I force stop the "com.bugsnag.android.mazerunner" Android app')
+  step('I start the "com.bugsnag.android.mazerunner" Android app using the "com.bugsnag.android.mazerunner.MainActivity" activity')
+end
+
+When("I configure the app to run in the {string} state") do |event_metadata|
+  step("I set environment variable \"EVENT_METADATA\" to \"#{event_metadata}\"")
+end

mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/Scenario.kt

@@ -50,11 +50,24 @@ abstract internal class Scenario(protected val config: Configuration,
      * Sets a NOP implementation for the Error Tracking API and the Session Tracking API,
      * preventing delivery
      */
+    @Deprecated("Disable via config instead, using the new delivery API")
     protected fun disableAllDelivery() {
         disableSessionDelivery()
         disableReportDelivery()
     }
 
+    protected fun disableAllDelivery(config: Configuration) {
+        config.delivery = object: Delivery {
+            override fun deliver(payload: SessionTrackingPayload?, config: Configuration?) {
+                throw DeliveryFailureException("Error Delivery NOP", RuntimeException("NOP"))
+            }
+
+            override fun deliver(report: Report?, config: Configuration?) {
+                throw DeliveryFailureException("Session Delivery NOP", RuntimeException("NOP"))
+            }
+        }
+    }
+
     /**
      * Returns a throwable with the message as the current classname
      */

mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/StartupCrashFlushScenario.kt

@@ -0,0 +1,42 @@
+package com.bugsnag.android.mazerunner.scenarios
+
+import android.content.Context
+import android.os.Handler
+import com.bugsnag.android.*
+
+/**
+ * Generates an uncaught exception, catches it, and persists it to disc, preventing any delivery.
+ *
+ * To generate a startup crash, set "eventMetaData" to "StartupCrash", otherwise the default
+ * behaviour is to report a normal crash.
+ *
+ * The mazerunner scenario that tests this works in 3 parts:
+ *
+ * 1. Generate and persist a normal crash on disk, by waiting longer than the threshold for
+ *    a startup crash
+ * 2. Generate and persist a startup crash on disk, by crashing as soon as the scenario starts
+ * 3. Send the stored reports. The startup crash should be delivered synchronously on the main thread,
+ * and the normal crash asynchronously.
+ */
+internal class StartupCrashFlushScenario(config: Configuration,
+                                         context: Context) : Scenario(config, context) {
+
+    override fun run() {
+        if ("CrashOfflineWithDelay" == eventMetaData) {
+            // Part 1 - Persist a regular crash to disk
+            disableAllDelivery(config)
+            super.run()
+            Handler().postDelayed({
+            	throw RuntimeException("Regular crash")
+			}, 6000)
+        } else if ("CrashOfflineAtStartup" == eventMetaData) {
+            // Part 2 - Persist a startup crash to disk
+            disableAllDelivery(config)
+            super.run()
+            throw RuntimeException("Startup crash")
+        } else {
+            // Part 3 - Online, no crashes: send any cached reports
+            super.run()
+        }
+    }
+}

sdk/src/main/java/com/bugsnag/android/ErrorStore.java

@@ -51,38 +51,42 @@ public int compare(File lhs, File rhs) {
     }
 
     void flushOnLaunch() {
-        final List<File> crashReports = findLaunchCrashReports();
-
-        if (crashReports.isEmpty() || config.getLaunchCrashThresholdMs() == 0) {
-            flushAsync(); // if disabled or no startup crash, flush async
-        } else {
-
-            // Block the main thread for a 2 second interval as the app may crash very soon.
-            // The request itself will run in a background thread and will continue after the 2
-            // second period until the request completes, or the app crashes.
-            flushOnLaunchCompleted = false;
-            Logger.info("Attempting to send launch crash reports");
-
-            Async.run(new Runnable() {
-                @Override
-                public void run() {
-                    flushReports(crashReports);
-                    flushOnLaunchCompleted = true;
-                }
-            });
-
-            long waitMs = 0;
-
-            while (!flushOnLaunchCompleted && waitMs < LAUNCH_CRASH_TIMEOUT_MS) {
-                try {
-                    Thread.sleep(LAUNCH_CRASH_POLL_MS);
-                    waitMs += LAUNCH_CRASH_POLL_MS;
-                } catch (InterruptedException exception) {
-                    Logger.warn("Interrupted while waiting for launch crash report request");
+        if (config.getLaunchCrashThresholdMs() != 0) {
+            List<File> storedFiles = findStoredFiles();
+            final List<File> crashReports = findLaunchCrashReports(storedFiles);
+
+            if (!crashReports.isEmpty()) {
+
+                // Block the main thread for a 2 second interval as the app may crash very soon.
+                // The request itself will run in a background thread and will continue after the 2
+                // second period until the request completes, or the app crashes.
+                flushOnLaunchCompleted = false;
+                Logger.info("Attempting to send launch crash reports");
+
+                Async.run(new Runnable() {
+                    @Override
+                    public void run() {
+                        flushReports(crashReports);
+                        flushOnLaunchCompleted = true;
+                    }
+                });
+
+                long waitMs = 0;
+
+                while (!flushOnLaunchCompleted && waitMs < LAUNCH_CRASH_TIMEOUT_MS) {
+                    try {
+                        Thread.sleep(LAUNCH_CRASH_POLL_MS);
+                        waitMs += LAUNCH_CRASH_POLL_MS;
+                    } catch (InterruptedException exception) {
+                        Logger.warn("Interrupted while waiting for launch crash report request");
+                    }
                 }
+                Logger.info("Continuing with Bugsnag initialisation");
             }
-            Logger.info("Continuing with Bugsnag initialisation");
+            cancelQueuedFiles(storedFiles); // cancel all previously found files
         }
+
+        flushAsync(); // flush any remaining errors async that weren't delivered
     }
 
     /**
@@ -130,7 +134,7 @@ private void flushErrorReport(File errorFile) {
         } catch (DeliveryFailureException exception) {
             cancelQueuedFiles(Collections.singleton(errorFile));
             Logger.warn("Could not send previously saved error(s)"
-                        + " to Bugsnag, will try again later", exception);
+                + " to Bugsnag, will try again later", exception);
         } catch (Exception exception) {
             deleteStoredFiles(Collections.singleton(errorFile));
             Logger.warn("Problem sending unsent error from disk", exception);
@@ -141,8 +145,7 @@ boolean isLaunchCrashReport(File file) {
         return file.getName().endsWith("_startupcrash.json");
     }
 
-    private List<File> findLaunchCrashReports() {
-        Collection<File> storedFiles = findStoredFiles();
+    private List<File> findLaunchCrashReports(Collection<File> storedFiles) {
         List<File> launchCrashes = new ArrayList<>();
 
         for (File file : storedFiles) {