fix: ConnectivityChecker should return false if offline and no permission

sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java

@@ -71,7 +71,8 @@ static void init(
 
     readDefaultOptionValues(options, context);
 
-    options.addEventProcessor(new DefaultAndroidEventProcessor(context, options));
+    options.addEventProcessor(
+        new DefaultAndroidEventProcessor(context, options, new BuildInfoProvider()));
 
     options.setSerializer(new AndroidSerializer(options.getLogger(), envelopeReader));
 

sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransportGate.java

@@ -12,20 +12,27 @@ final class AndroidTransportGate implements ITransportGate {
   private final Context context;
   private final @NotNull ILogger logger;
 
-  AndroidTransportGate(@NotNull Context context, @NotNull ILogger logger) {
+  AndroidTransportGate(final @NotNull Context context, final @NotNull ILogger logger) {
     this.context = context;
     this.logger = logger;
   }
 
   @Override
   public boolean isConnected() {
-    return isConnected(ConnectivityChecker.isConnected(context, logger));
+    return isConnected(ConnectivityChecker.getConnectionStatus(context, logger));
   }
 
   @TestOnly
-  boolean isConnected(Boolean connected) {
+  boolean isConnected(final @NotNull ConnectivityChecker.Status status) {
     // let's assume its connected if there's no permission or something as we can't really know
     // whether is online or not.
-    return connected != null ? connected : true;
+    switch (status) {
+      case CONNECTED:
+      case UNKNOWN:
+      case NO_PERMISSION:
+        return true;
+      default:
+        return false;
+    }
   }
 }

sentry-android-core/src/main/java/io/sentry/android/core/BuildInfoProvider.java

@@ -0,0 +1,19 @@
+package io.sentry.android.core;
+
+import android.os.Build;
+import org.jetbrains.annotations.ApiStatus;
+
+/** The Android Impl. of IBuildInfoProvider which returns the Build class info. */
+@ApiStatus.Internal
+public final class BuildInfoProvider implements IBuildInfoProvider {
+
+  /**
+   * Returns the Build.VERSION.SDK_INT
+   *
+   * @return the Build.VERSION.SDK_INT
+   */
+  @Override
+  public int getSdkInfoVersion() {
+    return Build.VERSION.SDK_INT;
+  }
+}

sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java

@@ -75,10 +75,16 @@ final class DefaultAndroidEventProcessor implements EventProcessor {
 
   @TestOnly final Future<Map<String, Object>> contextData;
 
+  private final @NotNull IBuildInfoProvider buildInfoProvider;
+
   public DefaultAndroidEventProcessor(
-      final @NotNull Context context, final @NotNull SentryOptions options) {
+      final @NotNull Context context,
+      final @NotNull SentryOptions options,
+      final @NotNull IBuildInfoProvider buildInfoProvider) {
     this.context = Objects.requireNonNull(context, "The application context is required.");
     this.options = Objects.requireNonNull(options, "The SentryOptions is required.");
+    this.buildInfoProvider =
+        Objects.requireNonNull(buildInfoProvider, "The BuildInfoProvider is required.");
 
     ExecutorService executorService = Executors.newSingleThreadExecutor();
     // dont ref. to method reference, theres a bug on it
@@ -287,7 +293,18 @@ private void setArchitectures(final @NotNull Device device) {
       device.setCharging(isCharging(batteryIntent));
       device.setBatteryTemperature(getBatteryTemperature(batteryIntent));
     }
-    device.setOnline(ConnectivityChecker.isConnected(context, options.getLogger()));
+    Boolean connected;
+    switch (ConnectivityChecker.getConnectionStatus(context, options.getLogger())) {
+      case NOT_CONNECTED:
+        connected = false;
+        break;
+      case CONNECTED:
+        connected = true;
+        break;
+      default:
+        connected = null;
+    }
+    device.setOnline(connected);
     device.setOrientation(getOrientation());
 
     try {
@@ -344,7 +361,8 @@ private void setArchitectures(final @NotNull Device device) {
     }
     if (device.getConnectionType() == null) {
       // wifi, ethernet or cellular, null if none
-      device.setConnectionType(ConnectivityChecker.getConnectionType(context, options.getLogger()));
+      device.setConnectionType(
+          ConnectivityChecker.getConnectionType(context, options.getLogger(), buildInfoProvider));
     }
 
     return device;

sentry-android-core/src/main/java/io/sentry/android/core/IBuildInfoProvider.java

@@ -0,0 +1,12 @@
+package io.sentry.android.core;
+
+/** To make SDK info classes testable */
+public interface IBuildInfoProvider {
+
+  /**
+   * Returns the SDK version of the given SDK
+   *
+   * @return the SDK Version
+   */
+  int getSdkInfoVersion();
+}

sentry-android-core/src/main/java/io/sentry/android/core/util/ConnectivityChecker.java

@@ -7,6 +7,7 @@
 import android.net.Network;
 import android.net.NetworkCapabilities;
 import android.os.Build;
+import io.sentry.android.core.IBuildInfoProvider;
 import io.sentry.core.ILogger;
 import io.sentry.core.SentryLevel;
 import org.jetbrains.annotations.ApiStatus;
@@ -16,98 +17,163 @@
 @ApiStatus.Internal
 public final class ConnectivityChecker {
 
+  public enum Status {
+    CONNECTED,
+    NOT_CONNECTED,
+    NO_PERMISSION,
+    UNKNOWN
+  }
+
   private ConnectivityChecker() {}
 
   /**
-   * Check whether the application has internet access at a point in time.
+   * Return the Connection status
    *
-   * @return true if the application has internet access
+   * @return the ConnectionStatus
    */
-  public static @Nullable Boolean isConnected(@NotNull Context context, @NotNull ILogger logger) {
-    ConnectivityManager connectivityManager = getConnectivityManager(context, logger);
+  public static @NotNull ConnectivityChecker.Status getConnectionStatus(
+      final @NotNull Context context, final @NotNull ILogger logger) {
+    final ConnectivityManager connectivityManager = getConnectivityManager(context, logger);
     if (connectivityManager == null) {
-      return null;
+      return Status.UNKNOWN;
     }
-    return isConnected(context, connectivityManager, logger);
+    return getConnectionStatus(context, connectivityManager, logger);
     // getActiveNetworkInfo might return null if VPN doesn't specify its
     // underlying network
 
     // when min. API 24, use:
     // connectivityManager.registerDefaultNetworkCallback(...)
   }
 
+  /**
+   * Return the Connection status
+   *
+   * @param context the Context
+   * @param connectivityManager the ConnectivityManager
+   * @param logger the Logger
+   * @return true if connected or no permission to check, false otherwise
+   */
   @SuppressWarnings({"deprecation", "MissingPermission"})
-  private static Boolean isConnected(
-      Context context, ConnectivityManager connectivityManager, ILogger logger) {
-    android.net.NetworkInfo activeNetwork =
-        getActiveNetworkInfo(context, connectivityManager, logger);
-
-    if (activeNetwork == null) {
-      logger.log(SentryLevel.INFO, "NetworkInfo is null and cannot check network status");
-      return null;
-    }
-    return activeNetwork.isConnected();
-  }
-
-  @SuppressWarnings({"deprecation", "MissingPermission"})
-  private static @Nullable android.net.NetworkInfo getActiveNetworkInfo(
-      @NotNull Context context,
-      @NotNull ConnectivityManager connectivityManager,
-      @NotNull ILogger logger) {
+  private static @NotNull ConnectivityChecker.Status getConnectionStatus(
+      final @NotNull Context context,
+      final @NotNull ConnectivityManager connectivityManager,
+      final @NotNull ILogger logger) {
     if (!Permissions.hasPermission(context, Manifest.permission.ACCESS_NETWORK_STATE)) {
       logger.log(SentryLevel.INFO, "No permission (ACCESS_NETWORK_STATE) to check network status.");
-      return null;
+      return Status.NO_PERMISSION;
     }
+    final android.net.NetworkInfo activeNetworkInfo = connectivityManager.getActiveNetworkInfo();
 
-    return connectivityManager.getActiveNetworkInfo();
+    if (activeNetworkInfo == null) {
+      logger.log(SentryLevel.INFO, "NetworkInfo is null, there's no active network.");
+      return Status.NOT_CONNECTED;
+    }
+    return activeNetworkInfo.isConnected() ? Status.CONNECTED : Status.NOT_CONNECTED;
   }
 
   /**
    * Check the connection type of the active network
    *
    * @param context the App. context
    * @param logger the logger from options
+   * @param buildInfoProvider the BuildInfoProvider provider
    * @return the connection type wifi, ethernet, cellular or null
    */
-  @SuppressLint({"ObsoleteSdkInt", "MissingPermission"})
-  public static String getConnectionType(@NotNull Context context, @NotNull ILogger logger) {
-    if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) {
-      ConnectivityManager connectivityManager = getConnectivityManager(context, logger);
-      if (connectivityManager == null) {
-        return null;
-      }
-      if (!Permissions.hasPermission(context, Manifest.permission.ACCESS_NETWORK_STATE)) {
-        logger.log(
-            SentryLevel.INFO, "No permission (ACCESS_NETWORK_STATE) to check network status.");
-        return null;
-      }
-      Network activeNetwork = connectivityManager.getActiveNetwork();
+  @SuppressLint({"ObsoleteSdkInt", "MissingPermission", "NewApi"})
+  public static @Nullable String getConnectionType(
+      final @NotNull Context context,
+      final @NotNull ILogger logger,
+      final @NotNull IBuildInfoProvider buildInfoProvider) {
+    final ConnectivityManager connectivityManager = getConnectivityManager(context, logger);
+    if (connectivityManager == null) {
+      return null;
+    }
+    if (!Permissions.hasPermission(context, Manifest.permission.ACCESS_NETWORK_STATE)) {
+      logger.log(SentryLevel.INFO, "No permission (ACCESS_NETWORK_STATE) to check network status.");
+      return null;
+    }
+
+    boolean ethernet = false;
+    boolean wifi = false;
+    boolean cellular = false;
+
+    if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.M) {
+      final Network activeNetwork = connectivityManager.getActiveNetwork();
       if (activeNetwork == null) {
         logger.log(SentryLevel.INFO, "Network is null and cannot check network status");
         return null;
       }
-      NetworkCapabilities networkCapabilities =
+      final NetworkCapabilities networkCapabilities =
           connectivityManager.getNetworkCapabilities(activeNetwork);
       if (networkCapabilities == null) {
         logger.log(SentryLevel.INFO, "NetworkCapabilities is null and cannot check network type");
         return null;
       }
-      if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) {
-        return "wifi";
-      }
       if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET)) {
-        return "ethernet";
+        ethernet = true;
+      }
+      if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI)) {
+        wifi = true;
       }
       if (networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR)) {
-        return "cellular";
+        cellular = true;
+      }
+    } else {
+      // ideally using connectivityManager.getAllNetworks(), but its >= Android L only
+
+      // for some reason linting didn't allow a single @SuppressWarnings("deprecation") on method
+      // signature
+      @SuppressWarnings("deprecation")
+      final android.net.NetworkInfo activeNetworkInfo = connectivityManager.getActiveNetworkInfo();
+
+      if (activeNetworkInfo == null) {
+        logger.log(SentryLevel.INFO, "NetworkInfo is null, there's no active network.");
+        return null;
+      }
+
+      @SuppressWarnings("deprecation")
+      final int type = activeNetworkInfo.getType();
+
+      @SuppressWarnings("deprecation")
+      final int TYPE_ETHERNET = ConnectivityManager.TYPE_ETHERNET;
+
+      @SuppressWarnings("deprecation")
+      final int TYPE_WIFI = ConnectivityManager.TYPE_WIFI;
+
+      @SuppressWarnings("deprecation")
+      final int TYPE_MOBILE = ConnectivityManager.TYPE_MOBILE;
+
+      switch (type) {
+        case TYPE_ETHERNET:
+          ethernet = true;
+          break;
+        case TYPE_WIFI:
+          wifi = true;
+          break;
+        case TYPE_MOBILE:
+          cellular = true;
+          break;
       }
     }
+
+    // TODO: change the protocol to be a list of transports as a device may have the capability of
+    // multiple
+    if (ethernet) {
+      return "ethernet";
+    }
+    if (wifi) {
+      return "wifi";
+    }
+    if (cellular) {
+      return "cellular";
+    }
+
     return null;
   }
 
   private static @Nullable ConnectivityManager getConnectivityManager(
-      @NotNull Context context, @NotNull ILogger logger) {
-    ConnectivityManager connectivityManager =
+      final @NotNull Context context, final @NotNull ILogger logger) {
+    final ConnectivityManager connectivityManager =
         (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
     if (connectivityManager == null) {
       logger.log(SentryLevel.INFO, "ConnectivityManager is null and cannot check network status");