do not log exceptions from user-defined callbacks

statsig-io · Sep 13, 2023 · 60fb9e3 · 60fb9e3
1 parent e91bab3
commit 60fb9e3
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 11 deletions.
diff --git a/src/main/java/com/statsig/androidsdk/ErrorBoundary.kt b/src/main/java/com/statsig/androidsdk/ErrorBoundary.kt
@@ -12,6 +12,9 @@ import kotlin.math.floor
 
 const val MAX_DIAGNOSTICS_MARKERS = 30
 const val SAMPLING_RATE = 10_000
+
+internal class ExternalException(message: String? = null) : Exception(message)
+
 internal class ErrorBoundary() {
     internal var urlString = "https://statsigapi.net/v1/sdk_exception"
 
@@ -47,7 +50,9 @@ internal class ErrorBoundary() {
     private fun handleException(exception: Throwable) {
         println("[Statsig]: An unexpected exception occurred.")
         println(exception)
-        this.logException(exception)
+        if (exception !is ExternalException) {
+            this.logException(exception)
+        }
     }
 
     fun getExceptionHandler(): CoroutineExceptionHandler {
@@ -63,8 +68,8 @@ internal class ErrorBoundary() {
             task()
             endMarker(tag, markerID, true, configName)
         } catch (e: Exception) {
-            handleException(e)
             endMarker(tag, markerID, false, configName)
+            handleException(e)
             recover?.let { it() }
         }
     }

diff --git a/src/main/java/com/statsig/androidsdk/StatsigClient.kt b/src/main/java/com/statsig/androidsdk/StatsigClient.kt
@@ -58,9 +58,14 @@ internal class StatsigClient() {
         val normalizedUser = setup(application, sdkKey, user, options)
         statsigScope.launch {
             val initDetails = setupAsync(normalizedUser)
-            // The scope's dispatcher may change in the future. This "withContext" will ensure we keep true to the documentation above.
+            // The scope's dispatcher may change in the future.
+            // This "withContext" will ensure that initialization is complete when the callback is invoked
             withContext(dispatcherProvider.main) {
-                callback?.onStatsigInitialize(initDetails)
+                try {
+                    callback?.onStatsigInitialize(initDetails)
+                } catch (e: Exception) {
+                    throw ExternalException(e.message)
+                }
             }
         }
     }
@@ -87,7 +92,8 @@ internal class StatsigClient() {
         return setupAsync(normalizedUser)
     }
 
-    private suspend fun setupAsync(user: StatsigUser): InitializationDetails {
+    @VisibleForTesting
+    internal suspend fun setupAsync(user: StatsigUser): InitializationDetails {
         return withContext(dispatcherProvider.io) {
             val initStartTime = System.currentTimeMillis()
             return@withContext Statsig.errorBoundary.captureAsync({
@@ -380,7 +386,11 @@ internal class StatsigClient() {
         statsigScope.launch {
             updateUserImpl()
             withContext(dispatcherProvider.main) {
-                callback?.onStatsigUpdateUser()
+                try {
+                    callback?.onStatsigUpdateUser()
+                } catch (e: Exception) {
+                    throw ExternalException(e.message)
+                }
             }
         }
     }

diff --git a/src/test/java/com/statsig/androidsdk/ErrorBoundaryTest.kt b/src/test/java/com/statsig/androidsdk/ErrorBoundaryTest.kt
@@ -150,26 +150,54 @@ class ErrorBoundaryTest {
 
     @Test
     fun testCoroutineExceptionHandler() {
+        // Expect exceptions thrown from coroutines without explicit capture statements
+        // are still caught by the ErrorBoundary via the CoroutineExceptionHandler
+        try {
+            runBlocking {
+                coEvery {
+                    Statsig.client.setupAsync(any())
+                } answers {
+                    throw IOException("Example exception in StatsigClient setupAsync")
+                }
+                Statsig.client.initializeAsync(app, "client-key", null)
+                Statsig.shutdown()
+            }
+        } catch (e: Throwable) {
+            assertTrue(false) // should not throw
+        }
+        verify(
+            1,
+            postRequestedFor(urlEqualTo("/v1/sdk_exception")).withHeader(
+                "STATSIG-API-KEY",
+                equalTo("client-key"),
+            ),
+        )
+    }
+
+    @Test
+    fun testExternalException() {
+        // Expect exceptions thrown from user defined callbacks to be caught
+        // by the ErrorBoundary but not logged
         val callback = object : IStatsigCallback {
             override fun onStatsigInitialize() {
-                // Generally, we don't expect exceptions from user defined callbacks to be caught
-                // in our error boundary, but until we implement a way to filter them out
-                // this test will use it as a way to test capturing via CoroutineExceptionHandler
                 throw IOException("Thrown from onStatsigInitialize")
             }
 
-            override fun onStatsigUpdateUser() {}
+            override fun onStatsigUpdateUser() {
+                throw IOException("Thrown from onStatsigUpdateUser")
+            }
         }
         try {
             runBlocking {
                 Statsig.client.initializeAsync(app, "client-key", null, callback)
+                Statsig.client.updateUserAsync(null, callback)
                 Statsig.shutdown()
             }
         } catch (e: Throwable) {
             assertTrue(false) // should not throw
         }
         verify(
-            1,
+            0,
             postRequestedFor(urlEqualTo("/v1/sdk_exception")).withHeader(
                 "STATSIG-API-KEY",
                 equalTo("client-key"),