Skip to content

Fix flaky SdkInitTests #4661

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
adinauer wants to merge 3 commits into from
Closed

Fix flaky SdkInitTests #4661

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
2d65f68
Fix flaky SdkInitTests
adinauer Aug 25, 2025
575097f
Format code
getsentry-bot Aug 25, 2025
d1184ba
Merge branch 'main' into 08-25-fix_flaky_sdkinittests
adinauer Aug 25, 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
40 changes: 26 additions & 14 deletions ...android-core/src/main/java/io/sentry/android/core/SystemEventsBreadcrumbsIntegration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.atomic.AtomicBoolean;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand Down Expand Up @@ -186,21 +187,32 @@ private void unregisterReceiver() {
return;
}

options
.getExecutorService()
.submit(
() -> {
final @Nullable SystemEventsBroadcastReceiver receiverRef;
try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) {
isStopped = true;
receiverRef = receiver;
receiver = null;
}
try {
options
.getExecutorService()
.submit(
() -> {
final @Nullable SystemEventsBroadcastReceiver receiverRef;
try (final @NotNull ISentryLifecycleToken ignored = receiverLock.acquire()) {
isStopped = true;
receiverRef = receiver;
receiver = null;
}

if (receiverRef != null) {
context.unregisterReceiver(receiverRef);
}
});
if (receiverRef != null) {
context.unregisterReceiver(receiverRef);
}
});
} catch (RejectedExecutionException e) {
if (options != null) {
options
.getLogger()
.log(
SentryLevel.DEBUG,
"SystemEventsBreadcrumbsIntegration was unable to unregister receiver.",
e);
}
}
Copy link

@cursor cursor bot Aug 25, 2025

Choose a reason for hiding this comment

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

Bug: Receiver Cleanup Fails on RejectedExecutionException

When unregisterReceiver() catches a RejectedExecutionException, the essential cleanup logic (unregistering the receiver, nullifying its reference, and updating isStopped) is skipped. This leaves the receiver registered with the Android system and the integration in an inconsistent state, potentially causing resource leaks.

Fix in Cursor Fix in Web

Copy link
Member

@romtsn romtsn Aug 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think this is legit - could you extract the logic into unregisterReceiverInternal or something and then call it in the catch block? In that case I think it's fine to still do it on the calling thread

}

@Override
Expand Down
57 changes: 56 additions & 1 deletion ...tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package io.sentry.uitest.android
import androidx.lifecycle.Lifecycle
import androidx.test.core.app.launchActivity
import androidx.test.ext.junit.runners.AndroidJUnit4
import io.sentry.IConnectionStatusProvider
import io.sentry.ProfilingTraceData
import io.sentry.Sentry
import io.sentry.SentryIntegrationPackageStorage
Expand All @@ -23,19 +24,51 @@ import shark.AndroidReferenceMatchers
import shark.IgnoredReferenceMatcher
import shark.ReferencePattern

/** Test connection status provider that always reports connected. */
private class AlwaysOnlineStatusProvider : IConnectionStatusProvider {
Copy link
Member

@romtsn romtsn Aug 25, 2025

Choose a reason for hiding this comment

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

hm, wondering if it's still going to fail because the device does not have internet connection, but we're saying here that it does? Shall we also try to enable it via e.g. executing adb shell svc wifi enable or something like that?

Copy link
Contributor

@stefanosiano stefanosiano Aug 26, 2025

Choose a reason for hiding this comment

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

i don't think it's a problem, because it's sending to localhost anyway, except for a single test (EnvelopeTests.sendProfiledTransaction) that we may even remove
But we block sending the envelopes if there's no connection, so this is good


override fun close() {
// No-op for test implementation
}

override fun getConnectionStatus(): IConnectionStatusProvider.ConnectionStatus {
return IConnectionStatusProvider.ConnectionStatus.CONNECTED
}

override fun getConnectionType(): String? {
return null
}

override fun addConnectionStatusObserver(
observer: IConnectionStatusProvider.IConnectionStatusObserver
): Boolean {
return false
}

override fun removeConnectionStatusObserver(
observer: IConnectionStatusProvider.IConnectionStatusObserver
) {
// no-op
}
}

