-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
* Block certain reports sent to ACRA * Update AnkiDroid/src/main/java/com/ichi2/anki/CrashReportService.kt Co-authored-by: David Allison <[email protected]> * feat: filter uncaught exceptions We want to filter all exception - that is ones that are uncaught as well as ones that are imperatively sent through CrashReportService That requires attaching filter to out Thread.defaultExceptionHandler chain - extract filter into service, use it from crash report service - implement default exception handler handling, init/reinit as needed - concrete types for the handlers and public uninstall/install for testability * feat: add exception type filter during exception handling * test: more heap for gradle test executors --------- Co-authored-by: David Allison <[email protected]> Co-authored-by: Mike Hardy <[email protected]>
- Loading branch information
3 people
authored
Dec 2, 2024
1 parent
0c5ea9d
commit 333ef64
Showing
8 changed files
with
247 additions
and
10 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -83,6 +83,7 @@ | |
| <w>quiro91</w> | ||
| <w>tarekkma</w> | ||
| <w>vianey</w> | ||
| <w>voczi</w> | ||
| <w>zanki</w> | ||
| </words> | ||
| </dictionary> | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
118 changes: 118 additions & 0 deletions
118
AnkiDroid/src/main/java/com/ichi2/anki/servicelayer/ThrowableFilterService.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,118 @@ | ||
| /**************************************************************************************** | ||
| * Copyright (c) 2024 voczi <[email protected]> | ||
| * Copyright (c) 2024 Mike Hardy <[email protected]> * | ||
| * * | ||
| * This program is free software; you can redistribute it and/or modify it under * | ||
| * the terms of the GNU General Public License as published by the Free Software * | ||
| * Foundation; either version 3 of the License, or (at your option) any later * | ||
| * version. * | ||
| * * | ||
| * This program is distributed in the hope that it will be useful, but WITHOUT ANY * | ||
| * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A * | ||
| * PARTICULAR PURPOSE. See the GNU General Public License for more details. * | ||
| * * | ||
| * You should have received a copy of the GNU General Public License along with * | ||
| * this program. If not, see <http://www.gnu.org/licenses/>. * | ||
| ****************************************************************************************/ | ||
| package com.ichi2.anki.servicelayer | ||
|
|
||
| import androidx.annotation.VisibleForTesting | ||
| import com.ichi2.anki.exception.StorageAccessException | ||
| import net.ankiweb.rsdroid.exceptions.BackendNetworkException | ||
| import net.ankiweb.rsdroid.exceptions.BackendSyncException | ||
| import net.ankiweb.rsdroid.exceptions.BackendSyncException.BackendSyncServerMessageException | ||
| import timber.log.Timber | ||
|
|
||
| object ThrowableFilterService { | ||
|
|
||
| @FunctionalInterface | ||
| fun interface FilteringExceptionHandler : Thread.UncaughtExceptionHandler | ||
|
|
||
| @VisibleForTesting | ||
| var originalUncaughtExceptionHandler: Thread.UncaughtExceptionHandler? = null | ||
|
|
||
| var uncaughtExceptionHandler = FilteringExceptionHandler { | ||
| thread: Thread?, throwable: Throwable -> | ||
| if (thread == null) { | ||
| Timber.w("unexpected: thread was null") | ||
| return@FilteringExceptionHandler | ||
| } | ||
| if (shouldDiscardThrowable(throwable)) { | ||
| Timber.i("discarding throwable") | ||
| return@FilteringExceptionHandler | ||
| } | ||
| originalUncaughtExceptionHandler?.uncaughtException(thread, throwable) | ||
| } | ||
|
|
||
| fun initialize() { | ||
| Timber.i("initialize()") | ||
| installDefaultExceptionHandler() | ||
| } | ||
|
|
||
| /** | ||
| * We want to filter any exceptions with PII, then chain to other handlers | ||
| */ | ||
| @Synchronized | ||
| @VisibleForTesting | ||
| fun installDefaultExceptionHandler() { | ||
| originalUncaughtExceptionHandler = Thread.getDefaultUncaughtExceptionHandler() | ||
| Timber.d("Chaining to uncaughtExceptionHandler (%s)", originalUncaughtExceptionHandler) | ||
| Thread.setDefaultUncaughtExceptionHandler(uncaughtExceptionHandler) | ||
| } | ||
|
|
||
| /** | ||
| * Reset the default exception handler | ||
| */ | ||
| @Synchronized | ||
| @VisibleForTesting | ||
| fun unInstallDefaultExceptionHandler() { | ||
| Thread.setDefaultUncaughtExceptionHandler(originalUncaughtExceptionHandler) | ||
| originalUncaughtExceptionHandler = null | ||
| } | ||
|
|
||
| /** | ||
| * Re-Initialize the throwable filter | ||
| */ | ||
| @Synchronized | ||
| fun reInitialize() { | ||
| // send any pending async hits, re-chain default exception handlers and re-init | ||
| Timber.i("reInitialize()") | ||
| unInstallDefaultExceptionHandler() | ||
| initialize() | ||
| } | ||
|
|
||
| fun shouldDiscardThrowable(t: Throwable): Boolean { | ||
| // Note that an exception may have a nested BackendSyncException, | ||
| // so we check if it is safe from PII despite also filtering by type | ||
| return exceptionIsUnwanted(t) || !t.safeFromPII() | ||
| } | ||
|
|
||
| // There are few exception types that are common, but are unwanted in | ||
| // our analytics or crash report service because they are not actionable | ||
| fun exceptionIsUnwanted(t: Throwable): Boolean { | ||
| Timber.v("exceptionIsUnwanted - examining %s", t.javaClass.simpleName) | ||
| when (t) { | ||
| is BackendNetworkException -> return true | ||
| is BackendSyncException -> return true | ||
| is StorageAccessException -> return true | ||
| } | ||
| Timber.v("exceptionIsUnwanted - exception was wanted") | ||
| return false | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the [Throwable] is safe from Personally Identifiable Information (PII) | ||
| * @return `false` if the [Throwable] contains PII, otherwise `true` | ||
| */ | ||
| fun Throwable.safeFromPII(): Boolean { | ||
| if (this.containsPIINonRecursive()) return false | ||
| return this.cause?.safeFromPII() != false | ||
| } | ||
|
|
||
| private fun Throwable.containsPIINonRecursive(): Boolean { | ||
| // BackendSyncServerMessage may contain PII and we do not want this leaked to ACRA. | ||
| // Related: https://github.com/ankidroid/Anki-Android/issues/17392 | ||
| // and also https://github.com/ankitects/anki/commit/ba1f5f4 | ||
| return this is BackendSyncServerMessageException | ||
| } | ||
| } |
65 changes: 65 additions & 0 deletions
65
AnkiDroid/src/test/java/com/ichi2/anki/ThrowableFilterServiceTest.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| /* | ||
| * Copyright (c) 2024 voczi <[email protected]> | ||
| * | ||
| * This program is free software; you can redistribute it and/or modify it under | ||
| * the terms of the GNU General Public License as published by the Free Software | ||
| * Foundation; either version 3 of the License, or (at your option) any later | ||
| * version. | ||
| * | ||
| * This program is distributed in the hope that it will be useful, but WITHOUT ANY | ||
| * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A | ||
| * PARTICULAR PURPOSE. See the GNU General Public License for more details. | ||
| * | ||
| * You should have received a copy of the GNU General Public License along with | ||
| * this program. If not, see <http://www.gnu.org/licenses/>. | ||
| */ | ||
|
|
||
| package com.ichi2.anki | ||
|
|
||
| import androidx.test.ext.junit.runners.AndroidJUnit4 | ||
| import anki.backend.BackendError | ||
| import com.ichi2.anki.exception.StorageAccessException | ||
| import com.ichi2.anki.servicelayer.ThrowableFilterService | ||
| import com.ichi2.anki.servicelayer.ThrowableFilterService.safeFromPII | ||
| import com.ichi2.testutils.JvmTest | ||
| import net.ankiweb.rsdroid.exceptions.BackendDeckIsFilteredException | ||
| import net.ankiweb.rsdroid.exceptions.BackendNetworkException | ||
| import net.ankiweb.rsdroid.exceptions.BackendSyncException | ||
| import net.ankiweb.rsdroid.exceptions.BackendSyncException.BackendSyncServerMessageException | ||
| import org.junit.Test | ||
| import org.junit.runner.RunWith | ||
| import kotlin.test.assertFalse | ||
| import kotlin.test.assertTrue | ||
|
|
||
| @RunWith(AndroidJUnit4::class) | ||
| class ThrowableFilterServiceTest : JvmTest() { | ||
|
|
||
| @Test | ||
| fun `Normal exceptions are flagged as PII-safe`() { | ||
| val exception = BackendDeckIsFilteredException(BackendError.newBuilder().build()) | ||
| assertTrue(exception.safeFromPII(), "Exception reported as safe from PII") | ||
| } | ||
|
|
||
| @Test | ||
| fun `BackendSyncServerMessage exceptions are flagged as PII-unsafe`() { | ||
| val exception1 = BackendSyncServerMessageException(BackendError.newBuilder().build()) | ||
| assertFalse(exception1.safeFromPII(), "Exception reported as not safe from PII") | ||
|
|
||
| val exception2 = Exception("", Exception("", exception1)) | ||
| assertFalse(exception2.safeFromPII(), "Nested exception reported as not safe from PII") | ||
| } | ||
|
|
||
| @Test | ||
| fun `exceptions are discarded correctly by type`() { | ||
| // regular exceptions should go through | ||
| assertFalse(ThrowableFilterService.shouldDiscardThrowable(Exception("wanted"))) | ||
|
|
||
| // exceptions of known unwanted types should not go through | ||
| val exception1 = BackendNetworkException(BackendError.newBuilder().build()) | ||
| assertTrue(ThrowableFilterService.shouldDiscardThrowable(exception1)) | ||
| val exception2 = BackendSyncException(BackendError.newBuilder().build()) | ||
| assertTrue(ThrowableFilterService.shouldDiscardThrowable(exception2)) | ||
| val exception3 = StorageAccessException("test exception") | ||
| assertTrue(ThrowableFilterService.shouldDiscardThrowable(exception3)) | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters