From 4f69f520332f83039d38feb90c008f8ab152bf4d Mon Sep 17 00:00:00 2001 From: Matthias Urhahn <matthias.urhahn@sap.com> Date: Fri, 8 Jan 2021 16:57:52 +0100 Subject: [PATCH] Catch uncaught exceptions and log them before forwarding and crashing. (#2054) This allows our internal logging to capture those errors too. --- .../de/rki/coronawarnapp/util/CWADebug.kt | 12 ++++ .../util/debug/UncaughtExceptionLogger.kt | 23 +++++++ .../util/debug/UncaughtExceptionLoggerTest.kt | 63 +++++++++++++++++++ 3 files changed, 98 insertions(+) create mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/debug/UncaughtExceptionLogger.kt create mode 100644 Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/debug/UncaughtExceptionLoggerTest.kt diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/CWADebug.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/CWADebug.kt index 4b959e3f8..15f4ffa54 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/CWADebug.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/CWADebug.kt @@ -2,9 +2,11 @@ package de.rki.coronawarnapp.util import android.app.Application import android.os.Build +import androidx.annotation.VisibleForTesting import de.rki.coronawarnapp.BuildConfig import de.rki.coronawarnapp.bugreporting.debuglog.DebugLogger import de.rki.coronawarnapp.util.debug.FileLogger +import de.rki.coronawarnapp.util.debug.UncaughtExceptionLogger import de.rki.coronawarnapp.util.di.ApplicationComponent import timber.log.Timber @@ -21,6 +23,8 @@ object CWADebug { fileLogger = FileLogger(application) } + setupExceptionHandler() + DebugLogger.init(application) logDeviceInfos() @@ -57,4 +61,12 @@ object CWADebug { Timber.i("CWA flavor: %s (%s)", BuildConfig.FLAVOR, BuildConfig.BUILD_TYPE) Timber.i("Build.FINGERPRINT: %s", Build.FINGERPRINT) } + + /** + * Allow internal logging via `DebugLogger` to log stacktraces for uncaught exceptions. + */ + @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) + internal fun setupExceptionHandler() { + UncaughtExceptionLogger.wrapCurrentHandler() + } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/debug/UncaughtExceptionLogger.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/debug/UncaughtExceptionLogger.kt new file mode 100644 index 000000000..a9887e63f --- /dev/null +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/debug/UncaughtExceptionLogger.kt @@ -0,0 +1,23 @@ +package de.rki.coronawarnapp.util.debug + +import timber.log.Timber + +class UncaughtExceptionLogger( + private val wrappedHandler: Thread.UncaughtExceptionHandler? +) : Thread.UncaughtExceptionHandler { + + init { + Timber.v("Wrapping exception handler: %s", wrappedHandler) + } + + override fun uncaughtException(thread: Thread, error: Throwable) { + Timber.tag(thread.name).e(error, "Uncaught exception!") + wrappedHandler?.uncaughtException(thread, error) + } + + companion object { + fun wrapCurrentHandler() = UncaughtExceptionLogger(Thread.getDefaultUncaughtExceptionHandler()).also { + Thread.setDefaultUncaughtExceptionHandler(it) + } + } +} diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/debug/UncaughtExceptionLoggerTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/debug/UncaughtExceptionLoggerTest.kt new file mode 100644 index 000000000..bc8edc659 --- /dev/null +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/util/debug/UncaughtExceptionLoggerTest.kt @@ -0,0 +1,63 @@ +package de.rki.coronawarnapp.util.debug + +import io.kotest.matchers.shouldBe +import io.mockk.Runs +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.verify +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import testhelpers.BaseTest + +class UncaughtExceptionLoggerTest : BaseTest() { + + var originalHandler: Thread.UncaughtExceptionHandler? = null + + @BeforeEach + fun setup() { + originalHandler = Thread.getDefaultUncaughtExceptionHandler() + } + + @AfterEach + fun teardown() { + Thread.setDefaultUncaughtExceptionHandler(originalHandler) + } + + @Test + fun `we wrap and call through to the original handler`() { + val wrappedHandler = mockk<Thread.UncaughtExceptionHandler>() + every { wrappedHandler.uncaughtException(any(), any()) } just Runs + + val instance = UncaughtExceptionLogger(wrappedHandler) + val testException = NotImplementedError() + instance.uncaughtException(Thread.currentThread(), testException) + + verify { wrappedHandler.uncaughtException(Thread.currentThread(), testException) } + } + + @Test + fun `auto setup replaces the current handler`() { + val wrappedHandler = mockk<Thread.UncaughtExceptionHandler>() + every { wrappedHandler.uncaughtException(any(), any()) } just Runs + + Thread.setDefaultUncaughtExceptionHandler(wrappedHandler) + Thread.getDefaultUncaughtExceptionHandler() shouldBe wrappedHandler + + val ourHandler = UncaughtExceptionLogger.wrapCurrentHandler() + Thread.getDefaultUncaughtExceptionHandler() shouldBe ourHandler + } + + @Test + fun `null handlers would be okay`() { + Thread.setDefaultUncaughtExceptionHandler(null) + Thread.getDefaultUncaughtExceptionHandler() shouldBe null + + val ourHandler = UncaughtExceptionLogger.wrapCurrentHandler() + Thread.getDefaultUncaughtExceptionHandler() shouldBe ourHandler + + val instance = UncaughtExceptionLogger(null) + instance.uncaughtException(Thread.currentThread(), NotImplementedError()) + } +} -- GitLab