fix: do not throw while evaluating flag, return proper error (#147)

do not throw while evaluating flag, return proper error
spotify · May 16, 2024 · d26d8e0 · d26d8e0
1 parent e960451
commit d26d8e0
Show file tree

Hide file tree

Showing 4 changed files with 52 additions and 43 deletions.
diff --git a/Confidence/src/main/java/com/spotify/confidence/Confidence.kt b/Confidence/src/main/java/com/spotify/confidence/Confidence.kt
@@ -79,10 +79,10 @@ class Confidence internal constructor(
 
     fun <T> getFlag(
         key: String,
-        defaultValue: T
+        default: T
     ): Evaluation<T> = cache.get().getEvaluation(
         key,
-        defaultValue,
+        default,
         getContext()
     ) { flagName, resolveToken ->
         // this lambda will be invoked inside the evaluation process

diff --git a/Confidence/src/main/java/com/spotify/confidence/ConfidenceFlagEvaluation.kt b/Confidence/src/main/java/com/spotify/confidence/ConfidenceFlagEvaluation.kt
@@ -1,8 +1,5 @@
 package com.spotify.confidence
 
-import kotlin.jvm.Throws
-
-@Throws(FlagNotFoundError::class, ParseError::class)
 fun <T> FlagResolution?.getEvaluation(
     flag: String,
     defaultValue: T,
@@ -19,9 +16,10 @@ fun <T> FlagResolution?.getEvaluation(
     }
     requireNotNull(this)
     val resolvedFlag = this.flags.firstOrNull { it.flag == parsedKey.flagName }
-        ?: throw FlagNotFoundError(
-            "Could not find flag named: ${parsedKey.flagName}",
-            parsedKey.flagName
+        ?: return Evaluation(
+            value = defaultValue,
+            reason = ResolveReason.ERROR,
+            errorCode = ErrorCode.FLAG_NOT_FOUND
         )
 
     if (resolvedFlag.reason != ResolveReason.RESOLVE_REASON_TARGETING_KEY_ERROR) {
@@ -40,12 +38,13 @@ fun <T> FlagResolution?.getEvaluation(
     val resolvedValue: ConfidenceValue =
         findValueFromValuePath(ConfidenceValue.Struct(flagValue), parsedKey.valuePath)
             ?: if (resolvedFlag.reason == ResolveReason.RESOLVE_REASON_MATCH) {
-                throw ParseError(
-                    "Unable to parse flag value: ${parsedKey.valuePath}",
-                    parsedKey.valuePath
+                return Evaluation(
+                    value = defaultValue,
+                    reason = resolvedFlag.reason,
+                    errorCode = ErrorCode.PARSE_ERROR
                 )
             } else {
-                return Evaluation(value = defaultValue, reason = ResolveReason.DEFAULT)
+                return Evaluation(value = defaultValue, reason = resolvedFlag.reason)
             }
 
     return when (resolvedFlag.reason) {
@@ -117,7 +116,8 @@ enum class ErrorCode {
     // The flag could not be found.
     FLAG_NOT_FOUND,
     INVALID_CONTEXT,
-    PROVIDER_NOT_READY
+    PROVIDER_NOT_READY,
+    PARSE_ERROR
 }
 
 data class ParseError(

diff --git a/Confidence/src/test/java/com/spotify/confidence/ConfidenceEvaluationTest.kt b/Confidence/src/test/java/com/spotify/confidence/ConfidenceEvaluationTest.kt
@@ -920,13 +920,15 @@ internal class ConfidenceEvaluationTest {
         val context = mapOf("key" to ConfidenceValue.String("foo"))
         val mockConfidence = getConfidence(testDispatcher, initialContext = context)
 
+        val reason = ResolveReason.RESOLVE_REASON_NO_TREATMENT_MATCH
+
         val resolvedNonMatchingFlags = Flags(
             listOf(
                 ResolvedFlag(
                     flag = "test-kotlin-flag-1",
                     variant = "",
                     mapOf(),
-                    ResolveReason.RESOLVE_REASON_NO_TREATMENT_MATCH
+                    reason
                 )
             )
         )
@@ -953,7 +955,7 @@ internal class ConfidenceEvaluationTest {
         Assert.assertNull(evalString.errorCode)
         Assert.assertNull(evalString.variant)
         TestCase.assertEquals("default", evalString.value)
-        TestCase.assertEquals(ResolveReason.DEFAULT, evalString.reason)
+        TestCase.assertEquals(reason, evalString.reason)
     }
 
     @Test
@@ -982,13 +984,11 @@ internal class ConfidenceEvaluationTest {
             "token2"
         )
         cache.refresh(cacheData)
-        val ex = Assert.assertThrows(FlagNotFoundError::class.java) {
-            mockConfidence.getFlag(
-                "test-kotlin-flag-2.mystring",
-                "default"
-            )
-        }
-        TestCase.assertEquals("Could not find flag named: test-kotlin-flag-2", ex.message)
+        val ex = mockConfidence.getFlag(
+            "test-kotlin-flag-2.mystring",
+            "default"
+        )
+        TestCase.assertEquals(ErrorCode.FLAG_NOT_FOUND, ex.errorCode)
     }
 
     @Test
@@ -1001,13 +1001,11 @@ internal class ConfidenceEvaluationTest {
         whenever(flagResolverClient.resolve(eq(listOf()), any())).thenThrow(Error(""))
         mockConfidence.fetchAndActivate()
         advanceUntilIdle()
-        val ex = Assert.assertThrows(FlagNotFoundError::class.java) {
-            mockConfidence.getFlag(
-                "test-kotlin-flag-2.mystring",
-                "default"
-            )
-        }
-        TestCase.assertEquals("Could not find flag named: test-kotlin-flag-2", ex.message)
+        val ex = mockConfidence.getFlag(
+            "test-kotlin-flag-2.mystring",
+            "default"
+        )
+        TestCase.assertEquals(ErrorCode.FLAG_NOT_FOUND, ex.errorCode)
     }
 
     @Test
@@ -1032,13 +1030,11 @@ internal class ConfidenceEvaluationTest {
         )
         mockConfidence.fetchAndActivate()
         advanceUntilIdle()
-        val ex = Assert.assertThrows(ParseError::class.java) {
-            mockConfidence.getFlag(
-                "test-kotlin-flag-1.wrongid",
-                "default"
-            )
-        }
-        TestCase.assertEquals("Unable to parse flag value: [wrongid]", ex.message)
+        val ex = mockConfidence.getFlag(
+            "test-kotlin-flag-1.wrongid",
+            "default"
+        )
+        TestCase.assertEquals(ErrorCode.PARSE_ERROR, ex.errorCode)
     }
 
     @Test
@@ -1058,13 +1054,11 @@ internal class ConfidenceEvaluationTest {
         )
         mockConfidence.fetchAndActivate()
         advanceUntilIdle()
-        val ex = Assert.assertThrows(ParseError::class.java) {
-            mockConfidence.getFlag(
-                "test-kotlin-flag-1.mystring.extrapath",
-                "default"
-            )
-        }
-        TestCase.assertEquals("Unable to parse flag value: [mystring, extrapath]", ex.message)
+        val ex = mockConfidence.getFlag(
+            "test-kotlin-flag-1.mystring.extrapath",
+            "default"
+        )
+        TestCase.assertEquals(ErrorCode.PARSE_ERROR, ex.errorCode)
     }
 }
 

diff --git a/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainActivity.kt b/ConfidenceDemoApp/src/main/java/com/example/confidencedemoapp/MainActivity.kt
@@ -15,19 +15,34 @@ import androidx.compose.material.MaterialTheme
 import androidx.compose.material.Surface
 import androidx.compose.material.Text
 import androidx.compose.runtime.Composable
+import androidx.compose.runtime.collectAsState
 import androidx.compose.runtime.livedata.observeAsState
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.graphics.Color
 import androidx.compose.ui.unit.dp
 import androidx.lifecycle.ViewModelProvider
 import com.example.confidencedemoapp.ui.theme.MyApplicationTheme
+import com.spotify.confidence.ConfidenceFactory
+import com.spotify.confidence.ConfidenceRegion
+import com.spotify.confidence.ConfidenceValue
+import kotlinx.coroutines.flow.flow
 
 class MainActivity : ComponentActivity() {
     private lateinit var vm: MainVm
 
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
         setContent {
+            val confidence = ConfidenceFactory.create(
+                applicationContext,
+                "clientSecret",
+                initialContext = mapOf("targeting_key" to ConfidenceValue.String("a98a4291-53b0-49d9-bae8-73d3f5da2070")),
+                ConfidenceRegion.EUROPE
+            )
+            val state = flow {
+                confidence.fetchAndActivate()
+                emit("READY")
+            }.collectAsState(initial ="LOADING")
             // These are observable states where changed will "update the view"
             val msgState = vm.message.observeAsState()
             val colorState = vm.color.observeAsState()