diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 2c1143c6ab..60c2a3cea1 100644 --- 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 index 703305c4fd..e5cf0cc94b 100644 --- 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 index 9070d7bbbe..a671898884 100644 --- a/bugsnag-android-core/detekt-baseline.xml +++ b/bugsnag-android-core/detekt-baseline.xml @@ -22,6 +22,7 @@ MagicNumber:DefaultDelivery.kt$DefaultDelivery$429 MagicNumber:DefaultDelivery.kt$DefaultDelivery$499 MagicNumber:LastRunInfoStore.kt$LastRunInfoStore$3 + MagicNumber:SessionFilenameInfo.kt$SessionFilenameInfo.Companion$35 MaxLineLength:LastRunInfo.kt$LastRunInfo$return "LastRunInfo(consecutiveLaunchCrashes=$consecutiveLaunchCrashes, crashed=$crashed, crashedDuringLaunch=$crashedDuringLaunch)" MaxLineLength:ThreadState.kt$ThreadState$"[${allThreads.size - maxThreadCount} threads omitted as the maxReportedThreads limit ($maxThreadCount) was exceeded]" ProtectedMemberInFinalClass:ConfigInternal.kt$ConfigInternal$protected val plugins = HashSet<Plugin>() @@ -38,7 +39,9 @@ SwallowedException:DeviceDataCollector.kt$DeviceDataCollector$catch (exception: Exception) { logger.w("Could not get battery status") } SwallowedException:DeviceDataCollector.kt$DeviceDataCollector$catch (exception: Exception) { logger.w("Could not get locationStatus") } SwallowedException:DeviceIdStore.kt$DeviceIdStore$catch (exc: OverlappingFileLockException) { Thread.sleep(FILE_LOCK_WAIT_MS) } + SwallowedException:EventFilenameInfo.kt$EventFilenameInfo.Companion$catch (e: Exception) { return -1 } SwallowedException:PluginClient.kt$PluginClient$catch (exc: ClassNotFoundException) { logger.d("Plugin '$clz' is not on the classpath - functionality will not be enabled.") null } + SwallowedException:SessionFilenameInfo.kt$SessionFilenameInfo.Companion$catch (e: Exception) { return 0 } TooManyFunctions:ConfigInternal.kt$ConfigInternal : CallbackAwareMetadataAwareUserAwareFeatureFlagAware TooManyFunctions:EventInternal.kt$EventInternal : FeatureFlagAwareStreamableMetadataAwareUserAware UnnecessaryAbstractClass:DependencyModule.kt$DependencyModule 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 index f32b49d6ae..11538b580d 100644 --- 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,6 +69,15 @@ public static Client start(@NonNull Context androidContext, @NonNull Configurati return client; } + /** + * Returns true if one of the start methods have been has been called and + * so Bugsnag is initialized; false if start 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."); } @@ -76,18 +85,19 @@ private static void logClientInitWarning() { /** * 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. - * + *

* 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. - * + *

* 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. - * + *

* You can use this to add or modify information attached to an Event * before it is sent to your dashboard. You can also return * false 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. - * + *

* For example: - * + *

* 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. - * + *

* You can use this to modify breadcrumbs before they are stored by Bugsnag. * You can also return false from any callback to ignore a breadcrumb. - * + *

* For example: - * + *

* 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. - * + *

* You can use this to modify sessions before they are stored by Bugsnag. * You can also return false from any callback to ignore a session. - * + *

* For example: - * + *

* 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() { * * stability score. * + * @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()}. - * + *

* 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 getBreadcrumbs() { /** * Retrieves information about the last launch of the application, if it has been run before. - * + *

* 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. - * + *

* 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 index 6d9ff766c9..f7cfd5f9af 100644 --- 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 ) { - /** - * 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 + ): 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 { + internal fun findErrorTypesInFilename(eventFile: File): Set { 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 { + internal fun findErrorTypesForEvent(obj: Any): Set { 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 diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStore.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStore.java index e1365644ae..5ae0c04c50 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStore.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/EventStore.java @@ -7,9 +7,11 @@ import java.io.File; import java.util.ArrayList; +import java.util.Calendar; import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.Date; import java.util.List; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; @@ -119,7 +121,7 @@ File findLaunchCrashReport(Collection storedFiles) { List launchCrashes = new ArrayList<>(); for (File file : storedFiles) { - EventFilenameInfo filenameInfo = EventFilenameInfo.Companion.fromFile(file, config); + EventFilenameInfo filenameInfo = EventFilenameInfo.fromFile(file, config); if (filenameInfo.isLaunchCrashReport()) { launchCrashes.add(file); } @@ -163,7 +165,7 @@ void flushReports(Collection storedReports) { private void flushEventFile(File eventFile) { try { - EventFilenameInfo eventInfo = EventFilenameInfo.Companion.fromFile(eventFile, config); + EventFilenameInfo eventInfo = EventFilenameInfo.fromFile(eventFile, config); String apiKey = eventInfo.getApiKey(); EventPayload payload = createEventPayload(eventFile, apiKey); @@ -188,9 +190,21 @@ private void deliverEventPayload(File eventFile, EventPayload payload) { logger.i("Deleting sent error file " + eventFile.getName()); break; case UNDELIVERED: - cancelQueuedFiles(Collections.singleton(eventFile)); - logger.w("Could not send previously saved error(s)" - + " to Bugsnag, will try again later"); + if (isTooBig(eventFile)) { + logger.w("Discarding over-sized event (" + + eventFile.length() + + ") after failed delivery"); + deleteStoredFiles(Collections.singleton(eventFile)); + } else if (isTooOld(eventFile)) { + logger.w("Discarding historical event (from " + + getCreationDate(eventFile) + + ") after failed delivery"); + deleteStoredFiles(Collections.singleton(eventFile)); + } else { + cancelQueuedFiles(Collections.singleton(eventFile)); + logger.w("Could not send previously saved error(s)" + + " to Bugsnag, will try again later"); + } break; case FAILURE: Exception exc = new RuntimeException("Failed to deliver event payload"); @@ -234,13 +248,29 @@ private void handleEventFlushFailure(Exception exc, File eventFile) { @Override String getFilename(Object object) { EventFilenameInfo eventInfo - = EventFilenameInfo.Companion.fromEvent(object, null, config); + = EventFilenameInfo.fromEvent(object, null, config); return eventInfo.encode(); } String getNdkFilename(Object object, String apiKey) { EventFilenameInfo eventInfo - = EventFilenameInfo.Companion.fromEvent(object, apiKey, config); + = EventFilenameInfo.fromEvent(object, apiKey, config); return eventInfo.encode(); } + + private static long oneMegabyte = 1024 * 1024; + + public boolean isTooBig(File file) { + return file.length() > oneMegabyte; + } + + public boolean isTooOld(File file) { + Calendar cal = Calendar.getInstance(); + cal.add(Calendar.DATE, -60); + return EventFilenameInfo.findTimestampInFilename(file) < cal.getTimeInMillis(); + } + + public Date getCreationDate(File file) { + return new Date(EventFilenameInfo.findTimestampInFilename(file)); + } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.java index cbafdac431..b613cea988 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/FileStore.java @@ -39,7 +39,7 @@ interface Delegate { private final Lock lock = new ReentrantLock(); private final Collection queuedFiles = new ConcurrentSkipListSet<>(); - private final Logger logger; + protected final Logger logger; private final EventStore.Delegate delegate; FileStore(@NonNull File storageDir, 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 index bfa3ed5d7f..1d6b2e45ed 100644 --- 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.21.0", + var version: String = "5.22.0", var url: String = "https://bugsnag.com" ) : JsonStream.Streamable { diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionFilenameInfo.kt b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionFilenameInfo.kt new file mode 100644 index 0000000000..04efa4081b --- /dev/null +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionFilenameInfo.kt @@ -0,0 +1,55 @@ +package com.bugsnag.android + +import java.io.File +import java.util.UUID + +/** + * Represents important information about a session filename. + * Currently the following information is encoded: + * + * uuid - to disambiguate stored error reports + * timestamp - to sort error reports by time of capture + */ +internal data class SessionFilenameInfo( + val timestamp: Long, + val uuid: String, +) { + + fun encode(): String { + return toFilename(timestamp, uuid) + } + + internal companion object { + + const val uuidLength = 36 + + /** + * Generates a filename for the session in the format + * "[UUID][timestamp]_v2.json" + */ + fun toFilename(timestamp: Long, uuid: String): String { + return "${uuid}${timestamp}_v2.json" + } + + @JvmStatic + fun defaultFilename(): String { + return toFilename(System.currentTimeMillis(), UUID.randomUUID().toString()) + } + + fun fromFile(file: File): SessionFilenameInfo { + return SessionFilenameInfo( + findTimestampInFilename(file), + findUuidInFilename(file) + ) + } + + private fun findUuidInFilename(file: File): String { + return file.name.substring(0, uuidLength - 1) + } + + @JvmStatic + fun findTimestampInFilename(file: File): Long { + return file.name.substring(uuidLength, file.name.indexOf("_")).toLongOrNull() ?: -1 + } + } +} diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionStore.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionStore.java index 0d84d8a677..a0238a5feb 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionStore.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionStore.java @@ -6,7 +6,9 @@ import androidx.annotation.Nullable; import java.io.File; +import java.util.Calendar; import java.util.Comparator; +import java.util.Date; import java.util.UUID; /** @@ -46,7 +48,16 @@ public int compare(File lhs, File rhs) { @NonNull @Override String getFilename(Object object) { - return UUID.randomUUID().toString() + System.currentTimeMillis() + "_v2.json"; + return SessionFilenameInfo.defaultFilename(); } + public boolean isTooOld(File file) { + Calendar cal = Calendar.getInstance(); + cal.add(Calendar.DATE, -60); + return SessionFilenameInfo.findTimestampInFilename(file) < cal.getTimeInMillis(); + } + + public Date getCreationDate(File file) { + return new Date(SessionFilenameInfo.findTimestampInFilename(file)); + } } diff --git a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionTracker.java b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionTracker.java index 2a5e95c7f1..ddb2f052d5 100644 --- a/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionTracker.java +++ b/bugsnag-android-core/src/main/java/com/bugsnag/android/SessionTracker.java @@ -270,8 +270,15 @@ void flushStoredSession(File storedFile) { logger.d("Sent 1 new session to Bugsnag"); break; case UNDELIVERED: - sessionStore.cancelQueuedFiles(Collections.singletonList(storedFile)); - logger.w("Leaving session payload for future delivery"); + if (sessionStore.isTooOld(storedFile)) { + logger.w("Discarding historical session (from {" + + sessionStore.getCreationDate(storedFile) + + "}) after failed delivery"); + sessionStore.deleteStoredFiles(Collections.singletonList(storedFile)); + } else { + sessionStore.cancelQueuedFiles(Collections.singletonList(storedFile)); + logger.w("Leaving session payload for future delivery"); + } break; case FAILURE: // drop bad data diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/BugsnagApiTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/BugsnagApiTest.kt index bb1b0b693b..8a4448b017 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/BugsnagApiTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/BugsnagApiTest.kt @@ -37,6 +37,17 @@ class BugsnagApiTest { verify(client, times(1)).context = "Bar" } + @Test + fun isStartedWhenStarted() { + assertEquals(true, Bugsnag.isStarted()) + } + + @Test + fun isStartedWhenNotStarted() { + Bugsnag.client = null + assertEquals(false, Bugsnag.isStarted()) + } + @Test fun getUser() { Bugsnag.getUser() diff --git a/bugsnag-android-core/src/test/java/com/bugsnag/android/BugsnagClientMirrorInterfaceTest.kt b/bugsnag-android-core/src/test/java/com/bugsnag/android/BugsnagClientMirrorInterfaceTest.kt index 47166196b1..a23892aa54 100644 --- a/bugsnag-android-core/src/test/java/com/bugsnag/android/BugsnagClientMirrorInterfaceTest.kt +++ b/bugsnag-android-core/src/test/java/com/bugsnag/android/BugsnagClientMirrorInterfaceTest.kt @@ -9,13 +9,18 @@ import java.lang.reflect.Method * signatures, and fails if the two are out of sync. * * This is intended to catch the case where a new API is added to one class but not the other. - * If a method genuinely only needs to be present on one of the classes, it can be added to a - * blacklist via [bugsnagBlacklist] or [clientBlacklist]. + * If a method genuinely only needs to be present on one of the classes, it should be added to + * [bugsnagAllowList] or [clientAllowList]. */ class BugsnagClientMirrorInterfaceTest { - private val bugsnagBlacklist = setOf("start", "getClient") - private val clientBlacklist = setOf( + private val bugsnagAllowList = setOf( + "start", + "getClient", + "isStarted" + ) + + private val clientAllowList = setOf( "update", "notifyObservers", "addObserver", @@ -35,14 +40,14 @@ class BugsnagClientMirrorInterfaceTest { @Test fun bugsnagHasSameMethodsAsClient() { val methods = clientMethods.subtract(bugsnagMethods) - .filter { !clientBlacklist.contains(it.name) } + .filter { !clientAllowList.contains(it.name) } assertTrue("Bugsnag is missing the following methods: $methods", methods.isEmpty()) } @Test fun clientHasSameMethodsAsBugsnag() { val methods = bugsnagMethods.subtract(clientMethods) - .filter { !bugsnagBlacklist.contains(it.name) } + .filter { !bugsnagAllowList.contains(it.name) } assertTrue("Client is missing the following methods: $methods", methods.isEmpty()) } } diff --git a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c index 648a533a73..df219af2ab 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.c @@ -58,6 +58,17 @@ bool bsg_run_on_error() { return true; } +bool bsg_begin_handling_crash() { + static bool expected = false; + return atomic_compare_exchange_strong(&bsg_global_env->handling_crash, + &expected, true); +} + +void bsg_finish_handling_crash() { + bsg_global_env->crash_handled = true; + bsg_global_env->handling_crash = false; +} + JNIEXPORT void JNICALL Java_com_bugsnag_android_NdkPlugin_enableCrashReporting( JNIEnv *env, jobject _this) { if (bsg_global_env == NULL) { @@ -143,6 +154,7 @@ JNIEXPORT void JNICALL Java_com_bugsnag_android_ndk_NativeBridge_install( bugsnag_env->report_header.version = BUGSNAG_EVENT_VERSION; bugsnag_env->consecutive_launch_crashes = consecutive_launch_crashes; bugsnag_env->send_threads = send_threads; + bugsnag_env->handling_crash = ATOMIC_VAR_INIT(false); // copy event path to env struct const char *event_path = bsg_safe_get_string_utf_chars(env, _event_path); diff --git a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.h b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.h index 66506884d4..592c9bb1d6 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.h +++ b/bugsnag-plugin-android-ndk/src/main/jni/bugsnag_ndk.h @@ -4,6 +4,7 @@ #ifndef BUGSNAG_NDK_H #define BUGSNAG_NDK_H +#include #include #include "../assets/include/bugsnag.h" @@ -53,7 +54,7 @@ typedef struct { * true if a crash is currently being handled. Disallows multiple crashes * from being processed simultaneously */ - bool handling_crash; + _Atomic bool handling_crash; /** * true if a handler has completed crash handling */ @@ -78,6 +79,22 @@ typedef struct { */ bool bsg_run_on_error(); +/** + * This must be called before handling a native crash to ensure that two crash + * handlers don't run simultaneously. If this function returns falls, DO NOT + * PROCEED WITH CRASH HANDLING! + * + * When done handling the crash, you must call bsg_finish_handling_crash() + * + * @return true if no other crash handler is already running. + */ +bool bsg_begin_handling_crash(); + +/** + * Let the system know that you've finished handling the crash. + */ +void bsg_finish_handling_crash(); + #ifdef __cplusplus } #endif diff --git a/bugsnag-plugin-android-ndk/src/main/jni/handlers/cpp_handler.cpp b/bugsnag-plugin-android-ndk/src/main/jni/handlers/cpp_handler.cpp index 61b1c80646..af8d7bf0f3 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/handlers/cpp_handler.cpp +++ b/bugsnag-plugin-android-ndk/src/main/jni/handlers/cpp_handler.cpp @@ -47,7 +47,10 @@ void bsg_handle_cpp_terminate() { if (bsg_global_env == NULL || bsg_global_env->handling_crash) return; - bsg_global_env->handling_crash = true; + if (!bsg_begin_handling_crash()) { + return; + } + bsg_populate_event_as(bsg_global_env); bsg_global_env->next_event.unhandled = true; bsg_global_env->next_event.error.frame_count = bsg_unwind_crash_stack( @@ -72,7 +75,8 @@ void bsg_handle_cpp_terminate() { bsg_serialize_event_to_file(bsg_global_env); bsg_serialize_last_run_info_to_file(bsg_global_env); } - bsg_global_env->crash_handled = true; + + bsg_finish_handling_crash(); bsg_handler_uninstall_cpp(); if (bsg_global_terminate_previous != NULL) { bsg_global_terminate_previous(); diff --git a/bugsnag-plugin-android-ndk/src/main/jni/handlers/signal_handler.c b/bugsnag-plugin-android-ndk/src/main/jni/handlers/signal_handler.c index 5c4f0cc42b..15afe15f70 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/handlers/signal_handler.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/handlers/signal_handler.c @@ -168,18 +168,19 @@ void bsg_handle_signal(int signum, siginfo_t *info, if (bsg_global_env == NULL) { return; } - if (bsg_global_env->handling_crash) { - if (bsg_global_env->crash_handled) { - // The C++ handler default action is to raise a fatal signal once - // handling is complete. The report is already generated so at this - // point, the handler only needs to be uninstalled. - bsg_handler_uninstall_signal(); - bsg_invoke_previous_signal_handler(signum, info, user_context); - } + if (!bsg_begin_handling_crash()) { + return; + } + + if (bsg_global_env->crash_handled) { + // The C++ handler default action is to raise a fatal signal once + // handling is complete. The report is already generated so at this + // point, the handler only needs to be uninstalled. + bsg_handler_uninstall_signal(); + bsg_invoke_previous_signal_handler(signum, info, user_context); return; } - bsg_global_env->handling_crash = true; bsg_global_env->next_event.unhandled = true; bsg_populate_event_as(bsg_global_env); bsg_global_env->next_event.error.frame_count = bsg_unwind_crash_stack( @@ -209,6 +210,8 @@ void bsg_handle_signal(int signum, siginfo_t *info, bsg_serialize_event_to_file(bsg_global_env); bsg_serialize_last_run_info_to_file(bsg_global_env); } + + bsg_finish_handling_crash(); bsg_handler_uninstall_signal(); bsg_invoke_previous_signal_handler(signum, info, user_context); } diff --git a/bugsnag-plugin-android-ndk/src/main/jni/utils/string.c b/bugsnag-plugin-android-ndk/src/main/jni/utils/string.c index cb193471f2..e1d853a9e7 100644 --- a/bugsnag-plugin-android-ndk/src/main/jni/utils/string.c +++ b/bugsnag-plugin-android-ndk/src/main/jni/utils/string.c @@ -16,9 +16,11 @@ size_t bsg_strlen(const char *str) { } void bsg_strncpy(char *dst, const char *src, size_t dst_size) { - if (src == NULL || dst == NULL || dst_size == 0) { + if (dst == NULL || dst_size == 0) { return; } dst[0] = '\0'; - strncat(dst, src, dst_size - 1); + if (src != NULL) { + strncat(dst, src, dst_size - 1); + } } diff --git a/bugsnag-plugin-android-ndk/src/test/cpp/test_utils_string.c b/bugsnag-plugin-android-ndk/src/test/cpp/test_utils_string.c index e8182c9ebf..711025fa71 100644 --- a/bugsnag-plugin-android-ndk/src/test/cpp/test_utils_string.c +++ b/bugsnag-plugin-android-ndk/src/test/cpp/test_utils_string.c @@ -12,6 +12,16 @@ TEST test_copy_empty_string(void) { PASS(); } +TEST test_copy_null_string(void) { + int dst_len = 10; + char *dst = calloc(sizeof(char), dst_len); + strcpy(dst, "C h a n g e"); + bsg_strncpy(dst, NULL, dst_len); + ASSERT(dst[0] == '\0'); + free(dst); + PASS(); +} + TEST test_copy_literal_string(void) { char *src = "C h a n g e"; int dst_len = 10; @@ -48,6 +58,7 @@ TEST length_null_string(void) { SUITE(suite_string_utils) { RUN_TEST(test_copy_empty_string); + RUN_TEST(test_copy_null_string); RUN_TEST(test_copy_literal_string); RUN_TEST(length_empty_string); RUN_TEST(length_literal_string); diff --git a/examples/sdk-app-example/app/build.gradle b/examples/sdk-app-example/app/build.gradle index 7ee14b7135..9117ef63d9 100644 --- a/examples/sdk-app-example/app/build.gradle +++ b/examples/sdk-app-example/app/build.gradle @@ -38,7 +38,7 @@ android { } dependencies { - implementation "com.bugsnag:bugsnag-android:5.20.0" + implementation "com.bugsnag:bugsnag-android:5.21.0" implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version" implementation "androidx.appcompat:appcompat:1.4.0" implementation "com.google.android.material:material:1.4.0" diff --git a/features/fixtures/mazerunner/app/src/main/java/com/bugsnag/android/mazerunner/MainActivity.kt b/features/fixtures/mazerunner/app/src/main/java/com/bugsnag/android/mazerunner/MainActivity.kt index 7bdf76f962..e2bdb86e12 100644 --- a/features/fixtures/mazerunner/app/src/main/java/com/bugsnag/android/mazerunner/MainActivity.kt +++ b/features/fixtures/mazerunner/app/src/main/java/com/bugsnag/android/mazerunner/MainActivity.kt @@ -10,6 +10,7 @@ import android.widget.Button import android.widget.EditText import com.bugsnag.android.Configuration import com.bugsnag.android.mazerunner.scenarios.Scenario +import java.io.File class MainActivity : Activity() { @@ -30,6 +31,15 @@ class MainActivity : Activity() { sendBroadcast(closeDialog) } + // Clear persistent data (used to stop scenarios bleeding into each other) + findViewById