From 002fd06b72bdde0b4fb584cfcbf48746e59d394c Mon Sep 17 00:00:00 2001 From: David Perez Date: Tue, 22 Oct 2024 12:37:03 -0500 Subject: [PATCH] This PR adds Timber to the app (#4116) --- README.md | 5 ++ app/build.gradle.kts | 4 ++ .../platform/manager/CrashLogsManagerImpl.kt | 16 ----- .../data/platform/manager/LogsManagerImpl.kt | 27 ++++++++ .../x8bit/bitwarden/BitwardenApplication.kt | 6 +- .../auth/repository/AuthRepositoryImpl.kt | 23 ++++++- .../repository/di/AuthRepositoryModule.kt | 3 + .../AccessibilityNodeInfoManagerImpl.kt | 6 +- .../data/autofill/di/AutofillModule.kt | 6 +- .../processor/AutofillProcessorImpl.kt | 6 +- .../datasource/network/core/ResultCall.kt | 17 +++-- .../network/retrofit/RetrofitsImpl.kt | 19 +----- .../platform/datasource/sdk/BaseSdkSource.kt | 9 +-- .../{CrashLogsManager.kt => LogsManager.kt} | 9 ++- .../manager/di/PlatformManagerModule.kt | 8 +-- .../feature/settings/about/AboutViewModel.kt | 8 +-- .../platform/manager/CrashLogsManagerImpl.kt | 35 ---------- .../data/platform/manager/LogsManagerImpl.kt | 67 +++++++++++++++++++ .../auth/repository/AuthRepositoryTest.kt | 5 ++ .../processor/AutofillProcessorTest.kt | 10 +-- .../network/util/CallExtensionsTest.kt | 13 ++++ .../settings/about/AboutViewModelTest.kt | 10 +-- gradle/libs.versions.toml | 2 + 23 files changed, 202 insertions(+), 112 deletions(-) delete mode 100644 app/src/fdroid/java/com/x8bit/bitwarden/data/platform/manager/CrashLogsManagerImpl.kt create mode 100644 app/src/fdroid/java/com/x8bit/bitwarden/data/platform/manager/LogsManagerImpl.kt rename app/src/main/java/com/x8bit/bitwarden/data/platform/manager/{CrashLogsManager.kt => LogsManager.kt} (67%) delete mode 100644 app/src/standard/java/com/x8bit/bitwarden/data/platform/manager/CrashLogsManagerImpl.kt create mode 100644 app/src/standard/java/com/x8bit/bitwarden/data/platform/manager/LogsManagerImpl.kt diff --git a/README.md b/README.md index b20a3341f..8b8874249 100644 --- a/README.md +++ b/README.md @@ -171,6 +171,11 @@ The following is a list of all third-party dependencies included as part of the - Purpose: A networking layer interface. - License: Apache 2.0 +- **Timber** + - https://github.com/JakeWharton/timber + - Purpose: Extensible logging library for Android. + - License: Apache 2.0 + - **zxcvbn4j** - https://github.com/nulab/zxcvbn4j - Purpose: Password strength estimation. diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 6fd3ce718..d43d67d84 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -75,6 +75,7 @@ android { isMinifyEnabled = false buildConfigField(type = "boolean", name = "HAS_DEBUG_MENU", value = "true") + buildConfigField(type = "boolean", name = "HAS_LOGS_ENABLED", value = "true") } // Beta and Release variants are identical except beta has a different package name @@ -88,6 +89,7 @@ android { ) buildConfigField(type = "boolean", name = "HAS_DEBUG_MENU", value = "false") + buildConfigField(type = "boolean", name = "HAS_LOGS_ENABLED", value = "false") } release { isDebuggable = false @@ -98,6 +100,7 @@ android { ) buildConfigField(type = "boolean", name = "HAS_DEBUG_MENU", value = "false") + buildConfigField(type = "boolean", name = "HAS_LOGS_ENABLED", value = "false") } } @@ -200,6 +203,7 @@ dependencies { implementation(platform(libs.square.retrofit.bom)) implementation(libs.square.retrofit) implementation(libs.square.retrofit.kotlinx.serialization) + implementation(libs.timber) implementation(libs.zxing.zxing.core) // For now we are restricted to running Compose tests for debug builds only diff --git a/app/src/fdroid/java/com/x8bit/bitwarden/data/platform/manager/CrashLogsManagerImpl.kt b/app/src/fdroid/java/com/x8bit/bitwarden/data/platform/manager/CrashLogsManagerImpl.kt deleted file mode 100644 index 398b7f165..000000000 --- a/app/src/fdroid/java/com/x8bit/bitwarden/data/platform/manager/CrashLogsManagerImpl.kt +++ /dev/null @@ -1,16 +0,0 @@ -package com.x8bit.bitwarden.data.platform.manager - -import com.x8bit.bitwarden.data.platform.datasource.disk.legacy.LegacyAppCenterMigrator -import com.x8bit.bitwarden.data.platform.repository.SettingsRepository - -/** - * CrashLogsManager implementation for F-droid flavor builds. - */ -class CrashLogsManagerImpl( - settingsRepository: SettingsRepository, - legacyAppCenterMigrator: LegacyAppCenterMigrator, -) : CrashLogsManager { - override var isEnabled: Boolean = true - - override fun trackNonFatalException(throwable: Throwable) = Unit -} diff --git a/app/src/fdroid/java/com/x8bit/bitwarden/data/platform/manager/LogsManagerImpl.kt b/app/src/fdroid/java/com/x8bit/bitwarden/data/platform/manager/LogsManagerImpl.kt new file mode 100644 index 000000000..387bd196f --- /dev/null +++ b/app/src/fdroid/java/com/x8bit/bitwarden/data/platform/manager/LogsManagerImpl.kt @@ -0,0 +1,27 @@ +package com.x8bit.bitwarden.data.platform.manager + +import com.x8bit.bitwarden.BuildConfig +import com.x8bit.bitwarden.data.platform.datasource.disk.legacy.LegacyAppCenterMigrator +import com.x8bit.bitwarden.data.platform.repository.SettingsRepository +import com.x8bit.bitwarden.data.platform.repository.model.Environment +import timber.log.Timber + +/** + * [LogsManager] implementation for F-droid flavor builds. + */ +class LogsManagerImpl( + settingsRepository: SettingsRepository, + legacyAppCenterMigrator: LegacyAppCenterMigrator, +) : LogsManager { + init { + if (BuildConfig.HAS_LOGS_ENABLED) { + Timber.plant(Timber.DebugTree()) + } + } + + override var isEnabled: Boolean = false + + override fun setUserData(userId: String?, environmentType: Environment.Type) = Unit + + override fun trackNonFatalException(throwable: Throwable) = Unit +} diff --git a/app/src/main/java/com/x8bit/bitwarden/BitwardenApplication.kt b/app/src/main/java/com/x8bit/bitwarden/BitwardenApplication.kt index efca8263e..4d3dd821a 100644 --- a/app/src/main/java/com/x8bit/bitwarden/BitwardenApplication.kt +++ b/app/src/main/java/com/x8bit/bitwarden/BitwardenApplication.kt @@ -3,7 +3,7 @@ package com.x8bit.bitwarden import android.app.Application import com.x8bit.bitwarden.data.auth.manager.AuthRequestNotificationManager import com.x8bit.bitwarden.data.platform.annotation.OmitFromCoverage -import com.x8bit.bitwarden.data.platform.manager.CrashLogsManager +import com.x8bit.bitwarden.data.platform.manager.LogsManager import com.x8bit.bitwarden.data.platform.manager.NetworkConfigManager import com.x8bit.bitwarden.data.platform.manager.event.OrganizationEventManager import com.x8bit.bitwarden.data.platform.manager.restriction.RestrictionManager @@ -19,10 +19,10 @@ class BitwardenApplication : Application() { // Inject classes here that must be triggered on startup but are not otherwise consumed by // other callers. @Inject - lateinit var networkConfigManager: NetworkConfigManager + lateinit var logsManager: LogsManager @Inject - lateinit var crashLogsManager: CrashLogsManager + lateinit var networkConfigManager: NetworkConfigManager @Inject lateinit var authRequestNotificationManager: AuthRequestNotificationManager diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt index 747f60163..19776db93 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryImpl.kt @@ -95,6 +95,7 @@ import com.x8bit.bitwarden.data.auth.util.toSdkParams import com.x8bit.bitwarden.data.platform.datasource.disk.ConfigDiskSource import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager +import com.x8bit.bitwarden.data.platform.manager.LogsManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.PushManager import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager @@ -104,6 +105,7 @@ import com.x8bit.bitwarden.data.platform.manager.util.getActivePolicies import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow +import com.x8bit.bitwarden.data.platform.repository.util.toEnvironmentUrls import com.x8bit.bitwarden.data.platform.util.asFailure import com.x8bit.bitwarden.data.platform.util.asSuccess import com.x8bit.bitwarden.data.platform.util.flatMap @@ -164,7 +166,8 @@ class AuthRepositoryImpl( private val userLogoutManager: UserLogoutManager, private val policyManager: PolicyManager, private val featureFlagManager: FeatureFlagManager, - private val firstTimeActionManager: FirstTimeActionManager, + firstTimeActionManager: FirstTimeActionManager, + logsManager: LogsManager, pushManager: PushManager, dispatcherManager: DispatcherManager, ) : AuthRepository, @@ -363,6 +366,24 @@ class AuthRepositoryImpl( featureFlagManager.getFeatureFlag(FlagKey.OnboardingCarousel) init { + combine( + mutableHasPendingAccountAdditionStateFlow, + authDiskSource.userStateFlow, + environmentRepository.environmentStateFlow, + ) { hasPendingAddition, userState, environment -> + logsManager.setUserData( + userId = userState?.activeUserId.takeUnless { hasPendingAddition }, + environmentType = userState + ?.activeAccount + ?.settings + ?.environmentUrlData + ?.toEnvironmentUrls() + ?.type + .takeUnless { hasPendingAddition } + ?: environment.type, + ) + } + .launchIn(unconfinedScope) pushManager .syncOrgKeysFlow .onEach { diff --git a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/di/AuthRepositoryModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/di/AuthRepositoryModule.kt index 4e07ab2e3..99bbe4566 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/di/AuthRepositoryModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/auth/repository/di/AuthRepositoryModule.kt @@ -16,6 +16,7 @@ import com.x8bit.bitwarden.data.auth.repository.AuthRepositoryImpl import com.x8bit.bitwarden.data.platform.datasource.disk.ConfigDiskSource import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager +import com.x8bit.bitwarden.data.platform.manager.LogsManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.PushManager import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager @@ -60,6 +61,7 @@ object AuthRepositoryModule { policyManager: PolicyManager, featureFlagManager: FeatureFlagManager, firstTimeActionManager: FirstTimeActionManager, + logsManager: LogsManager, ): AuthRepository = AuthRepositoryImpl( accountsService = accountsService, devicesService = devicesService, @@ -82,5 +84,6 @@ object AuthRepositoryModule { policyManager = policyManager, featureFlagManager = featureFlagManager, firstTimeActionManager = firstTimeActionManager, + logsManager = logsManager, ) } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityNodeInfoManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityNodeInfoManagerImpl.kt index 4c497530e..da2baaea0 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityNodeInfoManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/accessibility/manager/AccessibilityNodeInfoManagerImpl.kt @@ -1,11 +1,10 @@ package com.x8bit.bitwarden.data.autofill.accessibility.manager import android.net.Uri -import android.util.Log import android.view.accessibility.AccessibilityNodeInfo -import com.x8bit.bitwarden.BuildConfig import com.x8bit.bitwarden.data.autofill.accessibility.util.getKnownUsernameFieldNull import com.x8bit.bitwarden.data.autofill.accessibility.util.isUsername +import timber.log.Timber private const val MAX_NODE_COUNT: Int = 100 @@ -90,7 +89,6 @@ class AccessibilityNodeInfoManagerImpl : AccessibilityNodeInfoManager { ?.let { allNodes.getOrNull(index = allNodes.indexOf(element = it) - 1) } private fun log(message: String) { - if (!BuildConfig.DEBUG) return - Log.i("AccessibilityNodeInfoManager", message) + Timber.i(message) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/AutofillModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/AutofillModule.kt index a9309a3b9..fdb632791 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/AutofillModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/di/AutofillModule.kt @@ -21,7 +21,7 @@ import com.x8bit.bitwarden.data.autofill.processor.AutofillProcessor import com.x8bit.bitwarden.data.autofill.processor.AutofillProcessorImpl import com.x8bit.bitwarden.data.autofill.provider.AutofillCipherProvider import com.x8bit.bitwarden.data.autofill.provider.AutofillCipherProviderImpl -import com.x8bit.bitwarden.data.platform.manager.CrashLogsManager +import com.x8bit.bitwarden.data.platform.manager.LogsManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.ciphermatching.CipherMatchingManager import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager @@ -121,7 +121,7 @@ object AutofillModule { policyManager: PolicyManager, saveInfoBuilder: SaveInfoBuilder, settingsRepository: SettingsRepository, - crashLogsManager: CrashLogsManager, + logsManager: LogsManager, ): AutofillProcessor = AutofillProcessorImpl( dispatcherManager = dispatcherManager, @@ -131,7 +131,7 @@ object AutofillModule { policyManager = policyManager, saveInfoBuilder = saveInfoBuilder, settingsRepository = settingsRepository, - crashLogsManager = crashLogsManager, + logsManager = logsManager, ) @Singleton diff --git a/app/src/main/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorImpl.kt index 5e668c9fb..611e18000 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorImpl.kt @@ -13,7 +13,7 @@ import com.x8bit.bitwarden.data.autofill.model.AutofillRequest import com.x8bit.bitwarden.data.autofill.parser.AutofillParser import com.x8bit.bitwarden.data.autofill.util.createAutofillSavedItemIntentSender import com.x8bit.bitwarden.data.autofill.util.toAutofillSaveItem -import com.x8bit.bitwarden.data.platform.manager.CrashLogsManager +import com.x8bit.bitwarden.data.platform.manager.LogsManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager import com.x8bit.bitwarden.data.platform.repository.SettingsRepository @@ -35,7 +35,7 @@ class AutofillProcessorImpl( private val parser: AutofillParser, private val saveInfoBuilder: SaveInfoBuilder, private val settingsRepository: SettingsRepository, - private val crashLogsManager: CrashLogsManager, + private val logsManager: LogsManager, ) : AutofillProcessor { /** @@ -146,7 +146,7 @@ class AutofillProcessorImpl( } catch (e: RuntimeException) { // This is to catch any TransactionTooLargeExceptions that could occur here. // These exceptions get wrapped as a RuntimeException. - crashLogsManager.trackNonFatalException(e) + logsManager.trackNonFatalException(e) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/core/ResultCall.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/core/ResultCall.kt index 0d312c22c..7acc77c56 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/core/ResultCall.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/core/ResultCall.kt @@ -9,6 +9,7 @@ import retrofit2.Call import retrofit2.Callback import retrofit2.HttpException import retrofit2.Response +import timber.log.Timber import java.lang.reflect.Type /** @@ -19,6 +20,7 @@ private const val NO_CONTENT_RESPONSE_CODE: Int = 204 /** * A [Call] for wrapping a network request into a [Result]. */ +@Suppress("TooManyFunctions") class ResultCall( private val backingCall: Call, private val successType: Type, @@ -34,7 +36,7 @@ class ResultCall( } override fun onFailure(call: Call, t: Throwable) { - callback.onResponse(this@ResultCall, Response.success(t.asFailure())) + callback.onResponse(this@ResultCall, Response.success(t.toFailure())) } }, ) @@ -44,9 +46,9 @@ class ResultCall( try { Response.success(backingCall.execute().toResult()) } catch (ioException: IOException) { - Response.success(ioException.asFailure()) + Response.success(ioException.toFailure()) } catch (runtimeException: RuntimeException) { - Response.success(runtimeException.asFailure()) + Response.success(runtimeException.toFailure()) } override fun isCanceled(): Boolean = backingCall.isCanceled @@ -62,9 +64,14 @@ class ResultCall( */ fun executeForResult(): Result = requireNotNull(execute().body()) + private fun Throwable.toFailure(): Result = + this + .also { Timber.w(it, "Network Error: ${backingCall.request().url}") } + .asFailure() + private fun Response.toResult(): Result = if (!this.isSuccessful) { - HttpException(this).asFailure() + HttpException(this).toFailure() } else { val body = this.body() @Suppress("UNCHECKED_CAST") @@ -76,7 +83,7 @@ class ResultCall( // We allow null for 204's, just return null. this.code() == NO_CONTENT_RESPONSE_CODE -> (null as T).asSuccess() // All other null bodies result in an error. - else -> IllegalStateException("Unexpected null body!").asFailure() + else -> IllegalStateException("Unexpected null body!").toFailure() } } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/retrofit/RetrofitsImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/retrofit/RetrofitsImpl.kt index 59f2cb9d2..e681f484c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/retrofit/RetrofitsImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/network/retrofit/RetrofitsImpl.kt @@ -1,7 +1,5 @@ package com.x8bit.bitwarden.data.platform.datasource.network.retrofit -import android.util.Log -import com.x8bit.bitwarden.BuildConfig import com.x8bit.bitwarden.data.platform.datasource.network.authenticator.RefreshAuthenticator import com.x8bit.bitwarden.data.platform.datasource.network.core.ResultCallAdapterFactory import com.x8bit.bitwarden.data.platform.datasource.network.interceptor.AuthTokenInterceptor @@ -15,8 +13,7 @@ import okhttp3.OkHttpClient import okhttp3.logging.HttpLoggingInterceptor import retrofit2.Retrofit import retrofit2.converter.kotlinx.serialization.asConverterFactory - -private const val MAX_LOG_MESSAGE_LENGTH: Int = 4000 +import timber.log.Timber /** * Primary implementation of [Retrofits]. @@ -79,20 +76,10 @@ class RetrofitsImpl( //region Helper properties and functions private val loggingInterceptor: HttpLoggingInterceptor by lazy { - HttpLoggingInterceptor { message -> - message.chunked(size = MAX_LOG_MESSAGE_LENGTH).forEach { chunk -> - Log.d("BitwardenNetworkClient", chunk) - } - } + HttpLoggingInterceptor { message -> Timber.tag("BitwardenNetworkClient").d(message) } .apply { redactHeader(name = HEADER_KEY_AUTHORIZATION) - setLevel( - if (BuildConfig.DEBUG) { - HttpLoggingInterceptor.Level.BODY - } else { - HttpLoggingInterceptor.Level.NONE - }, - ) + setLevel(HttpLoggingInterceptor.Level.BODY) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/sdk/BaseSdkSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/sdk/BaseSdkSource.kt index d0ddb3ed5..9a88ae812 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/sdk/BaseSdkSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/datasource/sdk/BaseSdkSource.kt @@ -1,9 +1,8 @@ package com.x8bit.bitwarden.data.platform.datasource.sdk -import android.util.Log import com.bitwarden.sdk.Client -import com.x8bit.bitwarden.BuildConfig import com.x8bit.bitwarden.data.platform.manager.SdkClientManager +import timber.log.Timber /** * Base class for simplifying sdk interactions. @@ -27,9 +26,5 @@ abstract class BaseSdkSource( protected inline fun T.runCatchingWithLogs( block: T.() -> R, ): Result = runCatching(block = block) - .onFailure { - if (BuildConfig.DEBUG) { - Log.w(this@BaseSdkSource::class.java.simpleName, it) - } - } + .onFailure { Timber.w(it) } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/CrashLogsManager.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/LogsManager.kt similarity index 67% rename from app/src/main/java/com/x8bit/bitwarden/data/platform/manager/CrashLogsManager.kt rename to app/src/main/java/com/x8bit/bitwarden/data/platform/manager/LogsManager.kt index dd0ec0172..d191627c5 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/CrashLogsManager.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/LogsManager.kt @@ -1,10 +1,12 @@ package com.x8bit.bitwarden.data.platform.manager +import com.x8bit.bitwarden.data.platform.repository.model.Environment + /** * Implementations of this interface provide a way to enable or disable the collection of crash * logs, giving control over whether crash logs are generated and stored. */ -interface CrashLogsManager { +interface LogsManager { /** * Gets or sets whether the collection of crash logs is enabled. */ @@ -14,4 +16,9 @@ interface CrashLogsManager { * Tracks a [Throwable] if logs are enabled. */ fun trackNonFatalException(throwable: Throwable) + + /** + * Tracks the current user data. + */ + fun setUserData(userId: String?, environmentType: Environment.Type) } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt index 4bb8ee4b0..62ab14ea7 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/di/PlatformManagerModule.kt @@ -19,13 +19,13 @@ import com.x8bit.bitwarden.data.platform.manager.AssetManager import com.x8bit.bitwarden.data.platform.manager.AssetManagerImpl import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManager import com.x8bit.bitwarden.data.platform.manager.BiometricsEncryptionManagerImpl -import com.x8bit.bitwarden.data.platform.manager.CrashLogsManager -import com.x8bit.bitwarden.data.platform.manager.CrashLogsManagerImpl import com.x8bit.bitwarden.data.platform.manager.DebugMenuFeatureFlagManagerImpl import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManagerImpl import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManagerImpl +import com.x8bit.bitwarden.data.platform.manager.LogsManager +import com.x8bit.bitwarden.data.platform.manager.LogsManagerImpl import com.x8bit.bitwarden.data.platform.manager.NetworkConfigManager import com.x8bit.bitwarden.data.platform.manager.NetworkConfigManagerImpl import com.x8bit.bitwarden.data.platform.manager.NetworkConnectionManager @@ -238,10 +238,10 @@ object PlatformManagerModule { @Provides @Singleton - fun provideCrashLogsManager( + fun provideLogsManager( legacyAppCenterMigrator: LegacyAppCenterMigrator, settingsRepository: SettingsRepository, - ): CrashLogsManager = CrashLogsManagerImpl( + ): LogsManager = LogsManagerImpl( settingsRepository = settingsRepository, legacyAppCenterMigrator = legacyAppCenterMigrator, ) diff --git a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutViewModel.kt b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutViewModel.kt index 6b11035a7..a354e064b 100644 --- a/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutViewModel.kt +++ b/app/src/main/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutViewModel.kt @@ -5,7 +5,7 @@ import androidx.lifecycle.SavedStateHandle import androidx.lifecycle.viewModelScope import com.x8bit.bitwarden.BuildConfig import com.x8bit.bitwarden.R -import com.x8bit.bitwarden.data.platform.manager.CrashLogsManager +import com.x8bit.bitwarden.data.platform.manager.LogsManager import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager import com.x8bit.bitwarden.data.platform.repository.EnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.util.baseWebVaultUrlOrDefault @@ -33,12 +33,12 @@ class AboutViewModel @Inject constructor( private val savedStateHandle: SavedStateHandle, private val clipboardManager: BitwardenClipboardManager, clock: Clock, - private val crashLogsManager: CrashLogsManager, + private val logsManager: LogsManager, private val environmentRepository: EnvironmentRepository, ) : BaseViewModel( initialState = savedStateHandle[KEY_STATE] ?: createInitialState( clock = clock, - isCrashLoggingEnabled = crashLogsManager.isEnabled, + isCrashLoggingEnabled = logsManager.isEnabled, ), ) { init { @@ -84,7 +84,7 @@ class AboutViewModel @Inject constructor( } private fun handleSubmitCrashLogsClick(action: AboutAction.SubmitCrashLogsClick) { - crashLogsManager.isEnabled = action.enabled + logsManager.isEnabled = action.enabled mutableStateFlow.update { currentState -> currentState.copy(isSubmitCrashLogsEnabled = action.enabled) } diff --git a/app/src/standard/java/com/x8bit/bitwarden/data/platform/manager/CrashLogsManagerImpl.kt b/app/src/standard/java/com/x8bit/bitwarden/data/platform/manager/CrashLogsManagerImpl.kt deleted file mode 100644 index dceffe3b6..000000000 --- a/app/src/standard/java/com/x8bit/bitwarden/data/platform/manager/CrashLogsManagerImpl.kt +++ /dev/null @@ -1,35 +0,0 @@ -package com.x8bit.bitwarden.data.platform.manager - -import com.google.firebase.crashlytics.ktx.crashlytics -import com.google.firebase.ktx.Firebase -import com.x8bit.bitwarden.data.platform.annotation.OmitFromCoverage -import com.x8bit.bitwarden.data.platform.datasource.disk.legacy.LegacyAppCenterMigrator -import com.x8bit.bitwarden.data.platform.repository.SettingsRepository - -/** - * CrashLogsManager implementation for standard flavor builds. - */ -@OmitFromCoverage -class CrashLogsManagerImpl( - private val settingsRepository: SettingsRepository, - legacyAppCenterMigrator: LegacyAppCenterMigrator, -) : CrashLogsManager { - - override var isEnabled: Boolean - get() = settingsRepository.isCrashLoggingEnabled - set(value) { - settingsRepository.isCrashLoggingEnabled = value - Firebase.crashlytics.isCrashlyticsCollectionEnabled = value - } - - override fun trackNonFatalException(throwable: Throwable) { - if (settingsRepository.isCrashLoggingEnabled) { - Firebase.crashlytics.recordException(throwable) - } - } - - init { - legacyAppCenterMigrator.migrateIfNecessary() - isEnabled = settingsRepository.isCrashLoggingEnabled - } -} diff --git a/app/src/standard/java/com/x8bit/bitwarden/data/platform/manager/LogsManagerImpl.kt b/app/src/standard/java/com/x8bit/bitwarden/data/platform/manager/LogsManagerImpl.kt new file mode 100644 index 000000000..0947f8b25 --- /dev/null +++ b/app/src/standard/java/com/x8bit/bitwarden/data/platform/manager/LogsManagerImpl.kt @@ -0,0 +1,67 @@ +package com.x8bit.bitwarden.data.platform.manager + +import com.google.firebase.crashlytics.ktx.crashlytics +import com.google.firebase.ktx.Firebase +import com.x8bit.bitwarden.BuildConfig +import com.x8bit.bitwarden.data.platform.annotation.OmitFromCoverage +import com.x8bit.bitwarden.data.platform.datasource.disk.legacy.LegacyAppCenterMigrator +import com.x8bit.bitwarden.data.platform.repository.SettingsRepository +import com.x8bit.bitwarden.data.platform.repository.model.Environment +import timber.log.Timber + +/** + * [LogsManager] implementation for standard flavor builds. + */ +@OmitFromCoverage +class LogsManagerImpl( + private val settingsRepository: SettingsRepository, + legacyAppCenterMigrator: LegacyAppCenterMigrator, +) : LogsManager { + + private val nonfatalErrorTree: NonfatalErrorTree = NonfatalErrorTree() + + override var isEnabled: Boolean + get() = settingsRepository.isCrashLoggingEnabled + set(value) { + settingsRepository.isCrashLoggingEnabled = value + Firebase.crashlytics.isCrashlyticsCollectionEnabled = value + if (BuildConfig.HAS_LOGS_ENABLED) { + Timber.plant(Timber.DebugTree()) + } + if (value) { + Timber.plant(nonfatalErrorTree) + } else { + Timber.uproot(nonfatalErrorTree) + } + } + + override fun setUserData(userId: String?, environmentType: Environment.Type) { + Firebase.crashlytics.setUserId(userId.orEmpty()) + Firebase.crashlytics.setCustomKey( + if (userId == null) "PreAuthRegion" else "Region", + environmentType.toString(), + ) + } + + override fun trackNonFatalException(throwable: Throwable) { + if (isEnabled) { + Firebase.crashlytics.recordException(throwable) + } + } + + init { + legacyAppCenterMigrator.migrateIfNecessary() + isEnabled = settingsRepository.isCrashLoggingEnabled + } + + private inner class NonfatalErrorTree : Timber.Tree() { + override fun log(priority: Int, tag: String?, message: String, t: Throwable?) { + t?.let { trackNonFatalException(BitwardenNonfatalException(message, it)) } + } + } +} + +private class BitwardenNonfatalException( + message: String, + throwable: Throwable, +) : Exception(message, throwable) diff --git a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt index 12336587b..2f22d25be 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/auth/repository/AuthRepositoryTest.kt @@ -102,6 +102,7 @@ import com.x8bit.bitwarden.data.platform.datasource.disk.util.FakeConfigDiskSour import com.x8bit.bitwarden.data.platform.datasource.network.model.ConfigResponseJson import com.x8bit.bitwarden.data.platform.manager.FeatureFlagManager import com.x8bit.bitwarden.data.platform.manager.FirstTimeActionManager +import com.x8bit.bitwarden.data.platform.manager.LogsManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.manager.PushManager import com.x8bit.bitwarden.data.platform.manager.dispatcher.DispatcherManager @@ -249,6 +250,9 @@ class AuthRepositoryTest { every { currentOrDefaultUserFirstTimeState } returns FIRST_TIME_STATE every { firstTimeStateFlow } returns MutableStateFlow(FIRST_TIME_STATE) } + private val logsManager: LogsManager = mockk { + every { setUserData(userId = any(), environmentType = any()) } just runs + } private val repository = AuthRepositoryImpl( accountsService = accountsService, @@ -272,6 +276,7 @@ class AuthRepositoryTest { policyManager = policyManager, featureFlagManager = featureFlagManager, firstTimeActionManager = firstTimeActionManager, + logsManager = logsManager, ) @BeforeEach diff --git a/app/src/test/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorTest.kt index c27e523ab..ec85b9be9 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/autofill/processor/AutofillProcessorTest.kt @@ -22,7 +22,7 @@ import com.x8bit.bitwarden.data.autofill.parser.AutofillParser import com.x8bit.bitwarden.data.autofill.util.createAutofillSavedItemIntentSender import com.x8bit.bitwarden.data.autofill.util.toAutofillSaveItem import com.x8bit.bitwarden.data.platform.base.FakeDispatcherManager -import com.x8bit.bitwarden.data.platform.manager.CrashLogsManager +import com.x8bit.bitwarden.data.platform.manager.LogsManager import com.x8bit.bitwarden.data.platform.manager.PolicyManager import com.x8bit.bitwarden.data.platform.repository.SettingsRepository import com.x8bit.bitwarden.data.vault.datasource.network.model.PolicyTypeJson @@ -57,7 +57,7 @@ class AutofillProcessorTest { private val policyManager: PolicyManager = mockk() private val saveInfoBuilder: SaveInfoBuilder = mockk() private val settingsRepository: SettingsRepository = mockk() - private val crashLogsManager: CrashLogsManager = mockk() + private val logsManager: LogsManager = mockk() private val appInfo: AutofillAppInfo = AutofillAppInfo( context = mockk(), @@ -79,7 +79,7 @@ class AutofillProcessorTest { policyManager = policyManager, saveInfoBuilder = saveInfoBuilder, settingsRepository = settingsRepository, - crashLogsManager = crashLogsManager, + logsManager = logsManager, ) } @@ -246,7 +246,7 @@ class AutofillProcessorTest { } returns fillResponse val runtimeException = RuntimeException("TransactionToLarge") every { fillCallback.onSuccess(fillResponse) } throws runtimeException - every { crashLogsManager.trackNonFatalException(runtimeException) } just runs + every { logsManager.trackNonFatalException(runtimeException) } just runs // Test autofillProcessor.processFillRequest( @@ -271,7 +271,7 @@ class AutofillProcessorTest { saveInfo = saveInfo, ) fillCallback.onSuccess(fillResponse) - crashLogsManager.trackNonFatalException(runtimeException) + logsManager.trackNonFatalException(runtimeException) } } diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/util/CallExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/util/CallExtensionsTest.kt index b123f05b9..36ab10f53 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/util/CallExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/datasource/network/util/CallExtensionsTest.kt @@ -3,6 +3,7 @@ package com.x8bit.bitwarden.data.platform.datasource.network.util import com.x8bit.bitwarden.data.platform.util.asSuccess import io.mockk.every import io.mockk.mockk +import okhttp3.Request import okhttp3.ResponseBody.Companion.toResponseBody import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertTrue @@ -15,7 +16,11 @@ class CallExtensionsTest { @Test fun `executeForResult returns failure when execute throws IOException`() { + val request = mockk { + every { url } returns mockk() + } val call = mockk> { + every { request() } returns request every { execute() } throws IOException("Fail") } @@ -26,7 +31,11 @@ class CallExtensionsTest { @Test fun `executeForResult returns failure when execute throws RuntimeException`() { + val request = mockk { + every { url } returns mockk() + } val call = mockk> { + every { request() } returns request every { execute() } throws RuntimeException("Fail") } @@ -37,7 +46,11 @@ class CallExtensionsTest { @Test fun `executeForResult returns failure when response is failure`() { + val request = mockk { + every { url } returns mockk() + } val call = mockk> { + every { request() } returns request every { execute() } returns Response.error(400, "".toResponseBody()) } diff --git a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutViewModelTest.kt b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutViewModelTest.kt index f0f042719..075d13c4d 100644 --- a/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutViewModelTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/ui/platform/feature/settings/about/AboutViewModelTest.kt @@ -3,7 +3,7 @@ package com.x8bit.bitwarden.ui.platform.feature.settings.about import androidx.lifecycle.SavedStateHandle import app.cash.turbine.test import com.x8bit.bitwarden.BuildConfig -import com.x8bit.bitwarden.data.platform.manager.CrashLogsManager +import com.x8bit.bitwarden.data.platform.manager.LogsManager import com.x8bit.bitwarden.data.platform.manager.clipboard.BitwardenClipboardManager import com.x8bit.bitwarden.data.platform.repository.util.FakeEnvironmentRepository import com.x8bit.bitwarden.data.platform.repository.util.baseWebVaultUrlOrDefault @@ -30,7 +30,7 @@ class AboutViewModelTest : BaseViewModelTest() { private val environmentRepository = FakeEnvironmentRepository() private val clipboardManager: BitwardenClipboardManager = mockk() - private val crashLogsManager: CrashLogsManager = mockk { + private val logsManager: LogsManager = mockk { every { isEnabled } returns false every { isEnabled = any() } just runs } @@ -99,12 +99,12 @@ class AboutViewModelTest : BaseViewModelTest() { } @Test - fun `on SubmitCrashLogsClick should update crashLogsManager isEnabled`() = runTest { + fun `on SubmitCrashLogsClick should update LogsManager isEnabled`() = runTest { val viewModel = createViewModel(DEFAULT_ABOUT_STATE) viewModel.trySendAction(AboutAction.SubmitCrashLogsClick(true)) - coVerify(exactly = 1) { crashLogsManager.isEnabled = true } + coVerify(exactly = 1) { logsManager.isEnabled = true } } @Test @@ -153,7 +153,7 @@ class AboutViewModelTest : BaseViewModelTest() { clipboardManager = clipboardManager, clock = fixedClock, environmentRepository = environmentRepository, - crashLogsManager = crashLogsManager, + logsManager = logsManager, ) } diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 83ed8753e..944343bd2 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -45,6 +45,7 @@ okhttp = "4.12.0" retrofitBom = "2.11.0" robolectric = "4.13" sonarqube = "5.1.0.4882" +timber = "5.0.1" turbine = "1.2.0" zxcvbn4j = "1.9.0" zxing = "3.5.3" @@ -110,6 +111,7 @@ square-retrofit = { module = "com.squareup.retrofit2:retrofit" } square-retrofit-bom = { module = "com.squareup.retrofit2:retrofit-bom", version.ref = "retrofitBom" } square-retrofit-kotlinx-serialization = { module = "com.squareup.retrofit2:converter-kotlinx-serialization" } square-turbine = { module = "app.cash.turbine:turbine", version.ref = "turbine" } +timber = { module = "com.jakewharton.timber:timber", version.ref = "timber" } zxing-zxing-core = { module = "com.google.zxing:core", version.ref = "zxing" } [plugins]