diff --git a/app/src/gplay/java/eu/darken/capod/common/upgrade/core/billing/GplayServiceUnavailableException.kt b/app/src/gplay/java/eu/darken/capod/common/upgrade/core/billing/GplayServiceUnavailableException.kt index 2997ae6a..7fb93c96 100644 --- a/app/src/gplay/java/eu/darken/capod/common/upgrade/core/billing/GplayServiceUnavailableException.kt +++ b/app/src/gplay/java/eu/darken/capod/common/upgrade/core/billing/GplayServiceUnavailableException.kt @@ -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" } diff --git a/app/src/gplay/res/values/strings.xml b/app/src/gplay/res/values/strings.xml index 52494098..c45bb9ad 100644 --- a/app/src/gplay/res/values/strings.xml +++ b/app/src/gplay/res/values/strings.xml @@ -69,6 +69,8 @@ CAPod Pro Your upgrade and purchase status. 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. + + Google Play is not installed. Google Play error 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 Connection error diff --git a/app/src/main/java/eu/darken/capod/common/error/ErrorEventHandler.kt b/app/src/main/java/eu/darken/capod/common/error/ErrorEventHandler.kt index 54906c90..6bcd993a 100644 --- a/app/src/main/java/eu/darken/capod/common/error/ErrorEventHandler.kt +++ b/app/src/main/java/eu/darken/capod/common/error/ErrorEventHandler.kt @@ -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") diff --git a/app/src/test/java/eu/darken/capod/common/error/ComposeErrorDialogGuardTest.kt b/app/src/test/java/eu/darken/capod/common/error/ComposeErrorDialogGuardTest.kt new file mode 100644 index 00000000..de06b755 --- /dev/null +++ b/app/src/test/java/eu/darken/capod/common/error/ComposeErrorDialogGuardTest.kt @@ -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() + } + + 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" diff --git a/app/src/testGplay/java/eu/darken/capod/common/error/ComposeErrorDialogTest.kt b/app/src/testGplay/java/eu/darken/capod/common/error/ComposeErrorDialogTest.kt index 75e4e23a..7ac7d2d1 100644 --- a/app/src/testGplay/java/eu/darken/capod/common/error/ComposeErrorDialogTest.kt +++ b/app/src/testGplay/java/eu/darken/capod/common/error/ComposeErrorDialogTest.kt @@ -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) } diff --git a/app/src/testGplay/java/eu/darken/capod/common/upgrade/core/billing/GplayFixActionTest.kt b/app/src/testGplay/java/eu/darken/capod/common/upgrade/core/billing/GplayFixActionTest.kt new file mode 100644 index 00000000..c08080fc --- /dev/null +++ b/app/src/testGplay/java/eu/darken/capod/common/upgrade/core/billing/GplayFixActionTest.kt @@ -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 activityOf(clazz: Class): 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)) + } +}