Skip to content

fix(android): Treat an unpopulated connection cache as stale (JAVA-717) #6029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
runningcode wants to merge 3 commits into
base: no/java-572-timestamp-timing
Choose a base branch
from
+55 −32
Open

fix(android): Treat an unpopulated connection cache as stale (JAVA-717) #6029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2b5a8f7
test(android): Make the DISCONNECTED test mock a disconnected network
runningcode Sep 3, 2026
d091a4c
fix(android): Treat an unpopulated connection cache as stale (JAVA-717)
runningcode Sep 3, 2026
3fb8770
Add changelog entry for JAVA-717
runningcode Sep 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- Update `SentryTraced` so that it now honors `options.setIgnoredSpanOrigins` ([#6058](https://github.com/getsentry/sentry-java/pull/6058))
- `SentryTraced` now checks for its owning transaction dynamically rather than once per app process. The latter caused `SentryTraced` spans to be dropped process-wide once the original transaction finished ([#6057](https://github.com/getsentry/sentry-java/pull/6057))
- Fix typos in Spring GraphQL integration names (`GrahQL` to `GraphQL`) ([#6061](https://github.com/getsentry/sentry-java/pull/6061))
- 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))

### Internal

Expand Down
3 changes: 1 addition & 2 deletions sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.getMonotonicTicker()));
}

if (options.getCacheDirPath() != null) {
Expand Down
33 changes: 18 additions & 15 deletions ...e/src/main/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.MonotonicTicker;
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;
Expand All @@ -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 MonotonicTicker ticker;
private final @NotNull List<IConnectionStatusObserver> connectionStatusObservers;
private final @Nullable Handler handler;
private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock();
Expand All @@ -66,29 +68,30 @@ 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 MonotonicTicker ticker) {
this(context, options, buildInfoProvider, ticker, null);
}

@SuppressLint("InlinedApi")
public AndroidConnectionStatusProvider(
@NotNull Context context,
@NotNull SentryOptions options,
@NotNull BuildInfoProvider buildInfoProvider,
@NotNull ICurrentDateProvider timeProvider,
@NotNull MonotonicTicker ticker,
@Nullable Handler handler) {
this.context = ContextUtils.getApplicationContext(context);
this.options = options;
this.buildInfoProvider = buildInfoProvider;
this.timeProvider = timeProvider;
this.ticker = ticker;
this.cacheFreshUntil = Deadline.passed(ticker);
this.handler = handler;
this.connectionStatusObservers = new ArrayList<>();

Expand Down Expand Up @@ -231,7 +234,7 @@ private void clearCacheAndNotifyObservers() {
try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) {
cachedNetworkCapabilities = null;
currentNetwork = null;
lastCacheUpdateTime = timeProvider.getCurrentTimeMillis();
cacheFreshUntil = Deadline.after(ticker, CACHE_TTL_MINUTES, TimeUnit.MINUTES);

@0xadam-brown 0xadam-brown Sep 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing this in action, I wonder if there isn't a better name than Deadline.after()? ("after" makes me think that perhaps we're offsetting the start of the deadline until the time we pass to after())

Thoughts about until()? I prefer it b/c my mind defaults to thinking that whatever I'm returned is "valid"; a deadline that's passed isn't; and until() points to the valid time segment.

(I also considered at(), which reads nicely here, but (I think) mostly because my mind is sneaking in the idea of a clock, and not reading this as a pure duration.)

Happy to defer to you...

@runningcode runningcode Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally had it as in but that's a reserved keyword in kotlin so it was an awkward API once we get to the kotlin code. I don't see a big enough difference between until and after. My reasoning with after is that the deadline expires after the specified time.

We can change it later, but I'd rather start getting some PRs merged. Rebasing is getting tedious with all the stacked PRs.


options
.getLogger()
Expand Down Expand Up @@ -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.after(ticker, CACHE_TTL_MINUTES, TimeUnit.MINUTES);
return;
}

if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.M) {
cachedNetworkCapabilities = null;
lastCacheUpdateTime = timeProvider.getCurrentTimeMillis();
cacheFreshUntil = Deadline.after(ticker, CACHE_TTL_MINUTES, TimeUnit.MINUTES);
return;
}

