🐛 (RUMM-918): Prevent crash in registerNetworkCallback

DataDog · Dec 7, 2020 · 58c1adf · 58c1adf
1 parent c13a0e8
commit 58c1adf
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 2 deletions.
diff --git a/...src/main/kotlin/com/datadog/android/core/internal/net/info/CallbackNetworkInfoProvider.kt b/...src/main/kotlin/com/datadog/android/core/internal/net/info/CallbackNetworkInfoProvider.kt
@@ -49,6 +49,7 @@ internal class CallbackNetworkInfoProvider :
 
     //region NetworkInfoProvider
 
+    @Suppress("TooGenericExceptionCaught")
     override fun register(context: Context) {
         val connMgr = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
         try {
@@ -60,19 +61,30 @@ internal class CallbackNetworkInfoProvider :
             }
         } catch (e: SecurityException) {
             // RUMM-852 On some devices we get a SecurityException with message
-            // "package does not belong to 10411"
+            // "package does not belong to xxxx"
+            devLogger.e(ERROR_REGISTER, e)
+            networkInfo = NetworkInfo(NetworkInfo.Connectivity.NETWORK_OTHER)
+        } catch (e: RuntimeException) {
+            // RUMM-918 in some cases the device throws a IllegalArgumentException on register
+            // "Too many NetworkRequests filed" This happens when registerDefaultNetworkCallback is
+            // called too many times without matching unregisterNetworkCallback
             devLogger.e(ERROR_REGISTER, e)
             networkInfo = NetworkInfo(NetworkInfo.Connectivity.NETWORK_OTHER)
         }
     }
 
+    @Suppress("TooGenericExceptionCaught")
     override fun unregister(context: Context) {
         val connMgr = context.getSystemService(Context.CONNECTIVITY_SERVICE) as ConnectivityManager
         try {
             connMgr.unregisterNetworkCallback(this)
         } catch (e: SecurityException) {
             // RUMM-852 On some devices we get a SecurityException with message
-            // "package does not belong to 10411"
+            // "package does not belong to xxxx"
+            devLogger.e(ERROR_UNREGISTER, e)
+        } catch (e: RuntimeException) {
+            // RUMM-918 in some cases the device throws a IllegalArgumentException on unregister
+            // e.g. when the callback was not registered
             devLogger.e(ERROR_UNREGISTER, e)
         }
     }

diff --git a/...test/kotlin/com/datadog/android/core/internal/net/info/CallbackNetworkInfoProviderTest.kt b/...test/kotlin/com/datadog/android/core/internal/net/info/CallbackNetworkInfoProviderTest.kt
@@ -350,6 +350,29 @@ internal class CallbackNetworkInfoProviderTest {
             )
     }
 
+    @Test
+    fun `M warn developers W register() with RuntimeException`(
+        @StringForgery message: String
+    ) {
+        // RUMM-918 in some cases the device throws a IllegalArgumentException on register
+        // "Too many NetworkRequests filed" This happens when registerDefaultNetworkCallback is
+        // called too many times without matching unregisterNetworkCallback
+        val context = mock<Context>()
+        val manager = mock<ConnectivityManager>()
+        val exception = RuntimeException(message)
+        whenever(context.getSystemService(Context.CONNECTIVITY_SERVICE)) doReturn manager
+        whenever(manager.registerDefaultNetworkCallback(testedProvider)) doThrow exception
+
+        testedProvider.register(context)
+
+        verify(mockDevLogHandler)
+            .handleLog(
+                Log.ERROR,
+                CallbackNetworkInfoProvider.ERROR_REGISTER,
+                exception
+            )
+    }
+
     @Test
     fun `M assume network is available W register() with SecurityException + getLatestNetworkInfo`(
         @StringForgery message: String
@@ -372,6 +395,30 @@ internal class CallbackNetworkInfoProviderTest {
             .hasDownSpeed(-1)
     }
 
+    @Test
+    fun `M assume network is available W register() with RuntimeException + getLatestNetworkInfo`(
+        @StringForgery message: String
+    ) {
+        // RUMM-918 in some cases the device throws a IllegalArgumentException on register
+        // "Too many NetworkRequests filed" This happens when registerDefaultNetworkCallback is
+        // called too many times without matching unregisterNetworkCallback
+        val context = mock<Context>()
+        val manager = mock<ConnectivityManager>()
+        val exception = RuntimeException(message)
+        whenever(context.getSystemService(Context.CONNECTIVITY_SERVICE)) doReturn manager
+        whenever(manager.registerDefaultNetworkCallback(testedProvider)) doThrow exception
+
+        testedProvider.register(context)
+        val networkInfo = testedProvider.getLatestNetworkInfo()
+
+        assertThat(networkInfo)
+            .hasConnectivity(NetworkInfo.Connectivity.NETWORK_OTHER)
+            .hasCarrierName(null)
+            .hasCarrierId(-1)
+            .hasUpSpeed(-1)
+            .hasDownSpeed(-1)
+    }
+
     @Test
     fun `M unregister callback W unregister()`() {
         val context = mock<Context>()
@@ -420,4 +467,26 @@ internal class CallbackNetworkInfoProviderTest {
                 exception
             )
     }
+
+    @Test
+    fun `M warn developers W unregister() with RuntimeException`(
+        @StringForgery message: String
+    ) {
+        // RUMM-918 in some cases the device throws a IllegalArgumentException on unregister
+        // e.g. when the callback was not registered
+        val context = mock<Context>()
+        val manager = mock<ConnectivityManager>()
+        val exception = RuntimeException(message)
+        whenever(context.getSystemService(Context.CONNECTIVITY_SERVICE)) doReturn manager
+        whenever(manager.unregisterNetworkCallback(testedProvider)) doThrow exception
+
+        testedProvider.unregister(context)
+
+        verify(mockDevLogHandler)
+            .handleLog(
+                Log.ERROR,
+                CallbackNetworkInfoProvider.ERROR_UNREGISTER,
+                exception
+            )
+    }
 }