Merge branch 'master' into remove-obsolete-test

CHANGELOG.md

@@ -1,13 +1,19 @@
 # Changelog
 
+## 4.X.X (TBD)
+
+### Bug fixes
+
+* Prevent errors from leaving a self-referencing breadcrumb
+ [#391](https://github.com/bugsnag/bugsnag-android/pull/391)
+
 ## 4.9.3 (2018-11-29)
 
 ### Bug fixes
 
 * Handle null values in MetaData.mergeMaps, preventing potential NPE
  [#386](https://github.com/bugsnag/bugsnag-android/pull/386)
 
-
 ## 4.9.2 (2018-11-07)
 
 ### Bug fixes

examples/sdk-app-example/build.gradle

@@ -1,6 +1,7 @@
 buildscript {
     repositories {
         google()
+        mavenCentral()
         jcenter()
     }
     dependencies {
@@ -15,6 +16,7 @@ apply plugin: 'kotlin-android'
 
 repositories {
     google()
+    mavenCentral()
     jcenter()
 }
 

features/fixtures/mazerunner/build.gradle

@@ -1,5 +1,6 @@
 repositories {
     google()
+    mavenCentral()
     jcenter()
     flatDir {
         dirs 'libs'
@@ -10,6 +11,7 @@ apply plugin: 'com.android.application'
 buildscript {
     repositories {
         google()
+        mavenCentral()
         jcenter()
     }
     ext.kotlin_version = '1.2.30'

features/fixtures/mazerunner/src/main/java/com/bugsnag/android/mazerunner/scenarios/InForegroundScenario.kt

@@ -0,0 +1,32 @@
+package com.bugsnag.android.mazerunner.scenarios
+
+import android.content.Context
+import com.bugsnag.android.Bugsnag
+import com.bugsnag.android.Configuration
+import java.lang.Thread
+import android.os.Handler
+import android.os.HandlerThread
+
+/**
+ * Sends a handled exception to Bugsnag, which has a short delay to allow the app to remain
+ * in the foreground for ~1 second
+ */
+internal class InForegroundScenario(config: Configuration,
+                                    context: Context) : Scenario(config, context) {
+    init {
+        config.setAutoCaptureSessions(false)
+    }
+
+    override fun run() {
+        super.run()
+
+        val thread = HandlerThread("HandlerThread")
+        thread.start()
+        Handler(thread.looper).post {
+            Thread.sleep(5000)
+            Bugsnag.notify(generateException())
+        }
+
+    }
+
+}

features/in_foreground.feature

@@ -0,0 +1,18 @@
+Feature: In foreground field populates correctly
+
+Scenario: Test handled exception after delay
+    When I run "InForegroundScenario"
+    Then I should receive a request
+    And the request is a valid for the error reporting API
+    And the event "app.inForeground" is true
+
+#    Duration in foreground should be a non-zero integer
+    And the payload field "events.0.app.durationInForeground" is greater than 0
+
+Scenario: Test handled exception in background
+    When I run "InForegroundScenario" and press the home button
+    And I press the home button
+    Then I should receive a request
+    And the request is a valid for the error reporting API
+    And the event "app.inForeground" is false
+    And the payload field "events.0.app.durationInForeground" equals 0

features/steps/build_steps.rb

@@ -175,3 +175,8 @@
     assert_not_nil(frame['lineNumber'], "The lineNumber of frame #{index} is nil")
   end
 end
+
+Then(/^the payload field "(.+)" is greater than (\d+)(?: for request (\d+))?$/) do |field_path, int_value, request_index|
+  observed_value = read_key_path(find_request(request_index)[:body], field_path)
+  assert(observed_value > int_value)
+end

ndk/build.gradle

@@ -6,8 +6,9 @@ apply plugin: 'com.jfrog.bintray'
 archivesBaseName = "bugsnag-android-ndk"
 
 repositories {
-    jcenter()
     google()
+    mavenCentral()
+    jcenter()
 }
 
 android {

sdk/build.gradle

@@ -8,6 +8,7 @@ apply plugin: 'kotlin-android'
 
 repositories {
     google()
+    mavenCentral()
     jcenter()
 }
 

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

@@ -898,7 +898,9 @@ public void run() {
             default:
                 break;
         }
+    }
 
+    private void leaveErrorBreadcrumb(@NonNull Error error) {
         // Add a breadcrumb for this error occurring
         String exceptionMessage = error.getExceptionMessage();
         Map<String, String> message = Collections.singletonMap("message", exceptionMessage);
@@ -1222,10 +1224,12 @@ void deliver(@NonNull Report report, @NonNull Error error) {
         try {
             config.getDelivery().deliver(report, config);
             Logger.info("Sent 1 new error to Bugsnag");
+            leaveErrorBreadcrumb(error);
         } catch (DeliveryFailureException exception) {
             Logger.warn("Could not send error(s) to Bugsnag,"
                 + " saving to disk to send later", exception);
             errorStore.write(error);
+            leaveErrorBreadcrumb(error);
         } catch (Exception exception) {
             Logger.warn("Problem sending error to Bugsnag", exception);
         }

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

@@ -36,10 +36,10 @@ class SessionTracker extends Observable implements Application.ActivityLifecycle
     private final SessionStore sessionStore;
 
     // This most recent time an Activity was stopped.
-    private AtomicLong activityLastStoppedAtMs = new AtomicLong(0);
+    private AtomicLong lastExitedForegroundMs = new AtomicLong(0);
 
     // The first Activity in this 'session' was started at this time.
-    private AtomicLong activityFirstStartedAtMs = new AtomicLong(0);
+    private AtomicLong lastEnteredForegroundMs = new AtomicLong(0);
     private AtomicReference<Session> currentSession = new AtomicReference<>();
     private Semaphore flushingRequest = new Semaphore(1);
 
@@ -261,7 +261,7 @@ void startFirstSession(Activity activity) {
         Session session = currentSession.get();
         if (session == null) {
             long nowMs = System.currentTimeMillis();
-            activityFirstStartedAtMs.set(nowMs);
+            lastEnteredForegroundMs.set(nowMs);
             startNewSession(new Date(nowMs), client.getUser(), true);
             foregroundActivities.add(getActivityName(activity));
         }
@@ -282,20 +282,24 @@ void startFirstSession(Activity activity) {
      */
     void updateForegroundTracker(String activityName, boolean activityStarting, long nowMs) {
         if (activityStarting) {
-            long noActivityRunningForMs = nowMs - activityLastStoppedAtMs.get();
+            long noActivityRunningForMs = nowMs - lastExitedForegroundMs.get();
 
             //FUTURE:SM Race condition between isEmpty and put
-            if (foregroundActivities.isEmpty()
-                && noActivityRunningForMs >= timeoutMs
-                && configuration.shouldAutoCaptureSessions()) {
+            if (foregroundActivities.isEmpty()) {
+                lastEnteredForegroundMs.set(nowMs);
 
-                activityFirstStartedAtMs.set(nowMs);
-                startNewSession(new Date(nowMs), client.getUser(), true);
+                if (noActivityRunningForMs >= timeoutMs
+                    && configuration.shouldAutoCaptureSessions()) {
+                    startNewSession(new Date(nowMs), client.getUser(), true);
+                }
             }
             foregroundActivities.add(activityName);
         } else {
             foregroundActivities.remove(activityName);
-            activityLastStoppedAtMs.set(nowMs);
+
+            if (foregroundActivities.isEmpty()) {
+                lastExitedForegroundMs.set(nowMs);
+            }
         }
         setChanged();
         notifyObservers(new NativeInterface.Message(
@@ -310,7 +314,7 @@ boolean isInForeground() {
     //FUTURE:SM This shouldnt be here
     long getDurationInForegroundMs(long nowMs) {
         long durationMs = 0;
-        long sessionStartTimeMs = activityFirstStartedAtMs.get();
+        long sessionStartTimeMs = lastEnteredForegroundMs.get();
 
         if (isInForeground() && sessionStartTimeMs != 0) {
             durationMs = nowMs - sessionStartTimeMs;