From 500c36c43a27e2f53b1c51083ef42ec3df2f9c29 Mon Sep 17 00:00:00 2001 From: Hee Tatt Ooi <hee.tatt.ooi@sap.com> Date: Fri, 5 Jun 2020 00:47:05 +0200 Subject: [PATCH] Exception handler (#176) * global exception handler that restarts app with previous crash message. for both main thread and coroutines * function comment * naming * added stackrace to intent * removed unnecessary line * extract constants * call reporter for dialog show * comments and refactoring * enable show dialog on app crash * cleanup and refactor * comment * cleanup --- .../coronawarnapp/CoronaWarnApplication.kt | 17 ++++- .../exception/ErrorReportReceiver.kt | 40 ++++++----- .../exception/ExceptionReporter.kt | 2 +- .../handler/GlobalExceptionHandler.kt | 68 +++++++++++++++++++ .../GlobalExceptionHandlerConstants.kt | 10 +++ .../ui/ActivityErrorReporting.kt | 22 ++++++ .../rki/coronawarnapp/ui/LauncherActivity.kt | 27 +++++++- .../rki/coronawarnapp/ui/main/MainActivity.kt | 10 +-- .../ui/onboarding/OnboardingActivity.kt | 6 ++ 9 files changed, 169 insertions(+), 33 deletions(-) create mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/handler/GlobalExceptionHandler.kt create mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/handler/GlobalExceptionHandlerConstants.kt create mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/ActivityErrorReporting.kt diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/CoronaWarnApplication.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/CoronaWarnApplication.kt index c41efeec1..bf6dd5608 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/CoronaWarnApplication.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/CoronaWarnApplication.kt @@ -4,6 +4,7 @@ import android.annotation.SuppressLint import android.app.Activity import android.app.Application import android.content.Context +import android.content.IntentFilter import android.content.pm.ActivityInfo import android.os.Bundle import android.util.Log @@ -12,6 +13,10 @@ import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleObserver import androidx.lifecycle.OnLifecycleEvent import androidx.lifecycle.ProcessLifecycleOwner +import androidx.localbroadcastmanager.content.LocalBroadcastManager +import de.rki.coronawarnapp.exception.ErrorReportReceiver +import de.rki.coronawarnapp.exception.ReportingConstants.ERROR_REPORT_LOCAL_BROADCAST_CHANNEL +import de.rki.coronawarnapp.exception.handler.GlobalExceptionHandler import de.rki.coronawarnapp.notification.NotificationHelper import org.conscrypt.Conscrypt import java.security.Security @@ -34,12 +39,15 @@ class CoronaWarnApplication : Application(), LifecycleObserver, instance.applicationContext } + private lateinit var errorReceiver: ErrorReportReceiver + override fun onCreate() { + super.onCreate() + GlobalExceptionHandler(this) instance = this NotificationHelper.createNotificationChannel() // Enable Conscrypt for TLS1.3 Support below API Level 29 Security.insertProviderAt(Conscrypt.newProvider(), 1) - super.onCreate() ProcessLifecycleOwner.get().lifecycle.addObserver(this) registerActivityLifecycleCallbacks(this) } @@ -63,7 +71,8 @@ class CoronaWarnApplication : Application(), LifecycleObserver, } override fun onActivityPaused(activity: Activity) { - // does not override function. Empty on intention + // unregisters error receiver + LocalBroadcastManager.getInstance(this).unregisterReceiver(errorReceiver) } override fun onActivityStarted(activity: Activity) { @@ -94,6 +103,8 @@ class CoronaWarnApplication : Application(), LifecycleObserver, } override fun onActivityResumed(activity: Activity) { - // does not override function. Empty on intention + errorReceiver = ErrorReportReceiver(activity) + LocalBroadcastManager.getInstance(this) + .registerReceiver(errorReceiver, IntentFilter(ERROR_REPORT_LOCAL_BROADCAST_CHANNEL)) } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/ErrorReportReceiver.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/ErrorReportReceiver.kt index f9d52f4f9..958e8a1dd 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/ErrorReportReceiver.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/ErrorReportReceiver.kt @@ -13,6 +13,7 @@ class ErrorReportReceiver(private val activity: Activity) : BroadcastReceiver() companion object { private val TAG: String = ErrorReportReceiver::class.java.simpleName } + override fun onReceive(context: Context, intent: Intent) { val category = ExceptionCategory .valueOf(intent.getStringExtra(ReportingConstants.ERROR_REPORT_CATEGORY_EXTRA) ?: "") @@ -25,25 +26,28 @@ class ErrorReportReceiver(private val activity: Activity) : BroadcastReceiver() val confirm = context.resources.getString(R.string.errors_generic_button_positive) val details = context.resources.getString(R.string.errors_generic_button_negative) val detailsTitle = context.resources.getString(R.string.errors_generic_details_headline) + if (CoronaWarnApplication.isAppInForeground) { - DialogHelper.showDialog(DialogHelper.DialogInstance( - activity, - title, - message, - confirm, - details, - null, - {}, - { - DialogHelper.showDialog( - DialogHelper.DialogInstance( - activity, - title, - "$detailsTitle:\n$stack", - confirm - )).run {} - } - )) + DialogHelper.showDialog( + DialogHelper.DialogInstance( + activity, + title, + message, + confirm, + details, + null, + {}, + { + DialogHelper.showDialog( + DialogHelper.DialogInstance( + activity, + title, + "$detailsTitle:\n$stack", + confirm + ) + ).run {} + } + )) } Log.e( TAG, diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/ExceptionReporter.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/ExceptionReporter.kt index 65b1713f1..3c58048be 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/ExceptionReporter.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/ExceptionReporter.kt @@ -26,7 +26,7 @@ fun Throwable.report( LocalBroadcastManager.getInstance(CoronaWarnApplication.getAppContext()).sendBroadcast(intent) } -fun Throwable.reportGeneric( +fun reportGeneric( stackString: String ) { val intent = Intent(ReportingConstants.ERROR_REPORT_LOCAL_BROADCAST_CHANNEL) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/handler/GlobalExceptionHandler.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/handler/GlobalExceptionHandler.kt new file mode 100644 index 000000000..2fe17a4b6 --- /dev/null +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/handler/GlobalExceptionHandler.kt @@ -0,0 +1,68 @@ +package de.rki.coronawarnapp.exception.handler + +import android.content.Context +import android.content.Intent +import android.util.Log +import de.rki.coronawarnapp.CoronaWarnApplication +import de.rki.coronawarnapp.ui.LauncherActivity +import java.io.PrintWriter +import java.io.StringWriter +import java.lang.reflect.InvocationTargetException + +class GlobalExceptionHandler(private val application: CoronaWarnApplication) : + Thread.UncaughtExceptionHandler { + + companion object { + val TAG: String? = GlobalExceptionHandler::class.simpleName + } + + init { + Thread.setDefaultUncaughtExceptionHandler(this) + } + + override fun uncaughtException(thread: Thread, throwable: Throwable) { + try { + Log.i(TAG, "cause caught: " + throwable) + val cause = throwable.cause + + val stringWriter = StringWriter() + // Throwables from main thread are wrapped in an InvocationTargetException, + // unwrap the InvocationTargetException to get the original cause + if (cause is InvocationTargetException) { + cause.targetException.printStackTrace(PrintWriter(stringWriter)) + Log.i(TAG, "InvocationTargetException caught: " + cause.targetException) + } + // for errors thrown by coroutines, these are not wrapped in InvocationTargetException + else { + Log.i(TAG, "InvocationTargetException caught: " + throwable) + throwable.printStackTrace(PrintWriter(stringWriter)) + } + val stackTrace = stringWriter.toString() + triggerRestart(CoronaWarnApplication.getAppContext(), stackTrace) + } catch (e: Exception) { + Log.e(TAG, "GlobalExceptionHandler failing" + e) + } + } + + /** + * Restarts the app by sending an Intent to start LauncherActivitiy and + * terminating the JVM + * + * @see de.rki.coronawarnapp.ui.LauncherActivity + * + * @param context application context + * @param stackTrace exception that caused the crash + */ + private fun triggerRestart(context: Context, stackTrace: String) { + val intent = Intent(context, LauncherActivity::class.java) + intent.addFlags( + Intent.FLAG_ACTIVITY_CLEAR_TOP + or Intent.FLAG_ACTIVITY_CLEAR_TASK + or Intent.FLAG_ACTIVITY_NEW_TASK + ) + intent.putExtra(GlobalExceptionHandlerConstants.APP_CRASHED, true) + intent.putExtra(GlobalExceptionHandlerConstants.STACK_TRACE, stackTrace) + context.startActivity(intent) + Runtime.getRuntime().exit(0) + } +} diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/handler/GlobalExceptionHandlerConstants.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/handler/GlobalExceptionHandlerConstants.kt new file mode 100644 index 000000000..3d0509123 --- /dev/null +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/exception/handler/GlobalExceptionHandlerConstants.kt @@ -0,0 +1,10 @@ +package de.rki.coronawarnapp.exception.handler + +object GlobalExceptionHandlerConstants { + + // name of intent extra to described that an app has crashed. Intent extra is of type boolean + const val APP_CRASHED = "appCrashed" + + // name of intent extra that contains the stacktrace. Intent extra is of type boolean + const val STACK_TRACE = "stackTrace" +} diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/ActivityErrorReporting.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/ActivityErrorReporting.kt new file mode 100644 index 000000000..7a3db2f49 --- /dev/null +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/ActivityErrorReporting.kt @@ -0,0 +1,22 @@ +package de.rki.coronawarnapp.ui + +import androidx.appcompat.app.AppCompatActivity +import de.rki.coronawarnapp.exception.handler.GlobalExceptionHandlerConstants +import de.rki.coronawarnapp.exception.reportGeneric + +/** + * If the app crashed in the last instance and was restarted, the stacktrace is retrieved + * from the intent and displayed in a dialog report + * + * @see de.rki.coronawarnapp.exception.handler.GlobalExceptionHandler + */ +fun AppCompatActivity.showDialogWithStacktraceIfPreviouslyCrashed() { + val appCrashedAndWasRestarted = + intent.getBooleanExtra(GlobalExceptionHandlerConstants.APP_CRASHED, false) + if (appCrashedAndWasRestarted) { + val stackTrade = intent.getStringExtra(GlobalExceptionHandlerConstants.STACK_TRACE) + if (!stackTrade.isNullOrEmpty()) { + reportGeneric(stackTrade) + } + } +} diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/LauncherActivity.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/LauncherActivity.kt index 2f6612a78..ff3060508 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/LauncherActivity.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/LauncherActivity.kt @@ -5,6 +5,7 @@ import android.net.Uri import android.os.Bundle import android.widget.Toast import androidx.appcompat.app.AppCompatActivity +import de.rki.coronawarnapp.exception.handler.GlobalExceptionHandlerConstants import de.rki.coronawarnapp.http.DynamicURLs import de.rki.coronawarnapp.storage.LocalData import de.rki.coronawarnapp.ui.main.MainActivity @@ -17,7 +18,6 @@ class LauncherActivity : AppCompatActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) - retrieveCustomURLsFromSchema(intent.data) if (LocalData.isOnboarded()) { @@ -56,12 +56,33 @@ class LauncherActivity : AppCompatActivity() { } private fun startOnboardingActivity() { - startActivity(Intent(this, OnboardingActivity::class.java)) + val onboardingActivity = Intent(this, OnboardingActivity::class.java) + mapIntentExtras(onboardingActivity) + startActivity(onboardingActivity) finish() } private fun startMainActivity() { - startActivity(Intent(this, MainActivity::class.java)) + val mainActivityIntent = Intent(this, MainActivity::class.java) + mapIntentExtras(mainActivityIntent) + startActivity(mainActivityIntent) finish() } + + /** + * Maps the intentExtras for global exception handling to the next activity that is + * started + * + * @param intentForNextActivity + */ + private fun mapIntentExtras(intentForNextActivity: Intent) { + intentForNextActivity.putExtra( + GlobalExceptionHandlerConstants.APP_CRASHED, + intent.getBooleanExtra(GlobalExceptionHandlerConstants.APP_CRASHED, false) + ) + intentForNextActivity.putExtra( + GlobalExceptionHandlerConstants.STACK_TRACE, + intent.getStringExtra(GlobalExceptionHandlerConstants.STACK_TRACE) + ) + } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/MainActivity.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/MainActivity.kt index f6643d113..091353254 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/MainActivity.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/MainActivity.kt @@ -1,15 +1,13 @@ package de.rki.coronawarnapp.ui.main import android.content.Intent -import android.content.IntentFilter import android.os.Bundle import androidx.appcompat.app.AppCompatActivity import androidx.fragment.app.Fragment import androidx.fragment.app.FragmentManager import androidx.lifecycle.ViewModelProviders -import androidx.localbroadcastmanager.content.LocalBroadcastManager import de.rki.coronawarnapp.R -import de.rki.coronawarnapp.exception.ErrorReportReceiver +import de.rki.coronawarnapp.ui.showDialogWithStacktraceIfPreviouslyCrashed import de.rki.coronawarnapp.ui.viewmodel.SettingsViewModel import de.rki.coronawarnapp.util.ConnectivityHelper import de.rki.coronawarnapp.worker.BackgroundWorkScheduler @@ -32,8 +30,6 @@ class MainActivity : AppCompatActivity() { private lateinit var settingsViewModel: SettingsViewModel - private val errorReceiver = ErrorReportReceiver(this) - /** * Register connection callback. */ @@ -74,9 +70,9 @@ class MainActivity : AppCompatActivity() { */ override fun onResume() { super.onResume() - LocalBroadcastManager.getInstance(this).registerReceiver(errorReceiver, IntentFilter("error-report")) ConnectivityHelper.registerNetworkStatusCallback(this, callbackNetwork) ConnectivityHelper.registerBluetoothStatusCallback(this, callbackBluetooth) + showDialogWithStacktraceIfPreviouslyCrashed() } /** @@ -86,8 +82,6 @@ class MainActivity : AppCompatActivity() { super.onPause() ConnectivityHelper.unregisterNetworkStatusCallback(this, callbackNetwork) ConnectivityHelper.unregisterBluetoothStatusCallback(this, callbackBluetooth) - // Unregister since the activity is about to be closed. - LocalBroadcastManager.getInstance(this).unregisterReceiver(errorReceiver) } override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) { diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/onboarding/OnboardingActivity.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/onboarding/OnboardingActivity.kt index 203301626..871029e48 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/onboarding/OnboardingActivity.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/onboarding/OnboardingActivity.kt @@ -9,6 +9,7 @@ import androidx.lifecycle.LifecycleObserver import de.rki.coronawarnapp.R import de.rki.coronawarnapp.storage.LocalData import de.rki.coronawarnapp.ui.main.MainActivity +import de.rki.coronawarnapp.ui.showDialogWithStacktraceIfPreviouslyCrashed /** * This activity holds all the onboarding fragments and isn't used after a successful onboarding flow. @@ -40,6 +41,11 @@ class OnboardingActivity : AppCompatActivity(), LifecycleObserver { ) } + override fun onResume() { + super.onResume() + showDialogWithStacktraceIfPreviouslyCrashed() + } + fun completeOnboarding() { LocalData.isOnboarded(true) startActivity(Intent(this, MainActivity::class.java)) -- GitLab