Skip to content

Commit d983c00

Browse files
committed
refactor(common): extract CrashReportService interface to :common
Create CrashReporter interface and CrashReportService delegate object in :common (package com.ichi2.anki) so all modules can report crashes without depending on the app module. Move runCatchingWithReport to :common. Rename the ACRA-backed implementation to AcraCrashReporter (internal) in :AnkiDroid. Wire up via CrashReportService.setReporter() in AnkiDroidApp.onCreate(). Existing call sites keep using CrashReportService.sendExceptionReport with the same package — no import changes needed for ~40 files. Only 7 files using ACRA-specific methods updated to AcraCrashReporter.
1 parent 0b9f5ba commit d983c00

10 files changed

Lines changed: 163 additions & 82 deletions

File tree

AnkiDroid/src/androidTest/java/com/ichi2/anki/tests/ACRATest.kt

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ import androidx.annotation.StringRes
2121
import androidx.core.content.edit
2222
import androidx.test.annotation.UiThreadTest
2323
import androidx.test.ext.junit.runners.AndroidJUnit4
24+
import com.ichi2.anki.AcraCrashReporter
25+
import com.ichi2.anki.AcraCrashReporter.FEEDBACK_REPORT_ALWAYS
26+
import com.ichi2.anki.AcraCrashReporter.FEEDBACK_REPORT_ASK
2427
import com.ichi2.anki.AnkiDroidApp
25-
import com.ichi2.anki.CrashReportService
26-
import com.ichi2.anki.CrashReportService.FEEDBACK_REPORT_ALWAYS
27-
import com.ichi2.anki.CrashReportService.FEEDBACK_REPORT_ASK
2828
import com.ichi2.anki.R
2929
import com.ichi2.anki.analytics.UsageAnalytics
3030
import com.ichi2.anki.logging.ProductionCrashReportingTree
@@ -70,10 +70,10 @@ class ACRATest : InstrumentedTest() {
7070
@Throws(Exception::class)
7171
fun testDebugConfiguration() {
7272
// Debug mode overrides all saved state so no setup needed
73-
CrashReportService.setDebugACRAConfig(sharedPrefs)
73+
AcraCrashReporter.setDebugACRAConfig(sharedPrefs)
7474
assertArrayEquals(
7575
"Debug logcat arguments not set correctly",
76-
CrashReportService.acraCoreConfigBuilder
76+
AcraCrashReporter.acraCoreConfigBuilder
7777
.build()
7878
.logcatArguments
7979
.toTypedArray(),
@@ -90,21 +90,21 @@ class ACRATest : InstrumentedTest() {
9090
)
9191
assertEquals(
9292
"ACRA feedback was not turned off correctly",
93-
CrashReportService.FEEDBACK_REPORT_NEVER,
93+
AcraCrashReporter.FEEDBACK_REPORT_NEVER,
9494
sharedPrefs
95-
.getString(CrashReportService.FEEDBACK_REPORT_KEY, "undefined"),
95+
.getString(AcraCrashReporter.FEEDBACK_REPORT_KEY, "undefined"),
9696
)
9797
}
9898

9999
@Test
100100
@Throws(Exception::class)
101101
fun testProductionConfigurationUserDisabled() {
102102
// set up as if the user had prefs saved to disable completely
103-
setReportConfig(CrashReportService.FEEDBACK_REPORT_NEVER)
103+
setReportConfig(AcraCrashReporter.FEEDBACK_REPORT_NEVER)
104104

105105
// ACRA initializes production logcat via annotation and we can't mock Build.DEBUG
106106
// That means we are restricted from verifying production logcat args and this is the debug case again
107-
CrashReportService.setProductionACRAConfig(sharedPrefs)
107+
AcraCrashReporter.setProductionACRAConfig(sharedPrefs)
108108
verifyDebugACRAPreferences()
109109
}
110110

@@ -115,7 +115,7 @@ class ACRATest : InstrumentedTest() {
115115
setReportConfig(FEEDBACK_REPORT_ASK)
116116

117117
// If the user is set to ask, then it's production, with interaction mode dialog
118-
CrashReportService.setProductionACRAConfig(sharedPrefs)
118+
AcraCrashReporter.setProductionACRAConfig(sharedPrefs)
119119
verifyACRANotDisabled()
120120

121121
assertToastMessage(R.string.feedback_for_manual_toast_text)
@@ -134,7 +134,7 @@ class ACRATest : InstrumentedTest() {
134134

135135
// If the user is set to always, then it's production, with interaction mode toast
136136
// will be useful with ACRA 5.2.0
137-
CrashReportService.setProductionACRAConfig(sharedPrefs)
137+
AcraCrashReporter.setProductionACRAConfig(sharedPrefs)
138138

139139
// The same class/method combo is only sent once, so we face a new method each time (should test that system later)
140140
val crash = Exception("testCrashReportSend at " + System.currentTimeMillis())
@@ -153,12 +153,12 @@ class ACRATest : InstrumentedTest() {
153153
val crashData =
154154
CrashReportDataFactory(
155155
testContext,
156-
CrashReportService.acraCoreConfigBuilder.build(),
156+
AcraCrashReporter.acraCoreConfigBuilder.build(),
157157
).createCrashData(ReportBuilder().exception(crash))
158158
assertTrue(
159159
LimitingReportAdministrator().shouldSendReport(
160160
testContext,
161-
CrashReportService.acraCoreConfigBuilder.build(),
161+
AcraCrashReporter.acraCoreConfigBuilder.build(),
162162
crashData,
163163
),
164164
)
@@ -167,19 +167,19 @@ class ACRATest : InstrumentedTest() {
167167
assertFalse(
168168
LimitingReportAdministrator().shouldSendReport(
169169
testContext,
170-
CrashReportService.acraCoreConfigBuilder.build(),
170+
AcraCrashReporter.acraCoreConfigBuilder.build(),
171171
crashData,
172172
),
173173
)
174174

175175
// Now let's clear data
176-
CrashReportService.deleteACRALimiterData(testContext)
176+
AcraCrashReporter.deleteACRALimiterData(testContext)
177177

178178
// A third send should work again
179179
assertTrue(
180180
LimitingReportAdministrator().shouldSendReport(
181181
testContext,
182-
CrashReportService.acraCoreConfigBuilder.build(),
182+
AcraCrashReporter.acraCoreConfigBuilder.build(),
183183
crashData,
184184
),
185185
)
@@ -192,7 +192,7 @@ class ACRATest : InstrumentedTest() {
192192
setReportConfig(FEEDBACK_REPORT_ALWAYS)
193193

194194
// If the user is set to always, then it's production, with interaction mode toast
195-
CrashReportService.setProductionACRAConfig(sharedPrefs)
195+
AcraCrashReporter.setProductionACRAConfig(sharedPrefs)
196196
verifyACRANotDisabled()
197197

198198
assertToastMessage(R.string.feedback_auto_toast_text)
@@ -207,7 +207,7 @@ class ACRATest : InstrumentedTest() {
207207
setReportConfig(FEEDBACK_REPORT_ALWAYS)
208208

209209
// If the user is set to ask, then it's production, with interaction mode dialog
210-
CrashReportService.setProductionACRAConfig(sharedPrefs)
210+
AcraCrashReporter.setProductionACRAConfig(sharedPrefs)
211211
verifyACRANotDisabled()
212212

213213
assertDialogEnabledStatus("dialog should be disabled when status is ALWAYS", false)
@@ -226,7 +226,7 @@ class ACRATest : InstrumentedTest() {
226226
setReportConfig(FEEDBACK_REPORT_ASK)
227227

228228
// If the user is set to ask, then it's production, with interaction mode dialog
229-
CrashReportService.setProductionACRAConfig(sharedPrefs)
229+
AcraCrashReporter.setProductionACRAConfig(sharedPrefs)
230230
verifyACRANotDisabled()
231231

232232
assertToastMessage(R.string.feedback_for_manual_toast_text)
@@ -262,7 +262,7 @@ class ACRATest : InstrumentedTest() {
262262
ThrowableFilterService.installDefaultExceptionHandler()
263263

264264
// reinitialize things and make sure they came through correctly again
265-
CrashReportService.onPreferenceChanged(app!!.applicationContext, FEEDBACK_REPORT_ASK)
265+
AcraCrashReporter.onPreferenceChanged(app!!.applicationContext, FEEDBACK_REPORT_ASK)
266266
firstExceptionHandler = Thread.getDefaultUncaughtExceptionHandler()
267267
assertThat("First handler is ThrowableFilterService", firstExceptionHandler is ThrowableFilterService.FilteringExceptionHandler)
268268
ThrowableFilterService.unInstallDefaultExceptionHandler()
@@ -282,15 +282,15 @@ class ACRATest : InstrumentedTest() {
282282
}
283283

284284
private fun setAcraReportingMode(feedbackReportAlways: String) {
285-
CrashReportService.setAcraReportingMode(feedbackReportAlways)
285+
AcraCrashReporter.setAcraReportingMode(feedbackReportAlways)
286286
}
287287

288288
@Throws(ACRAConfigurationException::class)
289289
private fun assertDialogEnabledStatus(
290290
message: String,
291291
isEnabled: Boolean,
292292
) {
293-
val config = CrashReportService.acraCoreConfigBuilder.build()
293+
val config = AcraCrashReporter.acraCoreConfigBuilder.build()
294294
for (configuration in config.pluginConfigurations) {
295295
// Make sure the dialog is set to pop up
296296
if (configuration.javaClass.toString().contains("Dialog")) {
@@ -301,7 +301,7 @@ class ACRATest : InstrumentedTest() {
301301

302302
@Throws(ACRAConfigurationException::class)
303303
private fun assertToastIsEnabled() {
304-
val config = CrashReportService.acraCoreConfigBuilder.build()
304+
val config = AcraCrashReporter.acraCoreConfigBuilder.build()
305305
for (configuration in config.pluginConfigurations) {
306306
if (configuration.javaClass.toString().contains("Toast")) {
307307
assertThat("Toast should be enabled", configuration.enabled(), equalTo(true))
@@ -313,7 +313,7 @@ class ACRATest : InstrumentedTest() {
313313
private fun assertToastMessage(
314314
@StringRes res: Int,
315315
) {
316-
val config = CrashReportService.acraCoreConfigBuilder.build()
316+
val config = AcraCrashReporter.acraCoreConfigBuilder.build()
317317
for (configuration in config.pluginConfigurations) {
318318
if (configuration.javaClass.toString().contains("Toast")) {
319319
assertEquals(
@@ -333,7 +333,7 @@ class ACRATest : InstrumentedTest() {
333333
}
334334

335335
private fun setReportConfig(feedbackReportAsk: String) {
336-
sharedPrefs.edit { putString(CrashReportService.FEEDBACK_REPORT_KEY, feedbackReportAsk) }
336+
sharedPrefs.edit { putString(AcraCrashReporter.FEEDBACK_REPORT_KEY, feedbackReportAsk) }
337337
}
338338

339339
private val sharedPrefs: SharedPreferences

AnkiDroid/src/main/java/com/ichi2/anki/CrashReportService.kt renamed to AnkiDroid/src/main/java/com/ichi2/anki/AcraCrashReporter.kt

Lines changed: 20 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ import org.acra.config.ToastConfigurationBuilder
4343
import org.acra.sender.HttpSender
4444
import timber.log.Timber
4545

46-
object CrashReportService {
46+
internal object AcraCrashReporter : CrashReporter {
4747
// ACRA constants used for stored preferences
4848
const val FEEDBACK_REPORT_KEY = "reportErrorMode"
4949
const val FEEDBACK_REPORT_ASK = "2"
@@ -166,15 +166,15 @@ object CrashReportService {
166166
*/
167167
@JvmStatic
168168
fun initialize(application: Application) {
169-
CrashReportService.application = application
169+
this.application = application
170170
// FIXME ACRA needs to reinitialize after language is changed, but with the new language
171171
// this is difficult because the Application (AnkiDroidApp) does not change it's baseContext
172172
// perhaps a solution could be to change AnkiDroidApp to have a context wrapper that it sets
173173
// as baseContext, and that wrapper allows a resources/configuration update, then
174174
// in GeneralSettingsFragment for the language dialog change listener, the context wrapper
175175
// could be updated directly with the new locale code so that calling getString on would fetch
176176
// the new language string ?
177-
toastText = ToastType.AUTO_TOAST.getToastMessage(CrashReportService.application)
177+
toastText = ToastType.AUTO_TOAST.getToastMessage(application)
178178

179179
// Setup logging and crash reporting
180180
if (BuildConfig.DEBUG) {
@@ -262,17 +262,32 @@ object CrashReportService {
262262
}
263263

264264
/** Used when we don't have an exception to throw, but we know something is wrong and want to diagnose it */
265-
fun sendExceptionReport(
265+
override fun sendExceptionReport(
266266
message: String?,
267267
origin: String?,
268268
) = sendExceptionReport(ManuallyReportedException(message), origin)
269269

270+
override fun sendExceptionReport(
271+
e: Throwable,
272+
origin: String?,
273+
additionalInfo: String?,
274+
onlyIfSilent: Boolean,
275+
) = sendExceptionReportInternal(e, origin, additionalInfo, onlyIfSilent, application.applicationContext)
276+
270277
fun sendExceptionReport(
271278
e: Throwable,
272279
origin: String?,
273280
additionalInfo: String? = null,
274281
onlyIfSilent: Boolean = false,
275-
context: Context = application.applicationContext,
282+
context: Context,
283+
) = sendExceptionReportInternal(e, origin, additionalInfo, onlyIfSilent, context)
284+
285+
private fun sendExceptionReportInternal(
286+
e: Throwable,
287+
origin: String?,
288+
additionalInfo: String?,
289+
onlyIfSilent: Boolean,
290+
context: Context,
276291
) {
277292
sendAnalyticsException(e, false)
278293
AnkiDroidApp.sentExceptionReportHack = true
@@ -383,37 +398,3 @@ object CrashReportService {
383398
.filter { it.exceptionClass == UserSubmittedException::class.java.name }
384399
.maxOfOrNull { it.timestamp?.timeInMillis ?: -1L } ?: -1L
385400
}
386-
387-
/**
388-
* Runs the provided block, catching [Exception], logging it and reporting it to [CrashReportService]
389-
*
390-
* **Example**
391-
* ```
392-
* runCatchingWithReport("callingMethod", onlyIfSilent = true) {
393-
* doSomethingRisky()
394-
* }
395-
* ```
396-
*
397-
* **Note**: This differs from [runCatching] - `Error` is thrown
398-
*
399-
* @param origin Data logged to Timber, and provided as the 'origin' field in the error report
400-
* @param onlyIfSilent Skip crash report if the crash reporting service is not 'always accept'
401-
* @param block Code to execute
402-
*
403-
* @throws Error If raised, this will be reported and rethrown
404-
*
405-
* @return A Result containing either the successful result of [block] or the [Exception] thrown
406-
*/
407-
fun <T> runCatchingWithReport(
408-
origin: String?,
409-
onlyIfSilent: Boolean = false,
410-
block: () -> T,
411-
): Result<T> =
412-
try {
413-
Result.success(block())
414-
} catch (e: Throwable) {
415-
Timber.w(e, origin)
416-
CrashReportService.sendExceptionReport(e, origin, onlyIfSilent = onlyIfSilent)
417-
if (e is Error) throw e
418-
Result.failure(e)
419-
}

AnkiDroid/src/main/java/com/ichi2/anki/AnkiDroidApp.kt

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ import androidx.core.net.toUri
3434
import androidx.fragment.app.FragmentActivity
3535
import androidx.lifecycle.MutableLiveData
3636
import anki.collection.OpChanges
37+
import com.ichi2.anki.AcraCrashReporter.sendExceptionReport
3738
import com.ichi2.anki.AnkiDroidApp.Companion.sharedPreferencesTestingOverride
38-
import com.ichi2.anki.CrashReportService.sendExceptionReport
3939
import com.ichi2.anki.analytics.UsageAnalytics
4040
import com.ichi2.anki.browser.SharedPreferencesLastDeckIdRepository
4141
import com.ichi2.anki.common.annotations.LegacyNotifications
@@ -134,7 +134,8 @@ open class AnkiDroidApp :
134134
// Ensures any change is propagated to widgets
135135
ChangeManager.subscribe(this)
136136

137-
CrashReportService.initialize(this)
137+
AcraCrashReporter.initialize(this)
138+
CrashReportService.setReporter(AcraCrashReporter)
138139
val logType = LogType.value
139140
when (logType) {
140141
LogType.DEBUG -> Timber.plant(DebugTree())
@@ -165,7 +166,7 @@ open class AnkiDroidApp :
165166
}
166167

167168
// Stop after analytics and logging are initialised.
168-
if (CrashReportService.isProperServiceProcess()) {
169+
if (AcraCrashReporter.isProperServiceProcess()) {
169170
Timber.d("Skipping AnkiDroidApp.onCreate from ACRA sender process")
170171
return
171172
}

AnkiDroid/src/main/java/com/ichi2/anki/analytics/AnkiDroidCrashReportDialog.kt

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import android.content.DialogInterface
2222
import android.os.Bundle
2323
import android.view.View
2424
import androidx.core.content.edit
25-
import com.ichi2.anki.CrashReportService
25+
import com.ichi2.anki.AcraCrashReporter
2626
import com.ichi2.anki.R
2727
import com.ichi2.anki.databinding.DialogFeedbackBinding
2828
import com.ichi2.anki.preferences.sharedPrefs
@@ -87,11 +87,11 @@ class AnkiDroidCrashReportDialog :
8787
if (autoReport) {
8888
preferences.edit {
8989
putString(
90-
CrashReportService.FEEDBACK_REPORT_KEY,
91-
CrashReportService.FEEDBACK_REPORT_ALWAYS,
90+
AcraCrashReporter.FEEDBACK_REPORT_KEY,
91+
AcraCrashReporter.FEEDBACK_REPORT_ALWAYS,
9292
)
9393
}
94-
CrashReportService.setAcraReportingMode(CrashReportService.FEEDBACK_REPORT_ALWAYS)
94+
AcraCrashReporter.setAcraReportingMode(AcraCrashReporter.FEEDBACK_REPORT_ALWAYS)
9595
}
9696
// Send the crash report
9797
helper!!.sendCrash(binding.userComment.text.toString(), "")
@@ -100,7 +100,7 @@ class AnkiDroidCrashReportDialog :
100100
// The limiter persists it's limit info *before* the user cancels.
101101
// Therefore, on cancel, purge limits to make sure the user may actually send in future.
102102
// Better to maybe send to many reports than definitely too few.
103-
CrashReportService.deleteACRALimiterData(this)
103+
AcraCrashReporter.deleteACRALimiterData(this)
104104
helper!!.cancelReports()
105105
}
106106
finish()

AnkiDroid/src/main/java/com/ichi2/anki/dialogs/help/HelpItemActionsDispatcher.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@
1414
package com.ichi2.anki.dialogs.help
1515

1616
import androidx.annotation.StringRes
17+
import com.ichi2.anki.AcraCrashReporter
1718
import com.ichi2.anki.AnkiActivity
1819
import com.ichi2.anki.AnkiDroidApp
19-
import com.ichi2.anki.CrashReportService
2020
import com.ichi2.anki.R
2121
import com.ichi2.anki.snackbar.showSnackbar
2222
import com.ichi2.utils.AdaptionUtil
@@ -62,7 +62,7 @@ class AnkiActivityHelpActionsDispatcher(
6262
if (AdaptionUtil.isUserATestClient) {
6363
ankiActivity.showSnackbar(ankiActivity.getString(R.string.user_is_a_robot))
6464
} else {
65-
val wasReportSent = CrashReportService.sendReport(ankiActivity)
65+
val wasReportSent = AcraCrashReporter.sendReport(ankiActivity)
6666
if (!wasReportSent) {
6767
ankiActivity.showSnackbar(ankiActivity.getString(R.string.help_dialog_exception_report_debounce))
6868
return

AnkiDroid/src/main/java/com/ichi2/anki/preferences/DeveloperOptionsFragment.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ import androidx.core.app.ActivityCompat
2020
import androidx.core.content.edit
2121
import androidx.preference.Preference
2222
import androidx.preference.SwitchPreferenceCompat
23+
import com.ichi2.anki.AcraCrashReporter
2324
import com.ichi2.anki.AnkiDroidApp
2425
import com.ichi2.anki.BuildConfig
2526
import com.ichi2.anki.CollectionHelper
2627
import com.ichi2.anki.CollectionManager.withCol
27-
import com.ichi2.anki.CrashReportService
2828
import com.ichi2.anki.R
2929
import com.ichi2.anki.analytics.UsageAnalytics
3030
import com.ichi2.anki.dialogs.TtsVoicesDialogFragment
@@ -60,7 +60,7 @@ class DeveloperOptionsFragment : SettingsFragment() {
6060
requirePreference<Preference>(R.string.pref_trigger_crash_key).setOnPreferenceClickListener {
6161
// If we don't delete the limiter data, our test crash may not go through,
6262
// but we are triggering it very much on purpose, we want to see the crash in ACRA
63-
this.context?.let { c -> CrashReportService.deleteACRALimiterData(c) }
63+
this.context?.let { c -> AcraCrashReporter.deleteACRALimiterData(c) }
6464

6565
Timber.w("Crash triggered on purpose from advanced preferences in debug mode")
6666
throw RuntimeException("This is a test crash")

0 commit comments

Comments
 (0)