Catch NPEs and RuntimeExceptions thrown in getActiveNetworkInfo.

The framework seems to throw these on various devices and versions of Android, though mostly on Samsung between APIs 22 and 24. The deleted test was passing because of an exception in isConnected, not in registerReceiver so it was incorrect. There doesn’t seem to be a way to force Robolectric to throw when registering receivers.
bumptech · Dec 20, 2017 · c902730 · c902730
1 parent 2b4be8b
commit c902730
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 24 deletions.
diff --git a/library/src/main/java/com/bumptech/glide/manager/DefaultConnectivityMonitor.java b/library/src/main/java/com/bumptech/glide/manager/DefaultConnectivityMonitor.java
@@ -26,16 +26,7 @@ final class DefaultConnectivityMonitor implements ConnectivityMonitor {
     @Override
     public void onReceive(Context context, Intent intent) {
       boolean wasConnected = isConnected;
-      try {
-        isConnected = isConnected(context);
-      } catch (SecurityException e) {
-        // See #1405.
-        if (Log.isLoggable(TAG, Log.WARN)) {
-          Log.w(TAG, "Failed to determine connectivity status when connectivity changed", e);
-        }
-        // Default to true;
-        isConnected = true;
-      }
+      isConnected = isConnected(context);
       if (wasConnected != isConnected) {
         if (Log.isLoggable(TAG, Log.DEBUG)) {
           Log.d(TAG, "connectivity changed, isConnected: " + isConnected);
@@ -56,14 +47,15 @@ private void register() {
       return;
     }
 
+    // Initialize isConnected.
+    isConnected = isConnected(context);
     try {
       // See #1405
-      isConnected = isConnected(context);
       context.registerReceiver(connectivityReceiver,
           new IntentFilter(ConnectivityManager.CONNECTIVITY_ACTION));
       isRegistered = true;
     } catch (SecurityException e) {
-      // See #1417.
+      // See #1417, registering the receiver can throw SecurityException.
       if (Log.isLoggable(TAG, Log.WARN)) {
         Log.w(TAG, "Failed to register", e);
       }
@@ -85,9 +77,21 @@ private void unregister() {
   @SuppressLint("MissingPermission")
   boolean isConnected(Context context) {
     ConnectivityManager connectivityManager =
-        (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
-    NetworkInfo networkInfo =
-        Preconditions.checkNotNull(connectivityManager).getActiveNetworkInfo();
+        Preconditions.checkNotNull(
+            (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE));
+    NetworkInfo networkInfo;
+    try {
+      networkInfo = connectivityManager.getActiveNetworkInfo();
+    } catch (RuntimeException e) {
+      // #1405 shows that this throws a SecurityException.
+      // b/70869360 shows that this throws NullPointerException on APIs 22, 23, and 24.
+      // b/70869360 also shows that this throws RuntimeException on API 24 and 25.
+      if (Log.isLoggable(TAG, Log.WARN)) {
+        Log.w(TAG, "Failed to determine connectivity status when connectivity changed", e);
+      }
+      // Default to true;
+      return true;
+    }
     return networkInfo != null && networkInfo.isConnected();
   }
 

diff --git a/library/src/test/java/com/bumptech/glide/manager/DefaultConnectivityMonitorTest.java b/library/src/test/java/com/bumptech/glide/manager/DefaultConnectivityMonitorTest.java
@@ -125,15 +125,6 @@ public void register_withMissingPermission_doesNotThrow() {
     monitor.onStart();
   }
 
-  @Test
-  public void register_withMissingPermission_doesNotRegisterReceiver() {
-    harness.shadowConnectivityManager.isNetworkPermissionGranted = false;
-
-    monitor.onStart();
-
-    assertThat(getConnectivityReceivers()).isEmpty();
-  }
-
   @Test
   public void onReceive_withMissingPermission_doesNotThrow() {
     monitor.onStart();