Merge pull request #1643 from bugsnag/releases/v5.22.0

v5.22.0
bugsnag · Mar 31, 2022 · a5824a3 · a5824a3
2 parents bb01b06 + eeb518a
commit a5824a3
Show file tree

Hide file tree

Showing 90 changed files with 1,451 additions and 666 deletions.
diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
@@ -1,6 +1,6 @@
 name: instrumentation_tests
 
-on: [push, pull_request]
+on: [ push, pull_request ]
 
 env:
   GRADLE_OPTS: "-Dorg.gradle.jvmargs=-Xmx4g -Dorg.gradle.daemon=false -Dkotlin.incremental=false -Dorg.gradle.parallel=true"
@@ -16,16 +16,29 @@ jobs:
           - 29
 
     steps:
-      - uses: actions/checkout@v2.3.5
+      - uses: actions/checkout@v3
         with:
           submodules: recursive
-      - uses: gradle/wrapper-validation-action@v1.0.4
+      - uses: gradle/wrapper-validation-action@v1
 
-      - uses: actions/setup-java@v2
+      - uses: actions/setup-java@v3
         with:
           distribution: 'zulu'
           java-version: 11
 
+      - name: Gradle cache
+        uses: gradle/gradle-build-action@v2
+
+      - name: Create AVD and generate snapshot for caching
+        if: steps.avd-cache.outputs.cache-hit != 'true'
+        uses: reactivecircus/android-emulator-runner@v2
+        with:
+          api-level: ${{ matrix.api-level }}
+          force-avd-creation: false
+          emulator-options: -no-window -gpu swiftshader_indirect -noaudio -no-boot-anim -camera-back none
+          disable-animations: false
+          script: echo "Generated AVD snapshot for caching."
+
       - name: Run Tests
         uses: reactivecircus/android-emulator-runner@v2
         with:

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,22 @@
 # Changelog
 
