Skip to content

Fix connection status provider exceptions and api inconsistencies #4590

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

Closed
romtsn wants to merge 2 commits into from
Closed

Fix connection status provider exceptions and api inconsistencies #4590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9e65025
Handle exceptions in network callbacks and limit onUnavailable to API…
cursoragent Aug 1, 2025
02cfc15
Format code
getsentry-bot Aug 1, 2025
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
49 changes: 43 additions & 6 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 @@ -169,7 +169,16 @@ 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);
}
}
}
}
Expand All @@ -179,9 +188,22 @@ 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);
}
}
}
}
}
Expand All @@ -195,7 +217,13 @@ 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);
}
}
}
}
Expand Down Expand Up @@ -230,7 +258,16 @@ 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);
}
}
}
}
Expand Down
183 changes: 183 additions & 0 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 @@ -632,4 +632,187 @@ 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<NetworkCallback>()
val goodCallback1 = mock<NetworkCallback>()
val goodCallback2 = mock<NetworkCallback>()

// 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<Throwable>(),
)
}

@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<NetworkCallback>()
val goodCallback1 = mock<NetworkCallback>()
val goodCallback2 = mock<NetworkCallback>()

// 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<Throwable>(),
)
}

@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<NetworkCallback>()
val goodCallback1 = mock<NetworkCallback>()
val goodCallback2 = mock<NetworkCallback>()

// 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<Throwable>(),
)
}

@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<NetworkCallback>()
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<NetworkCallback>()
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<NetworkCallback>()
val goodCallback1 = mock<NetworkCallback>()
val goodCallback2 = mock<NetworkCallback>()

// 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<Throwable>(),
)
}
}