From 9e65025e3fe502859b0a0adc478f63c35c63d8d5 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 1 Aug 2025 13:20:24 +0000 Subject: [PATCH 1/2] Handle exceptions in network callbacks and limit onUnavailable to API 26+ Co-authored-by: roman.zavarnitsyn --- .../util/AndroidConnectionStatusProvider.java | 31 ++- .../AndroidConnectionStatusProviderTest.kt | 182 ++++++++++++++++++ 2 files changed, 207 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 16ccc0a2ee4..253c13ccae3 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 @@ -169,7 +169,11 @@ public void onAvailable(final @NotNull Network network) { try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { for (final @NotNull NetworkCallback cb : childCallbacks) { - cb.onAvailable(network); + try { + cb.onAvailable(network); + } catch (Throwable t) { + options.getLogger().log(SentryLevel.WARNING, "Exception in child NetworkCallback.onAvailable", t); + } } } } @@ -179,9 +183,16 @@ public void onAvailable(final @NotNull Network network) { public void onUnavailable() { clearCacheAndNotifyObservers(); - try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { - for (final @NotNull NetworkCallback cb : childCallbacks) { - cb.onUnavailable(); + // Only call onUnavailable on child callbacks if we're on API 26+ to maintain compatibility + if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.O) { + try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { + for (final @NotNull NetworkCallback cb : childCallbacks) { + try { + cb.onUnavailable(); + } catch (Throwable t) { + options.getLogger().log(SentryLevel.WARNING, "Exception in child NetworkCallback.onUnavailable", t); + } + } } } } @@ -195,7 +206,11 @@ public void onLost(final @NotNull Network network) { try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { for (final @NotNull NetworkCallback cb : childCallbacks) { - cb.onLost(network); + try { + cb.onLost(network); + } catch (Throwable t) { + options.getLogger().log(SentryLevel.WARNING, "Exception in child NetworkCallback.onLost", t); + } } } } @@ -230,7 +245,11 @@ public void onCapabilitiesChanged( try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { for (final @NotNull NetworkCallback cb : childCallbacks) { - cb.onCapabilitiesChanged(network, networkCapabilities); + try { + cb.onCapabilitiesChanged(network, networkCapabilities); + } catch (Throwable t) { + options.getLogger().log(SentryLevel.WARNING, "Exception in child NetworkCallback.onCapabilitiesChanged", t); + } } } } 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 6769ac15301..2ae5b0c0dda 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 @@ -632,4 +632,186 @@ class AndroidConnectionStatusProviderTest { mainCallback.onAvailable(network) verifyNoMoreInteractions(childCallback) } + + @Test + fun `exception in child callback onAvailable does not halt dispatch loop`() { + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val mainCallback = connectionStatusProvider.networkCallback + assertNotNull(mainCallback) + + // Register multiple child callbacks - one throws, others should still receive events + val throwingCallback = mock() + val goodCallback1 = mock() + val goodCallback2 = mock() + + // Configure the throwing callback to throw an exception + whenever(throwingCallback.onAvailable(any())).thenThrow(RuntimeException("Test exception")) + + AndroidConnectionStatusProvider.getChildCallbacks().addAll( + listOf(throwingCallback, goodCallback1, goodCallback2) + ) + + // Simulate event - should not fail despite throwing callback + mainCallback.onAvailable(network) + + // Verify all callbacks were called despite exception + verify(throwingCallback).onAvailable(network) + verify(goodCallback1).onAvailable(network) + verify(goodCallback2).onAvailable(network) + + // Verify the exception was logged + verify(logger).log( + eq(io.sentry.SentryLevel.WARNING), + eq("Exception in child NetworkCallback.onAvailable"), + any() + ) + } + + @Test + fun `exception in child callback onLost does not halt dispatch loop`() { + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val mainCallback = connectionStatusProvider.networkCallback + assertNotNull(mainCallback) + + // Set current network first + mainCallback.onAvailable(network) + + // Register multiple child callbacks - one throws, others should still receive events + val throwingCallback = mock() + val goodCallback1 = mock() + val goodCallback2 = mock() + + // Configure the throwing callback to throw an exception + whenever(throwingCallback.onLost(any())).thenThrow(RuntimeException("Test exception")) + + AndroidConnectionStatusProvider.getChildCallbacks().addAll( + listOf(throwingCallback, goodCallback1, goodCallback2) + ) + + // Simulate event - should not fail despite throwing callback + mainCallback.onLost(network) + + // Verify all callbacks were called despite exception + verify(throwingCallback).onLost(network) + verify(goodCallback1).onLost(network) + verify(goodCallback2).onLost(network) + + // Verify the exception was logged + verify(logger).log( + eq(io.sentry.SentryLevel.WARNING), + eq("Exception in child NetworkCallback.onLost"), + any() + ) + } + + @Test + fun `exception in child callback onCapabilitiesChanged does not halt dispatch loop`() { + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) + + val mainCallback = connectionStatusProvider.networkCallback + assertNotNull(mainCallback) + + // Set current network first + mainCallback.onAvailable(network) + + // Register multiple child callbacks - one throws, others should still receive events + val throwingCallback = mock() + val goodCallback1 = mock() + val goodCallback2 = mock() + + // Configure the throwing callback to throw an exception + whenever(throwingCallback.onCapabilitiesChanged(any(), any())).thenThrow(RuntimeException("Test exception")) + + AndroidConnectionStatusProvider.getChildCallbacks().addAll( + listOf(throwingCallback, goodCallback1, goodCallback2) + ) + + // Simulate event - should not fail despite throwing callback + mainCallback.onCapabilitiesChanged(network, networkCapabilities) + + // Verify all callbacks were called despite exception + verify(throwingCallback).onCapabilitiesChanged(network, networkCapabilities) + verify(goodCallback1).onCapabilitiesChanged(network, networkCapabilities) + verify(goodCallback2).onCapabilitiesChanged(network, networkCapabilities) + + // Verify the exception was logged + verify(logger).log( + eq(io.sentry.SentryLevel.WARNING), + eq("Exception in child NetworkCallback.onCapabilitiesChanged"), + any() + ) + } + + @Test + fun `onUnavailable calls child callbacks only on API 26 and above`() { + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.O) // API 26 + + val mainCallback = connectionStatusProvider.networkCallback + assertNotNull(mainCallback) + + // Register a child callback + val childCallback = mock() + AndroidConnectionStatusProvider.getChildCallbacks().add(childCallback) + + // Simulate onUnavailable event + mainCallback.onUnavailable() + + // Verify child callback received the event on API 26+ + verify(childCallback).onUnavailable() + } + + @Test + fun `onUnavailable does not call child callbacks on API 24-25`() { + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.N) // API 24 + + val mainCallback = connectionStatusProvider.networkCallback + assertNotNull(mainCallback) + + // Register a child callback + val childCallback = mock() + AndroidConnectionStatusProvider.getChildCallbacks().add(childCallback) + + // Simulate onUnavailable event + mainCallback.onUnavailable() + + // Verify child callback did NOT receive the event on API 24-25 + verify(childCallback, times(0)).onUnavailable() + } + + @Test + fun `exception in child callback onUnavailable does not halt dispatch loop on API 26+`() { + whenever(buildInfo.sdkInfoVersion).thenReturn(Build.VERSION_CODES.O) // API 26 + + val mainCallback = connectionStatusProvider.networkCallback + assertNotNull(mainCallback) + + // Register multiple child callbacks - one throws, others should still receive events + val throwingCallback = mock() + val goodCallback1 = mock() + val goodCallback2 = mock() + + // Configure the throwing callback to throw an exception + whenever(throwingCallback.onUnavailable()).thenThrow(RuntimeException("Test exception")) + + AndroidConnectionStatusProvider.getChildCallbacks().addAll( + listOf(throwingCallback, goodCallback1, goodCallback2) + ) + + // Simulate event - should not fail despite throwing callback + mainCallback.onUnavailable() + + // Verify all callbacks were called despite exception + verify(throwingCallback).onUnavailable() + verify(goodCallback1).onUnavailable() + verify(goodCallback2).onUnavailable() + + // Verify the exception was logged + verify(logger).log( + eq(io.sentry.SentryLevel.WARNING), + eq("Exception in child NetworkCallback.onUnavailable"), + any() + ) + } } From 02cfc157e910e30c6542d5b30a0eb5a1f14a67a5 Mon Sep 17 00:00:00 2001 From: Sentry Github Bot Date: Fri, 1 Aug 2025 13:23:14 +0000 Subject: [PATCH 2/2] Format code --- .../util/AndroidConnectionStatusProvider.java | 28 ++++++-- .../AndroidConnectionStatusProviderTest.kt | 67 ++++++++++--------- 2 files changed, 57 insertions(+), 38 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 253c13ccae3..c66a75cbf45 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 @@ -172,7 +172,12 @@ public void onAvailable(final @NotNull Network network) { try { cb.onAvailable(network); } catch (Throwable t) { - options.getLogger().log(SentryLevel.WARNING, "Exception in child NetworkCallback.onAvailable", t); + options + .getLogger() + .log( + SentryLevel.WARNING, + "Exception in child NetworkCallback.onAvailable", + t); } } } @@ -183,14 +188,20 @@ public void onAvailable(final @NotNull Network network) { public void onUnavailable() { clearCacheAndNotifyObservers(); - // Only call onUnavailable on child callbacks if we're on API 26+ to maintain compatibility + // Only call onUnavailable on child callbacks if we're on API 26+ to maintain + // compatibility if (buildInfoProvider.getSdkInfoVersion() >= Build.VERSION_CODES.O) { try (final @NotNull ISentryLifecycleToken ignored = childCallbacksLock.acquire()) { for (final @NotNull NetworkCallback cb : childCallbacks) { try { cb.onUnavailable(); } catch (Throwable t) { - options.getLogger().log(SentryLevel.WARNING, "Exception in child NetworkCallback.onUnavailable", t); + options + .getLogger() + .log( + SentryLevel.WARNING, + "Exception in child NetworkCallback.onUnavailable", + t); } } } @@ -209,7 +220,9 @@ public void onLost(final @NotNull Network network) { try { cb.onLost(network); } catch (Throwable t) { - options.getLogger().log(SentryLevel.WARNING, "Exception in child NetworkCallback.onLost", t); + options + .getLogger() + .log(SentryLevel.WARNING, "Exception in child NetworkCallback.onLost", t); } } } @@ -248,7 +261,12 @@ public void onCapabilitiesChanged( try { cb.onCapabilitiesChanged(network, networkCapabilities); } catch (Throwable t) { - options.getLogger().log(SentryLevel.WARNING, "Exception in child NetworkCallback.onCapabilitiesChanged", t); + options + .getLogger() + .log( + SentryLevel.WARNING, + "Exception in child NetworkCallback.onCapabilitiesChanged", + t); } } } 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 2ae5b0c0dda..0b992677db7 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 @@ -648,9 +648,8 @@ class AndroidConnectionStatusProviderTest { // Configure the throwing callback to throw an exception whenever(throwingCallback.onAvailable(any())).thenThrow(RuntimeException("Test exception")) - AndroidConnectionStatusProvider.getChildCallbacks().addAll( - listOf(throwingCallback, goodCallback1, goodCallback2) - ) + AndroidConnectionStatusProvider.getChildCallbacks() + .addAll(listOf(throwingCallback, goodCallback1, goodCallback2)) // Simulate event - should not fail despite throwing callback mainCallback.onAvailable(network) @@ -661,11 +660,12 @@ class AndroidConnectionStatusProviderTest { verify(goodCallback2).onAvailable(network) // Verify the exception was logged - verify(logger).log( - eq(io.sentry.SentryLevel.WARNING), - eq("Exception in child NetworkCallback.onAvailable"), - any() - ) + verify(logger) + .log( + eq(io.sentry.SentryLevel.WARNING), + eq("Exception in child NetworkCallback.onAvailable"), + any(), + ) } @Test @@ -686,9 +686,8 @@ class AndroidConnectionStatusProviderTest { // Configure the throwing callback to throw an exception whenever(throwingCallback.onLost(any())).thenThrow(RuntimeException("Test exception")) - AndroidConnectionStatusProvider.getChildCallbacks().addAll( - listOf(throwingCallback, goodCallback1, goodCallback2) - ) + AndroidConnectionStatusProvider.getChildCallbacks() + .addAll(listOf(throwingCallback, goodCallback1, goodCallback2)) // Simulate event - should not fail despite throwing callback mainCallback.onLost(network) @@ -699,11 +698,12 @@ class AndroidConnectionStatusProviderTest { verify(goodCallback2).onLost(network) // Verify the exception was logged - verify(logger).log( - eq(io.sentry.SentryLevel.WARNING), - eq("Exception in child NetworkCallback.onLost"), - any() - ) + verify(logger) + .log( + eq(io.sentry.SentryLevel.WARNING), + eq("Exception in child NetworkCallback.onLost"), + any(), + ) } @Test @@ -722,11 +722,11 @@ class AndroidConnectionStatusProviderTest { val goodCallback2 = mock() // Configure the throwing callback to throw an exception - whenever(throwingCallback.onCapabilitiesChanged(any(), any())).thenThrow(RuntimeException("Test exception")) + whenever(throwingCallback.onCapabilitiesChanged(any(), any())) + .thenThrow(RuntimeException("Test exception")) - AndroidConnectionStatusProvider.getChildCallbacks().addAll( - listOf(throwingCallback, goodCallback1, goodCallback2) - ) + AndroidConnectionStatusProvider.getChildCallbacks() + .addAll(listOf(throwingCallback, goodCallback1, goodCallback2)) // Simulate event - should not fail despite throwing callback mainCallback.onCapabilitiesChanged(network, networkCapabilities) @@ -737,11 +737,12 @@ class AndroidConnectionStatusProviderTest { verify(goodCallback2).onCapabilitiesChanged(network, networkCapabilities) // Verify the exception was logged - verify(logger).log( - eq(io.sentry.SentryLevel.WARNING), - eq("Exception in child NetworkCallback.onCapabilitiesChanged"), - any() - ) + verify(logger) + .log( + eq(io.sentry.SentryLevel.WARNING), + eq("Exception in child NetworkCallback.onCapabilitiesChanged"), + any(), + ) } @Test @@ -795,9 +796,8 @@ class AndroidConnectionStatusProviderTest { // Configure the throwing callback to throw an exception whenever(throwingCallback.onUnavailable()).thenThrow(RuntimeException("Test exception")) - AndroidConnectionStatusProvider.getChildCallbacks().addAll( - listOf(throwingCallback, goodCallback1, goodCallback2) - ) + AndroidConnectionStatusProvider.getChildCallbacks() + .addAll(listOf(throwingCallback, goodCallback1, goodCallback2)) // Simulate event - should not fail despite throwing callback mainCallback.onUnavailable() @@ -808,10 +808,11 @@ class AndroidConnectionStatusProviderTest { verify(goodCallback2).onUnavailable() // Verify the exception was logged - verify(logger).log( - eq(io.sentry.SentryLevel.WARNING), - eq("Exception in child NetworkCallback.onUnavailable"), - any() - ) + verify(logger) + .log( + eq(io.sentry.SentryLevel.WARNING), + eq("Exception in child NetworkCallback.onUnavailable"), + any(), + ) } }