mirror of
https://github.com/d4rken-org/capod.git
synced 2026-09-14 18:26:11 -04:00
fix: Harden the error dialog's Google Play fix action
The dialog's fix dispatch ran unguarded: a throwing action crashed the UI thread from inside a click handler and skipped onDismiss(), leaving the dialog latched on the current error. The dispatch is now wrapped in try/catch with onDismiss() in a finally block. Google Play fix action: - Drop FLAG_ACTIVITY_NEW_TASK. The action runs on an activity context, so the flag only detached Play's app info from the caller's task and back stack. - Catch SecurityException next to ActivityNotFoundException: Play can be installed but blocked (disabled app, restricted profile, guarding ROM), which denies the launch instead of failing to resolve it. - The fallback toast is now a translatable string resource instead of a hardcoded literal. New coverage: ComposeErrorDialogGuardTest pins that a throwing fix action still dismisses the dialog (shared source set, so both flavors run it), GplayFixActionTest pins the denied and unresolvable launches showing a toast instead of crashing, and ComposeErrorDialogTest now asserts the launch intent carries no NEW_TASK flag.
This commit is contained in:
+16
-8
@@ -1,5 +1,6 @@
|
||||
package eu.darken.capod.common.upgrade.core.billing
|
||||
|
||||
import android.app.Activity
|
||||
import android.content.ActivityNotFoundException
|
||||
import android.content.Context
|
||||
import android.content.Intent
|
||||
@@ -25,21 +26,28 @@ class GplayServiceUnavailableException(cause: Throwable) :
|
||||
// action is a GENERIC troubleshooting affordance (open Play's app info), not a diagnosis of
|
||||
// the cause. Harmless for a transient blip, and it matches the fleet's dialog.
|
||||
fixAction = { activity ->
|
||||
try {
|
||||
val intent = Intent().apply {
|
||||
action = Settings.ACTION_APPLICATION_DETAILS_SETTINGS
|
||||
data = Uri.fromParts("package", GPLAY_PKG, null)
|
||||
addFlags(Intent.FLAG_ACTIVITY_NEW_TASK)
|
||||
}
|
||||
val intent = Intent().apply {
|
||||
action = Settings.ACTION_APPLICATION_DETAILS_SETTINGS
|
||||
data = Uri.fromParts("package", GPLAY_PKG, null)
|
||||
}
|
||||
|
||||
try {
|
||||
activity.startActivity(intent)
|
||||
} catch (e: ActivityNotFoundException) {
|
||||
log(ERROR) { "Can't launch settings intent for Google Play: $e" }
|
||||
Toast.makeText(activity, "Google Play is not installed", Toast.LENGTH_SHORT).show()
|
||||
onLaunchFailed(activity, e)
|
||||
} catch (e: SecurityException) {
|
||||
// Play can be installed but unreachable: disabled app, work/restricted profile or a
|
||||
// ROM that guards the settings screen. The launch is denied, not unresolvable.
|
||||
onLaunchFailed(activity, e)
|
||||
}
|
||||
},
|
||||
)
|
||||
|
||||
private fun onLaunchFailed(activity: Activity, e: Exception) {
|
||||
log(ERROR) { "Can't launch settings intent for Google Play: $e" }
|
||||
Toast.makeText(activity, R.string.upgrades_gplay_not_installed_message, Toast.LENGTH_SHORT).show()
|
||||
}
|
||||
|
||||
companion object {
|
||||
private const val GPLAY_PKG = "com.android.vending"
|
||||
}
|
||||
|
||||
@@ -69,6 +69,8 @@
|
||||
<string name="settings_upgrade_status_label">CAPod Pro</string>
|
||||
<string name="settings_upgrade_status_description">Your upgrade and purchase status.</string>
|
||||
<string name="upgrades_gplay_unavailable_error_description">CAPod can\'t connect to Google Play. Is Google Play installed and up to date? Is your Google Account logged in? Try clearing the cache of the Google Play app and rebooting your device.</string>
|
||||
<!-- Toast shown when the "Google Play" button of an error dialog can't open Google Play's app info, e.g. Google Play is missing or blocked on this device. "Google Play" is a brand name, keep it untranslated. -->
|
||||
<string name="upgrades_gplay_not_installed_message">Google Play is not installed.</string>
|
||||
<string name="upgrades_gplay_internal_error_title">Google Play error</string>
|
||||
<string name="upgrades_gplay_internal_error_description">An internal Google Play error occurred. Please try the following:\n\n• Restart your device\n• Clear Google Play Store cache\n• Try again later</string>
|
||||
<string name="upgrades_gplay_network_error_title">Connection error</string>
|
||||
|
||||
@@ -13,6 +13,10 @@ import androidx.compose.runtime.setValue
|
||||
import androidx.compose.ui.platform.LocalContext
|
||||
import androidx.compose.ui.res.stringResource
|
||||
import eu.darken.capod.R
|
||||
import eu.darken.capod.common.debug.logging.Logging.Priority.ERROR
|
||||
import eu.darken.capod.common.debug.logging.asLog
|
||||
import eu.darken.capod.common.debug.logging.log
|
||||
import eu.darken.capod.common.debug.logging.logTag
|
||||
|
||||
@Composable
|
||||
fun ErrorEventHandler(source: ErrorEventSource2) {
|
||||
@@ -52,8 +56,16 @@ private fun ComposeErrorDialog(
|
||||
if (hasFix) {
|
||||
TextButton(
|
||||
onClick = {
|
||||
localizedError.fixAction!!.invoke(activity!!)
|
||||
onDismiss()
|
||||
// Error actions are arbitrary code (intent launches): a throw here would
|
||||
// crash the UI thread from inside a click handler, and skipping onDismiss()
|
||||
// would leave the dialog latched on the current error with no way out.
|
||||
try {
|
||||
localizedError.fixAction!!.invoke(activity!!)
|
||||
} catch (e: Exception) {
|
||||
log(TAG, ERROR) { "Error action failed: ${e.asLog()}" }
|
||||
} finally {
|
||||
onDismiss()
|
||||
}
|
||||
},
|
||||
) {
|
||||
Text(text = localizedError.fixActionLabel ?: stringResource(android.R.string.ok))
|
||||
@@ -75,3 +87,5 @@ private fun ComposeErrorDialog(
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
private val TAG = logTag("Error", "Dialog", "Compose")
|
||||
|
||||
@@ -0,0 +1,73 @@
|
||||
package eu.darken.capod.common.error
|
||||
|
||||
import android.app.Activity
|
||||
import android.content.Context
|
||||
import androidx.compose.ui.test.assertCountEquals
|
||||
import androidx.compose.ui.test.onAllNodesWithText
|
||||
import androidx.compose.ui.test.onNodeWithText
|
||||
import androidx.compose.ui.test.performClick
|
||||
import eu.darken.capod.common.compose.PreviewWrapper
|
||||
import eu.darken.capod.common.flow.SingleEventFlow
|
||||
import io.kotest.matchers.shouldBe
|
||||
import org.junit.Test
|
||||
import testhelpers.compose.BaseComposeRobolectricTest
|
||||
|
||||
/**
|
||||
* The shared error dialog dispatches arbitrary fix actions: one that blows up must never take the
|
||||
* UI — or the dialog's exit — down with it. Flavor-independent, so it runs on both legs.
|
||||
*/
|
||||
class ComposeErrorDialogGuardTest : BaseComposeRobolectricTest() {
|
||||
|
||||
private class FakeErrorSource : ErrorEventSource2 {
|
||||
override val errorEvents = SingleEventFlow<Throwable>()
|
||||
}
|
||||
|
||||
private class TestError(
|
||||
private val fixAction: (Activity) -> Unit,
|
||||
) : Exception(ERROR_BODY), HasLocalizedError {
|
||||
override fun getLocalizedError(context: Context): LocalizedError = LocalizedError(
|
||||
throwable = this,
|
||||
label = ERROR_TITLE,
|
||||
description = ERROR_BODY,
|
||||
fixActionLabel = FIX_LABEL,
|
||||
fixAction = fixAction,
|
||||
)
|
||||
}
|
||||
|
||||
private fun showError(error: Throwable) {
|
||||
val source = FakeErrorSource()
|
||||
composeRule.setContent {
|
||||
PreviewWrapper {
|
||||
ErrorEventHandler(source)
|
||||
}
|
||||
}
|
||||
// Buffered channel: the event survives until the handler's collector attaches.
|
||||
source.errorEvents.tryEmit(error)
|
||||
composeRule.waitForIdle()
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a throwing fix action still dismisses the dialog`() {
|
||||
var invoked = false
|
||||
showError(
|
||||
TestError {
|
||||
// Flag first: the assertion below has to distinguish "action ran and threw" from
|
||||
// "action was never dispatched".
|
||||
invoked = true
|
||||
throw IllegalStateException("fix action exploded")
|
||||
}
|
||||
)
|
||||
|
||||
composeRule.onNodeWithText(FIX_LABEL).performClick()
|
||||
composeRule.waitForIdle()
|
||||
|
||||
invoked shouldBe true
|
||||
// The handler latches on the current error: the dialog only goes away if the throw was
|
||||
// caught and onDismiss still ran.
|
||||
composeRule.onAllNodesWithText(FIX_LABEL).assertCountEquals(0)
|
||||
}
|
||||
}
|
||||
|
||||
private const val ERROR_TITLE = "Test error title"
|
||||
private const val ERROR_BODY = "Test error description"
|
||||
private const val FIX_LABEL = "Fix it"
|
||||
@@ -1,6 +1,7 @@
|
||||
package eu.darken.capod.common.error
|
||||
|
||||
import android.content.Context
|
||||
import android.content.Intent
|
||||
import android.provider.Settings
|
||||
import androidx.activity.ComponentActivity
|
||||
import androidx.compose.ui.test.assertCountEquals
|
||||
@@ -76,6 +77,9 @@ class ComposeErrorDialogTest : BaseTest() {
|
||||
val started = shadowOf(composeRule.activity).nextStartedActivity.shouldNotBeNull()
|
||||
started.action shouldBe Settings.ACTION_APPLICATION_DETAILS_SETTINGS
|
||||
started.data.toString() shouldBe "package:com.android.vending"
|
||||
// NEW_TASK on an activity context detaches Play's app info from our task: the user loses the
|
||||
// back path to the screen they came from and the settings screen lingers in recents.
|
||||
(started.flags and Intent.FLAG_ACTIVITY_NEW_TASK) shouldBe 0
|
||||
// The user acted: leaving the dialog up would greet them again on the way back.
|
||||
composeRule.onAllNodesWithText("Google Play").assertCountEquals(0)
|
||||
}
|
||||
|
||||
+57
@@ -0,0 +1,57 @@
|
||||
package eu.darken.capod.common.upgrade.core.billing
|
||||
|
||||
import android.app.Activity
|
||||
import android.content.ActivityNotFoundException
|
||||
import android.content.Intent
|
||||
import eu.darken.capod.R
|
||||
import io.kotest.matchers.nulls.shouldNotBeNull
|
||||
import io.kotest.matchers.shouldBe
|
||||
import org.junit.Test
|
||||
import org.junit.runner.RunWith
|
||||
import org.robolectric.Robolectric
|
||||
import org.robolectric.RobolectricTestRunner
|
||||
import org.robolectric.annotation.Config
|
||||
import org.robolectric.shadows.ShadowToast
|
||||
import testhelpers.BaseTest
|
||||
import testhelpers.TestApplication
|
||||
|
||||
/**
|
||||
* The error dialog's "Google Play" button runs on an activity context: a device where the launch is
|
||||
* refused must get a toast, not a crash. The successful launch is covered by ComposeErrorDialogTest.
|
||||
*/
|
||||
@RunWith(RobolectricTestRunner::class)
|
||||
@Config(sdk = [33], application = TestApplication::class)
|
||||
class GplayFixActionTest : BaseTest() {
|
||||
|
||||
/** Play is installed but unreachable: disabled app, restricted profile or a guarding ROM. */
|
||||
class DeniedLaunchActivity : Activity() {
|
||||
override fun startActivity(intent: Intent): Unit = throw SecurityException("Permission Denial")
|
||||
}
|
||||
|
||||
/** Play isn't on the device at all, so nothing resolves the app info intent. */
|
||||
class MissingPlayActivity : Activity() {
|
||||
override fun startActivity(intent: Intent): Unit = throw ActivityNotFoundException("No Activity found")
|
||||
}
|
||||
|
||||
private fun <T : Activity> activityOf(clazz: Class<T>): T = Robolectric.buildActivity(clazz).setup().get()
|
||||
|
||||
private fun assertToastInsteadOfCrash(activity: Activity) {
|
||||
val fixAction = GplayServiceUnavailableException(RuntimeException("Play hiccup"))
|
||||
.getLocalizedError(activity).fixAction.shouldNotBeNull()
|
||||
|
||||
fixAction.invoke(activity)
|
||||
|
||||
val expected = activity.getString(R.string.upgrades_gplay_not_installed_message)
|
||||
ShadowToast.getTextOfLatestToast() shouldBe expected
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a denied launch shows the not-installed toast instead of crashing`() {
|
||||
assertToastInsteadOfCrash(activityOf(DeniedLaunchActivity::class.java))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `an unresolvable launch shows the not-installed toast instead of crashing`() {
|
||||
assertToastInsteadOfCrash(activityOf(MissingPlayActivity::class.java))
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user