From 245baa3b13529beab42c98ac46eed0f0586ac0ee Mon Sep 17 00:00:00 2001 From: M66B Date: Fri, 27 Dec 2024 15:56:15 +0100 Subject: [PATCH] Updated Bugsnag --- app/build.gradle | 2 +- .../android/ActivityBreadcrumbCollector.kt | 7 +- app/src/main/java/com/bugsnag/android/App.kt | 12 +- .../java/com/bugsnag/android/AppWithState.kt | 62 +++++- .../com/bugsnag/android/BugsnagEventMapper.kt | 3 +- .../com/bugsnag/android/BugsnagStateModule.kt | 2 +- .../main/java/com/bugsnag/android/Client.java | 77 ++++---- .../com/bugsnag/android/ClientObservable.kt | 2 +- .../bugsnag/android/DataCollectionModule.kt | 24 +-- .../com/bugsnag/android/DefaultDelivery.kt | 70 +------ .../bugsnag/android/DefaultDelivery.kt.orig | 121 ++++++++++++ .../java/com/bugsnag/android/Deliverable.kt | 39 ++++ .../com/bugsnag/android/DeliveryHeaders.kt | 20 -- .../com/bugsnag/android/DeliveryStatus.kt | 20 +- .../bugsnag/android/DeviceDataCollector.kt | 25 +-- .../java/com/bugsnag/android/DeviceIdStore.kt | 53 +++-- .../java/com/bugsnag/android/EventPayload.kt | 107 ++++++++++- .../com/bugsnag/android/EventStorageModule.kt | 16 +- .../java/com/bugsnag/android/EventStore.kt | 34 +++- .../java/com/bugsnag/android/FileStore.kt | 39 ++-- .../android/InternalReportDelegate.java | 11 +- .../java/com/bugsnag/android/JsonStream.java | 6 +- .../com/bugsnag/android/LastRunInfoStore.kt | 1 + .../main/java/com/bugsnag/android/Notifier.kt | 2 +- .../java/com/bugsnag/android/Session.java | 14 +- .../bugsnag/android/SessionFilenameInfo.kt | 8 +- .../java/com/bugsnag/android/SessionStore.kt | 14 +- .../java/com/bugsnag/android/StorageModule.kt | 59 ++++-- .../java/com/bugsnag/android/TrackerModule.kt | 23 ++- .../java/com/bugsnag/android/UserStore.kt | 20 +- .../android/internal/BackgroundTaskService.kt | 19 +- .../android/internal/BugsnagStoreMigrator.kt | 13 +- .../android/internal/ForegroundDetector.kt | 6 + .../android/internal/ImmutableConfig.kt | 35 ++-- .../android/internal/dag/ConfigModule.kt | 3 +- .../android/internal/dag/ContextModule.kt | 6 +- .../android/internal/dag/DependencyModule.kt | 46 ++--- .../bugsnag/android/internal/dag/Provider.kt | 181 ++++++++++++++++++ .../internal/dag/SystemServiceModule.kt | 6 +- 39 files changed, 858 insertions(+), 350 deletions(-) create mode 100644 app/src/main/java/com/bugsnag/android/DefaultDelivery.kt.orig create mode 100644 app/src/main/java/com/bugsnag/android/Deliverable.kt create mode 100644 app/src/main/java/com/bugsnag/android/internal/dag/Provider.kt diff --git a/app/build.gradle b/app/build.gradle index 373afd2f4f..83a2df639e 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -593,7 +593,7 @@ dependencies { def minidns_version = "1.0.5" def openpgp_version = "12.0" def badge_version = "1.1.22" - def bugsnag_version = "6.7.0" + def bugsnag_version = "6.10.0" def biweekly_version = "0.6.8" def vcard_version = "0.12.1" def relinker_version = "1.4.5" diff --git a/app/src/main/java/com/bugsnag/android/ActivityBreadcrumbCollector.kt b/app/src/main/java/com/bugsnag/android/ActivityBreadcrumbCollector.kt index b97a2eb86a..0b031a729c 100644 --- a/app/src/main/java/com/bugsnag/android/ActivityBreadcrumbCollector.kt +++ b/app/src/main/java/com/bugsnag/android/ActivityBreadcrumbCollector.kt @@ -79,6 +79,11 @@ internal class ActivityBreadcrumbCollector( } set("hasData", intent.data != null) - set("hasExtras", intent.extras?.keySet()?.joinToString(", ") ?: false) + + try { + set("hasExtras", intent.extras?.keySet()?.joinToString(", ") ?: false) + } catch (re: Exception) { + // deliberately ignore + } } } diff --git a/app/src/main/java/com/bugsnag/android/App.kt b/app/src/main/java/com/bugsnag/android/App.kt index e603b2926e..78c9a3d5ab 100644 --- a/app/src/main/java/com/bugsnag/android/App.kt +++ b/app/src/main/java/com/bugsnag/android/App.kt @@ -1,6 +1,7 @@ package com.bugsnag.android import com.bugsnag.android.internal.ImmutableConfig +import com.bugsnag.android.internal.dag.Provider import java.io.IOException /** @@ -36,7 +37,7 @@ open class App internal constructor( /** * The unique identifier for the build of the application set in [Configuration.buildUuid] */ - var buildUuid: String?, + buildUuid: Provider?, /** * The application type set in [Configuration#version] @@ -49,6 +50,15 @@ open class App internal constructor( var versionCode: Number? ) : JsonStream.Streamable { + private var buildUuidProvider: Provider? = buildUuid + + var buildUuid: String? = null + get() = field ?: buildUuidProvider?.getOrNull() + set(value) { + field = value + buildUuidProvider = null + } + internal constructor( config: ImmutableConfig, binaryArch: String?, diff --git a/app/src/main/java/com/bugsnag/android/AppWithState.kt b/app/src/main/java/com/bugsnag/android/AppWithState.kt index bf05c4af6b..b087a2d8bd 100644 --- a/app/src/main/java/com/bugsnag/android/AppWithState.kt +++ b/app/src/main/java/com/bugsnag/android/AppWithState.kt @@ -1,18 +1,20 @@ package com.bugsnag.android import com.bugsnag.android.internal.ImmutableConfig +import com.bugsnag.android.internal.dag.Provider +import com.bugsnag.android.internal.dag.ValueProvider /** * Stateful information set by the notifier about your app can be found on this class. These values * can be accessed and amended if necessary. */ -class AppWithState( +class AppWithState internal constructor( binaryArch: String?, id: String?, releaseStage: String?, version: String?, codeBundleId: String?, - buildUuid: String?, + buildUuid: Provider?, type: String?, versionCode: Number?, @@ -36,7 +38,61 @@ class AppWithState( * Whether the application was launching when the event occurred */ var isLaunching: Boolean? -) : App(binaryArch, id, releaseStage, version, codeBundleId, buildUuid, type, versionCode) { +) : App( + binaryArch, + id, + releaseStage, + version, + codeBundleId, + buildUuid, + type, + versionCode +) { + + constructor( + binaryArch: String?, + id: String?, + releaseStage: String?, + version: String?, + codeBundleId: String?, + buildUuid: String?, + type: String?, + versionCode: Number?, + + /** + * The number of milliseconds the application was running before the event occurred + */ + duration: Number?, + + /** + * The number of milliseconds the application was running in the foreground before the + * event occurred + */ + durationInForeground: Number?, + + /** + * Whether the application was in the foreground when the event occurred + */ + inForeground: Boolean?, + + /** + * Whether the application was launching when the event occurred + */ + isLaunching: Boolean? + ) : this( + binaryArch, + id, + releaseStage, + version, + codeBundleId, + buildUuid?.let(::ValueProvider), + type, + versionCode, + duration, + durationInForeground, + inForeground, + isLaunching + ) internal constructor( config: ImmutableConfig, diff --git a/app/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt b/app/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt index b63606de39..4fae3a1fe4 100644 --- a/app/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt +++ b/app/src/main/java/com/bugsnag/android/BugsnagEventMapper.kt @@ -2,6 +2,7 @@ package com.bugsnag.android import com.bugsnag.android.internal.DateUtils import com.bugsnag.android.internal.InternalMetricsImpl +import com.bugsnag.android.internal.dag.ValueProvider import java.text.DateFormat import java.text.SimpleDateFormat import java.util.Date @@ -151,7 +152,7 @@ internal class BugsnagEventMapper( app["releaseStage"] as? String, app["version"] as? String, app["codeBundleId"] as? String, - app["buildUUID"] as? String, + (app["buildUUID"] as? String)?.let(::ValueProvider), app["type"] as? String, (app["versionCode"] as? Number)?.toInt(), (app["duration"] as? Number)?.toLong(), diff --git a/app/src/main/java/com/bugsnag/android/BugsnagStateModule.kt b/app/src/main/java/com/bugsnag/android/BugsnagStateModule.kt index 2aa375226d..51d565a83d 100644 --- a/app/src/main/java/com/bugsnag/android/BugsnagStateModule.kt +++ b/app/src/main/java/com/bugsnag/android/BugsnagStateModule.kt @@ -10,7 +10,7 @@ import com.bugsnag.android.internal.dag.DependencyModule internal class BugsnagStateModule( cfg: ImmutableConfig, configuration: Configuration -) : DependencyModule() { +) : DependencyModule { val clientObservable = ClientObservable() diff --git a/app/src/main/java/com/bugsnag/android/Client.java b/app/src/main/java/com/bugsnag/android/Client.java index afc6675044..cea27385ac 100644 --- a/app/src/main/java/com/bugsnag/android/Client.java +++ b/app/src/main/java/com/bugsnag/android/Client.java @@ -3,7 +3,6 @@ package com.bugsnag.android; import static com.bugsnag.android.SeverityReason.REASON_HANDLED_EXCEPTION; import com.bugsnag.android.internal.BackgroundTaskService; -import com.bugsnag.android.internal.BugsnagStoreMigrator; import com.bugsnag.android.internal.ForegroundDetector; import com.bugsnag.android.internal.ImmutableConfig; import com.bugsnag.android.internal.InternalMetrics; @@ -13,6 +12,7 @@ import com.bugsnag.android.internal.StateObserver; import com.bugsnag.android.internal.TaskType; import com.bugsnag.android.internal.dag.ConfigModule; import com.bugsnag.android.internal.dag.ContextModule; +import com.bugsnag.android.internal.dag.Provider; import com.bugsnag.android.internal.dag.SystemServiceModule; import android.app.Application; @@ -59,7 +59,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF private final InternalMetrics internalMetrics; private final ContextState contextState; private final CallbackState callbackState; - private final UserState userState; + private final Provider userState; private final Map configDifferences; final Context appContext; @@ -125,7 +125,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF * @param configuration a configuration for the Client */ public Client(@NonNull Context androidContext, @NonNull final Configuration configuration) { - ContextModule contextModule = new ContextModule(androidContext); + ContextModule contextModule = new ContextModule(androidContext, bgTaskService); appContext = contextModule.getCtx(); notifier = configuration.getNotifier(); @@ -165,19 +165,15 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF + "Bugsnag.start(context.getApplicationContext()). " + "For further info see: " + "https://docs.bugsnag.com/platforms/android/#basic-configuration"); - } - BugsnagStoreMigrator.moveToNewDirectory( - immutableConfig.getPersistenceDirectory().getValue()); - // setup storage as soon as possible final StorageModule storageModule = new StorageModule(appContext, - immutableConfig, logger); + immutableConfig, bgTaskService); // setup state trackers for bugsnag - BugsnagStateModule bugsnagStateModule = new BugsnagStateModule( - immutableConfig, configuration); + BugsnagStateModule bugsnagStateModule = + new BugsnagStateModule(immutableConfig, configuration); clientObservable = bugsnagStateModule.getClientObservable(); callbackState = bugsnagStateModule.getCallbackState(); breadcrumbState = bugsnagStateModule.getBreadcrumbState(); @@ -186,34 +182,26 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF featureFlagState = bugsnagStateModule.getFeatureFlagState(); // lookup system services - final SystemServiceModule systemServiceModule = new SystemServiceModule(contextModule); - - // block until storage module has resolved everything - storageModule.resolveDependencies(bgTaskService, TaskType.IO); + final SystemServiceModule systemServiceModule = + new SystemServiceModule(contextModule, bgTaskService); // setup further state trackers and data collection TrackerModule trackerModule = new TrackerModule(configModule, storageModule, this, bgTaskService, callbackState); - launchCrashTracker = trackerModule.getLaunchCrashTracker(); - sessionTracker = trackerModule.getSessionTracker(); DataCollectionModule dataCollectionModule = new DataCollectionModule(contextModule, configModule, systemServiceModule, trackerModule, - bgTaskService, connectivity, storageModule.getDeviceId(), - storageModule.getInternalDeviceId(), memoryTrimState); - dataCollectionModule.resolveDependencies(bgTaskService, TaskType.IO); - appDataCollector = dataCollectionModule.getAppDataCollector(); - deviceDataCollector = dataCollectionModule.getDeviceDataCollector(); + bgTaskService, connectivity, storageModule.getDeviceIdStore(), + memoryTrimState); // load the device + user information - userState = storageModule.getUserStore().load(configuration.getUser()); - storageModule.getSharedPrefMigrator().deleteLegacyPrefs(); + userState = storageModule.loadUser(configuration.getUser()); EventStorageModule eventStorageModule = new EventStorageModule(contextModule, configModule, dataCollectionModule, bgTaskService, trackerModule, systemServiceModule, notifier, callbackState); - eventStorageModule.resolveDependencies(bgTaskService, TaskType.IO); - eventStore = eventStorageModule.getEventStore(); + + eventStore = eventStorageModule.getEventStore().get(); deliveryDelegate = new DeliveryDelegate(logger, eventStore, immutableConfig, callbackState, notifier, bgTaskService); @@ -221,8 +209,13 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF exceptionHandler = new ExceptionHandler(this, logger); // load last run info - lastRunInfoStore = storageModule.getLastRunInfoStore(); - lastRunInfo = storageModule.getLastRunInfo(); + lastRunInfoStore = storageModule.getLastRunInfoStore().getOrNull(); + lastRunInfo = storageModule.getLastRunInfo().getOrNull(); + + launchCrashTracker = trackerModule.getLaunchCrashTracker(); + sessionTracker = trackerModule.getSessionTracker().get(); + appDataCollector = dataCollectionModule.getAppDataCollector().get(); + deviceDataCollector = dataCollectionModule.getDeviceDataCollector().get(); Set userPlugins = configuration.getPlugins(); pluginClient = new PluginClient(userPlugins, immutableConfig, logger); @@ -245,7 +238,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF MetadataState metadataState, ContextState contextState, CallbackState callbackState, - UserState userState, + Provider userState, FeatureFlagState featureFlagState, ClientObservable clientObservable, Context appContext, @@ -452,7 +445,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF breadcrumbState.addObserver(observer); sessionTracker.addObserver(observer); clientObservable.addObserver(observer); - userState.addObserver(observer); + userState.get().addObserver(observer); contextState.addObserver(observer); deliveryDelegate.addObserver(observer); launchCrashTracker.addObserver(observer); @@ -465,7 +458,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF breadcrumbState.removeObserver(observer); sessionTracker.removeObserver(observer); clientObservable.removeObserver(observer); - userState.removeObserver(observer); + userState.get().removeObserver(observer); contextState.removeObserver(observer); deliveryDelegate.removeObserver(observer); launchCrashTracker.removeObserver(observer); @@ -479,7 +472,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF void syncInitialState() { metadataState.emitObservableEvent(); contextState.emitObservableEvent(); - userState.emitObservableEvent(); + userState.get().emitObservableEvent(); memoryTrimState.emitObservableEvent(); featureFlagState.emitObservableEvent(); } @@ -539,11 +532,10 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF * * 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 boolean resumeSession() { return sessionTracker.resumeSession(); @@ -556,7 +548,8 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF * 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 String getContext() { + @Nullable + public String getContext() { return contextState.getContext(); } @@ -576,7 +569,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF */ @Override public void setUser(@Nullable String id, @Nullable String email, @Nullable String name) { - userState.setUser(new User(id, email, name)); + userState.get().setUser(new User(id, email, name)); } /** @@ -585,7 +578,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF @NonNull @Override public User getUser() { - return userState.getUser(); + return userState.get().getUser(); } /** @@ -621,6 +614,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF /** * Removes a previously added "on error" callback + * * @param onError the callback to remove */ @Override @@ -661,6 +655,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF /** * Removes a previously added "on breadcrumb" callback + * * @param onBreadcrumb the callback to remove */ @Override @@ -701,6 +696,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF /** * Removes a previously added "on session" callback + * * @param onSession the callback to remove */ @Override @@ -724,9 +720,9 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF /** * Notify Bugsnag of a handled exception * - * @param exc the exception to send to Bugsnag - * @param onError callback invoked on the generated error report for - * additional modification + * @param exc the exception to send to Bugsnag + * @param onError callback invoked on the generated error report for + * additional modification */ public void notify(@NonNull Throwable exc, @Nullable OnErrorCallback onError) { if (exc != null) { @@ -789,7 +785,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF event.setBreadcrumbs(breadcrumbState.copy()); // Attach user info to the event - User user = userState.getUser(); + User user = userState.get().getUser(); event.setUser(user.getId(), user.getEmail(), user.getName()); // Attach context to the event @@ -959,6 +955,7 @@ public class Client implements MetadataAware, CallbackAware, UserAware, FeatureF /** * 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 metadata Additional diagnostic information about the app environment * @param type A category for the breadcrumb diff --git a/app/src/main/java/com/bugsnag/android/ClientObservable.kt b/app/src/main/java/com/bugsnag/android/ClientObservable.kt index b099c7edc3..989ea30fba 100644 --- a/app/src/main/java/com/bugsnag/android/ClientObservable.kt +++ b/app/src/main/java/com/bugsnag/android/ClientObservable.kt @@ -18,7 +18,7 @@ internal class ClientObservable : BaseObservable() { conf.apiKey, conf.enabledErrorTypes.ndkCrashes, conf.appVersion, - conf.buildUuid, + conf.buildUuid?.getOrNull(), conf.releaseStage, lastRunInfoPath, consecutiveLaunchCrashes, diff --git a/app/src/main/java/com/bugsnag/android/DataCollectionModule.kt b/app/src/main/java/com/bugsnag/android/DataCollectionModule.kt index 70e9bd78f1..e8967de359 100644 --- a/app/src/main/java/com/bugsnag/android/DataCollectionModule.kt +++ b/app/src/main/java/com/bugsnag/android/DataCollectionModule.kt @@ -2,9 +2,10 @@ package com.bugsnag.android import android.os.Environment import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.dag.BackgroundDependencyModule import com.bugsnag.android.internal.dag.ConfigModule import com.bugsnag.android.internal.dag.ContextModule -import com.bugsnag.android.internal.dag.DependencyModule +import com.bugsnag.android.internal.dag.Provider import com.bugsnag.android.internal.dag.SystemServiceModule /** @@ -18,10 +19,9 @@ internal class DataCollectionModule( trackerModule: TrackerModule, bgTaskService: BackgroundTaskService, connectivity: Connectivity, - deviceId: String?, - internalDeviceId: String?, + deviceIdStore: Provider, memoryTrimState: MemoryTrimState -) : DependencyModule() { +) : BackgroundDependencyModule(bgTaskService) { private val ctx = contextModule.ctx private val cfg = configModule.config @@ -29,32 +29,32 @@ internal class DataCollectionModule( private val deviceBuildInfo: DeviceBuildInfo = DeviceBuildInfo.defaultInfo() private val dataDir = Environment.getDataDirectory() - val appDataCollector by future { + val appDataCollector = provider { AppDataCollector( ctx, ctx.packageManager, cfg, - trackerModule.sessionTracker, + trackerModule.sessionTracker.get(), systemServiceModule.activityManager, trackerModule.launchCrashTracker, memoryTrimState ) } - private val rootDetector by future { - RootDetector(logger = logger, deviceBuildInfo = deviceBuildInfo) + private val rootDetection = provider { + val rootDetector = RootDetector(logger = logger, deviceBuildInfo = deviceBuildInfo) + rootDetector.isRooted() } - val deviceDataCollector by future { + val deviceDataCollector = provider { DeviceDataCollector( connectivity, ctx, ctx.resources, - deviceId, - internalDeviceId, + deviceIdStore.map { it.load() }, deviceBuildInfo, dataDir, - rootDetector, + rootDetection, bgTaskService, logger ) diff --git a/app/src/main/java/com/bugsnag/android/DefaultDelivery.kt b/app/src/main/java/com/bugsnag/android/DefaultDelivery.kt index 1832989b7c..2d8861ef24 100644 --- a/app/src/main/java/com/bugsnag/android/DefaultDelivery.kt +++ b/app/src/main/java/com/bugsnag/android/DefaultDelivery.kt @@ -4,69 +4,27 @@ import android.net.TrafficStats import com.bugsnag.android.internal.JsonHelper import java.io.IOException import java.net.HttpURLConnection -import java.net.HttpURLConnection.HTTP_BAD_REQUEST -import java.net.HttpURLConnection.HTTP_CLIENT_TIMEOUT -import java.net.HttpURLConnection.HTTP_OK import java.net.URL internal class DefaultDelivery( private val connectivity: Connectivity?, - private val apiKey: String, - private val maxStringValueLength: Int, private val logger: Logger ) : Delivery { - companion object { - // 1MB with some fiddle room in case of encoding overhead - const val maxPayloadSize = 999700 - } - override fun deliver(payload: Session, deliveryParams: DeliveryParams): DeliveryStatus { val status = deliver( deliveryParams.endpoint, JsonHelper.serialize(payload), + payload.integrityToken, deliveryParams.headers ) logger.i("Session API request finished with status $status") return status } - private fun serializePayload(payload: EventPayload): ByteArray { - var json = JsonHelper.serialize(payload) - if (json.size <= maxPayloadSize) { - return json - } - - var event = payload.event - if (event == null) { - event = MarshalledEventSource(payload.eventFile!!, apiKey, logger).invoke() - payload.event = event - payload.apiKey = apiKey - } - - val (itemsTrimmed, dataTrimmed) = event.impl.trimMetadataStringsTo(maxStringValueLength) - event.impl.internalMetrics.setMetadataTrimMetrics( - itemsTrimmed, - dataTrimmed - ) - - json = JsonHelper.serialize(payload) - if (json.size <= maxPayloadSize) { - return json - } - - val breadcrumbAndBytesRemovedCounts = - event.impl.trimBreadcrumbsBy(json.size - maxPayloadSize) - event.impl.internalMetrics.setBreadcrumbTrimMetrics( - breadcrumbAndBytesRemovedCounts.itemsTrimmed, - breadcrumbAndBytesRemovedCounts.dataTrimmed - ) - return JsonHelper.serialize(payload) - } - override fun deliver(payload: EventPayload, deliveryParams: DeliveryParams): DeliveryStatus { - val json = serializePayload(payload) - val status = deliver(deliveryParams.endpoint, json, deliveryParams.headers) + val json = payload.trimToSize().toByteArray() + val status = deliver(deliveryParams.endpoint, json, payload.integrityToken, deliveryParams.headers) logger.i("Error API request finished with status $status") return status } @@ -74,6 +32,7 @@ internal class DefaultDelivery( fun deliver( urlString: String, json: ByteArray, + integrity: String?, headers: Map ): DeliveryStatus { @@ -84,11 +43,11 @@ internal class DefaultDelivery( var conn: HttpURLConnection? = null try { - conn = makeRequest(URL(urlString), json, headers) + conn = makeRequest(URL(urlString), json, integrity, headers) // End the request, get the response code val responseCode = conn.responseCode - val status = getDeliveryStatus(responseCode) + val status = DeliveryStatus.forHttpResponseCode(responseCode) logRequestInfo(responseCode, conn, status) return status } catch (oom: OutOfMemoryError) { @@ -111,6 +70,7 @@ internal class DefaultDelivery( private fun makeRequest( url: URL, json: ByteArray, + integrity: String?, headers: Map ): HttpURLConnection { val conn = url.openConnection() as HttpURLConnection @@ -120,8 +80,7 @@ internal class DefaultDelivery( // https://developer.android.com/reference/java/net/HttpURLConnection conn.setFixedLengthStreamingMode(json.size) - // calculate the SHA-1 digest and add all other headers - computeSha1Digest(json)?.let { digest -> + integrity?.let { digest -> conn.addRequestProperty(HEADER_BUGSNAG_INTEGRITY, digest) } headers.forEach { (key, value) -> @@ -159,17 +118,4 @@ internal class DefaultDelivery( } } } - - internal fun getDeliveryStatus(responseCode: Int): DeliveryStatus { - 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/app/src/main/java/com/bugsnag/android/DefaultDelivery.kt.orig b/app/src/main/java/com/bugsnag/android/DefaultDelivery.kt.orig new file mode 100644 index 0000000000..a186936579 --- /dev/null +++ b/app/src/main/java/com/bugsnag/android/DefaultDelivery.kt.orig @@ -0,0 +1,121 @@ +package com.bugsnag.android + +import android.net.TrafficStats +import com.bugsnag.android.internal.JsonHelper +import java.io.IOException +import java.net.HttpURLConnection +import java.net.URL + +internal class DefaultDelivery( + private val connectivity: Connectivity?, + private val logger: Logger +) : Delivery { + + override fun deliver(payload: Session, deliveryParams: DeliveryParams): DeliveryStatus { + val status = deliver( + deliveryParams.endpoint, + JsonHelper.serialize(payload), + payload.integrityToken, + deliveryParams.headers + ) + logger.i("Session API request finished with status $status") + return status + } + + override fun deliver(payload: EventPayload, deliveryParams: DeliveryParams): DeliveryStatus { + val json = payload.trimToSize().toByteArray() + val status = deliver(deliveryParams.endpoint, json, payload.integrityToken, deliveryParams.headers) + logger.i("Error API request finished with status $status") + return status + } + + fun deliver( + urlString: String, + json: ByteArray, + integrity: String?, + headers: Map + ): DeliveryStatus { + + TrafficStats.setThreadStatsTag(1) + if (connectivity != null && !connectivity.hasNetworkConnection()) { + return DeliveryStatus.UNDELIVERED + } + var conn: HttpURLConnection? = null + + try { + conn = makeRequest(URL(urlString), json, integrity, headers) + + // End the request, get the response code + val responseCode = conn.responseCode + val status = DeliveryStatus.forHttpResponseCode(responseCode) + logRequestInfo(responseCode, conn, status) + return status + } catch (oom: OutOfMemoryError) { + // attempt to persist the payload on disk. This approach uses streams to write to a + // file, which takes less memory than serializing the payload into a ByteArray, and + // therefore has a reasonable chance of retaining the payload for future delivery. + logger.w("Encountered OOM delivering payload, falling back to persist on disk", oom) + return DeliveryStatus.UNDELIVERED + } catch (exception: IOException) { + logger.w("IOException encountered in request", exception) + return DeliveryStatus.UNDELIVERED + } catch (exception: Exception) { + logger.w("Unexpected error delivering payload", exception) + return DeliveryStatus.FAILURE + } finally { + conn?.disconnect() + } + } + + private fun makeRequest( + url: URL, + json: ByteArray, + integrity: String?, + headers: Map + ): HttpURLConnection { + val conn = url.openConnection() as HttpURLConnection + conn.doOutput = true + + // avoids creating a buffer within HttpUrlConnection, see + // https://developer.android.com/reference/java/net/HttpURLConnection + conn.setFixedLengthStreamingMode(json.size) + + integrity?.let { digest -> + conn.addRequestProperty(HEADER_BUGSNAG_INTEGRITY, digest) + } + headers.forEach { (key, value) -> + if (value != null) { + conn.addRequestProperty(key, value) + } + } + + // write the JSON payload + conn.outputStream.use { + it.write(json) + } + return conn + } + + private fun logRequestInfo(code: Int, conn: HttpURLConnection, status: DeliveryStatus) { + 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()}") + } + } + + runCatching { + if (status != DeliveryStatus.DELIVERED) { + conn.errorStream.bufferedReader().use { + logger.w("Request error details: ${it.readText()}") + } + } + } + } +} diff --git a/app/src/main/java/com/bugsnag/android/Deliverable.kt b/app/src/main/java/com/bugsnag/android/Deliverable.kt new file mode 100644 index 0000000000..c41da5de5e --- /dev/null +++ b/app/src/main/java/com/bugsnag/android/Deliverable.kt @@ -0,0 +1,39 @@ +package com.bugsnag.android + +import java.io.IOException +import java.security.DigestOutputStream +import java.security.MessageDigest + +/** + * Denotes objects that are expected to be delivered over a network. + */ +interface Deliverable { + /** + * Return the byte representation of this `Deliverable`. + */ + @Throws(IOException::class) + fun toByteArray(): ByteArray + + /** + * The value of the "Bugsnag-Integrity" HTTP header returned as a String. This value is used + * to validate the payload and is expected by the standard BugSnag servers. + */ + val integrityToken: String? + get() { + runCatching { + val shaDigest = MessageDigest.getInstance("SHA-1") + val builder = StringBuilder("sha1 ") + + // Pipe the object through a no-op output stream + DigestOutputStream(NullOutputStream(), shaDigest).use { stream -> + stream.buffered().use { writer -> + writer.write(toByteArray()) + } + shaDigest.digest().forEach { byte -> + builder.append(String.format("%02x", byte)) + } + } + return builder.toString() + }.getOrElse { return null } + } +} diff --git a/app/src/main/java/com/bugsnag/android/DeliveryHeaders.kt b/app/src/main/java/com/bugsnag/android/DeliveryHeaders.kt index 89df18055d..c5d2d7fee7 100644 --- a/app/src/main/java/com/bugsnag/android/DeliveryHeaders.kt +++ b/app/src/main/java/com/bugsnag/android/DeliveryHeaders.kt @@ -2,8 +2,6 @@ package com.bugsnag.android import com.bugsnag.android.internal.DateUtils import java.io.OutputStream -import java.security.DigestOutputStream -import java.security.MessageDigest import java.util.Date private const val HEADER_API_PAYLOAD_VERSION = "Bugsnag-Payload-Version" @@ -60,24 +58,6 @@ internal fun sessionApiHeaders(apiKey: String): Map = mapOf( HEADER_BUGSNAG_SENT_AT to DateUtils.toIso8601(Date()) ) -internal fun computeSha1Digest(payload: ByteArray): String? { - runCatching { - val shaDigest = MessageDigest.getInstance("SHA-1") - val builder = StringBuilder("sha1 ") - - // Pipe the object through a no-op output stream - DigestOutputStream(NullOutputStream(), shaDigest).use { stream -> - stream.buffered().use { writer -> - writer.write(payload) - } - shaDigest.digest().forEach { byte -> - builder.append(String.format("%02x", byte)) - } - } - return builder.toString() - }.getOrElse { return null } -} - internal class NullOutputStream : OutputStream() { override fun write(b: Int) = Unit } diff --git a/app/src/main/java/com/bugsnag/android/DeliveryStatus.kt b/app/src/main/java/com/bugsnag/android/DeliveryStatus.kt index 9bbd4d8ccf..37c398805f 100644 --- a/app/src/main/java/com/bugsnag/android/DeliveryStatus.kt +++ b/app/src/main/java/com/bugsnag/android/DeliveryStatus.kt @@ -1,5 +1,9 @@ package com.bugsnag.android +import java.net.HttpURLConnection.HTTP_BAD_REQUEST +import java.net.HttpURLConnection.HTTP_CLIENT_TIMEOUT +import java.net.HttpURLConnection.HTTP_OK + /** * Return value for the status of a payload delivery. */ @@ -19,5 +23,19 @@ enum class DeliveryStatus { * * The payload was not delivered and should be deleted without attempting retry. */ - FAILURE + FAILURE; + + companion object { + @JvmStatic + fun forHttpResponseCode(responseCode: Int): DeliveryStatus { + return when { + responseCode in HTTP_OK..299 -> DELIVERED + responseCode in HTTP_BAD_REQUEST..499 && // 400-499 are considered unrecoverable + responseCode != HTTP_CLIENT_TIMEOUT && // except for 408 + responseCode != 429 -> FAILURE + + else -> UNDELIVERED + } + } + } } diff --git a/app/src/main/java/com/bugsnag/android/DeviceDataCollector.kt b/app/src/main/java/com/bugsnag/android/DeviceDataCollector.kt index 493763a493..b818523837 100644 --- a/app/src/main/java/com/bugsnag/android/DeviceDataCollector.kt +++ b/app/src/main/java/com/bugsnag/android/DeviceDataCollector.kt @@ -13,6 +13,7 @@ import android.os.Build import android.provider.Settings import com.bugsnag.android.internal.BackgroundTaskService import com.bugsnag.android.internal.TaskType +import com.bugsnag.android.internal.dag.Provider import java.io.File import java.util.Date import java.util.Locale @@ -28,11 +29,10 @@ internal class DeviceDataCollector( private val connectivity: Connectivity, private val appContext: Context, resources: Resources, - private val deviceId: String?, - private val internalDeviceId: String?, + private val deviceIdStore: Provider, private val buildInfo: DeviceBuildInfo, private val dataDirectory: File, - rootDetector: RootDetector, + private val rootedFuture: Provider?, private val bgTaskService: BackgroundTaskService, private val logger: Logger ) { @@ -45,7 +45,6 @@ internal class DeviceDataCollector( private val locale = Locale.getDefault().toString() private val cpuAbi = getCpuAbi() private var runtimeVersions: MutableMap - private val rootedFuture: Future? private val totalMemoryFuture: Future? = retrieveTotalDeviceMemory() private var orientation = AtomicInteger(resources.configuration.orientation) @@ -54,25 +53,13 @@ internal class DeviceDataCollector( buildInfo.apiLevel?.let { map["androidApiLevel"] = it } buildInfo.osBuild?.let { map["osBuild"] = it } runtimeVersions = map - - rootedFuture = try { - bgTaskService.submitTask( - TaskType.IO, - Callable { - rootDetector.isRooted() - } - ) - } catch (exc: RejectedExecutionException) { - logger.w("Failed to perform root detection checks", exc) - null - } } fun generateDevice() = Device( buildInfo, cpuAbi, checkIsRooted(), - deviceId, + deviceIdStore.get()?.deviceId, locale, totalMemoryFuture.runCatching { this?.get() }.getOrNull(), runtimeVersions.toMutableMap() @@ -81,7 +68,7 @@ internal class DeviceDataCollector( fun generateDeviceWithState(now: Long) = DeviceWithState( buildInfo, checkIsRooted(), - deviceId, + deviceIdStore.get()?.deviceId, locale, totalMemoryFuture.runCatching { this?.get() }.getOrNull(), runtimeVersions.toMutableMap(), @@ -94,7 +81,7 @@ internal class DeviceDataCollector( fun generateInternalDeviceWithState(now: Long) = DeviceWithState( buildInfo, checkIsRooted(), - internalDeviceId, + deviceIdStore.get()?.internalDeviceId, locale, totalMemoryFuture.runCatching { this?.get() }.getOrNull(), runtimeVersions.toMutableMap(), diff --git a/app/src/main/java/com/bugsnag/android/DeviceIdStore.kt b/app/src/main/java/com/bugsnag/android/DeviceIdStore.kt index 74dc3e87e9..a0a2cab06f 100644 --- a/app/src/main/java/com/bugsnag/android/DeviceIdStore.kt +++ b/app/src/main/java/com/bugsnag/android/DeviceIdStore.kt @@ -2,6 +2,7 @@ package com.bugsnag.android import android.content.Context import com.bugsnag.android.internal.ImmutableConfig +import com.bugsnag.android.internal.dag.Provider import java.io.File import java.util.UUID @@ -11,23 +12,19 @@ import java.util.UUID */ internal class DeviceIdStore @JvmOverloads @Suppress("LongParameterList") constructor( context: Context, - deviceIdfile: File = File(context.filesDir, "device-id"), - deviceIdGenerator: () -> UUID = { UUID.randomUUID() }, - internalDeviceIdfile: File = File(context.filesDir, "internal-device-id"), - internalDeviceIdGenerator: () -> UUID = { UUID.randomUUID() }, - private val sharedPrefMigrator: SharedPrefMigrator, + private val deviceIdFile: File = File(context.filesDir, "device-id"), + private val deviceIdGenerator: () -> UUID = { UUID.randomUUID() }, + private val internalDeviceIdFile: File = File(context.filesDir, "internal-device-id"), + private val internalDeviceIdGenerator: () -> UUID = { UUID.randomUUID() }, + private val sharedPrefMigrator: Provider, config: ImmutableConfig, - logger: Logger + private val logger: Logger ) { - private val persistence: DeviceIdPersistence - private val internalPersistence: DeviceIdPersistence + private lateinit var persistence: DeviceIdPersistence + private lateinit var internalPersistence: DeviceIdPersistence private val generateId = config.generateAnonymousId - - init { - persistence = DeviceIdFilePersistence(deviceIdfile, deviceIdGenerator, logger) - internalPersistence = DeviceIdFilePersistence(internalDeviceIdfile, internalDeviceIdGenerator, logger) - } + private var deviceIds: DeviceIds? = null /** * Loads the device ID from @@ -37,7 +34,7 @@ internal class DeviceIdStore @JvmOverloads @Suppress("LongParameterList") constr * If no device ID exists then the legacy value stored in [SharedPreferences] will * be used. If no value is present then a random UUID will be generated and persisted. */ - fun loadDeviceId(): String? { + private fun loadDeviceId(): String? { // If generateAnonymousId = false, return null // so that a previously persisted device ID is not returned, // or a new one is not generated and persisted @@ -48,14 +45,14 @@ internal class DeviceIdStore @JvmOverloads @Suppress("LongParameterList") constr if (result != null) { return result } - result = sharedPrefMigrator.loadDeviceId(false) + result = sharedPrefMigrator.get().loadDeviceId(false) if (result != null) { return result } return persistence.loadDeviceId(true) } - fun loadInternalDeviceId(): String? { + private fun loadInternalDeviceId(): String? { // If generateAnonymousId = false, return null // so that a previously persisted device ID is not returned, // or a new one is not generated and persisted @@ -64,4 +61,28 @@ internal class DeviceIdStore @JvmOverloads @Suppress("LongParameterList") constr } return internalPersistence.loadDeviceId(true) } + + fun load(): DeviceIds? { + if (deviceIds != null) { + return deviceIds + } + + persistence = DeviceIdFilePersistence(deviceIdFile, deviceIdGenerator, logger) + internalPersistence = + DeviceIdFilePersistence(internalDeviceIdFile, internalDeviceIdGenerator, logger) + + val deviceId = loadDeviceId() + val internalDeviceId = loadInternalDeviceId() + + if (deviceId != null || internalDeviceId != null) { + deviceIds = DeviceIds(deviceId, internalDeviceId) + } + + return deviceIds + } + + data class DeviceIds( + val deviceId: String?, + val internalDeviceId: String? + ) } diff --git a/app/src/main/java/com/bugsnag/android/EventPayload.kt b/app/src/main/java/com/bugsnag/android/EventPayload.kt index 51f213016d..01431aea8c 100644 --- a/app/src/main/java/com/bugsnag/android/EventPayload.kt +++ b/app/src/main/java/com/bugsnag/android/EventPayload.kt @@ -1,6 +1,8 @@ package com.bugsnag.android +import androidx.annotation.VisibleForTesting import com.bugsnag.android.internal.ImmutableConfig +import com.bugsnag.android.internal.JsonHelper import java.io.File import java.io.IOException @@ -13,13 +15,20 @@ import java.io.IOException class EventPayload @JvmOverloads internal constructor( var apiKey: String?, event: Event? = null, - internal val eventFile: File? = null, + eventFile: File? = null, notifier: Notifier, private val config: ImmutableConfig -) : JsonStream.Streamable { +) : JsonStream.Streamable, Deliverable { - var event = event - internal set(value) { field = value } + var event: Event? = event + internal set + + internal var eventFile: File? = eventFile + private set + + private var cachedBytes: ByteArray? = null + + private val logger: Logger get() = config.logger internal val notifier = Notifier(notifier.name, notifier.version, notifier.url).apply { dependencies = notifier.dependencies.toMutableList() @@ -27,11 +36,64 @@ class EventPayload @JvmOverloads internal constructor( internal fun getErrorTypes(): Set { val event = this.event - return when { - event != null -> event.impl.getErrorTypesFromStackframes() - eventFile != null -> EventFilenameInfo.fromFile(eventFile, config).errorTypes - else -> emptySet() + + return event?.impl?.getErrorTypesFromStackframes() ?: ( + eventFile?.let { EventFilenameInfo.fromFile(it, config).errorTypes } + ?: emptySet() + ) + } + + private fun decodedEvent(): Event { + val localEvent = event + if (localEvent != null) { + return localEvent } + + val eventSource = MarshalledEventSource(eventFile!!, apiKey ?: config.apiKey, logger) + val decodedEvent = eventSource() + + // cache the decoded Event object + event = decodedEvent + + return decodedEvent + } + + /** + * If required trim this `EventPayload` so that its [encoded data](toByteArray) will usually be + * less-than or equal to [maxSizeBytes]. This function may make no changes to the payload, and + * may also not achieve the requested [maxSizeBytes]. The default use of the function is + * configured to [DEFAULT_MAX_PAYLOAD_SIZE]. + * + * @return `this` for call chaining + */ + @JvmOverloads + fun trimToSize(maxSizeBytes: Int = DEFAULT_MAX_PAYLOAD_SIZE): EventPayload { + var json = toByteArray() + if (json.size <= maxSizeBytes) { + return this + } + + val event = decodedEvent() + val (itemsTrimmed, dataTrimmed) = event.impl.trimMetadataStringsTo(config.maxStringValueLength) + event.impl.internalMetrics.setMetadataTrimMetrics( + itemsTrimmed, + dataTrimmed + ) + + json = rebuildPayloadCache() + if (json.size <= maxSizeBytes) { + return this + } + + val breadcrumbAndBytesRemovedCounts = + event.impl.trimBreadcrumbsBy(json.size - maxSizeBytes) + event.impl.internalMetrics.setBreadcrumbTrimMetrics( + breadcrumbAndBytesRemovedCounts.itemsTrimmed, + breadcrumbAndBytesRemovedCounts.dataTrimmed + ) + + rebuildPayloadCache() + return this } @Throws(IOException::class) @@ -51,4 +113,33 @@ class EventPayload @JvmOverloads internal constructor( writer.endArray() writer.endObject() } + + /** + * Transform this `EventPayload` to a byte array suitable for delivery to a BugSnag event + * endpoint (typically configured using [EndpointConfiguration.notify]). + */ + @Throws(IOException::class) + override fun toByteArray(): ByteArray { + var payload = cachedBytes + if (payload == null) { + payload = JsonHelper.serialize(this) + cachedBytes = payload + } + return payload + } + + @VisibleForTesting + internal fun rebuildPayloadCache(): ByteArray { + cachedBytes = null + return toByteArray() + } + + companion object { + /** + * The default maximum payload size for [trimToSize], payloads larger than this will + * typically be rejected by BugSnag. + */ + // 1MB with some fiddle room in case of encoding overhead + const val DEFAULT_MAX_PAYLOAD_SIZE = 999700 + } } diff --git a/app/src/main/java/com/bugsnag/android/EventStorageModule.kt b/app/src/main/java/com/bugsnag/android/EventStorageModule.kt index bad4b68c39..2d519eb5b1 100644 --- a/app/src/main/java/com/bugsnag/android/EventStorageModule.kt +++ b/app/src/main/java/com/bugsnag/android/EventStorageModule.kt @@ -1,9 +1,9 @@ package com.bugsnag.android import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.dag.BackgroundDependencyModule import com.bugsnag.android.internal.dag.ConfigModule import com.bugsnag.android.internal.dag.ContextModule -import com.bugsnag.android.internal.dag.DependencyModule import com.bugsnag.android.internal.dag.SystemServiceModule /** @@ -18,32 +18,32 @@ internal class EventStorageModule( systemServiceModule: SystemServiceModule, notifier: Notifier, callbackState: CallbackState -) : DependencyModule() { +) : BackgroundDependencyModule(bgTaskService) { private val cfg = configModule.config - private val delegate by future { - if (cfg.telemetry.contains(Telemetry.INTERNAL_ERRORS) == true) + private val delegate = provider { + if (cfg.telemetry.contains(Telemetry.INTERNAL_ERRORS)) InternalReportDelegate( contextModule.ctx, cfg.logger, cfg, systemServiceModule.storageManager, - dataCollectionModule.appDataCollector, + dataCollectionModule.appDataCollector.get(), dataCollectionModule.deviceDataCollector, - trackerModule.sessionTracker, + trackerModule.sessionTracker.get(), notifier, bgTaskService ) else null } - val eventStore by future { + val eventStore = provider { EventStore( cfg, cfg.logger, notifier, bgTaskService, - delegate, + delegate.getOrNull(), callbackState ) } diff --git a/app/src/main/java/com/bugsnag/android/EventStore.kt b/app/src/main/java/com/bugsnag/android/EventStore.kt index e6d8401ae9..75300b3907 100644 --- a/app/src/main/java/com/bugsnag/android/EventStore.kt +++ b/app/src/main/java/com/bugsnag/android/EventStore.kt @@ -1,15 +1,16 @@ package com.bugsnag.android +import android.os.SystemClock import com.bugsnag.android.EventFilenameInfo.Companion.findTimestampInFilename import com.bugsnag.android.EventFilenameInfo.Companion.fromEvent import com.bugsnag.android.EventFilenameInfo.Companion.fromFile import com.bugsnag.android.JsonStream.Streamable import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.ForegroundDetector import com.bugsnag.android.internal.ImmutableConfig import com.bugsnag.android.internal.TaskType import java.io.File import java.util.Calendar -import java.util.Comparator import java.util.Date import java.util.concurrent.Callable import java.util.concurrent.ExecutionException @@ -19,8 +20,7 @@ import java.util.concurrent.TimeUnit import java.util.concurrent.TimeoutException /** - * Store and flush Event reports which couldn't be sent immediately due to - * lack of network connectivity. + * Store and flush Event reports. */ internal class EventStore( private val config: ImmutableConfig, @@ -32,7 +32,6 @@ internal class EventStore( ) : FileStore( File(config.persistenceDirectory.value, "bugsnag/errors"), config.maxPersistedEvents, - EVENT_COMPARATOR, logger, delegate ) { @@ -42,7 +41,8 @@ internal class EventStore( override val logger: Logger /** - * Flush startup crashes synchronously on the main thread + * Flush startup crashes synchronously on the main thread. Startup crashes block the main thread + * when being sent (subject to [Configuration.setSendLaunchCrashesSynchronously]) */ fun flushOnLaunch() { if (!config.sendLaunchCrashesSynchronously) { @@ -58,13 +58,28 @@ internal class EventStore( return } try { - future.get(LAUNCH_CRASH_TIMEOUT_MS, TimeUnit.MILLISECONDS) + // Calculate the maximum amount of time we are prepared to block while sending + // startup crashes, based on how long we think startup has taken so-far. + // This attempts to mitigate possible startup ANRs that can occur when other SDKs + // have blocked the main thread before this code is reached. + val currentStartupDuration = + SystemClock.elapsedRealtime() - ForegroundDetector.startupTime + var timeout = LAUNCH_CRASH_TIMEOUT_MS - currentStartupDuration + + if (timeout <= 0) { + // if Bugsnag.start is called too long after Application.onCreate is expected to + // have returned, we use a full LAUNCH_CRASH_TIMEOUT_MS instead of a calculated one + // assuming that the app is already fully started + timeout = LAUNCH_CRASH_TIMEOUT_MS + } + + future.get(timeout, TimeUnit.MILLISECONDS) } catch (exc: InterruptedException) { - logger.d("Failed to send launch crash reports within 2s timeout, continuing.", exc) + logger.d("Failed to send launch crash reports within timeout, continuing.", exc) } catch (exc: ExecutionException) { - logger.d("Failed to send launch crash reports within 2s timeout, continuing.", exc) + logger.d("Failed to send launch crash reports within timeout, continuing.", exc) } catch (exc: TimeoutException) { - logger.d("Failed to send launch crash reports within 2s timeout, continuing.", exc) + logger.d("Failed to send launch crash reports within timeout, continuing.", exc) } } @@ -159,6 +174,7 @@ internal class EventStore( deleteStoredFiles(setOf(eventFile)) logger.i("Deleting sent error file $eventFile.name") } + DeliveryStatus.UNDELIVERED -> undeliveredEventPayload(eventFile) DeliveryStatus.FAILURE -> { val exc: Exception = RuntimeException("Failed to deliver event payload") diff --git a/app/src/main/java/com/bugsnag/android/FileStore.kt b/app/src/main/java/com/bugsnag/android/FileStore.kt index 235ba699bf..69f7c77632 100644 --- a/app/src/main/java/com/bugsnag/android/FileStore.kt +++ b/app/src/main/java/com/bugsnag/android/FileStore.kt @@ -7,8 +7,6 @@ import java.io.FileNotFoundException import java.io.FileOutputStream import java.io.OutputStreamWriter import java.io.Writer -import java.util.Collections -import java.util.Comparator import java.util.concurrent.ConcurrentSkipListSet import java.util.concurrent.locks.Lock import java.util.concurrent.locks.ReentrantLock @@ -16,7 +14,6 @@ import java.util.concurrent.locks.ReentrantLock internal abstract class FileStore( val storageDir: File, private val maxStoreCount: Int, - private val comparator: Comparator, protected open val logger: Logger, protected val delegate: Delegate? ) { @@ -34,10 +31,6 @@ internal abstract class FileStore( private val lock: Lock = ReentrantLock() private val queuedFiles: MutableCollection = ConcurrentSkipListSet() - init { - isStorageDirValid(storageDir) - } - /** * Checks whether the storage directory is a writable directory. If it is not, * this method will attempt to create the directory. @@ -115,23 +108,21 @@ internal abstract class FileStore( // Limit number of saved payloads to prevent disk space issues if (isStorageDirValid(storageDir)) { val listFiles = storageDir.listFiles() ?: return - val files: ArrayList = arrayListOf(*listFiles) - if (files.size >= maxStoreCount) { - // Sort files then delete the first one (oldest timestamp) - Collections.sort(files, comparator) - var k = 0 - while (k < files.size && files.size >= maxStoreCount) { - val oldestFile = files[k] - if (!queuedFiles.contains(oldestFile)) { - logger.w( - "Discarding oldest error as stored " + - "error limit reached: '" + oldestFile.path + '\'' - ) - deleteStoredFiles(setOf(oldestFile)) - files.removeAt(k) - k-- - } - k++ + if (listFiles.size < maxStoreCount) return + val sortedListFiles = listFiles.sortedBy { it.lastModified() } + // Number of files to discard takes into account that a new file may need to be written + val numberToDiscard = listFiles.size - maxStoreCount + 1 + var discardedCount = 0 + for (file in sortedListFiles) { + if (discardedCount == numberToDiscard) { + return + } else if (!queuedFiles.contains(file)) { + logger.w( + "Discarding oldest error as stored error limit reached: '" + + file.path + '\'' + ) + deleteStoredFiles(setOf(file)) + discardedCount++ } } } diff --git a/app/src/main/java/com/bugsnag/android/InternalReportDelegate.java b/app/src/main/java/com/bugsnag/android/InternalReportDelegate.java index a9d6a08d93..ad1d791bdf 100644 --- a/app/src/main/java/com/bugsnag/android/InternalReportDelegate.java +++ b/app/src/main/java/com/bugsnag/android/InternalReportDelegate.java @@ -5,8 +5,8 @@ import static com.bugsnag.android.SeverityReason.REASON_UNHANDLED_EXCEPTION; import com.bugsnag.android.internal.BackgroundTaskService; import com.bugsnag.android.internal.ImmutableConfig; -import com.bugsnag.android.internal.JsonHelper; import com.bugsnag.android.internal.TaskType; +import com.bugsnag.android.internal.dag.Provider; import android.annotation.SuppressLint; import android.content.Context; @@ -33,7 +33,7 @@ class InternalReportDelegate implements EventStore.Delegate { final StorageManager storageManager; final AppDataCollector appDataCollector; - final DeviceDataCollector deviceDataCollector; + final Provider deviceDataCollector; final Context appContext; final SessionTracker sessionTracker; final Notifier notifier; @@ -44,7 +44,7 @@ class InternalReportDelegate implements EventStore.Delegate { ImmutableConfig immutableConfig, @Nullable StorageManager storageManager, AppDataCollector appDataCollector, - DeviceDataCollector deviceDataCollector, + Provider deviceDataCollector, SessionTracker sessionTracker, Notifier notifier, BackgroundTaskService backgroundTaskService) { @@ -102,7 +102,7 @@ class InternalReportDelegate implements EventStore.Delegate { */ void reportInternalBugsnagError(@NonNull Event event) { event.setApp(appDataCollector.generateAppWithState()); - event.setDevice(deviceDataCollector.generateDeviceWithState(new Date().getTime())); + event.setDevice(deviceDataCollector.get().generateDeviceWithState(new Date().getTime())); event.addMetadata(INTERNAL_DIAGNOSTICS_TAB, "notifierName", notifier.getName()); event.addMetadata(INTERNAL_DIAGNOSTICS_TAB, "notifierVersion", notifier.getVersion()); @@ -126,7 +126,8 @@ class InternalReportDelegate implements EventStore.Delegate { DefaultDelivery defaultDelivery = (DefaultDelivery) delivery; defaultDelivery.deliver( params.getEndpoint(), - JsonHelper.INSTANCE.serialize(payload), + payload.toByteArray(), + payload.getIntegrityToken(), headers ); } diff --git a/app/src/main/java/com/bugsnag/android/JsonStream.java b/app/src/main/java/com/bugsnag/android/JsonStream.java index 57072f22b6..a32413457c 100644 --- a/app/src/main/java/com/bugsnag/android/JsonStream.java +++ b/app/src/main/java/com/bugsnag/android/JsonStream.java @@ -64,7 +64,11 @@ public class JsonStream extends JsonWriter { * Collections, Maps, and arrays. */ public void value(@Nullable Object object) throws IOException { - value(object, false); + if (object instanceof File) { + value((File) object); + } else { + value(object, false); + } } /** diff --git a/app/src/main/java/com/bugsnag/android/LastRunInfoStore.kt b/app/src/main/java/com/bugsnag/android/LastRunInfoStore.kt index d5b1a93d42..89f7c4516f 100644 --- a/app/src/main/java/com/bugsnag/android/LastRunInfoStore.kt +++ b/app/src/main/java/com/bugsnag/android/LastRunInfoStore.kt @@ -36,6 +36,7 @@ internal class LastRunInfoStore(config: ImmutableConfig) { add(KEY_CRASHED, lastRunInfo.crashed) add(KEY_CRASHED_DURING_LAUNCH, lastRunInfo.crashedDuringLaunch) }.toString() + file.parentFile?.mkdirs() file.writeText(text) logger.d("Persisted: $text") } diff --git a/app/src/main/java/com/bugsnag/android/Notifier.kt b/app/src/main/java/com/bugsnag/android/Notifier.kt index 56e4794d8b..e8c0693cdd 100644 --- a/app/src/main/java/com/bugsnag/android/Notifier.kt +++ b/app/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 = "6.7.0", + var version: String = "6.10.0", var url: String = "https://bugsnag.com" ) : JsonStream.Streamable { diff --git a/app/src/main/java/com/bugsnag/android/Session.java b/app/src/main/java/com/bugsnag/android/Session.java index 5081ad22e7..f838aa37e2 100644 --- a/app/src/main/java/com/bugsnag/android/Session.java +++ b/app/src/main/java/com/bugsnag/android/Session.java @@ -1,6 +1,7 @@ package com.bugsnag.android; import com.bugsnag.android.internal.DateUtils; +import com.bugsnag.android.internal.JsonHelper; import androidx.annotation.NonNull; import androidx.annotation.Nullable; @@ -17,7 +18,7 @@ import java.util.concurrent.atomic.AtomicInteger; * Represents a contiguous session in an application. */ @SuppressWarnings("ConstantConditions") -public final class Session implements JsonStream.Streamable, UserAware { +public final class Session implements JsonStream.Streamable, Deliverable, UserAware { private final File file; private final Notifier notifier; @@ -258,6 +259,17 @@ public final class Session implements JsonStream.Streamable, UserAware { } } + @NonNull + public byte[] toByteArray() throws IOException { + return JsonHelper.INSTANCE.serialize(this); + } + + @Nullable + @Override + public String getIntegrityToken() { + return Deliverable.DefaultImpls.getIntegrityToken(this); + } + private void serializePayload(@NonNull JsonStream writer) throws IOException { writer.value(file); } diff --git a/app/src/main/java/com/bugsnag/android/SessionFilenameInfo.kt b/app/src/main/java/com/bugsnag/android/SessionFilenameInfo.kt index 3ea39917cd..6ce21ac750 100644 --- a/app/src/main/java/com/bugsnag/android/SessionFilenameInfo.kt +++ b/app/src/main/java/com/bugsnag/android/SessionFilenameInfo.kt @@ -1,6 +1,5 @@ package com.bugsnag.android -import com.bugsnag.android.internal.ImmutableConfig import java.io.File import java.util.UUID @@ -34,13 +33,10 @@ internal data class SessionFilenameInfo( } @JvmStatic - fun defaultFilename( - obj: Any?, - config: ImmutableConfig - ): SessionFilenameInfo { + fun defaultFilename(obj: Any?, apiKey: String): SessionFilenameInfo { val sanitizedApiKey = when (obj) { is Session -> obj.apiKey - else -> config.apiKey + else -> apiKey } return SessionFilenameInfo( diff --git a/app/src/main/java/com/bugsnag/android/SessionStore.kt b/app/src/main/java/com/bugsnag/android/SessionStore.kt index 86519ac5af..e6e0ce6aa8 100644 --- a/app/src/main/java/com/bugsnag/android/SessionStore.kt +++ b/app/src/main/java/com/bugsnag/android/SessionStore.kt @@ -2,7 +2,6 @@ package com.bugsnag.android import com.bugsnag.android.SessionFilenameInfo.Companion.defaultFilename import com.bugsnag.android.SessionFilenameInfo.Companion.findTimestampInFilename -import com.bugsnag.android.internal.ImmutableConfig import java.io.File import java.util.Calendar import java.util.Comparator @@ -13,15 +12,14 @@ import java.util.Date * lack of network connectivity. */ internal class SessionStore( - private val config: ImmutableConfig, + bugsnagDir: File, + maxPersistedSessions: Int, + private val apiKey: String, logger: Logger, delegate: Delegate? ) : FileStore( - File( - config.persistenceDirectory.value, "bugsnag/sessions" - ), - config.maxPersistedSessions, - SESSION_COMPARATOR, + File(bugsnagDir, "sessions"), + maxPersistedSessions, logger, delegate ) { @@ -53,7 +51,7 @@ internal class SessionStore( } override fun getFilename(obj: Any?): String { - val sessionInfo = defaultFilename(obj, config) + val sessionInfo = defaultFilename(obj, apiKey) return sessionInfo.encode() } } diff --git a/app/src/main/java/com/bugsnag/android/StorageModule.kt b/app/src/main/java/com/bugsnag/android/StorageModule.kt index 8defc58ac4..46a07da023 100644 --- a/app/src/main/java/com/bugsnag/android/StorageModule.kt +++ b/app/src/main/java/com/bugsnag/android/StorageModule.kt @@ -1,56 +1,73 @@ package com.bugsnag.android import android.content.Context +import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.BugsnagStoreMigrator.migrateLegacyFiles import com.bugsnag.android.internal.ImmutableConfig -import com.bugsnag.android.internal.dag.DependencyModule +import com.bugsnag.android.internal.TaskType +import com.bugsnag.android.internal.dag.BackgroundDependencyModule +import com.bugsnag.android.internal.dag.Provider /** * A dependency module which constructs the objects that store information to disk in Bugsnag. */ internal class StorageModule( appContext: Context, - immutableConfig: ImmutableConfig, - logger: Logger -) : DependencyModule() { + private val immutableConfig: ImmutableConfig, + bgTaskService: BackgroundTaskService +) : BackgroundDependencyModule(bgTaskService, TaskType.IO) { - val sharedPrefMigrator by future { SharedPrefMigrator(appContext) } + val bugsnagDir = provider { + migrateLegacyFiles(immutableConfig.persistenceDirectory) + } + + val sharedPrefMigrator = provider { + SharedPrefMigrator(appContext) + } - private val deviceIdStore by future { + val deviceIdStore = provider { DeviceIdStore( appContext, sharedPrefMigrator = sharedPrefMigrator, - logger = logger, + logger = immutableConfig.logger, config = immutableConfig ) } - val deviceId by future { deviceIdStore.loadDeviceId() } - - val internalDeviceId by future { deviceIdStore.loadInternalDeviceId() } - - val userStore by future { + val userStore = provider { UserStore( - immutableConfig, - deviceId, + immutableConfig.persistUser, + bugsnagDir, + deviceIdStore.map { it.load() }, sharedPrefMigrator = sharedPrefMigrator, - logger = logger + logger = immutableConfig.logger ) } - val lastRunInfoStore by future { LastRunInfoStore(immutableConfig) } + val lastRunInfoStore = provider { + LastRunInfoStore(immutableConfig) + } - val sessionStore by future { + val sessionStore = provider { SessionStore( - immutableConfig, - logger, + bugsnagDir.get(), + immutableConfig.maxPersistedSessions, + immutableConfig.apiKey, + immutableConfig.logger, null ) } - val lastRunInfo by future { + val lastRunInfo = lastRunInfoStore.map { lastRunInfoStore -> val info = lastRunInfoStore.load() val currentRunInfo = LastRunInfo(0, crashed = false, crashedDuringLaunch = false) lastRunInfoStore.persist(currentRunInfo) - info + return@map info + } + + fun loadUser(initialUser: User): Provider = provider { + val userState = userStore.get().load(initialUser) + sharedPrefMigrator.getOrNull()?.deleteLegacyPrefs() + return@provider userState } } diff --git a/app/src/main/java/com/bugsnag/android/TrackerModule.kt b/app/src/main/java/com/bugsnag/android/TrackerModule.kt index ae1db31f2a..8eb6c47607 100644 --- a/app/src/main/java/com/bugsnag/android/TrackerModule.kt +++ b/app/src/main/java/com/bugsnag/android/TrackerModule.kt @@ -1,8 +1,8 @@ package com.bugsnag.android import com.bugsnag.android.internal.BackgroundTaskService +import com.bugsnag.android.internal.dag.BackgroundDependencyModule import com.bugsnag.android.internal.dag.ConfigModule -import com.bugsnag.android.internal.dag.DependencyModule /** * A dependency module which constructs objects that track launch/session related information @@ -14,18 +14,21 @@ internal class TrackerModule( client: Client, bgTaskService: BackgroundTaskService, callbackState: CallbackState -) : DependencyModule() { +) : BackgroundDependencyModule(bgTaskService) { private val config = configModule.config val launchCrashTracker = LaunchCrashTracker(config) - val sessionTracker = SessionTracker( - config, - callbackState, - client, - storageModule.sessionStore, - config.logger, - bgTaskService - ) + val sessionTracker = provider { + client.config + SessionTracker( + config, + callbackState, + client, + storageModule.sessionStore.get(), + config.logger, + bgTaskService + ) + } } diff --git a/app/src/main/java/com/bugsnag/android/UserStore.kt b/app/src/main/java/com/bugsnag/android/UserStore.kt index 548bcdbc35..58bf10d7ed 100644 --- a/app/src/main/java/com/bugsnag/android/UserStore.kt +++ b/app/src/main/java/com/bugsnag/android/UserStore.kt @@ -1,23 +1,23 @@ package com.bugsnag.android -import com.bugsnag.android.internal.ImmutableConfig import com.bugsnag.android.internal.StateObserver +import com.bugsnag.android.internal.dag.Provider import java.io.File import java.util.concurrent.atomic.AtomicReference /** * This class is responsible for persisting and retrieving user information. */ -internal class UserStore @JvmOverloads constructor( - private val config: ImmutableConfig, - private val deviceId: String?, - file: File = File(config.persistenceDirectory.value, "bugsnag/user-info"), - private val sharedPrefMigrator: SharedPrefMigrator, +internal class UserStore( + private val persist: Boolean, + private val persistentDir: Provider, + private val deviceIdStore: Provider, + file: File = File(persistentDir.get(), "user-info"), + private val sharedPrefMigrator: Provider, private val logger: Logger ) { private val synchronizedStreamableStore: SynchronizedStreamableStore - private val persist = config.persistUser private val previousUser = AtomicReference(null) init { @@ -50,7 +50,7 @@ internal class UserStore @JvmOverloads constructor( loadedUser != null && validUser(loadedUser) -> UserState(loadedUser) // if generateAnonymousId config option is false, the deviceId should already be null // here - else -> UserState(User(deviceId, null, null)) + else -> UserState(User(deviceIdStore.get()?.deviceId, null, null)) } userState.addObserver( @@ -81,8 +81,8 @@ internal class UserStore @JvmOverloads constructor( user.id != null || user.name != null || user.email != null private fun loadPersistedUser(): User? { - return if (sharedPrefMigrator.hasPrefs()) { - val legacyUser = sharedPrefMigrator.loadUser(deviceId) + return if (sharedPrefMigrator.get().hasPrefs()) { + val legacyUser = sharedPrefMigrator.get().loadUser(deviceIdStore.get()?.deviceId) save(legacyUser) legacyUser } else if ( diff --git a/app/src/main/java/com/bugsnag/android/internal/BackgroundTaskService.kt b/app/src/main/java/com/bugsnag/android/internal/BackgroundTaskService.kt index ef3394fd52..738b2f7922 100644 --- a/app/src/main/java/com/bugsnag/android/internal/BackgroundTaskService.kt +++ b/app/src/main/java/com/bugsnag/android/internal/BackgroundTaskService.kt @@ -1,6 +1,7 @@ package com.bugsnag.android.internal import androidx.annotation.VisibleForTesting +import com.bugsnag.android.internal.dag.RunnableProvider import java.util.concurrent.BlockingQueue import java.util.concurrent.Callable import java.util.concurrent.ExecutorService @@ -152,7 +153,11 @@ class BackgroundTaskService( @Throws(RejectedExecutionException::class) fun submitTask(taskType: TaskType, callable: Callable): Future { val task = FutureTask(callable) + execute(taskType, task) + return SafeFuture(task, taskType) + } + fun execute(taskType: TaskType, task: Runnable) { when (taskType) { TaskType.ERROR_REQUEST -> errorExecutor.execute(task) TaskType.SESSION_REQUEST -> sessionExecutor.execute(task) @@ -160,8 +165,6 @@ class BackgroundTaskService( TaskType.INTERNAL_REPORT -> internalReportExecutor.execute(task) TaskType.DEFAULT -> defaultExecutor.execute(task) } - - return SafeFuture(task, taskType) } /** @@ -185,6 +188,18 @@ class BackgroundTaskService( ioExecutor.awaitTerminationSafe() } + inline fun provider( + taskType: TaskType, + crossinline provider: () -> R + ): RunnableProvider { + val task = object : RunnableProvider() { + override fun invoke(): R = provider() + } + + execute(taskType, task) + return task + } + private fun ExecutorService.awaitTerminationSafe() { try { awaitTermination(SHUTDOWN_WAIT_MS, TimeUnit.MILLISECONDS) diff --git a/app/src/main/java/com/bugsnag/android/internal/BugsnagStoreMigrator.kt b/app/src/main/java/com/bugsnag/android/internal/BugsnagStoreMigrator.kt index 87b223c511..7affbf4fca 100644 --- a/app/src/main/java/com/bugsnag/android/internal/BugsnagStoreMigrator.kt +++ b/app/src/main/java/com/bugsnag/android/internal/BugsnagStoreMigrator.kt @@ -5,8 +5,9 @@ import java.io.File internal object BugsnagStoreMigrator { @JvmStatic - fun moveToNewDirectory(persistenceDir: File) { - val bugsnagDir = File(persistenceDir, "bugsnag") + fun migrateLegacyFiles(persistenceDir: Lazy): File { + val originalDir = persistenceDir.value + val bugsnagDir = File(originalDir, "bugsnag") if (!bugsnagDir.isDirectory) { bugsnagDir.mkdirs() } @@ -19,12 +20,12 @@ internal object BugsnagStoreMigrator { ) filesToMove.forEach { (from, to) -> - val fromFile = File(persistenceDir, from) + val fromFile = File(originalDir, from) if (fromFile.exists()) { - fromFile.renameTo( - File(bugsnagDir, to) - ) + fromFile.renameTo(File(bugsnagDir, to)) } } + + return bugsnagDir } } diff --git a/app/src/main/java/com/bugsnag/android/internal/ForegroundDetector.kt b/app/src/main/java/com/bugsnag/android/internal/ForegroundDetector.kt index 4308c8ee9a..548c56ebdc 100644 --- a/app/src/main/java/com/bugsnag/android/internal/ForegroundDetector.kt +++ b/app/src/main/java/com/bugsnag/android/internal/ForegroundDetector.kt @@ -55,6 +55,12 @@ internal object ForegroundDetector : ActivityLifecycleCallbacks, Handler.Callbac private var waitingForActivityRestart: Boolean = false + /** + * Marks the timestamp (relative to [SystemClock.elapsedRealtime]) that we initialised for the + * first time. + */ + internal val startupTime = SystemClock.elapsedRealtime() + @VisibleForTesting internal var backgroundSent = true diff --git a/app/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt b/app/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt index 17e195727b..cb147f1f15 100644 --- a/app/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt +++ b/app/src/main/java/com/bugsnag/android/internal/ImmutableConfig.kt @@ -22,10 +22,11 @@ import com.bugsnag.android.Session import com.bugsnag.android.Telemetry import com.bugsnag.android.ThreadSendPolicy import com.bugsnag.android.errorApiHeaders +import com.bugsnag.android.internal.dag.Provider +import com.bugsnag.android.internal.dag.ValueProvider import com.bugsnag.android.safeUnrollCauses import com.bugsnag.android.sessionApiHeaders import java.io.File -import java.util.concurrent.Callable import java.util.regex.Pattern data class ImmutableConfig( @@ -40,7 +41,7 @@ data class ImmutableConfig( val enabledBreadcrumbTypes: Set?, val telemetry: Set, val releaseStage: String?, - val buildUuid: String?, + val buildUuid: Provider?, val appVersion: String?, val versionCode: Int?, val appType: String?, @@ -53,6 +54,7 @@ data class ImmutableConfig( val maxPersistedEvents: Int, val maxPersistedSessions: Int, val maxReportedThreads: Int, + val maxStringValueLength: Int, val threadCollectionTimeLimitMillis: Long, val persistenceDirectory: Lazy, val sendLaunchCrashesSynchronously: Boolean, @@ -140,7 +142,7 @@ data class ImmutableConfig( @JvmOverloads internal fun convertToImmutableConfig( config: Configuration, - buildUuid: String? = null, + buildUuid: Provider? = null, packageInfo: PackageInfo? = null, appInfo: ApplicationInfo? = null, persistenceDir: Lazy = lazy { requireNotNull(config.persistenceDirectory) } @@ -174,6 +176,7 @@ internal fun convertToImmutableConfig( maxPersistedEvents = config.maxPersistedEvents, maxPersistedSessions = config.maxPersistedSessions, maxReportedThreads = config.maxReportedThreads, + maxStringValueLength = config.maxStringValueLength, threadCollectionTimeLimitMillis = config.threadCollectionTimeLimitMillis, enabledBreadcrumbTypes = config.enabledBreadcrumbTypes?.toSet(), telemetry = config.telemetry.toSet(), @@ -256,12 +259,7 @@ internal fun sanitiseConfiguration( @Suppress("SENSELESS_COMPARISON") if (configuration.delivery == null) { - configuration.delivery = DefaultDelivery( - connectivity, - configuration.apiKey, - configuration.maxStringValueLength, - configuration.logger!! - ) + configuration.delivery = DefaultDelivery(connectivity, configuration.logger!!) } return convertToImmutableConfig( configuration, @@ -275,25 +273,16 @@ internal fun sanitiseConfiguration( private fun collectBuildUuid( appInfo: ApplicationInfo?, backgroundTaskService: BackgroundTaskService -): String? { +): Provider? { val bundle = appInfo?.metaData return when { - bundle?.containsKey(BUILD_UUID) == true -> { + bundle?.containsKey(BUILD_UUID) == true -> ValueProvider( (bundle.getString(BUILD_UUID) ?: bundle.getInt(BUILD_UUID).toString()) .takeIf { it.isNotEmpty() } - } + ) - appInfo != null -> { - try { - backgroundTaskService - .submitTask( - TaskType.IO, - Callable { DexBuildIdGenerator.generateBuildId(appInfo) } - ) - .get() - } catch (e: Exception) { - null - } + appInfo != null -> backgroundTaskService.provider(TaskType.IO) { + DexBuildIdGenerator.generateBuildId(appInfo) } else -> null diff --git a/app/src/main/java/com/bugsnag/android/internal/dag/ConfigModule.kt b/app/src/main/java/com/bugsnag/android/internal/dag/ConfigModule.kt index 842057c2bc..aece93e476 100644 --- a/app/src/main/java/com/bugsnag/android/internal/dag/ConfigModule.kt +++ b/app/src/main/java/com/bugsnag/android/internal/dag/ConfigModule.kt @@ -14,7 +14,6 @@ internal class ConfigModule( configuration: Configuration, connectivity: Connectivity, bgTaskExecutor: BackgroundTaskService -) : DependencyModule() { - +) : BackgroundDependencyModule(bgTaskExecutor) { val config = sanitiseConfiguration(contextModule.ctx, configuration, connectivity, bgTaskExecutor) } diff --git a/app/src/main/java/com/bugsnag/android/internal/dag/ContextModule.kt b/app/src/main/java/com/bugsnag/android/internal/dag/ContextModule.kt index 9ece4ff604..0d36ce64bf 100644 --- a/app/src/main/java/com/bugsnag/android/internal/dag/ContextModule.kt +++ b/app/src/main/java/com/bugsnag/android/internal/dag/ContextModule.kt @@ -1,14 +1,16 @@ package com.bugsnag.android.internal.dag import android.content.Context +import com.bugsnag.android.internal.BackgroundTaskService /** * A dependency module which accesses the application context object, falling back to the supplied * context if it is the base context. */ internal class ContextModule( - appContext: Context -) : DependencyModule() { + appContext: Context, + bgTaskService: BackgroundTaskService +) : BackgroundDependencyModule(bgTaskService) { val ctx: Context = when (appContext.applicationContext) { null -> appContext diff --git a/app/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt b/app/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt index 731e5adfc5..a44ca147f4 100644 --- a/app/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt +++ b/app/src/main/java/com/bugsnag/android/internal/dag/DependencyModule.kt @@ -3,35 +3,37 @@ package com.bugsnag.android.internal.dag import com.bugsnag.android.internal.BackgroundTaskService import com.bugsnag.android.internal.TaskType -internal abstract class DependencyModule { - - private val properties = mutableListOf>() +internal interface DependencyModule +internal abstract class BackgroundDependencyModule( + @JvmField + val bgTaskService: BackgroundTaskService, + @JvmField + val taskType: TaskType = TaskType.DEFAULT +) : DependencyModule { /** - * Creates a new [Lazy] property that is marked as an object that should be resolved off the - * main thread when [resolveDependencies] is called. + * Convenience function to create and schedule a `RunnableProvider` of [taskType] with + * [bgTaskService]. The returned `RunnableProvider` will be implemented using the `provider` + * lambda as its `invoke` implementation. */ - fun future(initializer: () -> T): Lazy { - val lazy = lazy { - initializer() - } - properties.add(lazy) - return lazy + inline fun provider(crossinline provider: () -> R): RunnableProvider { + return bgTaskService.provider(taskType, provider) } /** - * Blocks until all dependencies in the module have been constructed. This provides the option - * for modules to construct objects in a background thread, then have a user block on another - * thread until all the objects have been constructed. + * Return a `RunnableProvider` containing the result of applying the given [mapping] to + * this `Provider`. The `RunnableProvider` will be scheduled with [bgTaskService] as a + * [taskType] when this function returns. + * + * This function behaves similar to `List.map` or `Any.let` but with `Provider` encapsulation + * to handle value reuse and threading. */ - fun resolveDependencies(bgTaskService: BackgroundTaskService, taskType: TaskType) { - kotlin.runCatching { - bgTaskService.submitTask( - taskType, - Runnable { - properties.forEach { it.value } - } - ).get() + internal inline fun Provider.map(crossinline mapping: (E) -> R): RunnableProvider { + val task = object : RunnableProvider() { + override fun invoke(): R = mapping(this@map.get()) } + + bgTaskService.execute(taskType, task) + return task } } diff --git a/app/src/main/java/com/bugsnag/android/internal/dag/Provider.kt b/app/src/main/java/com/bugsnag/android/internal/dag/Provider.kt new file mode 100644 index 0000000000..6eb4408439 --- /dev/null +++ b/app/src/main/java/com/bugsnag/android/internal/dag/Provider.kt @@ -0,0 +1,181 @@ +package com.bugsnag.android.internal.dag + +import android.os.Looper +import androidx.annotation.VisibleForTesting +import java.util.concurrent.atomic.AtomicInteger + +/** + * A lightweight abstraction similar to `Lazy` or `Future` allowing values to be calculated on + * separate threads, or to be pre-computed. + */ +interface Provider { + /** + * Same as [get] but will return `null` instead of throwing an exception if the value could + * not be computed. + */ + fun getOrNull(): E? + + /** + * Return the value sourced from this provider, throwing an exception if the provider failed + * to calculate a value. Anything thrown from here will have been captured when attempting + * to calculate the value. + */ + fun get(): E +} + +/** + * The primary implementation of [Provider], usually created using the + * [BackgroundDependencyModule.provider] function. Similar conceptually to + * [java.util.concurrent.FutureTask] but with a more compact implementation. The implementation + * of [RunnableProvider.get] is special because it behaves more like [Lazy.value] in that getting + * a value that is still pending will cause it to be run on the current thread instead of waiting + * for it to be run "sometime in the future". This makes RunnableProviders less bug-prone when + * dealing with single-thread executors (such as those in [BackgroundTaskService]). RunnableProvider + * also has special handling for the main-thread, ensuring no computational work (such as IO) is + * done on the main thread. + */ +abstract class RunnableProvider : Provider, Runnable { + private val state = AtomicInteger(TASK_STATE_PENDING) + + @Volatile + private var value: Any? = null + + /** + * Calculate the value of this [Provider]. This function will be called at-most once by [run]. + * Do not call this function directly, instead use [get] and [getOrNull] which implement the + * correct threading behaviour and will reuse the value if it has been previously calculated. + */ + abstract operator fun invoke(): E + + override fun getOrNull(): E? { + return getOr { return null } + } + + override fun get(): E { + return getOr { throw value as Throwable } + } + + private inline fun getOr(failureHandler: () -> E): E { + while (true) { + when (state.get()) { + TASK_STATE_RUNNING -> awaitResult() + TASK_STATE_PENDING -> { + if (isMainThread()) { + // When the calling thread is the 'main' thread, we *always* wait for the + // background workers to [invoke] this Provider, assuming that the Provider + // is performing some kind of IO that should be kept away from the main + // thread. Ideally this doesn't happen, but this behaviour avoids the + // need for complicated callback mechanisms. + awaitResult() + } else { + // If the Provider has yet to be computed, we will try and run it on the + // current thread. This potentially causes run() to happen on a different + // Thread to the expected worker (TaskType), effectively like work-stealing. + run() + } + } + + TASK_STATE_COMPLETE -> @Suppress("UNCHECKED_CAST") return value as E + TASK_STATE_FAILED -> failureHandler() + } + } + } + + private fun isMainThread(): Boolean { + return Thread.currentThread() === mainThread + } + + /** + * Cause the current thread to wait (block) until this `Provider` [isComplete]. Upon returning + * the [isComplete] function will return `true`. + */ + private fun awaitResult() { + synchronized(this) { + while (!isComplete()) { + @Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN") + (this as Object).wait() + } + } + } + + private fun isComplete() = when (state.get()) { + TASK_STATE_PENDING, TASK_STATE_RUNNING -> false + else -> true + } + + /** + * The main entry point for a provider, typically called by a worker thread from + * [BackgroundTaskService]. If [run] has already been called this will be a no-op (including + * a reentrant thread), as such the task state *must* be checked after calling this. + * + * This should not be called, and instead [get] or [getOrNull] should be used to obtain the + * value produced by [invoke]. + */ + final override fun run() { + if (state.compareAndSet(TASK_STATE_PENDING, TASK_STATE_RUNNING)) { + try { + value = invoke() + state.set(TASK_STATE_COMPLETE) + } catch (ex: Throwable) { + value = ex + state.set(TASK_STATE_FAILED) + } finally { + synchronized(this) { + // wakeup any waiting threads + @Suppress("PLATFORM_CLASS_MAPPED_TO_KOTLIN") + (this as Object).notifyAll() + } + } + } + } + + @VisibleForTesting + internal companion object { + /** + * The `Provider` task state before the provider has started actually running. This state + * indicates that the task has been constructed, has typically been scheduled but has + * not actually started running yet. + */ + private const val TASK_STATE_PENDING = 0 + + /** + * The `Provider` task state when running. Once the [run] function returns the state will + * be either [TASK_STATE_COMPLETE] or [TASK_STATE_FAILED]. + */ + private const val TASK_STATE_RUNNING = 1 + + /** + * The `Provider` state of a successfully completed task. When this is the state the + * provider value can be obtained immediately without error. + */ + private const val TASK_STATE_COMPLETE = 2 + + /** + * The `Provider` state of a task where [invoke] failed with an error or exception. + */ + private const val TASK_STATE_FAILED = 999 + + /** + * We cache the main thread to avoid any locks within [Looper.getMainLooper]. This is + * settable for unit tests, so that there doesn't have to be a valid Looper when they run. + * + * Actually access is done via the [mainThread] property. + */ + @VisibleForTesting + @Suppress("ObjectPropertyNaming") // backing property from 'mainThread' + internal var _mainThread: Thread? = null + get() { + if (field == null) { + field = Looper.getMainLooper().thread + } + return field + } + + internal val mainThread: Thread get() = _mainThread!! + } +} + +data class ValueProvider(private val value: T) : Provider { + override fun getOrNull(): T? = get() + override fun get(): T = value +} diff --git a/app/src/main/java/com/bugsnag/android/internal/dag/SystemServiceModule.kt b/app/src/main/java/com/bugsnag/android/internal/dag/SystemServiceModule.kt index eef01cbc63..1c7cd6cf02 100644 --- a/app/src/main/java/com/bugsnag/android/internal/dag/SystemServiceModule.kt +++ b/app/src/main/java/com/bugsnag/android/internal/dag/SystemServiceModule.kt @@ -2,13 +2,15 @@ package com.bugsnag.android.internal.dag import com.bugsnag.android.getActivityManager import com.bugsnag.android.getStorageManager +import com.bugsnag.android.internal.BackgroundTaskService /** * A dependency module which provides a reference to Android system services. */ internal class SystemServiceModule( - contextModule: ContextModule -) : DependencyModule() { + contextModule: ContextModule, + bgTaskService: BackgroundTaskService +) : BackgroundDependencyModule(bgTaskService) { val storageManager = contextModule.ctx.getStorageManager() val activityManager = contextModule.ctx.getActivityManager()