Merge pull request #50 from statsig-io/fix_eb

fix: Fix eb behavior

Author: xinlili-statsig

build.gradle

@@ -78,6 +78,7 @@ dependencies {
     testImplementation 'com.github.tomakehurst:wiremock:2.27.2'
     testImplementation "org.slf4j:slf4j-simple:1.8.0-beta4"
     testImplementation 'org.junit.jupiter:junit-jupiter:5.8.1'
+    testImplementation 'com.squareup.okhttp3:mockwebserver:4.9.0'
 }
 
 
@@ -141,4 +142,3 @@ afterEvaluate {
         }
     }
 }
-

src/main/java/com/statsig/androidlocalevalsdk/ErrorBoundary.kt

@@ -2,6 +2,9 @@ package com.statsig.androidlocalevalsdk
 
 import android.util.Log
 import kotlinx.coroutines.CoroutineExceptionHandler
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.launch
 import java.io.DataOutputStream
 import java.lang.Math.floor
 import java.net.HttpURLConnection
@@ -45,6 +48,16 @@ internal class ErrorBoundary() {
         }
     }
 
+    fun getUrl(): String {
+        return urlString
+    }
+
+    fun getNoopExceptionHandler(): CoroutineExceptionHandler {
+        return CoroutineExceptionHandler { _, _ ->
+            // No-op
+        }
+    }
+
     fun <T> capture(task: () -> T, tag: String? = null, recover: (() -> T)? = null, configName: String? = null): T? {
         var markerID = ""
         return try {
@@ -79,39 +92,41 @@ internal class ErrorBoundary() {
 
     internal fun logException(exception: Throwable, message: String? = null, tag: String? = null, configName: String? = null) {
         try {
-            Log.e("STATSIG", "An unexpected exception occured: " + exception)
-            if (message != null) {
-                Log.e("STATSIG", message)
+            CoroutineScope(this.getNoopExceptionHandler() + Dispatchers.IO).launch {
+                Log.e("STATSIG", "An unexpected exception occured: " + exception)
+                if (message != null) {
+                    Log.e("STATSIG", message)
+                }
+                val name = exception.javaClass.canonicalName ?: exception.javaClass.name
+                if (seen.contains(name)) {
+                    return@launch
+                }
+                seen.add(name)
+
+                val metadata = statsigMetadata ?: StatsigMetadata("")
+                val body = mutableMapOf(
+                    "exception" to name,
+                    "info" to RuntimeException(exception).stackTraceToString(),
+                    "statsigMetadata" to metadata,
+                )
+                if (tag != null) {
+                    body["tag"] = tag
+                }
+                if (configName != null) {
+                    body["configName"] = configName
+                }
+                val postData = StatsigUtils.getGson().toJson(body)
+
+                val conn = URL(getUrl()).openConnection() as HttpURLConnection
+                conn.requestMethod = "POST"
+                conn.doOutput = true
+                conn.setRequestProperty("Content-Type", "application/json")
+                conn.setRequestProperty("STATSIG-API-KEY", apiKey)
+                conn.useCaches = false
+
+                DataOutputStream(conn.outputStream).use { it.writeBytes(postData) }
+                conn.responseCode // triggers request
             }
-            val name = exception.javaClass.canonicalName ?: exception.javaClass.name
-            if (seen.contains(name)) {
-                return
-            }
-            seen.add(name)
-
-            val metadata = statsigMetadata ?: StatsigMetadata("")
-            val body = mutableMapOf(
-                "exception" to name,
-                "info" to RuntimeException(exception).stackTraceToString(),
-                "statsigMetadata" to metadata,
-            )
-            if (tag != null) {
-                body["tag"] = tag
-            }
-            if (configName != null) {
-                body["configName"] = configName
-            }
-            val postData = StatsigUtils.getGson().toJson(body)
-
-            val conn = URL(urlString).openConnection() as HttpURLConnection
-            conn.requestMethod = "POST"
-            conn.doOutput = true
-            conn.setRequestProperty("Content-Type", "application/json")
-            conn.setRequestProperty("STATSIG-API-KEY", apiKey)
-            conn.useCaches = false
-
-            DataOutputStream(conn.outputStream).use { it.writeBytes(postData) }
-            conn.responseCode // triggers request
         } catch (e: Exception) {
         }
     }

src/main/java/com/statsig/androidlocalevalsdk/Evaluator.kt

@@ -10,7 +10,7 @@ internal data class PersistedEvaluationArgs(
     val user: StatsigUser,
     val unhashedName: String,
     val config: APIConfig,
-    val persistedValues: PersistedValues?
+    val persistedValues: PersistedValues?,
 )
 
 internal class Evaluator(private val specStore: Store, private val errorBoundary: ErrorBoundary, private val persistentStorage: StatsigUserPersistenStorageHelper?) {
@@ -41,7 +41,7 @@ internal class Evaluator(private val specStore: Store, private val errorBoundary
             return evaluateConfig(user, config)
         }
         return evaluateConfigWithPersistedValues(
-            PersistedEvaluationArgs(user, configName, config, persistedValues)
+            PersistedEvaluationArgs(user, configName, config, persistedValues),
         )
     }
 
@@ -57,7 +57,7 @@ internal class Evaluator(private val specStore: Store, private val errorBoundary
             return evaluateConfig(user, layer)
         }
         return evaluateLayerWithPersistedValues(
-            PersistedEvaluationArgs(user, layerName, layer, persistedValues)
+            PersistedEvaluationArgs(user, layerName, layer, persistedValues),
         )
     }
 
@@ -228,7 +228,7 @@ internal class Evaluator(private val specStore: Store, private val errorBoundary
             configDelegate = configDelegate,
             explicitParameters = config.explicitParameters,
             evaluationDetails = this.createEvaluationDetails(this.specStore.initReason),
-            isExperimentGroup = delegatedResult.isExperimentGroup
+            isExperimentGroup = delegatedResult.isExperimentGroup,
         )
         evaluation.undelegatedSecondaryExposures = undelegatedSecondaryExposures
         return evaluation

src/main/java/com/statsig/androidlocalevalsdk/Layer.kt

@@ -14,7 +14,7 @@ class Layer internal constructor(
     val allocatedExperiment: String = "",
     var evaluationDetails: EvaluationDetails? = null,
     private val onExposure: OnLayerExposureInternal? = null,
-    ) {
+) {
 
     companion object {
         fun empty(name: String): Layer {

src/main/java/com/statsig/androidlocalevalsdk/StatsigClient.kt

@@ -7,7 +7,6 @@ import android.content.pm.PackageInfo
 import android.content.pm.PackageManager
 import android.util.Log
 import androidx.annotation.VisibleForTesting
-import com.statsig.androidsdk.StatsigNetworkConnectivityListener
 import kotlinx.coroutines.CoroutineExceptionHandler
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
@@ -155,7 +154,7 @@ class StatsigClient {
     fun getExperiment(user: StatsigUser, experimentName: String, option: GetExperimentOptions? = null): DynamicConfig {
         var result = DynamicConfig.empty()
         if (!isInitialized("getExperiment")) {
-            result.evaluationDetails = EvaluationDetails(0,EvaluationReason.UNINITIALIZED)
+            result.evaluationDetails = EvaluationDetails(0, EvaluationReason.UNINITIALIZED)
             return result
         }
         errorBoundary.capture({
@@ -195,7 +194,7 @@ class StatsigClient {
     fun getConfig(user: StatsigUser, dynamicConfigName: String, option: GetConfigOptions? = null): DynamicConfig {
         var result = DynamicConfig.empty()
         if (!isInitialized("getExperiment")) {
-            result.evaluationDetails = EvaluationDetails(0,EvaluationReason.UNINITIALIZED)
+            result.evaluationDetails = EvaluationDetails(0, EvaluationReason.UNINITIALIZED)
             return result
         }
         errorBoundary.capture({
@@ -234,7 +233,7 @@ class StatsigClient {
     fun getLayer(user: StatsigUser, layerName: String, option: GetLayerOptions? = null): Layer {
         var result = Layer.empty(layerName)
         if (!isInitialized("getExperiment")) {
-            result.evaluationDetails = EvaluationDetails(0,EvaluationReason.UNINITIALIZED)
+            result.evaluationDetails = EvaluationDetails(0, EvaluationReason.UNINITIALIZED)
             return result
         }
         errorBoundary.capture({
@@ -365,7 +364,7 @@ class StatsigClient {
         diagnostics = Diagnostics(options.disableDiagnosticsLogging)
         if (!this::statsigNetwork.isInitialized) {
             // For testing purpose, prevent mocked network be overwritten
-            statsigNetwork = StatsigNetwork(application,sdkKey, options, sharedPrefs, diagnostics)
+            statsigNetwork = StatsigNetwork(application, sdkKey, options, sharedPrefs, diagnostics)
         }
         statsigMetadata = StatsigMetadata()
         populateStatsigMetadata()

src/main/java/com/statsig/androidlocalevalsdk/StatsigNetwork.kt

@@ -200,10 +200,9 @@ internal class StatsigNetwork(
                             statusCode = code,
                             error = errorMarker,
                             attempt = retryAttempt,
-                            hasNetwork = connectivityListener.isNetworkAvailable()
+                            hasNetwork = connectivityListener.isNetworkAvailable(),
                         ),
                     )
-
                     when (code) {
                         in 200..299 -> {
                             if (code == 204) {
@@ -237,7 +236,7 @@ internal class StatsigNetwork(
                     Marker(
                         error = Marker.ErrorMessage(message = e.message, name = e.javaClass.name),
                         attempt = retryAttempt,
-                        hasNetwork = connectivityListener.isNetworkAvailable()
+                        hasNetwork = connectivityListener.isNetworkAvailable(),
                     ),
                 )
                 // Leave to ErrorBoundary to handle

src/main/java/com/statsig/androidlocalevalsdk/StatsigNetworkConnectivityListener.kt

@@ -20,4 +20,4 @@ class StatsigNetworkConnectivityListener(
 
         return connectivityManager.activeNetworkInfo?.isConnectedOrConnecting == true
     }
-}
+}

src/test/java/com/statsig/androidlocalevalsdk/DiagnosticsTest.kt

@@ -71,7 +71,6 @@ class DiagnosticsTest {
         }
         client.shutdown()
         val apiMarkers = parseMarkers(ContextType.API_CALL)
-        println(apiMarkers.size)
         assert(apiMarkers.size == MAX_MARKERS)
     }
 

src/test/java/com/statsig/androidlocalevalsdk/StatsigInitializationTimeoutTest.kt

@@ -0,0 +1,99 @@
+package com.statsig.androidlocalevalsdk
+
+import android.app.Application
+import io.mockk.coEvery
+import io.mockk.every
+import io.mockk.mockk
+import io.mockk.mockkConstructor
+import io.mockk.spyk
+import kotlinx.coroutines.*
+import okhttp3.mockwebserver.Dispatcher
+import okhttp3.mockwebserver.MockResponse
+import okhttp3.mockwebserver.MockWebServer
+import okhttp3.mockwebserver.RecordedRequest
+import org.junit.After
+import org.junit.Assert.assertTrue
+import org.junit.Before
+import org.junit.Test
+import java.util.concurrent.CountDownLatch
+import java.util.concurrent.TimeUnit
+
+class StatsigInitializationTimeoutTest {
+
+    private var app: Application = mockk()
+    private lateinit var client: StatsigClient
+    private lateinit var network: StatsigNetwork
+    private lateinit var errorBoundary: ErrorBoundary
+    private lateinit var mockWebServer: MockWebServer
+    private val hitErrorBoundary = CountDownLatch(1)
+
+    @Before
+    fun setup() {
+        mockWebServer = MockWebServer()
+        val dispatcher = object : Dispatcher() {
+            override fun dispatch(request: RecordedRequest): MockResponse {
+                return if (request.path!!.contains("sdk_exception")) {
+                    hitErrorBoundary.countDown()
+                    runBlocking {
+                        delay(1000)
+                    }
+                    MockResponse()
+                        .setBody("{\"result\":\"error logged\"}")
+                        .setResponseCode(200)
+                } else {
+                    MockResponse().setResponseCode(404)
+                }
+            }
+        }
+        mockWebServer.dispatcher = dispatcher
+        mockWebServer.start()
+        client = spyk(StatsigClient(), recordPrivateCalls = true)
+        client.errorBoundary = spyk(client.errorBoundary)
+        errorBoundary = client.errorBoundary
+        network = TestUtil.mockNetwork()
+
+        TestUtil.mockDispatchers()
+        TestUtil.stubAppFunctions(app)
+
+        // Lets get a successful network response, and then trigger error boundary
+        // so we can test that eb does not block the initialization beyond the init timeout
+        mockkConstructor(Store::class)
+        coEvery {
+            anyConstructed<Store>().fetchAndSave()
+        } throws(Exception("trigger the error boundary"))
+
+        every {
+            errorBoundary.getUrl()
+        } returns mockWebServer.url("/v1/sdk_exception").toString()
+
+        client.statsigNetwork = network
+        client.errorBoundary = errorBoundary
+    }
+
+    @After
+    fun tearDown() {
+        mockWebServer.shutdown()
+    }
+
+    @Test
+    fun testInitializeAsyncWithSlowErrorBoundary() = runBlocking {
+        var initializationDetails: InitializationDetails? = null
+        var initTimeout = 500
+        runBlocking {
+            initializationDetails = client.initialize(app, "client-key", StatsigOptions(initTimeoutMs = initTimeout))
+        }
+        // initialize timeout was hit, we got a value back and we are considered initialized
+        assert(initializationDetails != null)
+        assert(client.isInitialized())
+
+        // error boundary was hit, but has not completed at this point, so the initialization timeout worked
+        assertTrue(hitErrorBoundary.await(1, TimeUnit.SECONDS))
+        assertTrue(
+            "initialization time ${initializationDetails!!.duration} not less than initTimeout $initTimeout",
+            initializationDetails!!.duration < initTimeout + 100L,
+        )
+
+        // error boundary was hit, but has not completed at this point, so the initialization timeout worked
+        assert(hitErrorBoundary.await(1, TimeUnit.SECONDS))
+    }
+}