@RunWith(AndroidJUnit4::class)
class SdkInitTests : BaseUiTest() {
@Test
fun doubleInitDoesNotThrow() {
initSentry(false) { options: SentryAndroidOptions ->
options.tracesSampleRate = 1.0
options.profilesSampleRate = 1.0
options.readTimeoutMillis = 500
options.connectionTimeoutMillis = 500
}
val transaction = Sentry.startTransaction("e2etests", "testInit")
val sampleScenario = launchActivity<EmptyActivity>()
initSentry(false) { options: SentryAndroidOptions ->
options.tracesSampleRate = 1.0
options.profilesSampleRate = 1.0
options.readTimeoutMillis = 500
options.connectionTimeoutMillis = 500
}
transaction.finish()
sampleScenario.moveToState(Lifecycle.State.DESTROYED)
Expand All @@ -53,6 +86,9 @@ class SdkInitTests : BaseUiTest() {
// We use the same executorService before and after closing the SDK
it.executorService = options.executorService
it.isDebug = true
it.readTimeoutMillis = 500
it.connectionTimeoutMillis = 500
it.connectionStatusProvider = AlwaysOnlineStatusProvider()
}
val transaction = Sentry.startTransaction("e2etests", "testInit")
val sampleScenario = launchActivity<EmptyActivity>()
Expand All @@ -62,6 +98,9 @@ class SdkInitTests : BaseUiTest() {
// We use the same executorService before and after closing the SDK
it.executorService = options.executorService
it.isDebug = true
it.readTimeoutMillis = 500
it.connectionTimeoutMillis = 500
it.connectionStatusProvider = AlwaysOnlineStatusProvider()
}
relayIdlingResource.increment()
relayIdlingResource.increment()
Expand Down Expand Up @@ -103,7 +142,12 @@ class SdkInitTests : BaseUiTest() {
// Let's make the first request timeout
relay.addTimeoutResponse()

initSentry(true) { options: SentryAndroidOptions -> options.tracesSampleRate = 1.0 }
initSentry(true) { options: SentryAndroidOptions ->
options.tracesSampleRate = 1.0
options.readTimeoutMillis = 500
options.connectionTimeoutMillis = 500
options.connectionStatusProvider = AlwaysOnlineStatusProvider()
}

Sentry.startTransaction("beforeRestart", "emptyTransaction").finish()

Expand All @@ -120,6 +164,9 @@ class SdkInitTests : BaseUiTest() {
initSentry(true) { options: SentryAndroidOptions ->
options.tracesSampleRate = 1.0
options.profilesSampleRate = 1.0
options.readTimeoutMillis = 500
options.connectionTimeoutMillis = 500
options.connectionStatusProvider = AlwaysOnlineStatusProvider()
}
val afterRestart = System.currentTimeMillis()
val restartMs = afterRestart - beforeRestart
Expand Down Expand Up @@ -156,6 +203,7 @@ class SdkInitTests : BaseUiTest() {
initSentry(true) { options: SentryAndroidOptions ->
options.tracesSampleRate = 1.0
options.flushTimeoutMillis = 3000
options.connectionStatusProvider = AlwaysOnlineStatusProvider()
}

Sentry.startTransaction("beforeRestart", "emptyTransaction").finish()
Expand All @@ -170,6 +218,7 @@ class SdkInitTests : BaseUiTest() {
initSentry(true) { options: SentryAndroidOptions ->
options.tracesSampleRate = 1.0
options.profilesSampleRate = 1.0
options.connectionStatusProvider = AlwaysOnlineStatusProvider()
}
val afterRestart = System.currentTimeMillis()
val restartMs = afterRestart - beforeRestart
Expand Down Expand Up @@ -224,6 +273,9 @@ class SdkInitTests : BaseUiTest() {
initSentry(false) { options: SentryAndroidOptions ->
options.tracesSampleRate = 1.0
options.profilesSampleRate = 1.0
options.readTimeoutMillis = 500
options.connectionTimeoutMillis = 500
options.connectionStatusProvider = AlwaysOnlineStatusProvider()
}
}
activityScenario.moveToState(Lifecycle.State.DESTROYED)
Expand All @@ -244,6 +296,9 @@ class SdkInitTests : BaseUiTest() {
initSentry(false) { options: SentryAndroidOptions ->
options.tracesSampleRate = 1.0
options.profilesSampleRate = 1.0
options.readTimeoutMillis = 500
options.connectionTimeoutMillis = 500
options.connectionStatusProvider = AlwaysOnlineStatusProvider()
}
initLatch.countDown()
}
Expand Down
Loading