From de695678129c94373e91e028279aa049a718bc5b Mon Sep 17 00:00:00 2001 From: Kevin Gorham Date: Wed, 10 Jun 2020 10:01:58 -0400 Subject: [PATCH] Explicitly disable all feedback, dev info and crash reporting. Addresses https://github.com/zcash/zcash-android-wallet/issues/143 by placing everything behind a user setting that can be enabled in the future by users who want to continue helping us improve the user experience. For the most part, this will just be turned on for internal company releases in order to continue learning and improving the app. --- app/build.gradle | 10 +- app/src/main/AndroidManifest.xml | 2 + .../java/cash/z/ecc/android/ZcashWalletApp.kt | 1 - .../cash/z/ecc/android/di/module/AppModule.kt | 20 ++- .../android/feedback/FeedbackCrashlytics.kt | 4 +- .../cash/z/ecc/android/feedback/Report.kt | 1 + .../ui/detail/TransactionViewHolder.kt | 124 +++++++++--------- .../z/ecc/android/ui/scan/ScanFragment.kt | 4 +- .../ecc/android/ui/send/SendFinalFragment.kt | 5 +- .../z/ecc/android/ui/send/SendViewModel.kt | 3 +- .../z/ecc/android/ui/setup/BackupFragment.kt | 2 - build.gradle | 2 +- .../java/cash/z/ecc/android/Dependencies.kt | 5 +- .../android/feedback/FeedbackCoordinator.kt | 1 + 14 files changed, 96 insertions(+), 88 deletions(-) diff --git a/app/build.gradle b/app/build.gradle index 1c44b4a..2293a2d 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -5,8 +5,7 @@ apply plugin: 'kotlin-android' apply plugin: 'kotlin-android-extensions' apply plugin: 'kotlin-kapt' apply plugin: 'com.google.gms.google-services' -apply plugin: 'io.fabric' -apply plugin: 'com.google.firebase.firebase-perf' +apply plugin: 'com.google.firebase.crashlytics' //apply plugin: 'com.github.ben-manes.versions' archivesBaseName = 'zcash-android-wallet' @@ -102,10 +101,6 @@ android { } } -crashlytics { - enableNdk true -} - dependencies { implementation fileTree(dir: 'libs', include: ['*.jar']) implementation project(':qrecycler') @@ -161,9 +156,6 @@ dependencies { // Analytics (for dogfooding/crash-reporting/feedback only on internal team builds) implementation Deps.Analytics.CRASHLYTICS - implementation Deps.Analytics.CRASHLYTICS_NDK - implementation Deps.Analytics.FIREBASE - implementation Deps.Analytics.FIREBASE_PERF implementation Deps.Analytics.MIXPANEL // Misc. diff --git a/app/src/main/AndroidManifest.xml b/app/src/main/AndroidManifest.xml index 7b5fa4f..099b1b7 100644 --- a/app/src/main/AndroidManifest.xml +++ b/app/src/main/AndroidManifest.xml @@ -33,6 +33,8 @@ + + diff --git a/app/src/main/java/cash/z/ecc/android/ZcashWalletApp.kt b/app/src/main/java/cash/z/ecc/android/ZcashWalletApp.kt index fc3ab61..44ee9f9 100644 --- a/app/src/main/java/cash/z/ecc/android/ZcashWalletApp.kt +++ b/app/src/main/java/cash/z/ecc/android/ZcashWalletApp.kt @@ -37,7 +37,6 @@ class ZcashWalletApp : Application(), CameraXConfig.Provider { override fun onCreate() { Thread.setDefaultUncaughtExceptionHandler(ExceptionReporter(Thread.getDefaultUncaughtExceptionHandler())) - Twig.plant(TroubleshootingTwig()) creationTime = System.currentTimeMillis() instance = this // Setup handler for uncaught exceptions. diff --git a/app/src/main/java/cash/z/ecc/android/di/module/AppModule.kt b/app/src/main/java/cash/z/ecc/android/di/module/AppModule.kt index 92870ce..948eabd 100644 --- a/app/src/main/java/cash/z/ecc/android/di/module/AppModule.kt +++ b/app/src/main/java/cash/z/ecc/android/di/module/AppModule.kt @@ -2,9 +2,14 @@ package cash.z.ecc.android.di.module import android.content.ClipboardManager import android.content.Context +import android.content.SharedPreferences import cash.z.ecc.android.ZcashWalletApp import cash.z.ecc.android.di.component.MainActivitySubcomponent import cash.z.ecc.android.feedback.* +import cash.z.ecc.android.sdk.ext.SilentTwig +import cash.z.ecc.android.sdk.ext.TroubleshootingTwig +import cash.z.ecc.android.sdk.ext.Twig +import com.google.firebase.crashlytics.FirebaseCrashlytics import dagger.Module import dagger.Provides import dagger.multibindings.IntoSet @@ -27,6 +32,11 @@ class AppModule { // Feedback // + @Provides + @Singleton + fun providePreferences(context: Context): SharedPreferences + = context.getSharedPreferences("Application", Context.MODE_PRIVATE) + @Provides @Singleton fun provideFeedback(): Feedback = Feedback() @@ -35,8 +45,16 @@ class AppModule { @Singleton fun provideFeedbackCoordinator( feedback: Feedback, + preferences: SharedPreferences, defaultObservers: Set<@JvmSuppressWildcards FeedbackCoordinator.FeedbackObserver> - ): FeedbackCoordinator = FeedbackCoordinator(feedback, defaultObservers) + ): FeedbackCoordinator { + return preferences.getBoolean(FeedbackCoordinator.ENABLED, false).let { isEnabled -> + // observe nothing unless feedback is enabled + FirebaseCrashlytics.getInstance().setCrashlyticsCollectionEnabled(isEnabled) + Twig.plant(if (isEnabled) TroubleshootingTwig() else SilentTwig()) + FeedbackCoordinator(feedback, if (isEnabled) defaultObservers else setOf()) + } + } // diff --git a/app/src/main/java/cash/z/ecc/android/feedback/FeedbackCrashlytics.kt b/app/src/main/java/cash/z/ecc/android/feedback/FeedbackCrashlytics.kt index d3e5d30..ba146fb 100644 --- a/app/src/main/java/cash/z/ecc/android/feedback/FeedbackCrashlytics.kt +++ b/app/src/main/java/cash/z/ecc/android/feedback/FeedbackCrashlytics.kt @@ -1,6 +1,6 @@ package cash.z.ecc.android.feedback -import com.crashlytics.android.Crashlytics +import com.google.firebase.crashlytics.FirebaseCrashlytics class FeedbackCrashlytics : FeedbackCoordinator.FeedbackObserver { /** @@ -18,7 +18,7 @@ class FeedbackCrashlytics : FeedbackCoordinator.FeedbackObserver { ) else -> null } - exception?.let { Crashlytics.logException(it) } + exception?.let { FirebaseCrashlytics.getInstance().recordException(it) } } private class ReorgException(errorHeight: Int, rewindHeight: Int, reorgMesssage: String) : diff --git a/app/src/main/java/cash/z/ecc/android/feedback/Report.kt b/app/src/main/java/cash/z/ecc/android/feedback/Report.kt index b6899b3..f43f7a9 100644 --- a/app/src/main/java/cash/z/ecc/android/feedback/Report.kt +++ b/app/src/main/java/cash/z/ecc/android/feedback/Report.kt @@ -63,6 +63,7 @@ object Report { val errorHeight: Int by propertyMap val rewindHeight: Int by propertyMap } + class TxUpdateFailed(t: Throwable) : Feedback.AppError("txupdate", t, false) } } diff --git a/app/src/main/java/cash/z/ecc/android/ui/detail/TransactionViewHolder.kt b/app/src/main/java/cash/z/ecc/android/ui/detail/TransactionViewHolder.kt index 2389a0c..d0adffe 100644 --- a/app/src/main/java/cash/z/ecc/android/ui/detail/TransactionViewHolder.kt +++ b/app/src/main/java/cash/z/ecc/android/ui/detail/TransactionViewHolder.kt @@ -29,73 +29,75 @@ class TransactionViewHolder(itemView: View) : Recycler private val formatter = SimpleDateFormat("M/d h:mma", Locale.getDefault()) private val addressRegex = """zs\d\w{65,}""".toRegex() - fun bindTo(transaction: T?) = (itemView.context as MainActivity).lifecycleScope.launch { - // update view - var lineOne: String = "" - var lineTwo: String = "" - var amountZec: String = "" - var amountDisplay: String = "" - var amountColor: Int = R.color.text_light_dimmed - var lineOneColor: Int = R.color.text_light - var lineTwoColor: Int = R.color.text_light_dimmed - var indicatorBackground: Int = R.drawable.background_indicator_unknown + fun bindTo(transaction: T?) { + (itemView.context as MainActivity).lifecycleScope.launch { + // update view + var lineOne: String = "" + var lineTwo: String = "" + var amountZec: String = "" + var amountDisplay: String = "" + var amountColor: Int = R.color.text_light_dimmed + var lineOneColor: Int = R.color.text_light + var lineTwoColor: Int = R.color.text_light_dimmed + var indicatorBackground: Int = R.drawable.background_indicator_unknown - transaction?.apply { - itemView.setOnClickListener { - onTransactionClicked(this) - } - itemView.setOnLongClickListener { - onTransactionLongPressed(this) - true - } - amountZec = value.convertZatoshiToZecString() - // TODO: these might be good extension functions - val timestamp = formatter.format(blockTimeInSeconds * 1000L) - val isMined = blockTimeInSeconds != 0L - when { - !toAddress.isNullOrEmpty() -> { - lineOne = "You paid ${toAddress?.toAbbreviatedAddress()}" - lineTwo = if (isMined) "Sent $timestamp" else "Pending confirmation" - amountDisplay = "- $amountZec" - if (isMined) { - amountColor = R.color.zcashRed - indicatorBackground = R.drawable.background_indicator_outbound - } else { - lineOneColor = R.color.text_light_dimmed - lineTwoColor = R.color.text_light + transaction?.apply { + itemView.setOnClickListener { + onTransactionClicked(this) + } + itemView.setOnLongClickListener { + onTransactionLongPressed(this) + true + } + amountZec = value.convertZatoshiToZecString() + // TODO: these might be good extension functions + val timestamp = formatter.format(blockTimeInSeconds * 1000L) + val isMined = blockTimeInSeconds != 0L + when { + !toAddress.isNullOrEmpty() -> { + lineOne = "You paid ${toAddress?.toAbbreviatedAddress()}" + lineTwo = if (isMined) "Sent $timestamp" else "Pending confirmation" + amountDisplay = "- $amountZec" + if (isMined) { + amountColor = R.color.zcashRed + indicatorBackground = R.drawable.background_indicator_outbound + } else { + lineOneColor = R.color.text_light_dimmed + lineTwoColor = R.color.text_light + } + } + toAddress.isNullOrEmpty() && value > 0L && minedHeight > 0 -> { + lineOne = getSender(transaction) + lineTwo = "Received $timestamp" + amountDisplay = "+ $amountZec" + amountColor = R.color.zcashGreen + indicatorBackground = R.drawable.background_indicator_inbound + } + else -> { + lineOne = "Unknown" + lineTwo = "Unknown" + amountDisplay = "$amountZec" + amountColor = R.color.text_light } } - toAddress.isNullOrEmpty() && value > 0L && minedHeight > 0 -> { - lineOne = getSender(transaction) - lineTwo = "Received $timestamp" - amountDisplay = "+ $amountZec" - amountColor = R.color.zcashGreen - indicatorBackground = R.drawable.background_indicator_inbound - } - else -> { - lineOne = "Unknown" - lineTwo = "Unknown" - amountDisplay = "$amountZec" - amountColor = R.color.text_light + // sanitize amount + if (value < ZcashSdk.MINERS_FEE_ZATOSHI) amountDisplay = "< 0.001" + else if (amountZec.length > 10) { // 10 allows 3 digits to the left and 6 to the right of the decimal + amountDisplay = "tap to view" } } - // sanitize amount - if (value < ZcashSdk.MINERS_FEE_ZATOSHI) amountDisplay = "< 0.001" - else if (amountZec.length > 10) { // 10 allows 3 digits to the left and 6 to the right of the decimal - amountDisplay = "tap to view" - } + + + topText.text = lineOne + bottomText.text = lineTwo + amountText.text = amountDisplay + amountText.setTextColor(amountColor.toAppColor()) + topText.setTextColor(lineOneColor.toAppColor()) + bottomText.setTextColor(lineTwoColor.toAppColor()) + val context = itemView.context + indicator.background = context.resources.getDrawable(indicatorBackground) + shieldIcon.goneIf((transaction?.raw != null || transaction?.expiryHeight != null) && !transaction?.toAddress.isShielded()) } - - - topText.text = lineOne - bottomText.text = lineTwo - amountText.text = amountDisplay - amountText.setTextColor(amountColor.toAppColor()) - topText.setTextColor(lineOneColor.toAppColor()) - bottomText.setTextColor(lineTwoColor.toAppColor()) - val context = itemView.context - indicator.background = context.resources.getDrawable(indicatorBackground) - shieldIcon.goneIf((transaction?.raw != null || transaction?.expiryHeight != null) && !transaction?.toAddress.isShielded()) } private suspend fun getSender(transaction: ConfirmedTransaction): String { diff --git a/app/src/main/java/cash/z/ecc/android/ui/scan/ScanFragment.kt b/app/src/main/java/cash/z/ecc/android/ui/scan/ScanFragment.kt index 67d22c6..07f152f 100644 --- a/app/src/main/java/cash/z/ecc/android/ui/scan/ScanFragment.kt +++ b/app/src/main/java/cash/z/ecc/android/ui/scan/ScanFragment.kt @@ -21,8 +21,8 @@ import cash.z.ecc.android.feedback.Report.Tap.SCAN_RECEIVE import cash.z.ecc.android.sdk.ext.twig import cash.z.ecc.android.ui.base.BaseFragment import cash.z.ecc.android.ui.send.SendViewModel -import com.crashlytics.android.Crashlytics import com.google.common.util.concurrent.ListenableFuture +import com.google.firebase.crashlytics.FirebaseCrashlytics import kotlinx.coroutines.launch import java.util.concurrent.ExecutorService import java.util.concurrent.Executors @@ -102,7 +102,7 @@ class ScanFragment : BaseFragment() { preview.setSurfaceProvider(binding.preview.createSurfaceProvider()) } catch (t: Throwable) { // TODO: consider bubbling this up to the user - Crashlytics.logException(t) + mainActivity?.feedback?.report(t) twig("Error while opening the camera: $t") } diff --git a/app/src/main/java/cash/z/ecc/android/ui/send/SendFinalFragment.kt b/app/src/main/java/cash/z/ecc/android/ui/send/SendFinalFragment.kt index 814c6b1..c4c11ea 100644 --- a/app/src/main/java/cash/z/ecc/android/ui/send/SendFinalFragment.kt +++ b/app/src/main/java/cash/z/ecc/android/ui/send/SendFinalFragment.kt @@ -16,7 +16,6 @@ import cash.z.ecc.android.sdk.db.entity.* import cash.z.ecc.android.sdk.ext.convertZatoshiToZecString import cash.z.ecc.android.sdk.ext.toAbbreviatedAddress import cash.z.ecc.android.sdk.ext.twig -import com.crashlytics.android.Crashlytics import kotlinx.coroutines.delay import kotlinx.coroutines.flow.flow import kotlinx.coroutines.flow.launchIn @@ -112,8 +111,8 @@ class SendFinalFragment : BaseFragment() { } catch(t: Throwable) { val message = "ERROR: error while handling pending transaction update! $t" twig(message) - Crashlytics.log(message) - Crashlytics.logException(t) + mainActivity?.feedback?.report(Report.Error.NonFatal.TxUpdateFailed(t)) + mainActivity?.feedback?.report(t) } } diff --git a/app/src/main/java/cash/z/ecc/android/ui/send/SendViewModel.kt b/app/src/main/java/cash/z/ecc/android/ui/send/SendViewModel.kt index 9756f4b..5a01a9f 100644 --- a/app/src/main/java/cash/z/ecc/android/ui/send/SendViewModel.kt +++ b/app/src/main/java/cash/z/ecc/android/ui/send/SendViewModel.kt @@ -21,7 +21,6 @@ import cash.z.ecc.android.sdk.ext.ZcashSdk import cash.z.ecc.android.sdk.ext.convertZatoshiToZecString import cash.z.ecc.android.sdk.ext.twig import cash.z.ecc.android.sdk.validate.AddressType -import com.crashlytics.android.Crashlytics import kotlinx.coroutines.Dispatchers.IO import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flow @@ -144,7 +143,7 @@ class SendViewModel @Inject constructor() : ViewModel() { report(metricId) } } catch (t: Throwable) { - Crashlytics.logException(RuntimeException("Error while updating Metrics", t)) + feedback.report(t) } } diff --git a/app/src/main/java/cash/z/ecc/android/ui/setup/BackupFragment.kt b/app/src/main/java/cash/z/ecc/android/ui/setup/BackupFragment.kt index 9d5952b..7990411 100644 --- a/app/src/main/java/cash/z/ecc/android/ui/setup/BackupFragment.kt +++ b/app/src/main/java/cash/z/ecc/android/ui/setup/BackupFragment.kt @@ -10,11 +10,9 @@ import android.widget.TextView import android.widget.Toast import androidx.activity.addCallback import androidx.lifecycle.lifecycleScope -import cash.z.ecc.android.R import cash.z.ecc.android.ZcashWalletApp import cash.z.ecc.android.databinding.FragmentBackupBinding import cash.z.ecc.android.di.viewmodel.activityViewModel -import cash.z.ecc.android.di.viewmodel.viewModel import cash.z.ecc.android.feedback.Report import cash.z.ecc.android.feedback.Report.MetricType.SEED_PHRASE_LOADED import cash.z.ecc.android.feedback.Report.Tap.BACKUP_DONE diff --git a/build.gradle b/build.gradle index 7584f5d..a335c0f 100644 --- a/build.gradle +++ b/build.gradle @@ -13,7 +13,7 @@ buildscript { classpath 'com.google.gms:google-services:4.3.3' classpath "org.jetbrains.kotlin:kotlin-gradle-plugin:${Deps.kotlinVersion}" classpath 'io.fabric.tools:gradle:1.31.2' - classpath 'com.google.firebase:perf-plugin:1.3.1' + classpath 'com.google.firebase:firebase-crashlytics-gradle:2.1.1' } } diff --git a/buildSrc/src/main/java/cash/z/ecc/android/Dependencies.kt b/buildSrc/src/main/java/cash/z/ecc/android/Dependencies.kt index 0642076..d091f1b 100644 --- a/buildSrc/src/main/java/cash/z/ecc/android/Dependencies.kt +++ b/buildSrc/src/main/java/cash/z/ecc/android/Dependencies.kt @@ -61,10 +61,7 @@ object Deps { val STUB = "io.grpc:grpc-stub:$version" } object Analytics { // for dogfooding/crash-reporting/feedback only on internal team builds - val CRASHLYTICS = "com.crashlytics.sdk.android:crashlytics:2.10.1" - val CRASHLYTICS_NDK = "com.crashlytics.sdk.android:crashlytics-ndk:2.1.1" - val FIREBASE = "com.google.firebase:firebase-analytics:17.4.3" - val FIREBASE_PERF = "com.google.firebase:firebase-perf:19.0.7" + val CRASHLYTICS = "com.google.firebase:firebase-crashlytics:17.0.1" val MIXPANEL = "com.mixpanel.android:mixpanel-android:5.6.3" } object JavaX { diff --git a/feedback/src/main/java/cash/z/ecc/android/feedback/FeedbackCoordinator.kt b/feedback/src/main/java/cash/z/ecc/android/feedback/FeedbackCoordinator.kt index 133f1f1..36f1359 100644 --- a/feedback/src/main/java/cash/z/ecc/android/feedback/FeedbackCoordinator.kt +++ b/feedback/src/main/java/cash/z/ecc/android/feedback/FeedbackCoordinator.kt @@ -125,6 +125,7 @@ class FeedbackCoordinator(val feedback: Feedback, defaultObservers: Set