+## 5.22.0 (2022-03-31)
+
+### Enhancements
+
+* Added `Bugsnag.isStarted()` to test whether the Bugsnag client is in the middle of initializing. This can be used to guard uses of the Bugsnag API that are either on separate threads early in the app's start-up and so not guaranteed to be executed after `Bugsnag.start` has completed, or where Bugsnag may not have been started at all due to some internal app logic.
+ [slack-jallen](https://github.com/slack-jallen):[#1621](https://github.com/bugsnag/bugsnag-android/pull/1621)
+ [#1640](https://github.com/bugsnag/bugsnag-android/pull/1640)
+
+* Events and Sessions will be discarded if they cannot be uploaded and are older than 60 days or larger than 1MB
+  [#1633](https://github.com/bugsnag/bugsnag-android/pull/1633)
+
+### Bug fixes
+
+* Fixed potentially [thread-unsafe access](https://github.com/bugsnag/bugsnag-android/issues/883) when invoking `Bugsnag` static methods across different threads whilst `Bugsnag.start` is still in-flight. It is now safe to call any `Bugsnag` static method once `Bugsnag.start` has _begun_ executing, as access to the client singleton is controlled by a lock, so the new `isStarted` method (see above) should only be required where it cannot be determined whether the call to `Bugsnag.start` has begun or you do not want to wait. [#1638](https://github.com/bugsnag/bugsnag-android/pull/1638)
+* Calling `bugsnag_event_set_context` with NULL `context` correctly clears the event context again
+  [#1637](https://github.com/bugsnag/bugsnag-android/pull/1637)
+
 ## 5.21.0 (2022-03-17)
 
 ### Enhancements

diff --git a/bugsnag-android-core/detekt-baseline.xml b/bugsnag-android-core/detekt-baseline.xml
@@ -22,6 +22,7 @@
     <ID>MagicNumber:DefaultDelivery.kt$DefaultDelivery$429</ID>
     <ID>MagicNumber:DefaultDelivery.kt$DefaultDelivery$499</ID>
     <ID>MagicNumber:LastRunInfoStore.kt$LastRunInfoStore$3</ID>
+    <ID>MagicNumber:SessionFilenameInfo.kt$SessionFilenameInfo.Companion$35</ID>
     <ID>MaxLineLength:LastRunInfo.kt$LastRunInfo$return "LastRunInfo(consecutiveLaunchCrashes=$consecutiveLaunchCrashes, crashed=$crashed, crashedDuringLaunch=$crashedDuringLaunch)"</ID>
     <ID>MaxLineLength:ThreadState.kt$ThreadState$"[${allThreads.size - maxThreadCount} threads omitted as the maxReportedThreads limit ($maxThreadCount) was exceeded]"</ID>
     <ID>ProtectedMemberInFinalClass:ConfigInternal.kt$ConfigInternal$protected val plugins = HashSet&lt;Plugin>()</ID>
@@ -38,7 +39,9 @@
     <ID>SwallowedException:DeviceDataCollector.kt$DeviceDataCollector$catch (exception: Exception) { logger.w("Could not get battery status") }</ID>
     <ID>SwallowedException:DeviceDataCollector.kt$DeviceDataCollector$catch (exception: Exception) { logger.w("Could not get locationStatus") }</ID>
     <ID>SwallowedException:DeviceIdStore.kt$DeviceIdStore$catch (exc: OverlappingFileLockException) { Thread.sleep(FILE_LOCK_WAIT_MS) }</ID>
+    <ID>SwallowedException:EventFilenameInfo.kt$EventFilenameInfo.Companion$catch (e: Exception) { return -1 }</ID>
     <ID>SwallowedException:PluginClient.kt$PluginClient$catch (exc: ClassNotFoundException) { logger.d("Plugin '$clz' is not on the classpath - functionality will not be enabled.") null }</ID>
+    <ID>SwallowedException:SessionFilenameInfo.kt$SessionFilenameInfo.Companion$catch (e: Exception) { return 0 }</ID>
     <ID>TooManyFunctions:ConfigInternal.kt$ConfigInternal : CallbackAwareMetadataAwareUserAwareFeatureFlagAware</ID>
     <ID>TooManyFunctions:EventInternal.kt$EventInternal : FeatureFlagAwareStreamableMetadataAwareUserAware</ID>
     <ID>UnnecessaryAbstractClass:DependencyModule.kt$DependencyModule</ID>

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/Bugsnag.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/Bugsnag.java
@@ -69,25 +69,35 @@ public static Client start(@NonNull Context androidContext, @NonNull Configurati
         return client;
     }
 
+    /**
+     * Returns true if one of the <code>start</code> methods have been has been called and
+     * so Bugsnag is initialized; false if <code>start</code> has not been called and the
+     * other methods will throw IllegalStateException.
+     */
+    public static boolean isStarted() {
+        return client != null;
+    }
+
     private static void logClientInitWarning() {
         getClient().logger.w("Multiple Bugsnag.start calls detected. Ignoring.");
     }
 
     /**
      * Bugsnag uses the concept of "contexts" to help display and group your errors. Contexts
      * represent what was happening in your application at the time an error occurs.
-     *
+     * <p>
      * In an android app the "context" is automatically set as the foreground Activity.
      * If you would like to set this value manually, you should alter this property.
      */
-    @Nullable public static String getContext() {
+    @Nullable
+    public static String getContext() {
         return getClient().getContext();
     }
 
     /**
      * Bugsnag uses the concept of "contexts" to help display and group your errors. Contexts
      * represent what was happening in your application at the time an error occurs.
-     *
+     * <p>
      * In an android app the "context" is automatically set as the foreground Activity.
      * If you would like to set this value manually, you should alter this property.
      */
@@ -115,15 +125,15 @@ public static User getUser() {
     /**
      * Add a "on error" callback, to execute code at the point where an error report is
      * captured in Bugsnag.
-     *
+     * <p>
      * You can use this to add or modify information attached to an Event
      * before it is sent to your dashboard. You can also return
      * <code>false</code> from any callback to prevent delivery. "on error"
      * callbacks do not run before reports generated in the event
      * of immediate app termination from crashes in C/C++ code.
-     *
+     * <p>
      * For example:
-     *
+     * <p>
      * Bugsnag.addOnError(new OnErrorCallback() {
      * public boolean run(Event event) {
      * event.setSeverity(Severity.INFO);
@@ -140,6 +150,7 @@ public static void addOnError(@NonNull OnErrorCallback onError) {
 
     /**
      * Removes a previously added "on error" callback
+     *
      * @param onError the callback to remove
      */
     public static void removeOnError(@NonNull OnErrorCallback onError) {
@@ -149,12 +160,12 @@ public static void removeOnError(@NonNull OnErrorCallback onError) {
     /**
      * Add an "on breadcrumb" callback, to execute code before every
      * breadcrumb captured by Bugsnag.
-     *
+     * <p>
      * You can use this to modify breadcrumbs before they are stored by Bugsnag.
      * You can also return <code>false</code> from any callback to ignore a breadcrumb.
-     *
+     * <p>
      * For example:
-     *
+     * <p>
      * Bugsnag.onBreadcrumb(new OnBreadcrumbCallback() {
      * public boolean run(Breadcrumb breadcrumb) {
      * return false; // ignore the breadcrumb
@@ -170,6 +181,7 @@ public static void addOnBreadcrumb(@NonNull final OnBreadcrumbCallback onBreadcr
 
     /**
      * Removes a previously added "on breadcrumb" callback
+     *
      * @param onBreadcrumb the callback to remove
      */
     public static void removeOnBreadcrumb(@NonNull OnBreadcrumbCallback onBreadcrumb) {
@@ -179,12 +191,12 @@ public static void removeOnBreadcrumb(@NonNull OnBreadcrumbCallback onBreadcrumb
     /**
      * Add an "on session" callback, to execute code before every
      * session captured by Bugsnag.
-     *
+     * <p>
      * You can use this to modify sessions before they are stored by Bugsnag.
      * You can also return <code>false</code> from any callback to ignore a session.
-     *
+     * <p>
      * For example:
-     *
+     * <p>
      * Bugsnag.onSession(new OnSessionCallback() {
      * public boolean run(Session session) {
      * return false; // ignore the session
@@ -200,6 +212,7 @@ public static void addOnSession(@NonNull OnSessionCallback onSession) {
 
     /**
      * Removes a previously added "on session" callback
+     *
      * @param onSession the callback to remove
      */
     public static void removeOnSession(@NonNull OnSessionCallback onSession) {
@@ -219,7 +232,7 @@ public static void notify(@NonNull final Throwable exception) {
      * Notify Bugsnag of a handled exception
      *
      * @param exception the exception to send to Bugsnag
-     * @param onError  callback invoked on the generated error report for
+     * @param onError   callback invoked on the generated error report for
      *                  additional modification
      */
     public static void notify(@NonNull final Throwable exception,
@@ -286,7 +299,8 @@ public static void leaveBreadcrumb(@NonNull String message) {
     /**
      * Leave a "breadcrumb" log message representing an action or event which
      * occurred in your app, to aid with debugging
-     * @param message     A short label
+     *
+     * @param message  A short label
      * @param metadata Additional diagnostic information about the app environment
      * @param type     A category for the breadcrumb
      */
@@ -332,11 +346,10 @@ public static void startSession() {
      * <a href="https://docs.bugsnag.com/product/releases/releases-dashboard/#stability-score">
      * stability score</a>.
      *
+     * @return true if a previous session was resumed, false if a new session was started.
      * @see #startSession()
      * @see #pauseSession()
      * @see Configuration#setAutoTrackSessions(boolean)
-     *
-     * @return true if a previous session was resumed, false if a new session was started.
      */
     public static boolean resumeSession() {
         return getClient().resumeSession();
@@ -365,7 +378,7 @@ public static void pauseSession() {
      * Returns the current buffer of breadcrumbs that will be sent with captured events. This
      * ordered list represents the most recent breadcrumbs to be captured up to the limit
      * set in {@link Configuration#getMaxBreadcrumbs()}.
-     *
+     * <p>
      * The returned collection is readonly and mutating the list will cause no effect on the
      * Client's state. If you wish to alter the breadcrumbs collected by the Client then you should
      * use {@link Configuration#setEnabledBreadcrumbTypes(Set)} and
@@ -380,7 +393,7 @@ public static List<Breadcrumb> getBreadcrumbs() {
 
     /**
      * Retrieves information about the last launch of the application, if it has been run before.
-     *
+     * <p>
      * For example, this allows checking whether the app crashed on its last launch, which could
      * be used to perform conditional behaviour to recover from crashes, such as clearing the
      * app data cache.
@@ -394,7 +407,7 @@ public static LastRunInfo getLastRunInfo() {
      * Informs Bugsnag that the application has finished launching. Once this has been called
      * {@link AppWithState#isLaunching()} will always be false in any new error reports,
      * and synchronous delivery will not be attempted on the next launch for any fatal crashes.
-     *
+     * <p>
      * By default this method will be called after Bugsnag is initialized when
      * {@link Configuration#getLaunchDurationMillis()} has elapsed. Invoking this method manually
      * has precedence over the value supplied via the launchDurationMillis configuration option.
@@ -462,8 +475,12 @@ public static void clearFeatureFlags() {
     @NonNull
     public static Client getClient() {
         if (client == null) {
-            throw new IllegalStateException("You must call Bugsnag.start before any"
-                + " other Bugsnag methods");
+            synchronized (lock) {
+                if (client == null) {
+                    throw new IllegalStateException("You must call Bugsnag.start before any"
+                            + " other Bugsnag methods");
+                }
+            }
         }
 
         return client;

diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventFilenameInfo.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventFilenameInfo.kt
@@ -22,12 +22,8 @@ internal data class EventFilenameInfo(
     val errorTypes: Set<ErrorType>
 ) {
 
-    /**
-     * Generates a filename for the Event in the format
-     * "[timestamp]_[apiKey]_[errorTypes]_[UUID]_[startupcrash|not-jvm].json"
-     */
     fun encode(): String {
-        return "${timestamp}_${apiKey}_${serializeErrorTypeHeader(errorTypes)}_${uuid}_$suffix.json"
+        return toFilename(apiKey, uuid, timestamp, suffix, errorTypes)
     }
 
     fun isLaunchCrashReport(): Boolean = suffix == STARTUP_CRASH
@@ -36,7 +32,21 @@ internal data class EventFilenameInfo(
         private const val STARTUP_CRASH = "startupcrash"
         private const val NON_JVM_CRASH = "not-jvm"
 
-        @JvmOverloads
+        /**
+         * Generates a filename for the Event in the format
+         * "[timestamp]_[apiKey]_[errorTypes]_[UUID]_[startupcrash|not-jvm].json"
+         */
+        fun toFilename(
+            apiKey: String,
+            uuid: String,
+            timestamp: Long,
+            suffix: String,
+            errorTypes: Set<ErrorType>
+        ): String {
+            return "${timestamp}_${apiKey}_${serializeErrorTypeHeader(errorTypes)}_${uuid}_$suffix.json"
+        }
+
+        @JvmOverloads @JvmStatic
         fun fromEvent(
             obj: Any,
             uuid: String = UUID.randomUUID().toString(),
@@ -63,11 +73,12 @@ internal data class EventFilenameInfo(
         /**
          * Reads event information from a filename.
          */
+        @JvmStatic
         fun fromFile(file: File, config: ImmutableConfig): EventFilenameInfo {
             return EventFilenameInfo(
                 findApiKeyInFilename(file, config),
                 "", // ignore UUID field when reading from file as unused
-                -1, // ignore timestamp when reading from file as unused
+                findTimestampInFilename(file),
                 findSuffixInFilename(file),
                 findErrorTypesInFilename(file)
             )
@@ -77,7 +88,7 @@ internal data class EventFilenameInfo(
          * Retrieves the api key encoded in the filename, or an empty string if this information
          * is not encoded for the given event
          */
-        private fun findApiKeyInFilename(file: File, config: ImmutableConfig): String {
+        internal fun findApiKeyInFilename(file: File, config: ImmutableConfig): String {
             val name = file.name.removeSuffix("_$STARTUP_CRASH.json")
             val start = name.indexOf("_") + 1
             val end = name.indexOf("_", start)
@@ -93,7 +104,7 @@ internal data class EventFilenameInfo(
          * Retrieves the error types encoded in the filename, or an empty string if this
          * information is not encoded for the given event
          */
-        private fun findErrorTypesInFilename(eventFile: File): Set<ErrorType> {
+        internal fun findErrorTypesInFilename(eventFile: File): Set<ErrorType> {
             val name = eventFile.name
             val end = name.lastIndexOf("_", name.lastIndexOf("_") - 1)
             val start = name.lastIndexOf("_", end - 1) + 1
@@ -111,7 +122,7 @@ internal data class EventFilenameInfo(
          * Retrieves the error types encoded in the filename, or an empty string if this
          * information is not encoded for the given event
          */
-        private fun findSuffixInFilename(eventFile: File): String {
+        internal fun findSuffixInFilename(eventFile: File): String {
             val name = eventFile.nameWithoutExtension
             val suffix = name.substring(name.lastIndexOf("_") + 1)
             return when (suffix) {
@@ -120,10 +131,20 @@ internal data class EventFilenameInfo(
             }
         }
 
+        /**
+         * Retrieves the error types encoded in the filename, or an empty string if this
+         * information is not encoded for the given event
+         */
+        @JvmStatic
+        fun findTimestampInFilename(eventFile: File): Long {
+            val name = eventFile.nameWithoutExtension
+            return name.substringBefore("_", missingDelimiterValue = "-1").toLongOrNull() ?: -1
+        }
+
         /**
          * Retrieves the error types for the given event
          */
-        private fun findErrorTypesForEvent(obj: Any): Set<ErrorType> {
+        internal fun findErrorTypesForEvent(obj: Any): Set<ErrorType> {
             return when (obj) {
                 is Event -> obj.impl.getErrorTypesFromStackframes()
                 else -> setOf(ErrorType.C)
@@ -133,7 +154,7 @@ internal data class EventFilenameInfo(
         /**
          * Calculates the suffix for the given event
          */
-        private fun findSuffixForEvent(obj: Any, launching: Boolean?): String {
+        internal fun findSuffixForEvent(obj: Any, launching: Boolean?): String {
             return when {
                 obj is Event && obj.app.isLaunching == true -> STARTUP_CRASH
                 launching == true -> STARTUP_CRASH