diff --git a/CHANGELOG.md b/CHANGELOG.md index d9bc29f4b32..4d7a68f409c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ - 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)) - Remove unused method in ManifestMetadataReader ([#4585](https://github.com/getsentry/sentry-java/pull/4585)) - +- Have single `NetworkCallback` registered at a time to reduce IPC calls ([#4562](https://github.com/getsentry/sentry-java/pull/4562)) ## 8.18.0 diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 0bf0d44427d..7e0cf42507b 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 04d927ff79e..33b003e9081 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 6308f704009..16ccc0a2ee4 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, @@ -161,11 +166,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 @@ -174,6 +192,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() { @@ -203,6 +227,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( @@ -410,6 +440,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; @@ -618,8 +651,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, @@ -646,7 +706,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) { @@ -681,7 +741,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..4f6ba7fc5f0 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/NetworkBreadcrumbsIntegrationTest.kt @@ -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 @@ -30,9 +27,7 @@ import kotlin.test.assertNull import kotlin.test.assertTrue import org.mockito.kotlin.KInOrder import org.mockito.kotlin.any -import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.check -import org.mockito.kotlin.eq import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock import org.mockito.kotlin.never @@ -46,7 +41,6 @@ class NetworkBreadcrumbsIntegrationTest { var options = SentryAndroidOptions() val scopes = mock() val mockBuildInfoProvider = mock() - val connectivityManager = mock() var nowMs: Long = 0 val network = mock() @@ -64,7 +58,7 @@ class NetworkBreadcrumbsIntegrationTest { SentryNanotimeDate(DateUtils.nanosToDate(nowNanos), nowNanos) } } - return NetworkBreadcrumbsIntegration(context, buildInfo, options.logger) + return NetworkBreadcrumbsIntegration(context, buildInfo) } } @@ -73,13 +67,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 +80,7 @@ class NetworkBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - verify(fixture.connectivityManager).registerDefaultNetworkCallback(any()) + assertFalse(AndroidConnectionStatusProvider.getChildCallbacks().isEmpty()) assertNotNull(sut.networkCallback) } @@ -98,7 +90,7 @@ class NetworkBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - verify(fixture.connectivityManager, never()).registerDefaultNetworkCallback(any()) + assertTrue(AndroidConnectionStatusProvider.getChildCallbacks().isEmpty()) assertNull(sut.networkCallback) } @@ -110,7 +102,7 @@ class NetworkBreadcrumbsIntegrationTest { sut.register(fixture.scopes, fixture.options) - verify(fixture.connectivityManager, never()).registerDefaultNetworkCallback(any()) + assertTrue(AndroidConnectionStatusProvider.getChildCallbacks().isEmpty()) assertNull(sut.networkCallback) } @@ -121,21 +113,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) } @@ -158,18 +136,6 @@ class NetworkBreadcrumbsIntegrationTest { ) } - @Test - fun `When connected to the same network without disconnecting from the previous one, only one breadcrumb is captured`() { - val sut = fixture.getSut() - sut.register(fixture.scopes, fixture.options) - val callback = sut.networkCallback - assertNotNull(callback) - callback.onAvailable(fixture.network) - callback.onAvailable(fixture.network) - - verify(fixture.scopes, times(1)).addBreadcrumb(any()) - } - @Test fun `When disconnected from a network, a breadcrumb is captured`() { val sut = fixture.getSut() @@ -192,17 +158,6 @@ class NetworkBreadcrumbsIntegrationTest { ) } - @Test - fun `When disconnected from a network, a breadcrumb is captured only if previously connected to that network`() { - val sut = fixture.getSut() - sut.register(fixture.scopes, fixture.options) - val callback = sut.networkCallback - assertNotNull(callback) - // callback.onAvailable(network) was not called, so no breadcrumb should be captured - callback.onLost(mock()) - verify(fixture.scopes, never()).addBreadcrumb(any()) - } - @Test fun `When a network connection detail changes, a breadcrumb is captured`() { val buildInfo = mock() @@ -241,17 +196,6 @@ class NetworkBreadcrumbsIntegrationTest { ) } - @Test - fun `When a network connection detail changes, a breadcrumb is captured only if previously connected to that network`() { - val sut = fixture.getSut() - sut.register(fixture.scopes, fixture.options) - val callback = sut.networkCallback - assertNotNull(callback) - // callback.onAvailable(network) was not called, so no breadcrumb should be captured - onCapabilitiesChanged(callback, mock()) - verify(fixture.scopes, never()).addBreadcrumb(any(), anyOrNull()) - } - @Test fun `When a network connection detail changes, a new breadcrumb is captured if vpn flag changes`() { val sut = fixture.getSut() @@ -503,22 +447,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); } }