Expand All @@ -387,7 +390,7 @@ private void updateCache(@Nullable NetworkCapabilities networkCapabilities) {
null; // Clear cached capabilities if connectivity manager is null
}
}
lastCacheUpdateTime = timeProvider.getCurrentTimeMillis();
cacheFreshUntil = Deadline.after(ticker, CACHE_TTL_MINUTES, TimeUnit.MINUTES);

options
.getLogger()
Expand All @@ -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.after(ticker, CACHE_TTL_MINUTES, TimeUnit.MINUTES);
}
}
}

private boolean isCacheValid() {
return (timeProvider.getCurrentTimeMillis() - lastCacheUpdateTime) < CACHE_TTL_MS;
return !cacheFreshUntil.hasPassed();

@0xadam-brown 0xadam-brown Sep 8, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

l: We could add a hasNotPassed() method to Deadline, as well. I bet it'll get a lot of use.

@runningcode runningcode Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Its a good point that there are a lot of callers that invert the condition. I don't like methods with negative words in the name. When you invert the condition it becomes harder to reason about.

}

@Override
Expand Down Expand Up @@ -459,7 +462,7 @@ private void unregisterNetworkCallback(final boolean clearObservers) {
// Clear cached state
cachedNetworkCapabilities = null;
currentNetwork = null;
lastCacheUpdateTime = 0;
cacheFreshUntil = Deadline.passed(ticker);
}
options.getLogger().log(SentryLevel.DEBUG, "Network callback unregistered");
}
Expand Down
50 changes: 35 additions & 15 deletions ...src/test/java/io/sentry/android/core/internal/util/AndroidConnectionStatusProviderTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.TestMonotonicTicker
import java.util.concurrent.TimeUnit.MINUTES
import kotlin.test.AfterTest
import kotlin.test.BeforeTest
import kotlin.test.Test
Expand Down Expand Up @@ -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 ticker: TestMonotonicTicker
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<ContextUtils>

private var currentTime = 1000L

@BeforeTest
fun beforeTest() {
contextMock = mock()
Expand All @@ -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 }
ticker = TestMonotonicTicker()

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
Expand All @@ -120,7 +115,7 @@ class AndroidConnectionStatusProviderTest {
AppState.getInstance().registerLifecycleObserver(options)

connectionStatusProvider =
AndroidConnectionStatusProvider(contextMock, options, buildInfo, timeProvider)
AndroidConnectionStatusProvider(contextMock, options, buildInfo, ticker)
}

@AfterTest
Expand All @@ -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,
Expand Down Expand Up @@ -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, ticker)

assertEquals(
IConnectionStatusProvider.ConnectionStatus.UNKNOWN,
Expand Down Expand Up @@ -306,6 +305,27 @@ 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, TestMonotonicTicker())

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
Expand All @@ -323,7 +343,7 @@ class AndroidConnectionStatusProviderTest {
mockingDetails(connectivityManager).invocations.count { it.method.name == "getActiveNetwork" }

// Advance time by 1 minute (less than 2 minute TTL)
currentTime += 60 * 1000L
ticker.advance(1, MINUTES)

// Second call should use cache - no additional calls to getActiveNetwork
val secondResult = connectionStatusProvider.connectionStatus
Expand All @@ -336,7 +356,7 @@ class AndroidConnectionStatusProviderTest {
assertEquals(initialCallCount, callCountAfterSecond, "Second call should use cache")

// Advance time beyond TTL (total 3 minutes)
currentTime += 2 * 60 * 1000L
ticker.advance(2, MINUTES)

// Third call should refresh cache - should make new calls to getActiveNetwork
val thirdResult = connectionStatusProvider.connectionStatus
Expand Down Expand Up @@ -543,7 +563,7 @@ class AndroidConnectionStatusProviderTest {
whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(goodCaps)

// Force cache invalidation by advancing time beyond TTL
currentTime += 3 * 60 * 1000L // 3 minutes
ticker.advance(3, MINUTES)

// Should return CONNECTED for good capabilities
assertEquals(
Expand All @@ -560,7 +580,7 @@ class AndroidConnectionStatusProviderTest {
whenever(connectivityManager.getNetworkCapabilities(any())).thenReturn(unvalidatedCaps)

// Force cache invalidation again
currentTime += 3 * 60 * 1000L
ticker.advance(3, MINUTES)

assertEquals(
IConnectionStatusProvider.ConnectionStatus.DISCONNECTED,
Expand Down
Loading