From 2a0d6da985ad283700ae95598c42e74915951254 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 16 Jul 2025 12:34:21 +0200 Subject: [PATCH 01/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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/16] 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 5b3266256330f8da1aca82b707fab065e5193d9f Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 23 Jul 2025 13:42:05 +0200 Subject: [PATCH 11/16] perf(integrations): Use single lifecycle observer --- .../core/AndroidOptionsInitializer.java | 1 + .../android/core/AppLifecycleIntegration.java | 97 +++------- .../java/io/sentry/android/core/AppState.java | 169 +++++++++++++++++- .../sentry/android/core/LifecycleWatcher.java | 14 +- .../SystemEventsBreadcrumbsIntegration.java | 147 ++++----------- .../SystemEventsBreadcrumbsIntegrationTest.kt | 86 +++------ 6 files changed, 251 insertions(+), 263 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 e4874b4309b..71050022f6f 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 @@ -128,6 +128,7 @@ static void loadDefaultAndMetadataOptions( options.setCacheDirPath(getCacheDir(context).getAbsolutePath()); readDefaultOptionValues(options, context, buildInfoProvider); + AppState.getInstance().addLifecycleObserver(options); } @TestOnly diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java index 92bf2203481..44a1ae1a5bd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java @@ -4,10 +4,12 @@ import androidx.lifecycle.ProcessLifecycleOwner; import io.sentry.IScopes; +import io.sentry.ISentryLifecycleToken; import io.sentry.Integration; import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.android.core.internal.util.AndroidThreadChecker; +import io.sentry.util.AutoClosableReentrantLock; import io.sentry.util.Objects; import java.io.Closeable; import java.io.IOException; @@ -17,20 +19,11 @@ public final class AppLifecycleIntegration implements Integration, Closeable { + private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); @TestOnly @Nullable volatile LifecycleWatcher watcher; private @Nullable SentryAndroidOptions options; - private final @NotNull MainLooperHandler handler; - - public AppLifecycleIntegration() { - this(new MainLooperHandler()); - } - - AppLifecycleIntegration(final @NotNull MainLooperHandler handler) { - this.handler = handler; - } - @Override public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions options) { Objects.requireNonNull(scopes, "Scopes are required"); @@ -55,85 +48,47 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions if (this.options.isEnableAutoSessionTracking() || this.options.isEnableAppLifecycleBreadcrumbs()) { - try { - Class.forName("androidx.lifecycle.DefaultLifecycleObserver"); - Class.forName("androidx.lifecycle.ProcessLifecycleOwner"); - if (AndroidThreadChecker.getInstance().isMainThread()) { - addObserver(scopes); - } else { - // some versions of the androidx lifecycle-process require this to be executed on the main - // thread. - handler.post(() -> addObserver(scopes)); + try (final ISentryLifecycleToken ignored = lock.acquire()) { + if (watcher != null) { + return; } - } catch (ClassNotFoundException e) { - options - .getLogger() - .log( - SentryLevel.WARNING, - "androidx.lifecycle is not available, AppLifecycleIntegration won't be installed"); - } catch (IllegalStateException e) { - options - .getLogger() - .log(SentryLevel.ERROR, "AppLifecycleIntegration could not be installed", e); - } - } - } - private void addObserver(final @NotNull IScopes scopes) { - // this should never happen, check added to avoid warnings from NullAway - if (this.options == null) { - return; - } + watcher = + new LifecycleWatcher( + scopes, + this.options.getSessionTrackingIntervalMillis(), + this.options.isEnableAutoSessionTracking(), + this.options.isEnableAppLifecycleBreadcrumbs()); - watcher = - new LifecycleWatcher( - scopes, - this.options.getSessionTrackingIntervalMillis(), - this.options.isEnableAutoSessionTracking(), - this.options.isEnableAppLifecycleBreadcrumbs()); + AppState.getInstance().addAppStateListener(watcher); + } - try { - ProcessLifecycleOwner.get().getLifecycle().addObserver(watcher); options.getLogger().log(SentryLevel.DEBUG, "AppLifecycleIntegration installed."); addIntegrationToSdkVersion("AppLifecycle"); - } catch (Throwable e) { - // This is to handle a potential 'AbstractMethodError' gracefully. The error is triggered in - // connection with conflicting dependencies of the androidx.lifecycle. - // //See the issue here: https://github.com/getsentry/sentry-java/pull/2228 - watcher = null; - options - .getLogger() - .log( - SentryLevel.ERROR, - "AppLifecycleIntegration failed to get Lifecycle and could not be installed.", - e); } } private void removeObserver() { - final @Nullable LifecycleWatcher watcherRef = watcher; + final @Nullable LifecycleWatcher watcherRef; + try (final ISentryLifecycleToken ignored = lock.acquire()) { + watcherRef = watcher; + watcher = null; + } + if (watcherRef != null) { - ProcessLifecycleOwner.get().getLifecycle().removeObserver(watcherRef); + AppState.getInstance().removeAppStateListener(watcherRef); if (options != null) { options.getLogger().log(SentryLevel.DEBUG, "AppLifecycleIntegration removed."); } } - watcher = null; } @Override public void close() throws IOException { - if (watcher == null) { - return; - } - if (AndroidThreadChecker.getInstance().isMainThread()) { - removeObserver(); - } else { - // some versions of the androidx lifecycle-process require this to be executed on the main - // thread. - // avoid method refs on Android due to some issues with older AGP setups - // noinspection Convert2MethodRef - handler.post(() -> removeObserver()); - } + removeObserver(); + // TODO: probably should move it to Scopes.close(), but that'd require a new interface and + // different implementations for Java and Android. This is probably fine like this too, because + // integrations are closed in the same place + AppState.getInstance().removeLifecycleObserver(); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java index d9633aed540..e15a69257cd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java @@ -1,7 +1,20 @@ package io.sentry.android.core; +import androidx.annotation.NonNull; +import androidx.lifecycle.DefaultLifecycleObserver; +import androidx.lifecycle.Lifecycle; +import androidx.lifecycle.LifecycleOwner; +import androidx.lifecycle.ProcessLifecycleOwner; +import io.sentry.ILogger; import io.sentry.ISentryLifecycleToken; +import io.sentry.NoOpLogger; +import io.sentry.SentryLevel; +import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.util.AutoClosableReentrantLock; +import java.io.Closeable; +import java.io.IOException; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -9,9 +22,11 @@ /** AppState holds the state of the App, e.g. whether the app is in background/foreground, etc. */ @ApiStatus.Internal -public final class AppState { +public final class AppState implements Closeable { private static @NotNull AppState instance = new AppState(); private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); + volatile LifecycleObserver lifecycleObserver; + MainLooperHandler handler = new MainLooperHandler(); private AppState() {} @@ -19,7 +34,7 @@ private AppState() {} return instance; } - private @Nullable Boolean inBackground = null; + private volatile @Nullable Boolean inBackground = null; @TestOnly void resetInstance() { @@ -31,8 +46,156 @@ void resetInstance() { } void setInBackground(final boolean inBackground) { + this.inBackground = inBackground; + } + + void addAppStateListener(final @NotNull AppStateListener listener) { + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + ensureLifecycleObserver(NoOpLogger.getInstance()); + + lifecycleObserver.listeners.add(listener); + } + } + + void removeAppStateListener(final @NotNull AppStateListener listener) { + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + if (lifecycleObserver != null) { + lifecycleObserver.listeners.remove(listener); + } + } + } + + void addLifecycleObserver(final @Nullable SentryAndroidOptions options) { + if (lifecycleObserver != null) { + return; + } + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { - this.inBackground = inBackground; + ensureLifecycleObserver(options != null ? options.getLogger() : NoOpLogger.getInstance()); + } + } + + private void ensureLifecycleObserver(final @NotNull ILogger logger) { + if (lifecycleObserver != null) { + return; } + try { + Class.forName("androidx.lifecycle.ProcessLifecycleOwner"); + // create it right away, so it's available in addAppStateListener in case it's posted to main thread + lifecycleObserver = new LifecycleObserver(); + + if (AndroidThreadChecker.getInstance().isMainThread()) { + addObserverInternal(logger); + } else { + // some versions of the androidx lifecycle-process require this to be executed on the main + // thread. + handler.post(() -> addObserverInternal(logger)); + } + } catch (ClassNotFoundException e) { + logger + .log( + SentryLevel.WARNING, + "androidx.lifecycle is not available, some features might not be properly working," + + "e.g. Session Tracking, Network and System Events breadcrumbs, etc."); + } catch (Throwable e) { + logger + .log( + SentryLevel.ERROR, + "AppState could not register lifecycle observer", + e); + } + } + + private void addObserverInternal(final @NotNull ILogger logger) { + final @Nullable LifecycleObserver observerRef = lifecycleObserver; + try { + // might already be unregistered/removed so we have to check for nullability + if (observerRef != null) { + ProcessLifecycleOwner.get().getLifecycle().addObserver(observerRef); + } + } catch (Throwable e) { + // This is to handle a potential 'AbstractMethodError' gracefully. The error is triggered in + // connection with conflicting dependencies of the androidx.lifecycle. + // //See the issue here: https://github.com/getsentry/sentry-java/pull/2228 + lifecycleObserver = null; + logger + .log( + SentryLevel.ERROR, + "AppState failed to get Lifecycle and could not install lifecycle observer.", + e); + } + } + + void removeLifecycleObserver() { + if (lifecycleObserver == null) { + return; + } + + final @Nullable LifecycleObserver ref; + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + ref = lifecycleObserver; + lifecycleObserver.listeners.clear(); + lifecycleObserver = null; + } + + if (AndroidThreadChecker.getInstance().isMainThread()) { + removeObserverInternal(ref); + } else { + // some versions of the androidx lifecycle-process require this to be executed on the main + // thread. + // avoid method refs on Android due to some issues with older AGP setups + // noinspection Convert2MethodRef + handler.post(() -> removeObserverInternal(ref)); + } + } + + private void removeObserverInternal(final @Nullable LifecycleObserver ref) { + if (ref != null) { + ProcessLifecycleOwner.get().getLifecycle().removeObserver(ref); + } + } + + @Override + public void close() throws IOException { + removeLifecycleObserver(); + } + + static final class LifecycleObserver implements DefaultLifecycleObserver { + final List listeners = new CopyOnWriteArrayList() { + @Override + public boolean add(AppStateListener appStateListener) { + // notify the listeners immediately to let them "catch up" with the current state (mimics the behavior of androidx.lifecycle) + Lifecycle.State currentState = ProcessLifecycleOwner.get().getLifecycle().getCurrentState(); + if (currentState.isAtLeast(Lifecycle.State.STARTED)) { + appStateListener.onForeground(); + } else { + appStateListener.onBackground(); + } + return super.add(appStateListener); + } + }; + + @Override + public void onStart(@NonNull LifecycleOwner owner) { + for (AppStateListener listener : listeners) { + listener.onForeground(); + } + AppState.getInstance().setInBackground(false); + } + + @Override + public void onStop(@NonNull LifecycleOwner owner) { + for (AppStateListener listener : listeners) { + listener.onBackground(); + } + AppState.getInstance().setInBackground(true); + } + } + + // If necessary, we can adjust this and add other callbacks in the future + public interface AppStateListener { + void onForeground(); + + void onBackground(); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java index 89d78193207..a5e4d398e74 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/LifecycleWatcher.java @@ -1,7 +1,5 @@ package io.sentry.android.core; -import androidx.lifecycle.DefaultLifecycleObserver; -import androidx.lifecycle.LifecycleOwner; import io.sentry.Breadcrumb; import io.sentry.IScopes; import io.sentry.ISentryLifecycleToken; @@ -17,7 +15,7 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; -final class LifecycleWatcher implements DefaultLifecycleObserver { +final class LifecycleWatcher implements AppState.AppStateListener { private final AtomicLong lastUpdatedSession = new AtomicLong(0L); @@ -58,15 +56,10 @@ final class LifecycleWatcher implements DefaultLifecycleObserver { this.currentDateProvider = currentDateProvider; } - // App goes to foreground @Override - public void onStart(final @NotNull LifecycleOwner owner) { + public void onForeground() { startSession(); addAppBreadcrumb("foreground"); - - // Consider using owner.getLifecycle().getCurrentState().isAtLeast(Lifecycle.State.RESUMED); - // in the future. - AppState.getInstance().setInBackground(false); } private void startSession() { @@ -99,14 +92,13 @@ private void startSession() { // App went to background and triggered this callback after 700ms // as no new screen was shown @Override - public void onStop(final @NotNull LifecycleOwner owner) { + public void onBackground() { final long currentTimeMillis = currentDateProvider.getCurrentTimeMillis(); this.lastUpdatedSession.set(currentTimeMillis); scopes.getOptions().getReplayController().pause(); scheduleEndSession(); - AppState.getInstance().setInBackground(true); addAppBreadcrumb("background"); } 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 d681f08429a..3ab4bb4e6c3 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 @@ -25,9 +25,6 @@ import android.content.Intent; import android.content.IntentFilter; import android.os.Bundle; -import androidx.annotation.NonNull; -import androidx.lifecycle.DefaultLifecycleObserver; -import androidx.lifecycle.LifecycleOwner; import androidx.lifecycle.ProcessLifecycleOwner; import io.sentry.Breadcrumb; import io.sentry.Hint; @@ -52,16 +49,13 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; -public final class SystemEventsBreadcrumbsIntegration implements Integration, Closeable { +public final class SystemEventsBreadcrumbsIntegration implements Integration, Closeable, + AppState.AppStateListener { private final @NotNull Context context; @TestOnly @Nullable volatile SystemEventsBroadcastReceiver receiver; - @TestOnly @Nullable volatile ReceiverLifecycleHandler lifecycleHandler; - - private final @NotNull MainLooperHandler handler; - private @Nullable SentryAndroidOptions options; private @Nullable IScopes scopes; @@ -76,18 +70,10 @@ public SystemEventsBreadcrumbsIntegration(final @NotNull Context context) { this(context, getDefaultActionsInternal()); } - private SystemEventsBreadcrumbsIntegration( - final @NotNull Context context, final @NotNull String[] actions) { - this(context, actions, new MainLooperHandler()); - } - SystemEventsBreadcrumbsIntegration( - final @NotNull Context context, - final @NotNull String[] actions, - final @NotNull MainLooperHandler handler) { + final @NotNull Context context, final @NotNull String[] actions) { this.context = ContextUtils.getApplicationContext(context); this.actions = actions; - this.handler = handler; } public SystemEventsBreadcrumbsIntegration( @@ -95,7 +81,6 @@ public SystemEventsBreadcrumbsIntegration( this.context = ContextUtils.getApplicationContext(context); this.actions = new String[actions.size()]; actions.toArray(this.actions); - this.handler = new MainLooperHandler(); } @Override @@ -115,7 +100,7 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions this.options.isEnableSystemEventBreadcrumbs()); if (this.options.isEnableSystemEventBreadcrumbs()) { - addLifecycleObserver(this.options); + AppState.getInstance().addAppStateListener(this); registerReceiver(this.scopes, this.options, /* reportAsNewIntegration= */ true); } } @@ -129,10 +114,8 @@ private void registerReceiver( return; } - try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) { - if (isClosed || isStopped || receiver != null) { - return; - } + if (isClosed || isStopped || receiver != null) { + return; } try { @@ -183,88 +166,22 @@ private void registerReceiver( } private void unregisterReceiver() { - final @Nullable SystemEventsBroadcastReceiver receiverRef; - try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) { - isStopped = true; - receiverRef = receiver; - receiver = null; - } - - if (receiverRef != null) { - context.unregisterReceiver(receiverRef); + if (options == null) { + return; } - } - // TODO: this duplicates a lot of AppLifecycleIntegration. We should register once on init - // and multiplex to different listeners rather. - private void addLifecycleObserver(final @NotNull SentryAndroidOptions options) { - try { - Class.forName("androidx.lifecycle.DefaultLifecycleObserver"); - Class.forName("androidx.lifecycle.ProcessLifecycleOwner"); - if (AndroidThreadChecker.getInstance().isMainThread()) { - addObserverInternal(options); - } else { - // some versions of the androidx lifecycle-process require this to be executed on the main - // thread. - handler.post(() -> addObserverInternal(options)); + options.getExecutorService().submit(() -> { + final @Nullable SystemEventsBroadcastReceiver receiverRef; + try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) { + isStopped = true; + receiverRef = receiver; + receiver = null; } - } catch (ClassNotFoundException e) { - options - .getLogger() - .log( - SentryLevel.WARNING, - "androidx.lifecycle is not available, SystemEventsBreadcrumbsIntegration won't be able" - + " to register/unregister an internal BroadcastReceiver. This may result in an" - + " increased ANR rate on Android 14 and above."); - } catch (Throwable e) { - options - .getLogger() - .log( - SentryLevel.ERROR, - "SystemEventsBreadcrumbsIntegration could not register lifecycle observer", - e); - } - } - private void addObserverInternal(final @NotNull SentryAndroidOptions options) { - lifecycleHandler = new ReceiverLifecycleHandler(); - - try { - ProcessLifecycleOwner.get().getLifecycle().addObserver(lifecycleHandler); - } catch (Throwable e) { - // This is to handle a potential 'AbstractMethodError' gracefully. The error is triggered in - // connection with conflicting dependencies of the androidx.lifecycle. - // //See the issue here: https://github.com/getsentry/sentry-java/pull/2228 - lifecycleHandler = null; - options - .getLogger() - .log( - SentryLevel.ERROR, - "SystemEventsBreadcrumbsIntegration failed to get Lifecycle and could not install lifecycle observer.", - e); - } - } - - private void removeLifecycleObserver() { - if (lifecycleHandler != null) { - if (AndroidThreadChecker.getInstance().isMainThread()) { - removeObserverInternal(); - } else { - // some versions of the androidx lifecycle-process require this to be executed on the main - // thread. - // avoid method refs on Android due to some issues with older AGP setups - // noinspection Convert2MethodRef - handler.post(() -> removeObserverInternal()); + if (receiverRef != null) { + context.unregisterReceiver(receiverRef); } - } - } - - private void removeObserverInternal() { - final @Nullable ReceiverLifecycleHandler watcherRef = lifecycleHandler; - if (watcherRef != null) { - ProcessLifecycleOwner.get().getLifecycle().removeObserver(watcherRef); - } - lifecycleHandler = null; + }); } @Override @@ -274,11 +191,11 @@ public void close() throws IOException { filter = null; } - removeLifecycleObserver(); + AppState.getInstance().removeAppStateListener(this); unregisterReceiver(); if (options != null) { - options.getLogger().log(SentryLevel.DEBUG, "SystemEventsBreadcrumbsIntegration remove."); + options.getLogger().log(SentryLevel.DEBUG, "SystemEventsBreadcrumbsIntegration removed."); } } @@ -311,24 +228,20 @@ public void close() throws IOException { return actions; } - final class ReceiverLifecycleHandler implements DefaultLifecycleObserver { - @Override - public void onStart(@NonNull LifecycleOwner owner) { - if (scopes == null || options == null) { - return; - } + @Override + public void onForeground() { + if (scopes == null || options == null) { + return; + } - try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) { - isStopped = false; - } + isStopped = false; - registerReceiver(scopes, options, /* reportAsNewIntegration= */ false); - } + registerReceiver(scopes, options, /* reportAsNewIntegration= */ false); + } - @Override - public void onStop(@NonNull LifecycleOwner owner) { - unregisterReceiver(); - } + @Override + public void onBackground() { + unregisterReceiver(); } static final class SystemEventsBroadcastReceiver extends BroadcastReceiver { 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 d35df7c8444..0784621c28a 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 @@ -30,6 +30,8 @@ import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever import org.robolectric.Shadows.shadowOf import org.robolectric.annotation.Config +import kotlin.test.BeforeTest +import kotlin.test.assertTrue @RunWith(AndroidJUnit4::class) @Config(sdk = [Build.VERSION_CODES.TIRAMISU]) @@ -38,14 +40,11 @@ class SystemEventsBreadcrumbsIntegrationTest { val context = mock() var options = SentryAndroidOptions() val scopes = mock() - lateinit var handler: MainLooperHandler fun getSut( enableSystemEventBreadcrumbs: Boolean = true, executorService: ISentryExecutorService = ImmediateExecutorService(), - mockHandler: Boolean = true, ): SystemEventsBreadcrumbsIntegration { - handler = if (mockHandler) mock() else MainLooperHandler() options = SentryAndroidOptions().apply { isEnableSystemEventBreadcrumbs = enableSystemEventBreadcrumbs @@ -54,13 +53,17 @@ class SystemEventsBreadcrumbsIntegrationTest { return SystemEventsBreadcrumbsIntegration( context, SystemEventsBreadcrumbsIntegration.getDefaultActions().toTypedArray(), - handler, ) } } private val fixture = Fixture() + @BeforeTest + fun `set up`() { + AppState.getInstance().resetInstance() + } + @Test fun `When system events breadcrumb is enabled, it registers callback`() { val sut = fixture.getSut() @@ -337,82 +340,44 @@ class SystemEventsBreadcrumbsIntegrationTest { } @Test - fun `When integration is added, lifecycle handler should be started`() { + fun `When integration is added, should subscribe for app state events`() { val sut = fixture.getSut() sut.register(fixture.scopes, fixture.options) - assertNotNull(sut.lifecycleHandler) + assertTrue(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) } @Test - fun `When system events breadcrumbs are disabled, lifecycle handler should not be started`() { + fun `When system events breadcrumbs are disabled, should not subscribe for app state events`() { val sut = fixture.getSut() fixture.options.apply { isEnableSystemEventBreadcrumbs = false } sut.register(fixture.scopes, fixture.options) - assertNull(sut.lifecycleHandler) + assertFalse(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) } @Test - fun `When integration is closed, lifecycle handler should be closed`() { + fun `When integration is closed, should unsubscribe from app state events`() { val sut = fixture.getSut() sut.register(fixture.scopes, fixture.options) - assertNotNull(sut.lifecycleHandler) + assertTrue(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) sut.close() - assertNull(sut.lifecycleHandler) + assertFalse(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) } @Test - fun `When integration is registered from a background thread, post on the main thread`() { - val sut = fixture.getSut() - val latch = CountDownLatch(1) - - Thread { - sut.register(fixture.scopes, fixture.options) - latch.countDown() - } - .start() - - latch.await() - - verify(fixture.handler).post(any()) - } - - @Test - fun `When integration is closed from a background thread, post on the main thread`() { + fun `When integration is closed from a background thread, unsubscribes from app events`() { val sut = fixture.getSut() val latch = CountDownLatch(1) sut.register(fixture.scopes, fixture.options) - assertNotNull(sut.lifecycleHandler) - - Thread { - sut.close() - latch.countDown() - } - .start() - - latch.await() - - verify(fixture.handler).post(any()) - } - - @Test - fun `When integration is closed from a background thread, watcher is set to null`() { - val sut = fixture.getSut(mockHandler = false) - val latch = CountDownLatch(1) - - sut.register(fixture.scopes, fixture.options) - - assertNotNull(sut.lifecycleHandler) - Thread { sut.close() latch.countDown() @@ -424,7 +389,7 @@ class SystemEventsBreadcrumbsIntegrationTest { // ensure all messages on main looper got processed shadowOf(Looper.getMainLooper()).idle() - assertNull(sut.lifecycleHandler) + assertFalse(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) } @Test @@ -433,7 +398,7 @@ class SystemEventsBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - sut.lifecycleHandler!!.onStop(mock()) + sut.onBackground() verify(fixture.context).unregisterReceiver(any()) assertNull(sut.receiver) @@ -446,8 +411,8 @@ class SystemEventsBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) verify(fixture.context).registerReceiver(any(), any(), any()) - sut.lifecycleHandler!!.onStop(mock()) - sut.lifecycleHandler!!.onStart(mock()) + sut.onBackground() + sut.onForeground() verify(fixture.context, times(2)).registerReceiver(any(), any(), any()) assertNotNull(sut.receiver) @@ -461,7 +426,7 @@ class SystemEventsBreadcrumbsIntegrationTest { verify(fixture.context).registerReceiver(any(), any(), any()) val receiver = sut.receiver - sut.lifecycleHandler!!.onStart(mock()) + sut.onForeground() assertEquals(receiver, sut.receiver) } @@ -473,10 +438,10 @@ class SystemEventsBreadcrumbsIntegrationTest { deferredExecutorService.runAll() assertNotNull(sut.receiver) - sut.lifecycleHandler!!.onStop(mock()) - sut.lifecycleHandler!!.onStart(mock()) + sut.onBackground() + sut.onForeground() assertNull(sut.receiver) - sut.lifecycleHandler!!.onStop(mock()) + sut.onBackground() deferredExecutorService.runAll() assertNull(sut.receiver) } @@ -486,7 +451,7 @@ class SystemEventsBreadcrumbsIntegrationTest { val deferredExecutorService = DeferredExecutorService() val latch = CountDownLatch(1) - val sut = fixture.getSut(executorService = deferredExecutorService, mockHandler = false) + val sut = fixture.getSut(executorService = deferredExecutorService) sut.register(fixture.scopes, fixture.options) deferredExecutorService.runAll() assertNotNull(sut.receiver) @@ -499,13 +464,12 @@ class SystemEventsBreadcrumbsIntegrationTest { latch.await() - sut.lifecycleHandler!!.onStart(mock()) + sut.onForeground() assertNull(sut.receiver) deferredExecutorService.runAll() shadowOf(Looper.getMainLooper()).idle() assertNull(sut.receiver) - assertNull(sut.lifecycleHandler) } } From 5c5238a9a27c61d531eb165839a1d7b09af32945 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 24 Jul 2025 12:41:36 +0200 Subject: [PATCH 12/16] Add tests --- .../api/sentry-android-core.api | 12 +- .../core/AndroidOptionsInitializer.java | 2 +- .../android/core/AppLifecycleIntegration.java | 4 +- .../java/io/sentry/android/core/AppState.java | 67 ++--- .../SystemEventsBreadcrumbsIntegration.java | 31 +- .../core/AppLifecycleIntegrationTest.kt | 54 +--- .../io/sentry/android/core/AppStateTest.kt | 283 ++++++++++++++++++ .../SystemEventsBreadcrumbsIntegrationTest.kt | 43 ++- 8 files changed, 393 insertions(+), 103 deletions(-) create mode 100644 sentry-android-core/src/test/java/io/sentry/android/core/AppStateTest.kt diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 987f13f540f..c3c71aa75f4 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -166,11 +166,17 @@ public final class io/sentry/android/core/AppLifecycleIntegration : io/sentry/In public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V } -public final class io/sentry/android/core/AppState { +public final class io/sentry/android/core/AppState : java/io/Closeable { + public fun close ()V public static fun getInstance ()Lio/sentry/android/core/AppState; public fun isInBackground ()Ljava/lang/Boolean; } +public abstract interface class io/sentry/android/core/AppState$AppStateListener { + public abstract fun onBackground ()V + public abstract fun onForeground ()V +} + public final class io/sentry/android/core/BuildConfig { public static final field BUILD_TYPE Ljava/lang/String; public static final field DEBUG Z @@ -420,11 +426,13 @@ public class io/sentry/android/core/SpanFrameMetricsCollector : io/sentry/IPerfo public fun onSpanStarted (Lio/sentry/ISpan;)V } -public final class io/sentry/android/core/SystemEventsBreadcrumbsIntegration : io/sentry/Integration, java/io/Closeable { +public final class io/sentry/android/core/SystemEventsBreadcrumbsIntegration : io/sentry/Integration, io/sentry/android/core/AppState$AppStateListener, java/io/Closeable { public fun (Landroid/content/Context;)V public fun (Landroid/content/Context;Ljava/util/List;)V public fun close ()V public static fun getDefaultActions ()Ljava/util/List; + public fun onBackground ()V + public fun onForeground ()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 71050022f6f..40a0727a817 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 @@ -128,7 +128,7 @@ static void loadDefaultAndMetadataOptions( options.setCacheDirPath(getCacheDir(context).getAbsolutePath()); readDefaultOptionValues(options, context, buildInfoProvider); - AppState.getInstance().addLifecycleObserver(options); + AppState.getInstance().registerLifecycleObserver(options); } @TestOnly diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java index 44a1ae1a5bd..9fd90b23099 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppLifecycleIntegration.java @@ -2,13 +2,11 @@ import static io.sentry.util.IntegrationUtils.addIntegrationToSdkVersion; -import androidx.lifecycle.ProcessLifecycleOwner; import io.sentry.IScopes; import io.sentry.ISentryLifecycleToken; import io.sentry.Integration; import io.sentry.SentryLevel; import io.sentry.SentryOptions; -import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.util.AutoClosableReentrantLock; import io.sentry.util.Objects; import java.io.Closeable; @@ -89,6 +87,6 @@ public void close() throws IOException { // TODO: probably should move it to Scopes.close(), but that'd require a new interface and // different implementations for Java and Android. This is probably fine like this too, because // integrations are closed in the same place - AppState.getInstance().removeLifecycleObserver(); + AppState.getInstance().unregisterLifecycleObserver(); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java index e15a69257cd..855385631d0 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java @@ -2,7 +2,6 @@ import androidx.annotation.NonNull; import androidx.lifecycle.DefaultLifecycleObserver; -import androidx.lifecycle.Lifecycle; import androidx.lifecycle.LifecycleOwner; import androidx.lifecycle.ProcessLifecycleOwner; import io.sentry.ILogger; @@ -65,7 +64,7 @@ void removeAppStateListener(final @NotNull AppStateListener listener) { } } - void addLifecycleObserver(final @Nullable SentryAndroidOptions options) { + void registerLifecycleObserver(final @Nullable SentryAndroidOptions options) { if (lifecycleObserver != null) { return; } @@ -81,7 +80,8 @@ private void ensureLifecycleObserver(final @NotNull ILogger logger) { } try { Class.forName("androidx.lifecycle.ProcessLifecycleOwner"); - // create it right away, so it's available in addAppStateListener in case it's posted to main thread + // create it right away, so it's available in addAppStateListener in case it's posted to main + // thread lifecycleObserver = new LifecycleObserver(); if (AndroidThreadChecker.getInstance().isMainThread()) { @@ -92,17 +92,12 @@ private void ensureLifecycleObserver(final @NotNull ILogger logger) { handler.post(() -> addObserverInternal(logger)); } } catch (ClassNotFoundException e) { - logger - .log( - SentryLevel.WARNING, - "androidx.lifecycle is not available, some features might not be properly working," - + "e.g. Session Tracking, Network and System Events breadcrumbs, etc."); + logger.log( + SentryLevel.WARNING, + "androidx.lifecycle is not available, some features might not be properly working," + + "e.g. Session Tracking, Network and System Events breadcrumbs, etc."); } catch (Throwable e) { - logger - .log( - SentryLevel.ERROR, - "AppState could not register lifecycle observer", - e); + logger.log(SentryLevel.ERROR, "AppState could not register lifecycle observer", e); } } @@ -118,15 +113,14 @@ private void addObserverInternal(final @NotNull ILogger logger) { // connection with conflicting dependencies of the androidx.lifecycle. // //See the issue here: https://github.com/getsentry/sentry-java/pull/2228 lifecycleObserver = null; - logger - .log( - SentryLevel.ERROR, - "AppState failed to get Lifecycle and could not install lifecycle observer.", - e); + logger.log( + SentryLevel.ERROR, + "AppState failed to get Lifecycle and could not install lifecycle observer.", + e); } } - void removeLifecycleObserver() { + void unregisterLifecycleObserver() { if (lifecycleObserver == null) { return; } @@ -157,30 +151,31 @@ private void removeObserverInternal(final @Nullable LifecycleObserver ref) { @Override public void close() throws IOException { - removeLifecycleObserver(); + unregisterLifecycleObserver(); } - static final class LifecycleObserver implements DefaultLifecycleObserver { - final List listeners = new CopyOnWriteArrayList() { - @Override - public boolean add(AppStateListener appStateListener) { - // notify the listeners immediately to let them "catch up" with the current state (mimics the behavior of androidx.lifecycle) - Lifecycle.State currentState = ProcessLifecycleOwner.get().getLifecycle().getCurrentState(); - if (currentState.isAtLeast(Lifecycle.State.STARTED)) { - appStateListener.onForeground(); - } else { - appStateListener.onBackground(); - } - return super.add(appStateListener); - } - }; + final class LifecycleObserver implements DefaultLifecycleObserver { + final List listeners = + new CopyOnWriteArrayList() { + @Override + public boolean add(AppStateListener appStateListener) { + // notify the listeners immediately to let them "catch up" with the current state + // (mimics the behavior of androidx.lifecycle) + if (Boolean.FALSE.equals(inBackground)) { + appStateListener.onForeground(); + } else if (Boolean.TRUE.equals(inBackground)) { + appStateListener.onBackground(); + } + return super.add(appStateListener); + } + }; @Override public void onStart(@NonNull LifecycleOwner owner) { for (AppStateListener listener : listeners) { listener.onForeground(); } - AppState.getInstance().setInBackground(false); + setInBackground(false); } @Override @@ -188,7 +183,7 @@ public void onStop(@NonNull LifecycleOwner owner) { for (AppStateListener listener : listeners) { listener.onBackground(); } - AppState.getInstance().setInBackground(true); + setInBackground(true); } } 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 3ab4bb4e6c3..802d40d3d28 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 @@ -25,7 +25,6 @@ import android.content.Intent; import android.content.IntentFilter; import android.os.Bundle; -import androidx.lifecycle.ProcessLifecycleOwner; import io.sentry.Breadcrumb; import io.sentry.Hint; import io.sentry.IScopes; @@ -34,7 +33,6 @@ import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.android.core.internal.util.AndroidCurrentDateProvider; -import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.android.core.internal.util.Debouncer; import io.sentry.util.AutoClosableReentrantLock; import io.sentry.util.Objects; @@ -49,8 +47,8 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; -public final class SystemEventsBreadcrumbsIntegration implements Integration, Closeable, - AppState.AppStateListener { +public final class SystemEventsBreadcrumbsIntegration + implements Integration, Closeable, AppState.AppStateListener { private final @NotNull Context context; @@ -170,18 +168,21 @@ private void unregisterReceiver() { return; } - options.getExecutorService().submit(() -> { - final @Nullable SystemEventsBroadcastReceiver receiverRef; - try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) { - isStopped = true; - receiverRef = receiver; - receiver = null; - } + options + .getExecutorService() + .submit( + () -> { + final @Nullable SystemEventsBroadcastReceiver receiverRef; + try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) { + isStopped = true; + receiverRef = receiver; + receiver = null; + } - if (receiverRef != null) { - context.unregisterReceiver(receiverRef); - } - }); + if (receiverRef != null) { + context.unregisterReceiver(receiverRef); + } + }); } @Override diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AppLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AppLifecycleIntegrationTest.kt index 6b2cafabe7f..896673085c2 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AppLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AppLifecycleIntegrationTest.kt @@ -8,21 +8,17 @@ import kotlin.test.Test import kotlin.test.assertNotNull import kotlin.test.assertNull import org.junit.runner.RunWith -import org.mockito.kotlin.any import org.mockito.kotlin.mock -import org.mockito.kotlin.verify import org.robolectric.Shadows.shadowOf @RunWith(AndroidJUnit4::class) class AppLifecycleIntegrationTest { private class Fixture { val scopes = mock() - lateinit var handler: MainLooperHandler val options = SentryAndroidOptions() - fun getSut(mockHandler: Boolean = true): AppLifecycleIntegration { - handler = if (mockHandler) mock() else MainLooperHandler() - return AppLifecycleIntegration(handler) + fun getSut(): AppLifecycleIntegration { + return AppLifecycleIntegration() } } @@ -64,23 +60,7 @@ class AppLifecycleIntegrationTest { } @Test - fun `When AppLifecycleIntegration is registered from a background thread, post on the main thread`() { - val sut = fixture.getSut() - val latch = CountDownLatch(1) - - Thread { - sut.register(fixture.scopes, fixture.options) - latch.countDown() - } - .start() - - latch.await() - - verify(fixture.handler).post(any()) - } - - @Test - fun `When AppLifecycleIntegration is closed from a background thread, post on the main thread`() { + fun `When AppLifecycleIntegration is closed from a background thread, watcher is set to null`() { val sut = fixture.getSut() val latch = CountDownLatch(1) @@ -96,29 +76,25 @@ class AppLifecycleIntegrationTest { latch.await() - verify(fixture.handler).post(any()) + // ensure all messages on main looper got processed + shadowOf(Looper.getMainLooper()).idle() + + assertNull(sut.watcher) } @Test - fun `When AppLifecycleIntegration is closed from a background thread, watcher is set to null`() { - val sut = fixture.getSut(mockHandler = false) - val latch = CountDownLatch(1) + fun `When AppLifecycleIntegration is closed, AppState unregisterLifecycleObserver is called`() { + val sut = fixture.getSut() + val appState = AppState.getInstance() sut.register(fixture.scopes, fixture.options) - assertNotNull(sut.watcher) + // Verify that lifecycleObserver is not null after registration + assertNotNull(appState.lifecycleObserver) - Thread { - sut.close() - latch.countDown() - } - .start() - - latch.await() - - // ensure all messages on main looper got processed - shadowOf(Looper.getMainLooper()).idle() + sut.close() - assertNull(sut.watcher) + // Verify that lifecycleObserver is null after unregistering + assertNull(appState.lifecycleObserver) } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AppStateTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AppStateTest.kt new file mode 100644 index 00000000000..efef483daf4 --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AppStateTest.kt @@ -0,0 +1,283 @@ +package io.sentry.android.core + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.android.core.AppState.AppStateListener +import io.sentry.android.core.internal.util.AndroidThreadChecker +import java.util.concurrent.CountDownLatch +import kotlin.test.AfterTest +import kotlin.test.BeforeTest +import kotlin.test.assertFalse +import kotlin.test.assertNotNull +import kotlin.test.assertNull +import kotlin.test.assertSame +import kotlin.test.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.MockedStatic +import org.mockito.Mockito.mockStatic +import org.mockito.kotlin.any +import org.mockito.kotlin.mock +import org.mockito.kotlin.never +import org.mockito.kotlin.times +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever + +@RunWith(AndroidJUnit4::class) +class AppStateTest { + + private class Fixture { + val mockThreadChecker: AndroidThreadChecker = mock() + val mockHandler: MainLooperHandler = mock() + val options = SentryAndroidOptions() + val listener: AppStateListener = mock() + lateinit var androidThreadCheckerMock: MockedStatic + + fun getSut(isMainThread: Boolean = true): AppState { + val appState = AppState.getInstance() + whenever(mockThreadChecker.isMainThread).thenReturn(isMainThread) + appState.handler = mockHandler + return appState + } + + fun createListener(): AppStateListener = mock() + } + + private val fixture = Fixture() + + @BeforeTest + fun `set up`() { + AppState.getInstance().resetInstance() + + // Mock AndroidThreadChecker + fixture.androidThreadCheckerMock = mockStatic(AndroidThreadChecker::class.java) + fixture.androidThreadCheckerMock + .`when` { AndroidThreadChecker.getInstance() } + .thenReturn(fixture.mockThreadChecker) + } + + @AfterTest + fun `tear down`() { + fixture.androidThreadCheckerMock.close() + } + + @Test + fun `getInstance returns singleton instance`() { + val instance1 = fixture.getSut() + val instance2 = fixture.getSut() + + assertSame(instance1, instance2) + } + + @Test + fun `resetInstance creates new instance`() { + val sut = fixture.getSut() + sut.setInBackground(true) + + sut.resetInstance() + + val newInstance = fixture.getSut() + assertNull(newInstance.isInBackground()) + } + + @Test + fun `isInBackground returns null initially`() { + val sut = fixture.getSut() + + assertNull(sut.isInBackground()) + } + + @Test + fun `setInBackground updates state`() { + val sut = fixture.getSut() + + sut.setInBackground(true) + assertTrue(sut.isInBackground()!!) + + sut.setInBackground(false) + assertFalse(sut.isInBackground()!!) + } + + @Test + fun `addAppStateListener creates lifecycle observer if needed`() { + val sut = fixture.getSut() + + sut.addAppStateListener(fixture.listener) + + assertNotNull(sut.lifecycleObserver) + } + + @Test + fun `addAppStateListener from background thread posts to main thread`() { + val sut = fixture.getSut(isMainThread = false) + + sut.addAppStateListener(fixture.listener) + + verify(fixture.mockHandler).post(any()) + } + + @Test + fun `addAppStateListener notifies listener with onForeground when in foreground state`() { + val sut = fixture.getSut() + + sut.setInBackground(false) + sut.addAppStateListener(fixture.listener) + + verify(fixture.listener).onForeground() + verify(fixture.listener, never()).onBackground() + } + + @Test + fun `addAppStateListener notifies listener with onBackground when in background state`() { + val sut = fixture.getSut() + + sut.setInBackground(true) + sut.addAppStateListener(fixture.listener) + + verify(fixture.listener).onBackground() + verify(fixture.listener, never()).onForeground() + } + + @Test + fun `addAppStateListener does not notify listener when state is unknown`() { + val sut = fixture.getSut() + + // State is null (unknown) by default + sut.addAppStateListener(fixture.listener) + + verify(fixture.listener, never()).onForeground() + verify(fixture.listener, never()).onBackground() + } + + @Test + fun `removeAppStateListener removes listener`() { + val sut = fixture.getSut() + + sut.addAppStateListener(fixture.listener) + val observer = sut.lifecycleObserver + // Check that listener was added + assertNotNull(observer) + + sut.removeAppStateListener(fixture.listener) + // Listener should be removed but observer still exists + assertNotNull(sut.lifecycleObserver) + } + + @Test + fun `removeAppStateListener handles null lifecycle observer`() { + val sut = fixture.getSut() + + // Should not throw when lifecycleObserver is null + sut.removeAppStateListener(fixture.listener) + } + + @Test + fun `registerLifecycleObserver does nothing if already registered`() { + val sut = fixture.getSut() + + sut.registerLifecycleObserver(fixture.options) + val firstObserver = sut.lifecycleObserver + + sut.registerLifecycleObserver(fixture.options) + val secondObserver = sut.lifecycleObserver + + assertSame(firstObserver, secondObserver) + } + + @Test + fun `unregisterLifecycleObserver clears listeners and nulls observer`() { + val sut = fixture.getSut() + + sut.addAppStateListener(fixture.listener) + assertNotNull(sut.lifecycleObserver) + + sut.unregisterLifecycleObserver() + + assertNull(sut.lifecycleObserver) + } + + @Test + fun `unregisterLifecycleObserver handles null observer`() { + val sut = fixture.getSut() + + // Should not throw when lifecycleObserver is already null + sut.unregisterLifecycleObserver() + } + + @Test + fun `unregisterLifecycleObserver from background thread posts to main thread`() { + val sut = fixture.getSut(isMainThread = false) + + sut.registerLifecycleObserver(fixture.options) + + sut.unregisterLifecycleObserver() + + // 2 times - register and unregister + verify(fixture.mockHandler, times(2)).post(any()) + } + + @Test + fun `close calls unregisterLifecycleObserver`() { + val sut = fixture.getSut() + sut.addAppStateListener(fixture.listener) + + sut.close() + + assertNull(sut.lifecycleObserver) + } + + @Test + fun `LifecycleObserver onStart notifies all listeners and sets foreground`() { + val listener1 = fixture.createListener() + val listener2 = fixture.createListener() + val sut = fixture.getSut() + + // Add listeners to create observer + sut.addAppStateListener(listener1) + sut.addAppStateListener(listener2) + + val observer = sut.lifecycleObserver!! + observer.onStart(mock()) + + verify(listener1).onForeground() + verify(listener2).onForeground() + assertFalse(sut.isInBackground()!!) + } + + @Test + fun `LifecycleObserver onStop notifies all listeners and sets background`() { + val listener1 = fixture.createListener() + val listener2 = fixture.createListener() + val sut = fixture.getSut() + + // Add listeners to create observer + sut.addAppStateListener(listener1) + sut.addAppStateListener(listener2) + + val observer = sut.lifecycleObserver!! + observer.onStop(mock()) + + verify(listener1).onBackground() + verify(listener2).onBackground() + assertTrue(sut.isInBackground()!!) + } + + @Test + fun `thread safety - concurrent access is handled`() { + val listeners = (1..5).map { fixture.createListener() } + val sut = fixture.getSut() + val latch = CountDownLatch(5) + + // Add listeners concurrently + listeners.forEach { listener -> + Thread { + sut.addAppStateListener(listener) + latch.countDown() + } + .start() + } + latch.await() + + val observer = sut.lifecycleObserver + assertNotNull(observer) + } +} 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 0784621c28a..7cae0d67c67 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 @@ -13,11 +13,14 @@ import io.sentry.SentryLevel import io.sentry.test.DeferredExecutorService import io.sentry.test.ImmediateExecutorService import java.util.concurrent.CountDownLatch +import kotlin.test.AfterTest +import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull +import kotlin.test.assertTrue import org.junit.runner.RunWith import org.mockito.kotlin.any import org.mockito.kotlin.anyOrNull @@ -30,8 +33,6 @@ import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever import org.robolectric.Shadows.shadowOf import org.robolectric.annotation.Config -import kotlin.test.BeforeTest -import kotlin.test.assertTrue @RunWith(AndroidJUnit4::class) @Config(sdk = [Build.VERSION_CODES.TIRAMISU]) @@ -62,6 +63,12 @@ class SystemEventsBreadcrumbsIntegrationTest { @BeforeTest fun `set up`() { AppState.getInstance().resetInstance() + AppState.getInstance().registerLifecycleObserver(fixture.options) + } + + @AfterTest + fun `tear down`() { + AppState.getInstance().unregisterLifecycleObserver() } @Test @@ -345,7 +352,11 @@ class SystemEventsBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - assertTrue(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) + assertTrue( + AppState.getInstance().lifecycleObserver.listeners.any { + it is SystemEventsBreadcrumbsIntegration + } + ) } @Test @@ -355,7 +366,11 @@ class SystemEventsBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - assertFalse(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) + assertFalse( + AppState.getInstance().lifecycleObserver.listeners.any { + it is SystemEventsBreadcrumbsIntegration + } + ) } @Test @@ -364,11 +379,19 @@ class SystemEventsBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - assertTrue(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) + assertTrue( + AppState.getInstance().lifecycleObserver.listeners.any { + it is SystemEventsBreadcrumbsIntegration + } + ) sut.close() - assertFalse(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) + assertFalse( + AppState.getInstance().lifecycleObserver.listeners.any { + it is SystemEventsBreadcrumbsIntegration + } + ) } @Test @@ -389,7 +412,11 @@ class SystemEventsBreadcrumbsIntegrationTest { // ensure all messages on main looper got processed shadowOf(Looper.getMainLooper()).idle() - assertFalse(AppState.getInstance().lifecycleObserver.listeners.any { it is SystemEventsBreadcrumbsIntegration }) + assertFalse( + AppState.getInstance().lifecycleObserver.listeners.any { + it is SystemEventsBreadcrumbsIntegration + } + ) } @Test @@ -440,6 +467,7 @@ class SystemEventsBreadcrumbsIntegrationTest { sut.onBackground() sut.onForeground() + deferredExecutorService.runAll() assertNull(sut.receiver) sut.onBackground() deferredExecutorService.runAll() @@ -463,6 +491,7 @@ class SystemEventsBreadcrumbsIntegrationTest { .start() latch.await() + deferredExecutorService.runAll() sut.onForeground() assertNull(sut.receiver) From d2263b85223404f369c40ac59144806a6ec76da4 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 24 Jul 2025 12:54:58 +0200 Subject: [PATCH 13/16] Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9832c9cb7db..06b9cd1e029 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Internal + +- Use single `LifecycleObserver` and multi-cast it to the integrations interested in lifecycle states ([#4567](https://github.com/getsentry/sentry-java/pull/4567)) + ### Fixes - Allow multiple UncaughtExceptionHandlerIntegrations to be active at the same time ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) From 42ff8e98f30fa888a0f4c87e639c2999e43d18c7 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 24 Jul 2025 13:19:47 +0200 Subject: [PATCH 14/16] Fix tests --- .../android/core/LifecycleWatcherTest.kt | 64 +++++++------------ .../core/SessionTrackingIntegrationTest.kt | 11 ++-- 2 files changed, 27 insertions(+), 48 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt index 446dfa3330a..5149f167129 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/LifecycleWatcherTest.kt @@ -1,6 +1,5 @@ package io.sentry.android.core -import androidx.lifecycle.LifecycleOwner import io.sentry.Breadcrumb import io.sentry.DateUtils import io.sentry.IContinuousProfiler @@ -16,10 +15,8 @@ import io.sentry.transport.ICurrentDateProvider import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull -import kotlin.test.assertTrue import org.mockito.ArgumentCaptor import org.mockito.kotlin.any import org.mockito.kotlin.check @@ -33,7 +30,6 @@ import org.mockito.kotlin.whenever class LifecycleWatcherTest { private class Fixture { - val ownerMock = mock() val scopes = mock() val dateProvider = mock() val options = SentryOptions() @@ -77,7 +73,7 @@ class LifecycleWatcherTest { @Test fun `if last started session is 0, start new session`() { val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.scopes).startSession() verify(fixture.replayController).start() } @@ -86,8 +82,8 @@ class LifecycleWatcherTest { fun `if last started session is after interval, start new session`() { val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false) whenever(fixture.dateProvider.currentTimeMillis).thenReturn(1L, 2L) - watcher.onStart(fixture.ownerMock) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() + watcher.onForeground() verify(fixture.scopes, times(2)).startSession() verify(fixture.replayController, times(2)).start() } @@ -96,8 +92,8 @@ class LifecycleWatcherTest { fun `if last started session is before interval, it should not start a new session`() { val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false) whenever(fixture.dateProvider.currentTimeMillis).thenReturn(2L, 1L) - watcher.onStart(fixture.ownerMock) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() + watcher.onForeground() verify(fixture.scopes).startSession() verify(fixture.replayController).start() } @@ -105,8 +101,8 @@ class LifecycleWatcherTest { @Test fun `if app goes to background, end session after interval`() { val watcher = fixture.getSUT(enableAppLifecycleBreadcrumbs = false) - watcher.onStart(fixture.ownerMock) - watcher.onStop(fixture.ownerMock) + watcher.onForeground() + watcher.onBackground() verify(fixture.scopes, timeout(10000)).endSession() verify(fixture.replayController, timeout(10000)).stop() verify(fixture.continuousProfiler, timeout(10000)).close(eq(false)) @@ -116,12 +112,12 @@ class LifecycleWatcherTest { fun `if app goes to background and foreground again, dont end the session`() { val watcher = fixture.getSUT(sessionIntervalMillis = 30000L, enableAppLifecycleBreadcrumbs = false) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() - watcher.onStop(fixture.ownerMock) + watcher.onBackground() assertNotNull(watcher.timerTask) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() assertNull(watcher.timerTask) verify(fixture.scopes, never()).endSession() @@ -132,7 +128,7 @@ class LifecycleWatcherTest { fun `When session tracking is disabled, do not start session`() { val watcher = fixture.getSUT(enableAutoSessionTracking = false, enableAppLifecycleBreadcrumbs = false) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.scopes, never()).startSession() } @@ -140,14 +136,14 @@ class LifecycleWatcherTest { fun `When session tracking is disabled, do not end session`() { val watcher = fixture.getSUT(enableAutoSessionTracking = false, enableAppLifecycleBreadcrumbs = false) - watcher.onStop(fixture.ownerMock) + watcher.onBackground() verify(fixture.scopes, never()).endSession() } @Test fun `When app lifecycle breadcrumbs is enabled, add breadcrumb on start`() { val watcher = fixture.getSUT(enableAutoSessionTracking = false) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.scopes) .addBreadcrumb( check { @@ -163,14 +159,14 @@ class LifecycleWatcherTest { fun `When app lifecycle breadcrumbs is disabled, do not add breadcrumb on start`() { val watcher = fixture.getSUT(enableAutoSessionTracking = false, enableAppLifecycleBreadcrumbs = false) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.scopes, never()).addBreadcrumb(any()) } @Test fun `When app lifecycle breadcrumbs is enabled, add breadcrumb on stop`() { val watcher = fixture.getSUT(enableAutoSessionTracking = false) - watcher.onStop(fixture.ownerMock) + watcher.onBackground() verify(fixture.scopes) .addBreadcrumb( check { @@ -186,7 +182,7 @@ class LifecycleWatcherTest { fun `When app lifecycle breadcrumbs is disabled, do not add breadcrumb on stop`() { val watcher = fixture.getSUT(enableAutoSessionTracking = false, enableAppLifecycleBreadcrumbs = false) - watcher.onStop(fixture.ownerMock) + watcher.onBackground() verify(fixture.scopes, never()).addBreadcrumb(any()) } @@ -221,7 +217,7 @@ class LifecycleWatcherTest { ), ) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.scopes, never()).startSession() verify(fixture.replayController, never()).start() } @@ -250,25 +246,11 @@ class LifecycleWatcherTest { ), ) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.scopes).startSession() verify(fixture.replayController).start() } - @Test - fun `When app goes into foreground, sets isBackground to false for AppState`() { - val watcher = fixture.getSUT() - watcher.onStart(fixture.ownerMock) - assertFalse(AppState.getInstance().isInBackground!!) - } - - @Test - fun `When app goes into background, sets isBackground to true for AppState`() { - val watcher = fixture.getSUT() - watcher.onStop(fixture.ownerMock) - assertTrue(AppState.getInstance().isInBackground!!) - } - @Test fun `if the hub has already a fresh session running, resumes replay to invalidate isManualPause flag`() { val watcher = @@ -293,7 +275,7 @@ class LifecycleWatcherTest { ), ) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.replayController).resume() } @@ -301,16 +283,16 @@ class LifecycleWatcherTest { fun `background-foreground replay`() { whenever(fixture.dateProvider.currentTimeMillis).thenReturn(1L) val watcher = fixture.getSUT(sessionIntervalMillis = 2L, enableAppLifecycleBreadcrumbs = false) - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.replayController).start() - watcher.onStop(fixture.ownerMock) + watcher.onBackground() verify(fixture.replayController).pause() - watcher.onStart(fixture.ownerMock) + watcher.onForeground() verify(fixture.replayController, times(2)).resume() - watcher.onStop(fixture.ownerMock) + watcher.onBackground() verify(fixture.replayController, timeout(10000)).stop() } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SessionTrackingIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SessionTrackingIntegrationTest.kt index d4264d9831b..bdb328e2421 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SessionTrackingIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SessionTrackingIntegrationTest.kt @@ -18,7 +18,6 @@ import io.sentry.SentryEnvelope import io.sentry.SentryEvent import io.sentry.SentryLogEvent import io.sentry.SentryLogEvents -import io.sentry.SentryOptions import io.sentry.SentryReplayEvent import io.sentry.Session import io.sentry.TraceContext @@ -41,6 +40,7 @@ class SessionTrackingIntegrationTest { @BeforeTest fun `set up`() { + AppState.getInstance().resetInstance() context = ApplicationProvider.getApplicationContext() } @@ -56,7 +56,7 @@ class SessionTrackingIntegrationTest { } val client = CapturingSentryClient() Sentry.bindClient(client) - val lifecycle = setupLifecycle(options) + val lifecycle = setupLifecycle() val initSid = lastSessionId() lifecycle.handleLifecycleEvent(ON_START) @@ -115,12 +115,9 @@ class SessionTrackingIntegrationTest { return sid } - private fun setupLifecycle(options: SentryOptions): LifecycleRegistry { + private fun setupLifecycle(): LifecycleRegistry { val lifecycle = LifecycleRegistry(ProcessLifecycleOwner.get()) - val lifecycleWatcher = - (options.integrations.find { it is AppLifecycleIntegration } as AppLifecycleIntegration) - .watcher - lifecycle.addObserver(lifecycleWatcher!!) + lifecycle.addObserver(AppState.getInstance().lifecycleObserver) return lifecycle } From da939a8ae013b2e8f61c1daa1f40bcb53943e199 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Mon, 4 Aug 2025 14:33:06 +0200 Subject: [PATCH 15/16] Improve callback handling and test visibility (#4593) --- .../java/io/sentry/android/core/AppState.java | 22 +++++-- .../io/sentry/android/core/AppStateTest.kt | 63 ++++++++++++++++++- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java index 855385631d0..ebd640ed4cd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java @@ -2,6 +2,7 @@ import androidx.annotation.NonNull; import androidx.lifecycle.DefaultLifecycleObserver; +import androidx.lifecycle.LifecycleObserver; import androidx.lifecycle.LifecycleOwner; import androidx.lifecycle.ProcessLifecycleOwner; import io.sentry.ILogger; @@ -24,8 +25,8 @@ public final class AppState implements Closeable { private static @NotNull AppState instance = new AppState(); private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); - volatile LifecycleObserver lifecycleObserver; - MainLooperHandler handler = new MainLooperHandler(); + private volatile LifecycleObserver lifecycleObserver; + private MainLooperHandler handler = new MainLooperHandler(); private AppState() {} @@ -35,6 +36,16 @@ private AppState() {} private volatile @Nullable Boolean inBackground = null; + @TestOnly + LifecycleObserver getLifecycleObserver() { + return lifecycleObserver; + } + + @TestOnly + void setHandler(final @NotNull MainLooperHandler handler) { + this.handler = handler; + } + @TestOnly void resetInstance() { instance = new AppState(); @@ -159,6 +170,7 @@ final class LifecycleObserver implements DefaultLifecycleObserver { new CopyOnWriteArrayList() { @Override public boolean add(AppStateListener appStateListener) { + final boolean addResult = super.add(appStateListener); // notify the listeners immediately to let them "catch up" with the current state // (mimics the behavior of androidx.lifecycle) if (Boolean.FALSE.equals(inBackground)) { @@ -166,24 +178,24 @@ public boolean add(AppStateListener appStateListener) { } else if (Boolean.TRUE.equals(inBackground)) { appStateListener.onBackground(); } - return super.add(appStateListener); + return addResult; } }; @Override public void onStart(@NonNull LifecycleOwner owner) { + setInBackground(false); for (AppStateListener listener : listeners) { listener.onForeground(); } - setInBackground(false); } @Override public void onStop(@NonNull LifecycleOwner owner) { + setInBackground(true); for (AppStateListener listener : listeners) { listener.onBackground(); } - setInBackground(true); } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AppStateTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AppStateTest.kt index efef483daf4..4fe39b20f2e 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AppStateTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AppStateTest.kt @@ -6,6 +6,7 @@ import io.sentry.android.core.internal.util.AndroidThreadChecker import java.util.concurrent.CountDownLatch import kotlin.test.AfterTest import kotlin.test.BeforeTest +import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertNull @@ -35,7 +36,7 @@ class AppStateTest { fun getSut(isMainThread: Boolean = true): AppState { val appState = AppState.getInstance() whenever(mockThreadChecker.isMainThread).thenReturn(isMainThread) - appState.handler = mockHandler + appState.setHandler(mockHandler) return appState } @@ -261,6 +262,66 @@ class AppStateTest { assertTrue(sut.isInBackground()!!) } + @Test + fun `a listener can be unregistered within a callback`() { + val sut = fixture.getSut() + + var onForegroundCalled = false + val listener = + object : AppStateListener { + override fun onForeground() { + sut.removeAppStateListener(this) + onForegroundCalled = true + } + + override fun onBackground() { + // ignored + } + } + + sut.registerLifecycleObserver(fixture.options) + val observer = sut.lifecycleObserver!! + observer.onStart(mock()) + + // if an observer is added + sut.addAppStateListener(listener) + + // it should be notified + assertTrue(onForegroundCalled) + + // and removed from the list of listeners if it unregisters itself within the callback + assertEquals(sut.lifecycleObserver?.listeners?.size, 0) + } + + @Test + fun `state is correct within onStart and onStop callbacks`() { + val sut = fixture.getSut() + + var onForegroundCalled = false + var onBackgroundCalled = false + val listener = + object : AppStateListener { + override fun onForeground() { + assertFalse(sut.isInBackground!!) + onForegroundCalled = true + } + + override fun onBackground() { + assertTrue(sut.isInBackground!!) + onBackgroundCalled = true + } + } + + sut.addAppStateListener(listener) + + val observer = sut.lifecycleObserver!! + observer.onStart(mock()) + observer.onStop(mock()) + + assertTrue(onForegroundCalled) + assertTrue(onBackgroundCalled) + } + @Test fun `thread safety - concurrent access is handled`() { val listeners = (1..5).map { fixture.createListener() } From 018c55b34dc933ce4cde565b4cf93c28111423d1 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Tue, 5 Aug 2025 11:23:48 +0200 Subject: [PATCH 16/16] Null-check lifecycleObserver --- .../src/main/java/io/sentry/android/core/AppState.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java index ebd640ed4cd..8fc1c8ab0b8 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppState.java @@ -63,7 +63,9 @@ void addAppStateListener(final @NotNull AppStateListener listener) { try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { ensureLifecycleObserver(NoOpLogger.getInstance()); - lifecycleObserver.listeners.add(listener); + if (lifecycleObserver != null) { + lifecycleObserver.listeners.add(listener); + } } }