From 2a0d6da985ad283700ae95598c42e74915951254 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 16 Jul 2025 12:34:21 +0200 Subject: [PATCH 01/11] perf(connectivity): Cache network capabilities and status to reduce IPC calls --- .../core/AndroidOptionsInitializer.java | 4 +- .../util/AndroidConnectionStatusProvider.java | 443 ++++++++++++--- .../AndroidConnectionStatusProviderTest.kt | 515 ++++++++++++++---- sentry/api/sentry.api | 3 +- .../io/sentry/IConnectionStatusProvider.java | 3 +- .../sentry/NoOpConnectionStatusProvider.java | 6 + sentry/src/main/java/io/sentry/Scopes.java | 1 + .../test/java/io/sentry/SentryOptionsTest.kt | 2 + 8 files changed, 819 insertions(+), 158 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 302f4627f20..c3303cf4698 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -28,6 +28,7 @@ import io.sentry.android.core.internal.gestures.AndroidViewGestureTargetLocator; import io.sentry.android.core.internal.modules.AssetsModulesLoader; import io.sentry.android.core.internal.util.AndroidConnectionStatusProvider; +import io.sentry.android.core.internal.util.AndroidCurrentDateProvider; import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.android.core.internal.util.SentryFrameMetricsCollector; import io.sentry.android.core.performance.AppStartMetrics; @@ -157,7 +158,8 @@ static void initializeIntegrationsAndProcessors( if (options.getConnectionStatusProvider() instanceof NoOpConnectionStatusProvider) { options.setConnectionStatusProvider( - new AndroidConnectionStatusProvider(context, options.getLogger(), buildInfoProvider)); + new AndroidConnectionStatusProvider( + context, options, buildInfoProvider, AndroidCurrentDateProvider.getInstance())); } if (options.getCacheDirPath() != null) { diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java index ed8948e0a5a..3df49171d32 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java @@ -8,12 +8,15 @@ import android.net.Network; import android.net.NetworkCapabilities; import android.os.Build; +import androidx.annotation.NonNull; import io.sentry.IConnectionStatusProvider; import io.sentry.ILogger; import io.sentry.ISentryLifecycleToken; import io.sentry.SentryLevel; +import io.sentry.SentryOptions; import io.sentry.android.core.BuildInfoProvider; import io.sentry.android.core.ContextUtils; +import io.sentry.transport.ICurrentDateProvider; import io.sentry.util.AutoClosableReentrantLock; import java.util.ArrayList; import java.util.List; @@ -31,104 +34,402 @@ public final class AndroidConnectionStatusProvider implements IConnectionStatusProvider { private final @NotNull Context context; - private final @NotNull ILogger logger; + private final @NotNull SentryOptions options; private final @NotNull BuildInfoProvider buildInfoProvider; + private final @NotNull ICurrentDateProvider timeProvider; private final @NotNull List connectionStatusObservers; private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); private volatile @Nullable NetworkCallback networkCallback; + private static final @NotNull AutoClosableReentrantLock connectivityManagerLock = + new AutoClosableReentrantLock(); + private static volatile @Nullable ConnectivityManager connectivityManager; + + private static final int[] transports = { + NetworkCapabilities.TRANSPORT_WIFI, + NetworkCapabilities.TRANSPORT_CELLULAR, + NetworkCapabilities.TRANSPORT_ETHERNET + }; + + private static final int[] capabilities = new int[2]; + + private final @NotNull Thread initThread; + private volatile @Nullable NetworkCapabilities cachedNetworkCapabilities; + private volatile @Nullable Network currentNetwork; + private volatile long lastCacheUpdateTime = 0; + private static final long CACHE_TTL_MS = 2 * 60 * 1000L; // 2 minutes + + @SuppressLint("InlinedApi") public AndroidConnectionStatusProvider( @NotNull Context context, - @NotNull ILogger logger, - @NotNull BuildInfoProvider buildInfoProvider) { + @NotNull SentryOptions options, + @NotNull BuildInfoProvider buildInfoProvider, + @NotNull ICurrentDateProvider timeProvider) { this.context = ContextUtils.getApplicationContext(context); - this.logger = logger; + this.options = options; this.buildInfoProvider = buildInfoProvider; + this.timeProvider = timeProvider; this.connectionStatusObservers = new ArrayList<>(); + + capabilities[0] = NetworkCapabilities.NET_CAPABILITY_INTERNET; + if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.M) { + capabilities[1] = NetworkCapabilities.NET_CAPABILITY_VALIDATED; + } + + // Register network callback immediately for caching + //noinspection Convert2MethodRef + initThread = new Thread(() -> ensureNetworkCallbackRegistered()); + initThread.start(); } - @Override - public @NotNull ConnectionStatus getConnectionStatus() { - final ConnectivityManager connectivityManager = getConnectivityManager(context, logger); - if (connectivityManager == null) { - return ConnectionStatus.UNKNOWN; + /** + * Enhanced network connectivity check for Android 15. Checks for NET_CAPABILITY_INTERNET, + * NET_CAPABILITY_VALIDATED, and proper transport types. + * https://medium.com/@doronkakuli/adapting-your-network-connectivity-checks-for-android-15-a-practical-guide-2b1850619294 + */ + @SuppressLint("InlinedApi") + private boolean isNetworkEffectivelyConnected( + final @Nullable NetworkCapabilities networkCapabilities) { + if (networkCapabilities == null) { + return false; + } + + // Check for general internet capability AND validated status + boolean hasInternetAndValidated = + networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_INTERNET); + if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.M) { + hasInternetAndValidated = + hasInternetAndValidated + && networkCapabilities.hasCapability(NetworkCapabilities.NET_CAPABILITY_VALIDATED); } - return getConnectionStatus(context, connectivityManager, logger); - // getActiveNetworkInfo might return null if VPN doesn't specify its - // underlying network - // when min. API 24, use: - // connectivityManager.registerDefaultNetworkCallback(...) + if (!hasInternetAndValidated) { + return false; + } + + // Additionally, ensure it's a recognized transport type for general internet access + return networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_WIFI) + || networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_CELLULAR) + || networkCapabilities.hasTransport(NetworkCapabilities.TRANSPORT_ETHERNET); } - @Override - public @Nullable String getConnectionType() { - return getConnectionType(context, logger, buildInfoProvider); + /** Get connection status from cached NetworkCapabilities or fallback to legacy method. */ + private @NotNull ConnectionStatus getConnectionStatusFromCache() { + if (cachedNetworkCapabilities != null) { + return isNetworkEffectivelyConnected(cachedNetworkCapabilities) + ? ConnectionStatus.CONNECTED + : ConnectionStatus.DISCONNECTED; + } + + // Fallback to legacy method when NetworkCapabilities not available + final ConnectivityManager connectivityManager = + getConnectivityManager(context, options.getLogger()); + if (connectivityManager != null) { + return getConnectionStatus(context, connectivityManager, options.getLogger()); + } + + return ConnectionStatus.UNKNOWN; } - @Override - public boolean addConnectionStatusObserver(final @NotNull IConnectionStatusObserver observer) { - try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - connectionStatusObservers.add(observer); + /** Get connection type from cached NetworkCapabilities or fallback to legacy method. */ + private @Nullable String getConnectionTypeFromCache() { + final NetworkCapabilities capabilities = cachedNetworkCapabilities; + if (capabilities != null) { + return getConnectionType(capabilities); } - if (networkCallback == null) { - try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - if (networkCallback == null) { - final @NotNull NetworkCallback newNetworkCallback = - new NetworkCallback() { - @Override - public void onAvailable(final @NotNull Network network) { - updateObservers(); - } + // Fallback to legacy method when NetworkCapabilities not available + return getConnectionType(context, options.getLogger(), buildInfoProvider); + } - @Override - public void onUnavailable() { - updateObservers(); - } + private void ensureNetworkCallbackRegistered() { + if (networkCallback != null) { + return; // Already registered + } - @Override - public void onLost(final @NotNull Network network) { - updateObservers(); - } + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + if (networkCallback != null) { + return; + } - public void updateObservers() { - final @NotNull ConnectionStatus status = getConnectionStatus(); - try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - for (final @NotNull IConnectionStatusObserver observer : - connectionStatusObservers) { - observer.onConnectionStatusChanged(status); - } + final @NotNull NetworkCallback callback = + new NetworkCallback() { + @Override + public void onAvailable(final @NotNull Network network) { + currentNetwork = network; + } + + @Override + public void onUnavailable() { + clearCacheAndNotifyObservers(); + } + + @Override + public void onLost(final @NotNull Network network) { + if (!network.equals(currentNetwork)) { + return; + } + clearCacheAndNotifyObservers(); + } + + private void clearCacheAndNotifyObservers() { + // Clear cached capabilities and network reference atomically + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + cachedNetworkCapabilities = null; + currentNetwork = null; + lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); + + options + .getLogger() + .log(SentryLevel.DEBUG, "Cache cleared - network lost/unavailable"); + + // Notify all observers with DISCONNECTED status directly + // No need to query ConnectivityManager - we know the network is gone + for (final @NotNull IConnectionStatusObserver observer : + connectionStatusObservers) { + observer.onConnectionStatusChanged(ConnectionStatus.DISCONNECTED); + } + } + } + + @Override + public void onCapabilitiesChanged( + @NonNull Network network, @NonNull NetworkCapabilities networkCapabilities) { + if (!network.equals(currentNetwork)) { + return; + } + updateCacheAndNotifyObservers(network, networkCapabilities); + } + + private void updateCacheAndNotifyObservers( + @Nullable Network network, @Nullable NetworkCapabilities networkCapabilities) { + // Check if this change is meaningful before notifying observers + final boolean shouldUpdate = isSignificantChange(networkCapabilities); + + // Only notify observers if something meaningful changed + if (shouldUpdate) { + updateCache(networkCapabilities); + + final @NotNull ConnectionStatus status = getConnectionStatusFromCache(); + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + for (final @NotNull IConnectionStatusObserver observer : + connectionStatusObservers) { + observer.onConnectionStatusChanged(status); } } - }; + } + } + + /** + * Check if NetworkCapabilities change is significant for our observers. Only notify for + * changes that affect connectivity status or connection type. + */ + private boolean isSignificantChange(@Nullable NetworkCapabilities newCapabilities) { + final NetworkCapabilities oldCapabilities = cachedNetworkCapabilities; + + // Always significant if transitioning between null and non-null + if ((oldCapabilities == null) != (newCapabilities == null)) { + return true; + } + + // If both null, no change + if (oldCapabilities == null && newCapabilities == null) { + return false; + } + + // Check significant capability changes + if (hasSignificantCapabilityChanges(oldCapabilities, newCapabilities)) { + return true; + } + + // Check significant transport changes + if (hasSignificantTransportChanges(oldCapabilities, newCapabilities)) { + return true; + } + + return false; + } + + /** Check if capabilities that affect connectivity status changed. */ + private boolean hasSignificantCapabilityChanges( + @NotNull NetworkCapabilities old, @NotNull NetworkCapabilities new_) { + // Check capabilities we care about for connectivity determination + for (int capability : capabilities) { + if (capability != 0 + && old.hasCapability(capability) != new_.hasCapability(capability)) { + return true; + } + } + + return false; + } + + /** Check if transport types that affect connection type changed. */ + private boolean hasSignificantTransportChanges( + @NotNull NetworkCapabilities old, @NotNull NetworkCapabilities new_) { + // Check transports we care about for connection type determination + for (int transport : transports) { + if (old.hasTransport(transport) != new_.hasTransport(transport)) { + return true; + } + } + + return false; + } + }; - if (registerNetworkCallback(context, logger, buildInfoProvider, newNetworkCallback)) { - networkCallback = newNetworkCallback; - return true; + if (registerNetworkCallback(context, options.getLogger(), buildInfoProvider, callback)) { + networkCallback = callback; + options.getLogger().log(SentryLevel.DEBUG, "Network callback registered successfully"); + } else { + options.getLogger().log(SentryLevel.WARNING, "Failed to register network callback"); + } + } + } + + @SuppressLint({"NewApi", "MissingPermission"}) + private void updateCache(@Nullable NetworkCapabilities networkCapabilities) { + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + try { + if (networkCapabilities != null) { + cachedNetworkCapabilities = networkCapabilities; + } else { + if (!Permissions.hasPermission(context, Manifest.permission.ACCESS_NETWORK_STATE)) { + options + .getLogger() + .log( + SentryLevel.INFO, + "No permission (ACCESS_NETWORK_STATE) to check network status."); + cachedNetworkCapabilities = null; + lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); + return; + } + + if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.M) { + cachedNetworkCapabilities = null; + lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); + return; + } + + // Fallback: query current active network + final ConnectivityManager connectivityManager = + getConnectivityManager(context, options.getLogger()); + if (connectivityManager != null) { + final Network activeNetwork = connectivityManager.getActiveNetwork(); + + cachedNetworkCapabilities = + activeNetwork != null + ? connectivityManager.getNetworkCapabilities(activeNetwork) + : null; } else { - return false; + cachedNetworkCapabilities = + null; // Clear cached capabilities if connectivity manager is null } } + lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); + + options + .getLogger() + .log( + SentryLevel.DEBUG, + "Cache updated - Status: " + + getConnectionStatusFromCache() + + ", Type: " + + getConnectionTypeFromCache()); + } catch (Throwable t) { + options.getLogger().log(SentryLevel.WARNING, "Failed to update connection status cache", t); + cachedNetworkCapabilities = null; + lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); } } - // networkCallback is already registered, so we can safely return true - return true; + } + + private boolean isCacheValid() { + return (timeProvider.getCurrentTimeMillis() - lastCacheUpdateTime) < CACHE_TTL_MS; + } + + @Override + public @NotNull ConnectionStatus getConnectionStatus() { + if (!isCacheValid()) { + updateCache(null); + } + return getConnectionStatusFromCache(); + } + + @Override + public @Nullable String getConnectionType() { + if (!isCacheValid()) { + updateCache(null); + } + return getConnectionTypeFromCache(); + } + + @Override + public boolean addConnectionStatusObserver(final @NotNull IConnectionStatusObserver observer) { + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + connectionStatusObservers.add(observer); + } + ensureNetworkCallbackRegistered(); + + // Network callback is already registered during initialization + return networkCallback != null; } @Override public void removeConnectionStatusObserver(final @NotNull IConnectionStatusObserver observer) { try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { connectionStatusObservers.remove(observer); - if (connectionStatusObservers.isEmpty()) { - if (networkCallback != null) { - unregisterNetworkCallback(context, logger, networkCallback); - networkCallback = null; - } - } + // Keep the callback registered for caching even if no observers + } + } + + /** Clean up resources - should be called when the provider is no longer needed */ + @Override + public void close() { + try { + options + .getExecutorService() + .submit( + () -> { + final NetworkCallback callbackRef; + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + connectionStatusObservers.clear(); + + callbackRef = networkCallback; + networkCallback = null; + + if (callbackRef != null) { + unregisterNetworkCallback(context, options.getLogger(), callbackRef); + } + // Clear cached state + cachedNetworkCapabilities = null; + currentNetwork = null; + lastCacheUpdateTime = 0; + } + try (final @NotNull ISentryLifecycleToken ignored = + connectivityManagerLock.acquire()) { + connectivityManager = null; + } + }); + } catch (Throwable t) { + options + .getLogger() + .log(SentryLevel.ERROR, "Error submitting AndroidConnectionStatusProvider task", t); } } + /** + * Get the cached NetworkCapabilities for advanced use cases. Returns null if cache is stale or no + * capabilities are available. + * + * @return cached NetworkCapabilities or null + */ + @TestOnly + @Nullable + public NetworkCapabilities getCachedNetworkCapabilities() { + return cachedNetworkCapabilities; + } + /** * Return the Connection status * @@ -295,12 +596,22 @@ public void removeConnectionStatusObserver(final @NotNull IConnectionStatusObser private static @Nullable ConnectivityManager getConnectivityManager( 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"); + if (connectivityManager != null) { + return connectivityManager; + } + + try (final @NotNull ISentryLifecycleToken ignored = connectivityManagerLock.acquire()) { + if (connectivityManager != null) { + return connectivityManager; // Double-checked locking + } + + connectivityManager = + (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE); + if (connectivityManager == null) { + logger.log(SentryLevel.INFO, "ConnectivityManager is null and cannot check network status"); + } + return connectivityManager; } - return connectivityManager; } @SuppressLint({"MissingPermission", "NewApi"}) @@ -358,4 +669,10 @@ public List getStatusObservers() { public NetworkCallback getNetworkCallback() { return networkCallback; } + + @TestOnly + @NotNull + public Thread getInitThread() { + return initThread; + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt index b15ab4e605d..a362885d635 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt @@ -1,18 +1,27 @@ package io.sentry.android.core +import android.Manifest import android.content.Context import android.content.pm.PackageManager.PERMISSION_DENIED +import android.content.pm.PackageManager.PERMISSION_GRANTED import android.net.ConnectivityManager import android.net.ConnectivityManager.NetworkCallback import android.net.Network import android.net.NetworkCapabilities +import android.net.NetworkCapabilities.NET_CAPABILITY_INTERNET +import android.net.NetworkCapabilities.NET_CAPABILITY_VALIDATED import android.net.NetworkCapabilities.TRANSPORT_CELLULAR import android.net.NetworkCapabilities.TRANSPORT_ETHERNET import android.net.NetworkCapabilities.TRANSPORT_WIFI import android.net.NetworkInfo import android.os.Build import io.sentry.IConnectionStatusProvider +import io.sentry.ILogger +import io.sentry.SentryOptions import io.sentry.android.core.internal.util.AndroidConnectionStatusProvider +import io.sentry.test.ImmediateExecutorService +import io.sentry.transport.ICurrentDateProvider +import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals @@ -21,11 +30,13 @@ import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue import org.mockito.kotlin.any +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.eq import org.mockito.kotlin.mock -import org.mockito.kotlin.never +import org.mockito.kotlin.mockingDetails import org.mockito.kotlin.times import org.mockito.kotlin.verify +import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.whenever class AndroidConnectionStatusProviderTest { @@ -34,14 +45,24 @@ class AndroidConnectionStatusProviderTest { private lateinit var connectivityManager: ConnectivityManager private lateinit var networkInfo: NetworkInfo private lateinit var buildInfo: BuildInfoProvider + private lateinit var timeProvider: ICurrentDateProvider + private lateinit var options: SentryOptions private lateinit var network: Network private lateinit var networkCapabilities: NetworkCapabilities + private lateinit var logger: ILogger + + private var currentTime = 1000L @BeforeTest fun beforeTest() { contextMock = mock() connectivityManager = mock() - whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager) + whenever(contextMock.getSystemService(Context.CONNECTIVITY_SERVICE)) + .thenReturn(connectivityManager) + whenever( + contextMock.checkPermission(eq(Manifest.permission.ACCESS_NETWORK_STATE), any(), any()) + ) + .thenReturn(PERMISSION_GRANTED) networkInfo = mock() whenever(connectivityManager.activeNetworkInfo).thenReturn(networkInfo) @@ -54,13 +75,38 @@ class AndroidConnectionStatusProviderTest { networkCapabilities = mock() whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(networkCapabilities) + whenever(networkCapabilities.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(networkCapabilities.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + + timeProvider = mock() + whenever(timeProvider.currentTimeMillis).thenAnswer { currentTime } + + logger = mock() + options = SentryOptions() + options.setLogger(logger) + options.executorService = ImmediateExecutorService() + + // Reset current time for each test to ensure cache isolation + currentTime = 1000L - connectionStatusProvider = AndroidConnectionStatusProvider(contextMock, mock(), buildInfo) + connectionStatusProvider = + AndroidConnectionStatusProvider(contextMock, options, buildInfo, timeProvider) + + // Wait for async callback registration to complete + connectionStatusProvider.initThread.join() + } + + @AfterTest + fun `tear down`() { + // clear the cache and ensure proper cleanup + connectionStatusProvider.close() } @Test fun `When network is active and connected with permission, return CONNECTED for isConnected`() { whenever(networkInfo.isConnected).thenReturn(true) + assertEquals( IConnectionStatusProvider.ConnectionStatus.CONNECTED, connectionStatusProvider.connectionStatus, @@ -89,6 +135,8 @@ class AndroidConnectionStatusProviderTest { @Test fun `When network is not active, return DISCONNECTED for isConnected`() { + whenever(connectivityManager.activeNetwork).thenReturn(null) + assertEquals( IConnectionStatusProvider.ConnectionStatus.DISCONNECTED, connectionStatusProvider.connectionStatus, @@ -97,98 +145,86 @@ class AndroidConnectionStatusProviderTest { @Test fun `When ConnectivityManager is not available, return UNKNOWN for isConnected`() { - whenever(contextMock.getSystemService(any())).thenReturn(null) + // First close the existing provider to clean up static state + connectionStatusProvider.close() + + // Create a fresh context mock that returns null for ConnectivityManager + val nullConnectivityContext = mock() + whenever(nullConnectivityContext.getSystemService(any())).thenReturn(null) + whenever( + nullConnectivityContext.checkPermission( + eq(Manifest.permission.ACCESS_NETWORK_STATE), + any(), + any(), + ) + ) + .thenReturn(PERMISSION_GRANTED) + + // Create a new provider with the null connectivity manager + val providerWithNullConnectivity = + AndroidConnectionStatusProvider(nullConnectivityContext, options, buildInfo, timeProvider) + providerWithNullConnectivity.initThread.join() // Wait for async init to complete + assertEquals( IConnectionStatusProvider.ConnectionStatus.UNKNOWN, - connectionStatusProvider.connectionStatus, + providerWithNullConnectivity.connectionStatus, ) + + providerWithNullConnectivity.close() } @Test fun `When ConnectivityManager is not available, return null for getConnectionType`() { - assertNull(AndroidConnectionStatusProvider.getConnectionType(mock(), mock(), buildInfo)) + whenever(contextMock.getSystemService(any())).thenReturn(null) + assertNull(connectionStatusProvider.connectionType) } @Test fun `When there's no permission, return null for getConnectionType`() { whenever(contextMock.checkPermission(any(), any(), any())).thenReturn(PERMISSION_DENIED) - assertNull(AndroidConnectionStatusProvider.getConnectionType(contextMock, mock(), buildInfo)) + assertNull(connectionStatusProvider.connectionType) } @Test fun `When network is not active, return null for getConnectionType`() { - whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager) + whenever(connectivityManager.activeNetwork).thenReturn(null) - assertNull(AndroidConnectionStatusProvider.getConnectionType(contextMock, mock(), buildInfo)) + assertNull(connectionStatusProvider.connectionType) } @Test fun `When network capabilities are not available, return null for getConnectionType`() { - assertNull(AndroidConnectionStatusProvider.getConnectionType(contextMock, mock(), buildInfo)) + whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(null) + + assertNull(connectionStatusProvider.connectionType) } @Test fun `When network capabilities has TRANSPORT_WIFI, return wifi`() { whenever(networkCapabilities.hasTransport(eq(TRANSPORT_WIFI))).thenReturn(true) + whenever(networkCapabilities.hasTransport(eq(TRANSPORT_CELLULAR))).thenReturn(false) + whenever(networkCapabilities.hasTransport(eq(TRANSPORT_ETHERNET))).thenReturn(false) - assertEquals( - "wifi", - AndroidConnectionStatusProvider.getConnectionType(contextMock, mock(), buildInfo), - ) + assertEquals("wifi", connectionStatusProvider.connectionType) } @Test fun `When network capabilities has TRANSPORT_ETHERNET, return ethernet`() { whenever(networkCapabilities.hasTransport(eq(TRANSPORT_ETHERNET))).thenReturn(true) + whenever(networkCapabilities.hasTransport(eq(TRANSPORT_WIFI))).thenReturn(false) + whenever(networkCapabilities.hasTransport(eq(TRANSPORT_CELLULAR))).thenReturn(false) - assertEquals( - "ethernet", - AndroidConnectionStatusProvider.getConnectionType(contextMock, mock(), buildInfo), - ) + assertEquals("ethernet", connectionStatusProvider.connectionType) } @Test fun `When network capabilities has TRANSPORT_CELLULAR, return cellular`() { + whenever(networkCapabilities.hasTransport(eq(TRANSPORT_WIFI))).thenReturn(false) + whenever(networkCapabilities.hasTransport(eq(TRANSPORT_ETHERNET))).thenReturn(false) whenever(networkCapabilities.hasTransport(eq(TRANSPORT_CELLULAR))).thenReturn(true) - assertEquals( - "cellular", - AndroidConnectionStatusProvider.getConnectionType(contextMock, mock(), buildInfo), - ) - } - - @Test - fun `When there's no permission, do not register any NetworkCallback`() { - val buildInfo = mock() - whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) - whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager) - whenever(contextMock.checkPermission(any(), any(), any())).thenReturn(PERMISSION_DENIED) - val registered = - AndroidConnectionStatusProvider.registerNetworkCallback( - contextMock, - mock(), - buildInfo, - mock(), - ) - - assertFalse(registered) - verify(connectivityManager, never()).registerDefaultNetworkCallback(any()) - } - - @Test - fun `When sdkInfoVersion is not min N, do not register any NetworkCallback`() { - whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager) - val registered = - AndroidConnectionStatusProvider.registerNetworkCallback( - contextMock, - mock(), - buildInfo, - mock(), - ) - - assertFalse(registered) - verify(connectivityManager, never()).registerDefaultNetworkCallback(any()) + assertEquals("cellular", connectionStatusProvider.connectionType) } @Test @@ -199,7 +235,7 @@ class AndroidConnectionStatusProviderTest { val registered = AndroidConnectionStatusProvider.registerNetworkCallback( contextMock, - mock(), + logger, buildInfo, mock(), ) @@ -211,17 +247,16 @@ class AndroidConnectionStatusProviderTest { @Test fun `unregisterNetworkCallback calls connectivityManager unregisterDefaultNetworkCallback`() { whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager) - AndroidConnectionStatusProvider.unregisterNetworkCallback(contextMock, mock(), mock()) + AndroidConnectionStatusProvider.unregisterNetworkCallback(contextMock, logger, mock()) verify(connectivityManager).unregisterNetworkCallback(any()) } @Test fun `When connectivityManager getActiveNetwork throws an exception, getConnectionType returns null`() { - whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.S) whenever(connectivityManager.activeNetwork).thenThrow(SecurityException("Android OS Bug")) - assertNull(AndroidConnectionStatusProvider.getConnectionType(contextMock, mock(), buildInfo)) + assertNull(connectionStatusProvider.connectionType) } @Test @@ -231,27 +266,13 @@ class AndroidConnectionStatusProviderTest { assertFalse( AndroidConnectionStatusProvider.registerNetworkCallback( contextMock, - mock(), + logger, buildInfo, mock(), ) ) } - @Test - fun `When connectivityManager unregisterDefaultCallback throws an exception, it gets swallowed`() { - whenever(connectivityManager.registerDefaultNetworkCallback(any())) - .thenThrow(SecurityException("Android OS Bug")) - - var failed = false - try { - AndroidConnectionStatusProvider.unregisterNetworkCallback(contextMock, mock(), mock()) - } catch (t: Throwable) { - failed = true - } - assertFalse(failed) - } - @Test fun `connectionStatus returns NO_PERMISSIONS when context does not hold the permission`() { whenever(contextMock.checkPermission(any(), any(), any())).thenReturn(PERMISSION_DENIED) @@ -261,12 +282,6 @@ class AndroidConnectionStatusProviderTest { ) } - @Test - fun `connectionStatus returns ethernet when underlying mechanism provides ethernet`() { - whenever(networkCapabilities.hasTransport(eq(TRANSPORT_ETHERNET))).thenReturn(true) - assertEquals("ethernet", connectionStatusProvider.connectionType) - } - @Test fun `adding and removing an observer works correctly`() { whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) @@ -279,25 +294,341 @@ class AndroidConnectionStatusProviderTest { connectionStatusProvider.removeConnectionStatusObserver(observer) assertTrue(connectionStatusProvider.statusObservers.isEmpty()) - assertNull(connectionStatusProvider.networkCallback) } @Test - fun `underlying callbacks correctly trigger update`() { + fun `cache TTL works correctly`() { + // Setup: Mock network info to return connected + whenever(networkInfo.isConnected).thenReturn(true) + + // For API level M, the code uses getActiveNetwork() and getNetworkCapabilities() + // Let's track calls to these methods to verify caching behavior + + // Make the first call to establish baseline + val firstResult = connectionStatusProvider.connectionStatus + assertEquals(IConnectionStatusProvider.ConnectionStatus.CONNECTED, firstResult) + + // Count how many times getActiveNetwork was called so far (includes any initialization calls) + val initialCallCount = + mockingDetails(connectivityManager).invocations.count { it.method.name == "getActiveNetwork" } + + // Advance time by 1 minute (less than 2 minute TTL) + currentTime += 60 * 1000L + + // Second call should use cache - no additional calls to getActiveNetwork + val secondResult = connectionStatusProvider.connectionStatus + assertEquals(IConnectionStatusProvider.ConnectionStatus.CONNECTED, secondResult) + + val callCountAfterSecond = + mockingDetails(connectivityManager).invocations.count { it.method.name == "getActiveNetwork" } + + // Verify no additional calls were made (cache was used) + assertEquals(initialCallCount, callCountAfterSecond, "Second call should use cache") + + // Advance time beyond TTL (total 3 minutes) + currentTime += 2 * 60 * 1000L + + // Third call should refresh cache - should make new calls to getActiveNetwork + val thirdResult = connectionStatusProvider.connectionStatus + assertEquals(IConnectionStatusProvider.ConnectionStatus.CONNECTED, thirdResult) + + val callCountAfterThird = + mockingDetails(connectivityManager).invocations.count { it.method.name == "getActiveNetwork" } + + // Verify that new calls were made (cache was refreshed) + assertTrue(callCountAfterThird > callCountAfterSecond, "Third call should refresh cache") + + // All results should be consistent + assertEquals(firstResult, secondResult) + assertEquals(secondResult, thirdResult) + } + + @Test + fun `observers are only notified for significant changes`() { whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + // Get the callback that was registered + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + // IMPORTANT: Set network as current first + callback.onAvailable(network) + + // Create network capabilities for testing + val oldCaps = mock() + whenever(oldCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(oldCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(oldCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(oldCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) + whenever(oldCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + val newCaps = mock() + whenever(newCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(newCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(newCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(newCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) + whenever(newCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + // First callback with capabilities - should notify + callback.onCapabilitiesChanged(network, oldCaps) + + // Second callback with same significant capabilities - should NOT notify additional times + callback.onCapabilitiesChanged(network, newCaps) + + // Only first change should trigger notification + verify(observer, times(1)).onConnectionStatusChanged(any()) + } - var callback: NetworkCallback? = null - whenever(connectivityManager.registerDefaultNetworkCallback(any())).then { invocation -> - callback = invocation.getArgument(0, NetworkCallback::class.java) as NetworkCallback - Unit - } + @Test + fun `observers are notified when significant capabilities change`() { + // Create a new provider with API level N for network callback support + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) val observer = mock() connectionStatusProvider.addConnectionStatusObserver(observer) - callback!!.onAvailable(mock()) - callback!!.onUnavailable() - callback!!.onLost(mock()) - connectionStatusProvider.removeConnectionStatusObserver(observer) - verify(observer, times(3)).onConnectionStatusChanged(any()) + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + // IMPORTANT: Set network as current first + callback.onAvailable(network) + + val oldCaps = mock() + whenever(oldCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(oldCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(false) // Not validated + whenever(oldCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(oldCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) + whenever(oldCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + val newCaps = mock() + whenever(newCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(newCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) // Now validated + whenever(newCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(newCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) + whenever(newCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + callback.onCapabilitiesChanged(network, oldCaps) + callback.onCapabilitiesChanged(network, newCaps) + + // Should be notified for both changes (validation state changed) + verify(observer, times(2)).onConnectionStatusChanged(any()) + } + + @Test + fun `observers are notified when transport changes`() { + // Create a new provider with API level N for network callback support + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + // IMPORTANT: Set network as current first + callback.onAvailable(network) + + val wifiCaps = mock() + whenever(wifiCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(wifiCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(wifiCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(wifiCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) + whenever(wifiCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + val cellularCaps = mock() + whenever(cellularCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(cellularCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(cellularCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(false) + whenever(cellularCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true) + whenever(cellularCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + callback.onCapabilitiesChanged(network, wifiCaps) + callback.onCapabilitiesChanged(network, cellularCaps) + + // Should be notified for both changes (transport changed) + verify(observer, times(2)).onConnectionStatusChanged(any()) + } + + @Test + fun `onLost clears cache and notifies with DISCONNECTED`() { + // Create a new provider with API level N for network callback support + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + // Set current network + callback.onAvailable(network) + + // Lose the network + callback.onLost(network) + + assertNull(connectionStatusProvider.cachedNetworkCapabilities) + verify(observer) + .onConnectionStatusChanged(IConnectionStatusProvider.ConnectionStatus.DISCONNECTED) + } + + @Test + fun `onUnavailable clears cache and notifies with DISCONNECTED`() { + // Create a new provider with API level N for network callback support + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + callback.onUnavailable() + + assertNull(connectionStatusProvider.cachedNetworkCapabilities) + verify(observer) + .onConnectionStatusChanged(IConnectionStatusProvider.ConnectionStatus.DISCONNECTED) + } + + @Test + fun `onLost for different network is ignored`() { + // Create a new provider with API level N for network callback support + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + val network1 = mock() + val network2 = mock() + + // Set current network + callback.onAvailable(network1) + + // Lose a different network - should be ignored + callback.onLost(network2) + + verifyNoInteractions(observer) + } + + @Test + fun `isNetworkEffectivelyConnected works correctly for Android 15`() { + // Test case: has internet and validated capabilities with good transport + val goodCaps = mock() + whenever(goodCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(goodCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(goodCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(goodCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) + whenever(goodCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + // Override the mock to return good capabilities + whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(goodCaps) + + // Force cache invalidation by advancing time beyond TTL + currentTime += 3 * 60 * 1000L // 3 minutes + + // Should return CONNECTED for good capabilities + assertEquals( + IConnectionStatusProvider.ConnectionStatus.CONNECTED, + connectionStatusProvider.connectionStatus, + ) + + // Test case: missing validated capability + val unvalidatedCaps = mock() + whenever(unvalidatedCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(unvalidatedCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(false) + whenever(unvalidatedCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + + whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(unvalidatedCaps) + + // Force cache invalidation again + currentTime += 3 * 60 * 1000L + + assertEquals( + IConnectionStatusProvider.ConnectionStatus.DISCONNECTED, + connectionStatusProvider.connectionStatus, + ) + } + + @Test + fun `API level below M falls back to legacy method`() { + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.LOLLIPOP) + whenever(networkInfo.isConnected).thenReturn(true) + + assertEquals( + IConnectionStatusProvider.ConnectionStatus.CONNECTED, + connectionStatusProvider.connectionStatus, + ) + } + + @Test + fun `onCapabilitiesChanged updates cache`() { + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val observer = mock() + connectionStatusProvider.addConnectionStatusObserver(observer) + + val callbackCaptor = argumentCaptor() + verify(connectivityManager).registerDefaultNetworkCallback(callbackCaptor.capture()) + val callback = callbackCaptor.firstValue + + // Set network as current first + callback.onAvailable(network) + + // Create initial capabilities - CONNECTED state (wifi + validated) + val initialCaps = mock() + whenever(initialCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(initialCaps.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) + whenever(initialCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(true) + whenever(initialCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(false) + whenever(initialCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + // First callback with initial capabilities + callback.onCapabilitiesChanged(network, initialCaps) + + // Verify cache contains the initial capabilities + assertEquals(initialCaps, connectionStatusProvider.cachedNetworkCapabilities) + + // Verify initial state - should be CONNECTED with wifi + assertEquals( + IConnectionStatusProvider.ConnectionStatus.CONNECTED, + connectionStatusProvider.connectionStatus, + ) + assertEquals("wifi", connectionStatusProvider.connectionType) + + // Create new capabilities - DISCONNECTED state (cellular but not validated) + val newCaps = mock() + whenever(newCaps.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(true) + whenever(newCaps.hasCapability(NET_CAPABILITY_VALIDATED)) + .thenReturn(false) // Not validated = DISCONNECTED + whenever(newCaps.hasTransport(TRANSPORT_WIFI)).thenReturn(false) + whenever(newCaps.hasTransport(TRANSPORT_CELLULAR)).thenReturn(true) + whenever(newCaps.hasTransport(TRANSPORT_ETHERNET)).thenReturn(false) + + // Second callback with changed capabilities + callback.onCapabilitiesChanged(network, newCaps) + + // Verify cache is updated with new capabilities + assertEquals(newCaps, connectionStatusProvider.cachedNetworkCapabilities) + + // Verify that subsequent calls use the updated cache + // Both connection status AND type should change + assertEquals( + IConnectionStatusProvider.ConnectionStatus.DISCONNECTED, + connectionStatusProvider.connectionStatus, + ) + assertEquals("cellular", connectionStatusProvider.connectionType) + + // Verify observer was notified of the changes (both calls should notify since capabilities + // changed significantly) + verify(observer, times(2)).onConnectionStatusChanged(any()) } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 171cd7eaa88..b5dcbe2caf6 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -738,7 +738,7 @@ public final class io/sentry/HubScopesWrapper : io/sentry/IHub { public fun withScope (Lio/sentry/ScopeCallback;)V } -public abstract interface class io/sentry/IConnectionStatusProvider { +public abstract interface class io/sentry/IConnectionStatusProvider : java/io/Closeable { public abstract fun addConnectionStatusObserver (Lio/sentry/IConnectionStatusProvider$IConnectionStatusObserver;)Z public abstract fun getConnectionStatus ()Lio/sentry/IConnectionStatusProvider$ConnectionStatus; public abstract fun getConnectionType ()Ljava/lang/String; @@ -1452,6 +1452,7 @@ public final class io/sentry/NoOpCompositePerformanceCollector : io/sentry/Compo public final class io/sentry/NoOpConnectionStatusProvider : io/sentry/IConnectionStatusProvider { public fun ()V public fun addConnectionStatusObserver (Lio/sentry/IConnectionStatusProvider$IConnectionStatusObserver;)Z + public fun close ()V public fun getConnectionStatus ()Lio/sentry/IConnectionStatusProvider$ConnectionStatus; public fun getConnectionType ()Ljava/lang/String; public fun removeConnectionStatusObserver (Lio/sentry/IConnectionStatusProvider$IConnectionStatusObserver;)V diff --git a/sentry/src/main/java/io/sentry/IConnectionStatusProvider.java b/sentry/src/main/java/io/sentry/IConnectionStatusProvider.java index 1d75098e564..bd897882d7a 100644 --- a/sentry/src/main/java/io/sentry/IConnectionStatusProvider.java +++ b/sentry/src/main/java/io/sentry/IConnectionStatusProvider.java @@ -1,11 +1,12 @@ package io.sentry; +import java.io.Closeable; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @ApiStatus.Internal -public interface IConnectionStatusProvider { +public interface IConnectionStatusProvider extends Closeable { enum ConnectionStatus { UNKNOWN, diff --git a/sentry/src/main/java/io/sentry/NoOpConnectionStatusProvider.java b/sentry/src/main/java/io/sentry/NoOpConnectionStatusProvider.java index a1d66c9115b..765c2c0537d 100644 --- a/sentry/src/main/java/io/sentry/NoOpConnectionStatusProvider.java +++ b/sentry/src/main/java/io/sentry/NoOpConnectionStatusProvider.java @@ -1,5 +1,6 @@ package io.sentry; +import java.io.IOException; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -25,4 +26,9 @@ public boolean addConnectionStatusObserver(@NotNull IConnectionStatusObserver ob public void removeConnectionStatusObserver(@NotNull IConnectionStatusObserver observer) { // no-op } + + @Override + public void close() throws IOException { + // no-op + } } diff --git a/sentry/src/main/java/io/sentry/Scopes.java b/sentry/src/main/java/io/sentry/Scopes.java index fa1b7c2a81e..4b16d71b931 100644 --- a/sentry/src/main/java/io/sentry/Scopes.java +++ b/sentry/src/main/java/io/sentry/Scopes.java @@ -446,6 +446,7 @@ public void close(final boolean isRestarting) { getOptions().getTransactionProfiler().close(); getOptions().getContinuousProfiler().close(true); getOptions().getCompositePerformanceCollector().close(); + getOptions().getConnectionStatusProvider().close(); final @NotNull ISentryExecutorService executorService = getOptions().getExecutorService(); if (isRestarting) { executorService.submit( diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index f132807e428..770f7bea877 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -590,6 +590,8 @@ class SentryOptionsTest { val options = SentryOptions() val customProvider = object : IConnectionStatusProvider { + override fun close() = Unit + override fun getConnectionStatus(): IConnectionStatusProvider.ConnectionStatus { return IConnectionStatusProvider.ConnectionStatus.UNKNOWN } From 14bb2d54660ae01e0bc1e48617a49d369ff242ab Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 16 Jul 2025 17:40:40 +0200 Subject: [PATCH 02/11] changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1bb8a6656f..f73d9c22b56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Allow multiple UncaughtExceptionHandlerIntegrations to be active at the same time ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) +- Cache network capabilities and status to reduce IPC calls ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) ## 8.17.0 @@ -3539,7 +3540,7 @@ TBD Packages were released on [bintray](https://dl.bintray.com/getsentry/maven/io/sentry/) > Note: This release marks the unification of the Java and Android Sentry codebases based on the core of the Android SDK (version 2.x). -Previous releases for the Android SDK (version 2.x) can be found on the now archived: https://github.com/getsentry/sentry-android/ +> Previous releases for the Android SDK (version 2.x) can be found on the now archived: https://github.com/getsentry/sentry-android/ ## 3.0.0-alpha.1 From c82586d72541c20fd17d5157d78a9b8a57e50782 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 16 Jul 2025 17:43:23 +0200 Subject: [PATCH 03/11] Changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f73d9c22b56..62c8f5e17d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ ### Fixes - Allow multiple UncaughtExceptionHandlerIntegrations to be active at the same time ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) -- Cache network capabilities and status to reduce IPC calls ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) +- Cache network capabilities and status to reduce IPC calls ([#4560](https://github.com/getsentry/sentry-java/pull/4560)) ## 8.17.0 From 7418781778065cce406b3edeb32d03092995cf1e Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 16 Jul 2025 17:45:23 +0200 Subject: [PATCH 04/11] revert --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62c8f5e17d8..29dfad2e336 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3540,7 +3540,7 @@ TBD Packages were released on [bintray](https://dl.bintray.com/getsentry/maven/io/sentry/) > Note: This release marks the unification of the Java and Android Sentry codebases based on the core of the Android SDK (version 2.x). -> Previous releases for the Android SDK (version 2.x) can be found on the now archived: https://github.com/getsentry/sentry-android/ +Previous releases for the Android SDK (version 2.x) can be found on the now archived: https://github.com/getsentry/sentry-android/ ## 3.0.0-alpha.1 From f739d004dea8776c72cd36067b2bd95d928de5dd Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 17 Jul 2025 00:00:25 +0200 Subject: [PATCH 05/11] fix(breadcrumbs): Deduplicate battery breadcrumbs --- .../SystemEventsBreadcrumbsIntegration.java | 70 ++++++++++++++--- .../SystemEventsBreadcrumbsIntegrationTest.kt | 77 +++++++++++++++++++ 2 files changed, 137 insertions(+), 10 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index 27dcbbbd725..b073ab1a32e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -339,6 +339,43 @@ static final class SystemEventsBroadcastReceiver extends BroadcastReceiver { private final @NotNull Debouncer batteryChangedDebouncer = new Debouncer(AndroidCurrentDateProvider.getInstance(), DEBOUNCE_WAIT_TIME_MS, 0); + // Track previous battery state to avoid duplicate breadcrumbs when values haven't changed + private @Nullable BatteryState previousBatteryState; + + static final class BatteryState { + private final @Nullable Float level; + private final @Nullable Boolean charging; + + BatteryState(final @Nullable Float level, final @Nullable Boolean charging) { + this.level = level; + this.charging = charging; + } + + @Override + public boolean equals(final @Nullable Object other) { + if (!(other instanceof BatteryState)) return false; + BatteryState that = (BatteryState) other; + return isSimilarLevel(level, that.level) && Objects.equals(charging, that.charging); + } + + @Override + public int hashCode() { + // Use rounded level for hash consistency + Float roundedLevel = level != null ? Math.round(level * 100f) / 100f : null; + return Objects.hash(roundedLevel, charging); + } + + private boolean isSimilarLevel(final @Nullable Float level1, final @Nullable Float level2) { + if (level1 == null && level2 == null) return true; + if (level1 == null || level2 == null) return false; + + // Round both levels to 2 decimal places and compare + float rounded1 = Math.round(level1 * 100f) / 100f; + float rounded2 = Math.round(level2 * 100f) / 100f; + return Float.compare(rounded1, rounded2) == 0; + } + } + SystemEventsBroadcastReceiver( final @NotNull IScopes scopes, final @NotNull SentryAndroidOptions options) { this.scopes = scopes; @@ -355,14 +392,29 @@ public void onReceive(final Context context, final @NotNull Intent intent) { return; } + // For battery changes, check if the actual values have changed + @Nullable BatteryState batteryState = null; + if (isBatteryChanged) { + final @Nullable Float currentBatteryLevel = DeviceInfoUtil.getBatteryLevel(intent, options); + final @Nullable Boolean currentChargingState = DeviceInfoUtil.isCharging(intent, options); + batteryState = new BatteryState(currentBatteryLevel, currentChargingState); + + // Only create breadcrumb if battery state has actually changed + if (batteryState.equals(previousBatteryState)) { + return; + } + + previousBatteryState = batteryState; + } + + final BatteryState state = batteryState; final long now = System.currentTimeMillis(); try { options .getExecutorService() .submit( () -> { - final Breadcrumb breadcrumb = - createBreadcrumb(now, intent, action, isBatteryChanged); + final Breadcrumb breadcrumb = createBreadcrumb(now, intent, action, state); final Hint hint = new Hint(); hint.set(ANDROID_INTENT, intent); scopes.addBreadcrumb(breadcrumb, hint); @@ -411,7 +463,7 @@ String getStringAfterDotFast(final @Nullable String str) { final long timeMs, final @NotNull Intent intent, final @Nullable String action, - boolean isBatteryChanged) { + final @Nullable BatteryState batteryState) { final Breadcrumb breadcrumb = new Breadcrumb(timeMs); breadcrumb.setType("system"); breadcrumb.setCategory("device.event"); @@ -420,14 +472,12 @@ String getStringAfterDotFast(final @Nullable String str) { breadcrumb.setData("action", shortAction); } - if (isBatteryChanged) { - final Float batteryLevel = DeviceInfoUtil.getBatteryLevel(intent, options); - if (batteryLevel != null) { - breadcrumb.setData("level", batteryLevel); + if (batteryState != null) { + if (batteryState.level != null) { + breadcrumb.setData("level", batteryState.level); } - final Boolean isCharging = DeviceInfoUtil.isCharging(intent, options); - if (isCharging != null) { - breadcrumb.setData("charging", isCharging); + if (batteryState.charging != null) { + breadcrumb.setData("charging", batteryState.charging); } } else { final Bundle extras = intent.getExtras(); diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt index 99a80f361c5..d35df7c8444 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegrationTest.kt @@ -197,6 +197,83 @@ class SystemEventsBreadcrumbsIntegrationTest { verifyNoMoreInteractions(fixture.scopes) } + @Test + fun `battery changes with identical values do not generate breadcrumbs`() { + val sut = fixture.getSut() + sut.register(fixture.scopes, fixture.options) + + val intent1 = + Intent().apply { + action = Intent.ACTION_BATTERY_CHANGED + putExtra(BatteryManager.EXTRA_LEVEL, 80) + putExtra(BatteryManager.EXTRA_SCALE, 100) + putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB) + } + val intent2 = + Intent().apply { + action = Intent.ACTION_BATTERY_CHANGED + putExtra(BatteryManager.EXTRA_LEVEL, 80) + putExtra(BatteryManager.EXTRA_SCALE, 100) + putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB) + } + + // Receive first battery change + sut.receiver!!.onReceive(fixture.context, intent1) + + // Receive second battery change with identical values + sut.receiver!!.onReceive(fixture.context, intent2) + + // should only add the first crumb since values are identical + verify(fixture.scopes) + .addBreadcrumb( + check { + assertEquals(it.data["level"], 80f) + assertEquals(it.data["charging"], true) + }, + anyOrNull(), + ) + verifyNoMoreInteractions(fixture.scopes) + } + + @Test + fun `battery changes with minor level differences do not generate breadcrumbs`() { + val sut = fixture.getSut() + sut.register(fixture.scopes, fixture.options) + + val intent1 = + Intent().apply { + action = Intent.ACTION_BATTERY_CHANGED + putExtra(BatteryManager.EXTRA_LEVEL, 80001) // 80.001% + putExtra(BatteryManager.EXTRA_SCALE, 100000) + putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB) + } + val intent2 = + Intent().apply { + action = Intent.ACTION_BATTERY_CHANGED + putExtra(BatteryManager.EXTRA_LEVEL, 80002) // 80.002% + putExtra(BatteryManager.EXTRA_SCALE, 100000) + putExtra(BatteryManager.EXTRA_PLUGGED, BatteryManager.BATTERY_PLUGGED_USB) + } + + // Receive first battery change + sut.receiver!!.onReceive(fixture.context, intent1) + + // Receive second battery change with very minor level difference (rounds to same 3 decimal + // places) + sut.receiver!!.onReceive(fixture.context, intent2) + + // should only add the first crumb since both round to 80.000% + verify(fixture.scopes) + .addBreadcrumb( + check { + assertEquals(it.data["level"], 80.001f) + assertEquals(it.data["charging"], true) + }, + anyOrNull(), + ) + verifyNoMoreInteractions(fixture.scopes) + } + @Test fun `Do not crash if registerReceiver throws exception`() { val sut = fixture.getSut() From 833b026812ad6ae9634da7f724cae4234a87d64a Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 17 Jul 2025 00:05:03 +0200 Subject: [PATCH 06/11] ref --- .../core/SystemEventsBreadcrumbsIntegration.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index b073ab1a32e..7578ed6bf56 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -387,14 +387,14 @@ public void onReceive(final Context context, final @NotNull Intent intent) { final @Nullable String action = intent.getAction(); final boolean isBatteryChanged = ACTION_BATTERY_CHANGED.equals(action); - // aligning with iOS which only captures battery status changes every minute at maximum - if (isBatteryChanged && batteryChangedDebouncer.checkForDebounce()) { - return; - } - - // For battery changes, check if the actual values have changed @Nullable BatteryState batteryState = null; if (isBatteryChanged) { + if (batteryChangedDebouncer.checkForDebounce()) { + // aligning with iOS which only captures battery status changes every minute at maximum + return; + } + + // For battery changes, check if the actual values have changed final @Nullable Float currentBatteryLevel = DeviceInfoUtil.getBatteryLevel(intent, options); final @Nullable Boolean currentChargingState = DeviceInfoUtil.isCharging(intent, options); batteryState = new BatteryState(currentBatteryLevel, currentChargingState); From e4596fffb98c8f07e1b36c60755a39452908b3a0 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 17 Jul 2025 00:28:56 +0200 Subject: [PATCH 07/11] Changelog --- CHANGELOG.md | 1 + .../sentry/android/core/SystemEventsBreadcrumbsIntegration.java | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29dfad2e336..c2a6fbf4978 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Allow multiple UncaughtExceptionHandlerIntegrations to be active at the same time ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) - Cache network capabilities and status to reduce IPC calls ([#4560](https://github.com/getsentry/sentry-java/pull/4560)) +- Deduplicate battery breadcrumbs ([#4561](https://github.com/getsentry/sentry-java/pull/4561)) ## 8.17.0 diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java index 7578ed6bf56..d681f08429a 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java @@ -372,7 +372,7 @@ private boolean isSimilarLevel(final @Nullable Float level1, final @Nullable Flo // Round both levels to 2 decimal places and compare float rounded1 = Math.round(level1 * 100f) / 100f; float rounded2 = Math.round(level2 * 100f) / 100f; - return Float.compare(rounded1, rounded2) == 0; + return rounded1 == rounded2; } } From 0153ab5f73c7d213d3062396c7813949028b0cec Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 17 Jul 2025 09:17:38 +0200 Subject: [PATCH 08/11] Fix test --- .../util/AndroidConnectionStatusProvider.java | 5 +++++ .../core/NetworkBreadcrumbsIntegrationTest.kt | 21 +++++++++++++------ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java index 3df49171d32..0ea2e1638ab 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java @@ -675,4 +675,9 @@ public NetworkCallback getNetworkCallback() { public Thread getInitThread() { return initThread; } + + @TestOnly + public static void setConnectivityManager(final @Nullable ConnectivityManager cm) { + connectivityManager = cm; + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt index fd18e3d75b8..768fe87fbbd 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt @@ -16,9 +16,12 @@ import io.sentry.SentryNanotimeDate import io.sentry.TypeCheckHint import io.sentry.android.core.NetworkBreadcrumbsIntegration.NetworkBreadcrumbConnectionDetail import io.sentry.android.core.NetworkBreadcrumbsIntegration.NetworkBreadcrumbsNetworkCallback +import io.sentry.android.core.internal.util.AndroidConnectionStatusProvider import io.sentry.test.DeferredExecutorService import io.sentry.test.ImmediateExecutorService import java.util.concurrent.TimeUnit +import kotlin.test.AfterTest +import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -47,12 +50,6 @@ class NetworkBreadcrumbsIntegrationTest { var nowMs: Long = 0 val network = mock() - init { - whenever(mockBuildInfoProvider.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) - whenever(context.getSystemService(eq(Context.CONNECTIVITY_SERVICE))) - .thenReturn(connectivityManager) - } - fun getSut( enableNetworkEventBreadcrumbs: Boolean = true, buildInfo: BuildInfoProvider = mockBuildInfoProvider, @@ -73,6 +70,18 @@ class NetworkBreadcrumbsIntegrationTest { private val fixture = Fixture() + @BeforeTest + fun `set up`() { + whenever(fixture.mockBuildInfoProvider.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + whenever(fixture.context.getSystemService(eq(Context.CONNECTIVITY_SERVICE))) + .thenReturn(fixture.connectivityManager) + } + + @AfterTest + fun `tear down`() { + AndroidConnectionStatusProvider.setConnectivityManager(null) + } + @Test fun `When network events breadcrumb is enabled, it registers callback`() { val sut = fixture.getSut() From c31d64874a182bafced477fc1d32a165441d15f4 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 18 Jul 2025 16:18:07 +0200 Subject: [PATCH 09/11] perf(connectivity): Have only one NetworkCallback active at a time --- .../api/sentry-android-core.api | 2 +- .../core/AndroidOptionsInitializer.java | 3 +- .../core/NetworkBreadcrumbsIntegration.java | 109 ++++++------------ .../core/SendCachedEnvelopeIntegration.java | 4 +- .../util/AndroidConnectionStatusProvider.java | 69 ++++++++++- .../core/NetworkBreadcrumbsIntegrationTest.kt | 49 +------- .../AndroidConnectionStatusProviderTest.kt | 53 ++++----- .../android/replay/capture/CaptureStrategy.kt | 25 +++- ...achedEnvelopeFireAndForgetIntegration.java | 4 +- 9 files changed, 159 insertions(+), 159 deletions(-) rename sentry-android-core/src/test/java/io/sentry/android/core/{ => internal/util}/AndroidConnectionStatusProviderTest.kt (96%) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index ea67ed5bcb7..987f13f540f 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -263,7 +263,7 @@ public final class io/sentry/android/core/NdkIntegration : io/sentry/Integration } public final class io/sentry/android/core/NetworkBreadcrumbsIntegration : io/sentry/Integration, java/io/Closeable { - public fun (Landroid/content/Context;Lio/sentry/android/core/BuildInfoProvider;Lio/sentry/ILogger;)V + public fun (Landroid/content/Context;Lio/sentry/android/core/BuildInfoProvider;)V public fun close ()V public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index c3303cf4698..e4874b4309b 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -382,8 +382,7 @@ static void installDefaultIntegrations( } options.addIntegration(new AppComponentsBreadcrumbsIntegration(context)); options.addIntegration(new SystemEventsBreadcrumbsIntegration(context)); - options.addIntegration( - new NetworkBreadcrumbsIntegration(context, buildInfoProvider, options.getLogger())); + options.addIntegration(new NetworkBreadcrumbsIntegration(context, buildInfoProvider)); if (isReplayAvailable) { final ReplayIntegration replay = new ReplayIntegration(context, CurrentDateProvider.getInstance()); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/NetworkBreadcrumbsIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/NetworkBreadcrumbsIntegration.java index 5cfb5df2235..c7f3182b97d 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/NetworkBreadcrumbsIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/NetworkBreadcrumbsIntegration.java @@ -12,7 +12,6 @@ import io.sentry.Breadcrumb; import io.sentry.DateUtils; import io.sentry.Hint; -import io.sentry.ILogger; import io.sentry.IScopes; import io.sentry.ISentryLifecycleToken; import io.sentry.Integration; @@ -33,22 +32,17 @@ public final class NetworkBreadcrumbsIntegration implements Integration, Closeab private final @NotNull Context context; private final @NotNull BuildInfoProvider buildInfoProvider; - private final @NotNull ILogger logger; private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); - private volatile boolean isClosed; private @Nullable SentryOptions options; @TestOnly @Nullable volatile NetworkBreadcrumbsNetworkCallback networkCallback; public NetworkBreadcrumbsIntegration( - final @NotNull Context context, - final @NotNull BuildInfoProvider buildInfoProvider, - final @NotNull ILogger logger) { + final @NotNull Context context, final @NotNull BuildInfoProvider buildInfoProvider) { this.context = Objects.requireNonNull(ContextUtils.getApplicationContext(context), "Context is required"); this.buildInfoProvider = Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required"); - this.logger = Objects.requireNonNull(logger, "ILogger is required"); } @Override @@ -59,78 +53,54 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions (options instanceof SentryAndroidOptions) ? (SentryAndroidOptions) options : null, "SentryAndroidOptions is required"); - logger.log( - SentryLevel.DEBUG, - "NetworkBreadcrumbsIntegration enabled: %s", - androidOptions.isEnableNetworkEventBreadcrumbs()); - this.options = options; + options + .getLogger() + .log( + SentryLevel.DEBUG, + "NetworkBreadcrumbsIntegration enabled: %s", + androidOptions.isEnableNetworkEventBreadcrumbs()); + if (androidOptions.isEnableNetworkEventBreadcrumbs()) { // The specific error is logged in the ConnectivityChecker method if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.N) { - logger.log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+."); + options.getLogger().log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+."); return; } - try { - options - .getExecutorService() - .submit( - new Runnable() { - @Override - public void run() { - // in case integration is closed before the task is executed, simply return - if (isClosed) { - return; - } - - try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - networkCallback = - new NetworkBreadcrumbsNetworkCallback( - scopes, buildInfoProvider, options.getDateProvider()); - - final boolean registered = - AndroidConnectionStatusProvider.registerNetworkCallback( - context, logger, buildInfoProvider, networkCallback); - if (registered) { - logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration installed."); - addIntegrationToSdkVersion("NetworkBreadcrumbs"); - } else { - logger.log( - SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration not installed."); - // The specific error is logged by AndroidConnectionStatusProvider - } - } - } - }); - } catch (Throwable t) { - logger.log(SentryLevel.ERROR, "Error submitting NetworkBreadcrumbsIntegration task.", t); + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + networkCallback = + new NetworkBreadcrumbsNetworkCallback( + scopes, buildInfoProvider, options.getDateProvider()); + + final boolean registered = + AndroidConnectionStatusProvider.addNetworkCallback( + context, options.getLogger(), buildInfoProvider, networkCallback); + if (registered) { + options.getLogger().log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration installed."); + addIntegrationToSdkVersion("NetworkBreadcrumbs"); + } else { + options + .getLogger() + .log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration not installed."); + // The specific error is logged by AndroidConnectionStatusProvider + } } } } @Override public void close() throws IOException { - isClosed = true; + final @Nullable ConnectivityManager.NetworkCallback callbackRef; + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + callbackRef = networkCallback; + networkCallback = null; + } - try { - Objects.requireNonNull(options, "Options is required") - .getExecutorService() - .submit( - () -> { - try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - if (networkCallback != null) { - AndroidConnectionStatusProvider.unregisterNetworkCallback( - context, logger, networkCallback); - logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration removed."); - } - networkCallback = null; - } - }); - } catch (Throwable t) { - logger.log(SentryLevel.ERROR, "Error submitting NetworkBreadcrumbsIntegration task.", t); + if (callbackRef != null) { + AndroidConnectionStatusProvider.removeNetworkCallback(callbackRef); } } @@ -138,8 +108,6 @@ static final class NetworkBreadcrumbsNetworkCallback extends ConnectivityManager final @NotNull IScopes scopes; final @NotNull BuildInfoProvider buildInfoProvider; - @Nullable Network currentNetwork = null; - @Nullable NetworkCapabilities lastCapabilities = null; long lastCapabilityNanos = 0; final @NotNull SentryDateProvider dateProvider; @@ -156,21 +124,14 @@ static final class NetworkBreadcrumbsNetworkCallback extends ConnectivityManager @Override public void onAvailable(final @NonNull Network network) { - if (network.equals(currentNetwork)) { - return; - } final Breadcrumb breadcrumb = createBreadcrumb("NETWORK_AVAILABLE"); scopes.addBreadcrumb(breadcrumb); - currentNetwork = network; lastCapabilities = null; } @Override public void onCapabilitiesChanged( final @NonNull Network network, final @NonNull NetworkCapabilities networkCapabilities) { - if (!network.equals(currentNetwork)) { - return; - } final long nowNanos = dateProvider.now().nanoTimestamp(); final @Nullable NetworkBreadcrumbConnectionDetail connectionDetail = getNewConnectionDetails( @@ -195,12 +156,8 @@ public void onCapabilitiesChanged( @Override public void onLost(final @NonNull Network network) { - if (!network.equals(currentNetwork)) { - return; - } final Breadcrumb breadcrumb = createBreadcrumb("NETWORK_LOST"); scopes.addBreadcrumb(breadcrumb); - currentNetwork = null; lastCapabilities = null; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java index 41f4f838bf5..8eea2d71047 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java @@ -75,7 +75,9 @@ public void close() throws IOException { @Override public void onConnectionStatusChanged( final @NotNull IConnectionStatusProvider.ConnectionStatus status) { - if (scopes != null && options != null) { + if (scopes != null + && options != null + && status != IConnectionStatusProvider.ConnectionStatus.DISCONNECTED) { sendCachedEnvelopes(scopes, options); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java index 0ea2e1638ab..40e96eeb922 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java @@ -9,6 +9,7 @@ import android.net.NetworkCapabilities; import android.os.Build; import androidx.annotation.NonNull; +import androidx.annotation.RequiresApi; import io.sentry.IConnectionStatusProvider; import io.sentry.ILogger; import io.sentry.ISentryLifecycleToken; @@ -45,6 +46,10 @@ public final class AndroidConnectionStatusProvider implements IConnectionStatusP new AutoClosableReentrantLock(); private static volatile @Nullable ConnectivityManager connectivityManager; + private static final @NotNull AutoClosableReentrantLock childCallbacksLock = + new AutoClosableReentrantLock(); + private static final @NotNull List childCallbacks = new ArrayList<>(); + private static final int[] transports = { NetworkCapabilities.TRANSPORT_WIFI, NetworkCapabilities.TRANSPORT_CELLULAR, @@ -157,11 +162,24 @@ private void ensureNetworkCallbackRegistered() { @Override public void onAvailable(final @NotNull Network network) { currentNetwork = network; + + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + for (final @NotNull NetworkCallback cb : childCallbacks) { + cb.onAvailable(network); + } + } } + @RequiresApi(Build.VERSION_CODES.O) @Override public void onUnavailable() { clearCacheAndNotifyObservers(); + + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + for (final @NotNull NetworkCallback cb : childCallbacks) { + cb.onUnavailable(); + } + } } @Override @@ -170,6 +188,12 @@ public void onLost(final @NotNull Network network) { return; } clearCacheAndNotifyObservers(); + + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + for (final @NotNull NetworkCallback cb : childCallbacks) { + cb.onLost(network); + } + } } private void clearCacheAndNotifyObservers() { @@ -199,6 +223,12 @@ public void onCapabilitiesChanged( return; } updateCacheAndNotifyObservers(network, networkCapabilities); + + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + for (final @NotNull NetworkCallback cb : childCallbacks) { + cb.onCapabilitiesChanged(network, networkCapabilities); + } + } } private void updateCacheAndNotifyObservers( @@ -406,6 +436,9 @@ public void close() { currentNetwork = null; lastCacheUpdateTime = 0; } + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + childCallbacks.clear(); + } try (final @NotNull ISentryLifecycleToken ignored = connectivityManagerLock.acquire()) { connectivityManager = null; @@ -614,8 +647,35 @@ public NetworkCapabilities getCachedNetworkCapabilities() { } } + public static boolean addNetworkCallback( + final @NotNull Context context, + final @NotNull ILogger logger, + final @NotNull BuildInfoProvider buildInfoProvider, + final @NotNull NetworkCallback networkCallback) { + if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.N) { + logger.log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+."); + return false; + } + + if (!Permissions.hasPermission(context, Manifest.permission.ACCESS_NETWORK_STATE)) { + logger.log(SentryLevel.INFO, "No permission (ACCESS_NETWORK_STATE) to check network status."); + return false; + } + + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + childCallbacks.add(networkCallback); + } + return true; + } + + public static void removeNetworkCallback(final @NotNull NetworkCallback networkCallback) { + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + childCallbacks.remove(networkCallback); + } + } + @SuppressLint({"MissingPermission", "NewApi"}) - public static boolean registerNetworkCallback( + static boolean registerNetworkCallback( final @NotNull Context context, final @NotNull ILogger logger, final @NotNull BuildInfoProvider buildInfoProvider, @@ -642,7 +702,7 @@ public static boolean registerNetworkCallback( } @SuppressLint("NewApi") - public static void unregisterNetworkCallback( + static void unregisterNetworkCallback( final @NotNull Context context, final @NotNull ILogger logger, final @NotNull NetworkCallback networkCallback) { @@ -677,7 +737,8 @@ public Thread getInitThread() { } @TestOnly - public static void setConnectivityManager(final @Nullable ConnectivityManager cm) { - connectivityManager = cm; + @NotNull + public static List getChildCallbacks() { + return childCallbacks; } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt index 768fe87fbbd..cbea3499d85 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt @@ -1,8 +1,6 @@ package io.sentry.android.core import android.content.Context -import android.net.ConnectivityManager -import android.net.ConnectivityManager.NetworkCallback import android.net.Network import android.net.NetworkCapabilities import android.os.Build @@ -17,7 +15,6 @@ import io.sentry.TypeCheckHint import io.sentry.android.core.NetworkBreadcrumbsIntegration.NetworkBreadcrumbConnectionDetail import io.sentry.android.core.NetworkBreadcrumbsIntegration.NetworkBreadcrumbsNetworkCallback import io.sentry.android.core.internal.util.AndroidConnectionStatusProvider -import io.sentry.test.DeferredExecutorService import io.sentry.test.ImmediateExecutorService import java.util.concurrent.TimeUnit import kotlin.test.AfterTest @@ -32,7 +29,6 @@ import org.mockito.kotlin.KInOrder import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check -import org.mockito.kotlin.eq import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock import org.mockito.kotlin.never @@ -46,7 +42,6 @@ class NetworkBreadcrumbsIntegrationTest { var options = SentryAndroidOptions() val scopes = mock() val mockBuildInfoProvider = mock() - val connectivityManager = mock() var nowMs: Long = 0 val network = mock() @@ -64,7 +59,7 @@ class NetworkBreadcrumbsIntegrationTest { SentryNanotimeDate(DateUtils.nanosToDate(nowNanos), nowNanos) } } - return NetworkBreadcrumbsIntegration(context, buildInfo, options.logger) + return NetworkBreadcrumbsIntegration(context, buildInfo) } } @@ -73,13 +68,11 @@ class NetworkBreadcrumbsIntegrationTest { @BeforeTest fun `set up`() { whenever(fixture.mockBuildInfoProvider.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) - whenever(fixture.context.getSystemService(eq(Context.CONNECTIVITY_SERVICE))) - .thenReturn(fixture.connectivityManager) } @AfterTest fun `tear down`() { - AndroidConnectionStatusProvider.setConnectivityManager(null) + AndroidConnectionStatusProvider.getChildCallbacks().clear() } @Test @@ -88,7 +81,7 @@ class NetworkBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - verify(fixture.connectivityManager).registerDefaultNetworkCallback(any()) + assertFalse(AndroidConnectionStatusProvider.getChildCallbacks().isEmpty()) assertNotNull(sut.networkCallback) } @@ -98,7 +91,7 @@ class NetworkBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - verify(fixture.connectivityManager, never()).registerDefaultNetworkCallback(any()) + assertTrue(AndroidConnectionStatusProvider.getChildCallbacks().isEmpty()) assertNull(sut.networkCallback) } @@ -110,7 +103,7 @@ class NetworkBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - verify(fixture.connectivityManager, never()).registerDefaultNetworkCallback(any()) + assertTrue(AndroidConnectionStatusProvider.getChildCallbacks().isEmpty()) assertNull(sut.networkCallback) } @@ -121,21 +114,7 @@ class NetworkBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) sut.close() - verify(fixture.connectivityManager).unregisterNetworkCallback(any()) - assertNull(sut.networkCallback) - } - - @Test - fun `When NetworkBreadcrumbsIntegration is closed, it's ignored if not on Android N+`() { - val buildInfo = mock() - whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.M) - val sut = fixture.getSut(buildInfo = buildInfo) - assertNull(sut.networkCallback) - - sut.register(fixture.scopes, fixture.options) - sut.close() - - verify(fixture.connectivityManager, never()).unregisterNetworkCallback(any()) + assertTrue(AndroidConnectionStatusProvider.getChildCallbacks().isEmpty()) assertNull(sut.networkCallback) } @@ -503,22 +482,6 @@ class NetworkBreadcrumbsIntegrationTest { } } - @Test - fun `If integration is opened and closed immediately it still properly unregisters`() { - val executor = DeferredExecutorService() - val sut = fixture.getSut(executor = executor) - - sut.register(fixture.scopes, fixture.options) - sut.close() - - executor.runAll() - - assertNull(sut.networkCallback) - verify(fixture.connectivityManager, never()) - .registerDefaultNetworkCallback(any()) - verify(fixture.connectivityManager, never()).unregisterNetworkCallback(any()) - } - private fun KInOrder.verifyBreadcrumbInOrder( check: (detail: NetworkBreadcrumbConnectionDetail) -> Unit ) { diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt similarity index 96% rename from sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt rename to sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt index a362885d635..6769ac15301 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidConnectionStatusProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt @@ -1,4 +1,4 @@ -package io.sentry.android.core +package io.sentry.android.core.internal.util import android.Manifest import android.content.Context @@ -18,7 +18,7 @@ import android.os.Build import io.sentry.IConnectionStatusProvider import io.sentry.ILogger import io.sentry.SentryOptions -import io.sentry.android.core.internal.util.AndroidConnectionStatusProvider +import io.sentry.android.core.BuildInfoProvider import io.sentry.test.ImmediateExecutorService import io.sentry.transport.ICurrentDateProvider import kotlin.test.AfterTest @@ -37,6 +37,7 @@ import org.mockito.kotlin.mockingDetails import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions +import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever class AndroidConnectionStatusProviderTest { @@ -68,7 +69,7 @@ class AndroidConnectionStatusProviderTest { whenever(connectivityManager.activeNetworkInfo).thenReturn(networkInfo) buildInfo = mock() - whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.M) + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) network = mock() whenever(connectivityManager.activeNetwork).thenReturn(network) @@ -173,12 +174,6 @@ class AndroidConnectionStatusProviderTest { providerWithNullConnectivity.close() } - @Test - fun `When ConnectivityManager is not available, return null for getConnectionType`() { - whenever(contextMock.getSystemService(any())).thenReturn(null) - assertNull(connectionStatusProvider.connectionType) - } - @Test fun `When there's no permission, return null for getConnectionType`() { whenever(contextMock.checkPermission(any(), any(), any())).thenReturn(PERMISSION_DENIED) @@ -227,23 +222,6 @@ class AndroidConnectionStatusProviderTest { assertEquals("cellular", connectionStatusProvider.connectionType) } - @Test - fun `registerNetworkCallback calls connectivityManager registerDefaultNetworkCallback`() { - val buildInfo = mock() - whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) - whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager) - val registered = - AndroidConnectionStatusProvider.registerNetworkCallback( - contextMock, - logger, - buildInfo, - mock(), - ) - - assertTrue(registered) - verify(connectivityManager).registerDefaultNetworkCallback(any()) - } - @Test fun `unregisterNetworkCallback calls connectivityManager unregisterDefaultNetworkCallback`() { whenever(contextMock.getSystemService(any())).thenReturn(connectivityManager) @@ -631,4 +609,27 @@ class AndroidConnectionStatusProviderTest { // changed significantly) verify(observer, times(2)).onConnectionStatusChanged(any()) } + + @Test + fun `childCallbacks receive network events dispatched by provider`() { + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val mainCallback = connectionStatusProvider.networkCallback + assertNotNull(mainCallback) + + // Register a mock child callback + val childCallback = mock() + AndroidConnectionStatusProvider.getChildCallbacks().add(childCallback) + + // Simulate event on available + mainCallback.onAvailable(network) + + // Assert child callback received the event + verify(childCallback).onAvailable(network) + + // Remove it and ensure it no longer receives events + AndroidConnectionStatusProvider.getChildCallbacks().remove(childCallback) + mainCallback.onAvailable(network) + verifyNoMoreInteractions(childCallback) + } } diff --git a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt index 90d830eee47..2ae12c03c3a 100644 --- a/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt +++ b/sentry-android-replay/src/main/java/io/sentry/android/replay/capture/CaptureStrategy.kt @@ -54,7 +54,15 @@ internal interface CaptureStrategy { fun convert(): CaptureStrategy companion object { - private const val BREADCRUMB_START_OFFSET = 100L + private fun Breadcrumb?.isNetworkAvailable(): Boolean = + this != null && + category == "network.event" && + data.getOrElse("action", { null }) == "NETWORK_AVAILABLE" + + private fun Breadcrumb.isNetworkConnectivity(): Boolean = + category == "network.event" && data.containsKey("network_type") + + private const val NETWORK_BREADCRUMB_START_OFFSET = 5000L // 5 minutes, otherwise relay will just drop it. Can prevent the case where the device // time is wrong and the segment is too long. @@ -168,12 +176,18 @@ internal interface CaptureStrategy { } val urls = LinkedList() + var previousCrumb: Breadcrumb? = null breadcrumbs.forEach { breadcrumb -> - // we add some fixed breadcrumb offset to make sure we don't miss any - // breadcrumbs that might be relevant for the current segment, but just happened - // earlier than the current segment (e.g. network connectivity changed) + // we special-case network-reconnected breadcrumb, because there's usually some delay after + // we receive onConnected callback and we resume ongoing replay recording. We still want + // this breadcrumb to be sent with the current segment, hence we give it more room to make + // it into the replay + val isAfterNetworkReconnected = + previousCrumb?.isNetworkAvailable() == true && + breadcrumb.isNetworkConnectivity() && + breadcrumb.timestamp.time + NETWORK_BREADCRUMB_START_OFFSET >= segmentTimestamp.time if ( - (breadcrumb.timestamp.time + BREADCRUMB_START_OFFSET) >= segmentTimestamp.time && + (breadcrumb.timestamp.time >= segmentTimestamp.time || isAfterNetworkReconnected) && breadcrumb.timestamp.time < endTimestamp.time ) { val rrwebEvent = options.replayController.breadcrumbConverter.convert(breadcrumb) @@ -190,6 +204,7 @@ internal interface CaptureStrategy { } } } + previousCrumb = breadcrumb } if (screenAtStart != null && urls.firstOrNull() != screenAtStart) { diff --git a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java index 37ba75783a4..b08e140392c 100644 --- a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java +++ b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java @@ -97,7 +97,9 @@ public void close() throws IOException { @Override public void onConnectionStatusChanged( final @NotNull IConnectionStatusProvider.ConnectionStatus status) { - if (scopes != null && options != null) { + if (scopes != null + && options != null + && status != IConnectionStatusProvider.ConnectionStatus.DISCONNECTED) { sendCachedEnvelopes(scopes, options); } } From 83d80d7bafd9e1aa7ce6a3f703f6c81608db7810 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 18 Jul 2025 16:32:35 +0200 Subject: [PATCH 10/11] Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c2a6fbf4978..9832c9cb7db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - Allow multiple UncaughtExceptionHandlerIntegrations to be active at the same time ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) - Cache network capabilities and status to reduce IPC calls ([#4560](https://github.com/getsentry/sentry-java/pull/4560)) - Deduplicate battery breadcrumbs ([#4561](https://github.com/getsentry/sentry-java/pull/4561)) +- Have single `NetworkCallback` registered at a time to reduce IPC calls ([#4562](https://github.com/getsentry/sentry-java/pull/4562)) ## 8.17.0 From d4d78162e0cb9583ddc4a08d9d246e39bd239218 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Fri, 18 Jul 2025 22:36:51 +0200 Subject: [PATCH 11/11] Remove obsolete tests --- .../core/NetworkBreadcrumbsIntegrationTest.kt | 35 ------------------- 1 file changed, 35 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt index cbea3499d85..4f6ba7fc5f0 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt @@ -27,7 +27,6 @@ import kotlin.test.assertNull import kotlin.test.assertTrue import org.mockito.kotlin.KInOrder import org.mockito.kotlin.any -import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock @@ -137,18 +136,6 @@ class NetworkBreadcrumbsIntegrationTest { ) } - @Test - fun `When connected to the same network without disconnecting from the previous one, only one breadcrumb is captured`() { - val sut = fixture.getSut() - sut.register(fixture.scopes, fixture.options) - val callback = sut.networkCallback - assertNotNull(callback) - callback.onAvailable(fixture.network) - callback.onAvailable(fixture.network) - - verify(fixture.scopes, times(1)).addBreadcrumb(any()) - } - @Test fun `When disconnected from a network, a breadcrumb is captured`() { val sut = fixture.getSut() @@ -171,17 +158,6 @@ class NetworkBreadcrumbsIntegrationTest { ) } - @Test - fun `When disconnected from a network, a breadcrumb is captured only if previously connected to that network`() { - val sut = fixture.getSut() - sut.register(fixture.scopes, fixture.options) - val callback = sut.networkCallback - assertNotNull(callback) - // callback.onAvailable(network) was not called, so no breadcrumb should be captured - callback.onLost(mock()) - verify(fixture.scopes, never()).addBreadcrumb(any()) - } - @Test fun `When a network connection detail changes, a breadcrumb is captured`() { val buildInfo = mock() @@ -220,17 +196,6 @@ class NetworkBreadcrumbsIntegrationTest { ) } - @Test - fun `When a network connection detail changes, a breadcrumb is captured only if previously connected to that network`() { - val sut = fixture.getSut() - sut.register(fixture.scopes, fixture.options) - val callback = sut.networkCallback - assertNotNull(callback) - // callback.onAvailable(network) was not called, so no breadcrumb should be captured - onCapabilitiesChanged(callback, mock()) - verify(fixture.scopes, never()).addBreadcrumb(any(), anyOrNull()) - } - @Test fun `When a network connection detail changes, a new breadcrumb is captured if vpn flag changes`() { val sut = fixture.getSut()