For mozilla-mobile#8989 - Don't catch/rethrow prompt cast exceptions

This try-catch was added in mozilla-mobile#6687 as an effort to better individualize prompt cast exceptions in our crash reporting platforms to better asses their frequency and hopefully get more data to debug and fix the underlying issue. The original issue is resolved in the first commit of this patch so there is no need to keep the try-catch.
eliserichards · Jun 2, 2021 · 1a9b3ea · 1a9b3ea
1 parent 95f4cb0
commit 1a9b3ea
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 78 deletions.
diff --git a/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt b/components/feature/prompts/src/main/java/mozilla/components/feature/prompts/PromptFeature.kt
@@ -468,62 +468,55 @@ class PromptFeature private constructor(
     @Suppress("UNCHECKED_CAST", "ComplexMethod")
     override fun onConfirm(sessionId: String, promptRequestUID: String, value: Any?) {
         store.consumePromptFrom(sessionId, promptRequestUID, activePrompt) {
-            try {
-                when (it) {
-                    is TimeSelection -> it.onConfirm(value as Date)
-                    is Color -> it.onConfirm(value as String)
-                    is Alert -> {
-                        val shouldNotShowMoreDialogs = value as Boolean
-                        promptAbuserDetector.userWantsMoreDialogs(!shouldNotShowMoreDialogs)
-                        it.onConfirm(!shouldNotShowMoreDialogs)
-                    }
-                    is SingleChoice -> it.onConfirm(value as Choice)
-                    is MenuChoice -> it.onConfirm(value as Choice)
-                    is BeforeUnload -> it.onLeave()
-                    is Popup -> {
-                        val shouldNotShowMoreDialogs = value as Boolean
-                        promptAbuserDetector.userWantsMoreDialogs(!shouldNotShowMoreDialogs)
-                        it.onAllow()
-                    }
-                    is MultipleChoice -> it.onConfirm(value as Array<Choice>)
+            when (it) {
+                is TimeSelection -> it.onConfirm(value as Date)
+                is Color -> it.onConfirm(value as String)
+                is Alert -> {
+                    val shouldNotShowMoreDialogs = value as Boolean
+                    promptAbuserDetector.userWantsMoreDialogs(!shouldNotShowMoreDialogs)
+                    it.onConfirm(!shouldNotShowMoreDialogs)
+                }
+                is SingleChoice -> it.onConfirm(value as Choice)
+                is MenuChoice -> it.onConfirm(value as Choice)
+                is BeforeUnload -> it.onLeave()
+                is Popup -> {
+                    val shouldNotShowMoreDialogs = value as Boolean
+                    promptAbuserDetector.userWantsMoreDialogs(!shouldNotShowMoreDialogs)
+                    it.onAllow()
+                }
+                is MultipleChoice -> it.onConfirm(value as Array<Choice>)
 
-                    is Authentication -> {
-                        val (user, password) = value as Pair<String, String>
-                        it.onConfirm(user, password)
-                    }
+                is Authentication -> {
+                    val (user, password) = value as Pair<String, String>
+                    it.onConfirm(user, password)
+                }
 
-                    is TextPrompt -> {
-                        val (shouldNotShowMoreDialogs, text) = value as Pair<Boolean, String>
+                is TextPrompt -> {
+                    val (shouldNotShowMoreDialogs, text) = value as Pair<Boolean, String>
 
-                        promptAbuserDetector.userWantsMoreDialogs(!shouldNotShowMoreDialogs)
-                        it.onConfirm(!shouldNotShowMoreDialogs, text)
-                    }
+                    promptAbuserDetector.userWantsMoreDialogs(!shouldNotShowMoreDialogs)
+                    it.onConfirm(!shouldNotShowMoreDialogs, text)
+                }
 
-                    is Share -> it.onSuccess()
-
-                    is SaveLoginPrompt -> it.onConfirm(value as Login)
-
-                    is Confirm -> {
-                        val (isCheckBoxChecked, buttonType) =
-                            value as Pair<Boolean, MultiButtonDialogFragment.ButtonType>
-                        promptAbuserDetector.userWantsMoreDialogs(!isCheckBoxChecked)
-                        when (buttonType) {
-                            MultiButtonDialogFragment.ButtonType.POSITIVE ->
-                                it.onConfirmPositiveButton(!isCheckBoxChecked)
-                            MultiButtonDialogFragment.ButtonType.NEGATIVE ->
-                                it.onConfirmNegativeButton(!isCheckBoxChecked)
-                            MultiButtonDialogFragment.ButtonType.NEUTRAL ->
-                                it.onConfirmNeutralButton(!isCheckBoxChecked)
-                        }
+                is Share -> it.onSuccess()
+
+                is SaveLoginPrompt -> it.onConfirm(value as Login)
+
+                is Confirm -> {
+                    val (isCheckBoxChecked, buttonType) =
+                        value as Pair<Boolean, MultiButtonDialogFragment.ButtonType>
+                    promptAbuserDetector.userWantsMoreDialogs(!isCheckBoxChecked)
+                    when (buttonType) {
+                        MultiButtonDialogFragment.ButtonType.POSITIVE ->
+                            it.onConfirmPositiveButton(!isCheckBoxChecked)
+                        MultiButtonDialogFragment.ButtonType.NEGATIVE ->
+                            it.onConfirmNegativeButton(!isCheckBoxChecked)
+                        MultiButtonDialogFragment.ButtonType.NEUTRAL ->
+                            it.onConfirmNeutralButton(!isCheckBoxChecked)
                     }
-
-                    is Repost -> it.onConfirm()
                 }
-            } catch (e: ClassCastException) {
-                throw IllegalArgumentException(
-                    "PromptFeature onConsume cast failed with ${it.javaClass}",
-                    e
-                )
+
+                is Repost -> it.onConfirm()
             }
         }
     }

diff --git a/...nts/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt b/...nts/feature/prompts/src/test/java/mozilla/components/feature/prompts/PromptFeatureTest.kt
@@ -1752,34 +1752,6 @@ class PromptFeatureTest {
         assertFalse(prompt!!.shouldDismissOnLoad)
     }
 
-    @Test
-    fun `PromptFeature throws IllegalArgumentException when ClassCastException is triggered`() {
-        val feature = PromptFeature(
-            activity = mock(),
-            store = store,
-            fragmentManager = fragmentManager
-        ) { }
-        feature.start()
-
-        val singleChoiceRequest = SingleChoice(arrayOf()) {}
-        var illegalArgumentExceptionThrown = false
-        store.dispatch(ContentAction.UpdatePromptRequestAction(tabId, singleChoiceRequest))
-            .joinBlocking()
-
-        try {
-            feature.onConfirm(tabId, singleChoiceRequest.uid, "wrong")
-        } catch (e: IllegalArgumentException) {
-            illegalArgumentExceptionThrown = true
-            assertEquals(
-                "PromptFeature onConsume cast failed with class mozilla.components.concept.engine.prompt.PromptRequest\$SingleChoice",
-                e.message
-            )
-        }
-
-        store.waitUntilIdle()
-        assert(illegalArgumentExceptionThrown)
-    }
-
     @Test
     fun `A Repost PromptRequest prompt will be shown as a ConfirmDialogFragment`() {
         val feature = PromptFeature(