diff --git a/CHANGELOG.md b/CHANGELOG.md index f46dbcec85..8d67ca7262 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ ### Fixes +- Populate the Android connection status cache during the first two minutes after boot, instead of treating the empty cache as up to date ([#6029](https://github.com/getsentry/sentry-java/pull/6029)) - Keep dropped tombstone and ANR events dropped, instead of reporting the same app exit again at every app start ([#6002](https://github.com/getsentry/sentry-java/pull/6002)) - Apply `Sentry.withScope` and `Sentry.withIsolationScope` data to events captured inside the callback when `globalHubMode` is enabled ([#6004](https://github.com/getsentry/sentry-java/pull/6004)) - `globalHubMode` is enabled by default on Android, where tags, extras, contexts and level set inside the callback were silently dropped 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 c7c590d624..bae3f8c7c4 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 @@ -35,7 +35,6 @@ 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; @@ -178,7 +177,7 @@ static void initializeIntegrationsAndProcessors( if (options.getConnectionStatusProvider() instanceof NoOpConnectionStatusProvider) { options.setConnectionStatusProvider( new AndroidConnectionStatusProvider( - context, options, buildInfoProvider, AndroidCurrentDateProvider.getInstance())); + context, options, buildInfoProvider, options.getElapsedRealtimeClock())); } 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 3f05beeceb..e7230aa621 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 @@ -19,10 +19,12 @@ import io.sentry.android.core.AppState; import io.sentry.android.core.BuildInfoProvider; import io.sentry.android.core.ContextUtils; -import io.sentry.transport.ICurrentDateProvider; +import io.sentry.time.Deadline; +import io.sentry.time.ElapsedRealtimeClock; import io.sentry.util.AutoClosableReentrantLock; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -41,7 +43,7 @@ public final class AndroidConnectionStatusProvider private final @NotNull Context context; private final @NotNull SentryOptions options; private final @NotNull BuildInfoProvider buildInfoProvider; - private final @NotNull ICurrentDateProvider timeProvider; + private final @NotNull ElapsedRealtimeClock clock; private final @NotNull List connectionStatusObservers; private final @Nullable Handler handler; private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); @@ -66,16 +68,16 @@ public final class AndroidConnectionStatusProvider 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 + private volatile @NotNull Deadline cacheFreshUntil; + private static final long CACHE_TTL_MINUTES = 2; private final @NotNull AtomicBoolean isConnected = new AtomicBoolean(false); public AndroidConnectionStatusProvider( @NotNull Context context, @NotNull SentryOptions options, @NotNull BuildInfoProvider buildInfoProvider, - @NotNull ICurrentDateProvider timeProvider) { - this(context, options, buildInfoProvider, timeProvider, null); + @NotNull ElapsedRealtimeClock clock) { + this(context, options, buildInfoProvider, clock, null); } @SuppressLint("InlinedApi") @@ -83,12 +85,13 @@ public AndroidConnectionStatusProvider( @NotNull Context context, @NotNull SentryOptions options, @NotNull BuildInfoProvider buildInfoProvider, - @NotNull ICurrentDateProvider timeProvider, + @NotNull ElapsedRealtimeClock clock, @Nullable Handler handler) { this.context = ContextUtils.getApplicationContext(context); this.options = options; this.buildInfoProvider = buildInfoProvider; - this.timeProvider = timeProvider; + this.clock = clock; + this.cacheFreshUntil = Deadline.passed(clock); this.handler = handler; this.connectionStatusObservers = new ArrayList<>(); @@ -231,7 +234,7 @@ private void clearCacheAndNotifyObservers() { try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { cachedNetworkCapabilities = null; currentNetwork = null; - lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); + cacheFreshUntil = Deadline.in(clock, CACHE_TTL_MINUTES, TimeUnit.MINUTES); options .getLogger() @@ -362,13 +365,13 @@ private void updateCache(@Nullable NetworkCapabilities networkCapabilities) { SentryLevel.INFO, "No permission (ACCESS_NETWORK_STATE) to check network status."); cachedNetworkCapabilities = null; - lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); + cacheFreshUntil = Deadline.in(clock, CACHE_TTL_MINUTES, TimeUnit.MINUTES); return; } if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.M) { cachedNetworkCapabilities = null; - lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); + cacheFreshUntil = Deadline.in(clock, CACHE_TTL_MINUTES, TimeUnit.MINUTES); return; } @@ -387,7 +390,7 @@ private void updateCache(@Nullable NetworkCapabilities networkCapabilities) { null; // Clear cached capabilities if connectivity manager is null } } - lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); + cacheFreshUntil = Deadline.in(clock, CACHE_TTL_MINUTES, TimeUnit.MINUTES); options .getLogger() @@ -400,13 +403,13 @@ private void updateCache(@Nullable NetworkCapabilities networkCapabilities) { } catch (Throwable t) { options.getLogger().log(SentryLevel.WARNING, "Failed to update connection status cache", t); cachedNetworkCapabilities = null; - lastCacheUpdateTime = timeProvider.getCurrentTimeMillis(); + cacheFreshUntil = Deadline.in(clock, CACHE_TTL_MINUTES, TimeUnit.MINUTES); } } } private boolean isCacheValid() { - return (timeProvider.getCurrentTimeMillis() - lastCacheUpdateTime) < CACHE_TTL_MS; + return !cacheFreshUntil.hasPassed(); } @Override @@ -459,7 +462,7 @@ private void unregisterNetworkCallback(final boolean clearObservers) { // Clear cached state cachedNetworkCapabilities = null; currentNetwork = null; - lastCacheUpdateTime = 0; + cacheFreshUntil = Deadline.passed(clock); } options.getLogger().log(SentryLevel.DEBUG, "Network callback unregistered"); } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt index 4dd8062464..ab948efdcb 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt @@ -26,7 +26,8 @@ import io.sentry.android.core.BuildInfoProvider import io.sentry.android.core.ContextUtils import io.sentry.android.core.SystemEventsBreadcrumbsIntegration import io.sentry.test.ImmediateExecutorService -import io.sentry.transport.ICurrentDateProvider +import io.sentry.time.TestTicker +import java.util.concurrent.TimeUnit.MINUTES import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test @@ -61,15 +62,13 @@ 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 clock: TestTicker private lateinit var options: SentryOptions private lateinit var network: Network private lateinit var networkCapabilities: NetworkCapabilities private lateinit var logger: ILogger private lateinit var contextUtilsStaticMock: MockedStatic - private var currentTime = 1000L - @BeforeTest fun beforeTest() { contextMock = mock() @@ -96,17 +95,13 @@ class AndroidConnectionStatusProviderTest { whenever(networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(true) whenever(networkCapabilities.hasTransport(TRANSPORT_WIFI)).thenReturn(true) - timeProvider = mock() - whenever(timeProvider.currentTimeMillis).thenAnswer { currentTime } + clock = TestTicker() logger = mock() options = SentryOptions() options.setLogger(logger) options.executorService = ImmediateExecutorService() - // Reset current time for each test to ensure cache isolation - currentTime = 1000L - // Mock ContextUtils to return foreground importance contextUtilsStaticMock = mockStatic(ContextUtils::class.java) contextUtilsStaticMock @@ -120,7 +115,7 @@ class AndroidConnectionStatusProviderTest { AppState.getInstance().registerLifecycleObserver(options) connectionStatusProvider = - AndroidConnectionStatusProvider(contextMock, options, buildInfo, timeProvider) + AndroidConnectionStatusProvider(contextMock, options, buildInfo, clock) } @AfterTest @@ -144,6 +139,10 @@ class AndroidConnectionStatusProviderTest { @Test fun `When network is active but not connected with permission, return DISCONNECTED for isConnected`() { whenever(networkInfo.isConnected).thenReturn(false) + // buildInfo reports API 24, so the provider reads NetworkCapabilities rather than the legacy + // activeNetworkInfo. The active network has to report it cannot reach the internet too. + whenever(networkCapabilities.hasCapability(NET_CAPABILITY_INTERNET)).thenReturn(false) + whenever(networkCapabilities.hasCapability(NET_CAPABILITY_VALIDATED)).thenReturn(false) assertEquals( IConnectionStatusProvider.ConnectionStatus.DISCONNECTED, @@ -195,7 +194,7 @@ class AndroidConnectionStatusProviderTest { // Create a new provider with the null connectivity manager val providerWithNullConnectivity = - AndroidConnectionStatusProvider(nullConnectivityContext, options, buildInfo, timeProvider) + AndroidConnectionStatusProvider(nullConnectivityContext, options, buildInfo, clock) assertEquals( IConnectionStatusProvider.ConnectionStatus.UNKNOWN, @@ -306,6 +305,26 @@ class AndroidConnectionStatusProviderTest { assertTrue(connectionStatusProvider.statusObservers.isEmpty()) } + @Test + fun `an unpopulated cache is not treated as fresh shortly after boot`() { + whenever(networkInfo.isConnected).thenReturn(true) + + // elapsedRealtimeNanos() counts from boot, so a provider created moments after boot sees a + // tick near zero. The cache is still empty and must not be read as up to date. + val provider = AndroidConnectionStatusProvider(contextMock, options, buildInfo, TestTicker()) + + val callsBefore = + mockingDetails(connectivityManager).invocations.count { it.method.name == "getActiveNetwork" } + + assertEquals(IConnectionStatusProvider.ConnectionStatus.CONNECTED, provider.connectionStatus) + + val callsAfter = + mockingDetails(connectivityManager).invocations.count { it.method.name == "getActiveNetwork" } + assertTrue(callsAfter > callsBefore, "An empty cache must be populated before it is read") + + provider.close() + } + @Test fun `cache TTL works correctly`() { // Setup: Mock network info to return connected @@ -323,7 +342,7 @@ class AndroidConnectionStatusProviderTest { mockingDetails(connectivityManager).invocations.count { it.method.name == "getActiveNetwork" } // Advance time by 1 minute (less than 2 minute TTL) - currentTime += 60 * 1000L + clock.advance(1, MINUTES) // Second call should use cache - no additional calls to getActiveNetwork val secondResult = connectionStatusProvider.connectionStatus @@ -336,7 +355,7 @@ class AndroidConnectionStatusProviderTest { assertEquals(initialCallCount, callCountAfterSecond, "Second call should use cache") // Advance time beyond TTL (total 3 minutes) - currentTime += 2 * 60 * 1000L + clock.advance(2, MINUTES) // Third call should refresh cache - should make new calls to getActiveNetwork val thirdResult = connectionStatusProvider.connectionStatus @@ -543,7 +562,7 @@ class AndroidConnectionStatusProviderTest { whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(goodCaps) // Force cache invalidation by advancing time beyond TTL - currentTime += 3 * 60 * 1000L // 3 minutes + clock.advance(3, MINUTES) // Should return CONNECTED for good capabilities assertEquals( @@ -560,7 +579,7 @@ class AndroidConnectionStatusProviderTest { whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(unvalidatedCaps) // Force cache invalidation again - currentTime += 3 * 60 * 1000L + clock.advance(3, MINUTES) assertEquals( IConnectionStatusProvider.ConnectionStatus.DISCONNECTED,