diff --git a/CHANGELOG.md b/CHANGELOG.md index 570cd9cc00a..546c2287608 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### Fixes +- Avoid StrictMode warnings (followup) ([#4809](https://github.com/getsentry/sentry-java/pull/4809)) - Avoid StrictMode warnings ([#4724](https://github.com/getsentry/sentry-java/pull/4724)) - Use logger from options for JVM profiler ([#4771](https://github.com/getsentry/sentry-java/pull/4771)) - Session Replay: Avoid deadlock when pausing replay if no connection ([#4788](https://github.com/getsentry/sentry-java/pull/4788)) 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 02006d0924b..7f98b9fbb14 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 @@ -449,7 +449,8 @@ private static void readDefaultOptionValues( if (options.getDistinctId() == null) { try { - options.setDistinctId(Installation.id(context)); + options.setDistinctId( + options.getRuntimeManager().runWithRelaxedPolicy(() -> Installation.id(context))); } catch (RuntimeException e) { options.getLogger().log(SentryLevel.ERROR, "Could not generate distinct Id.", e); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java index 4710b2506da..91bae9d7fe7 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AnrV2EventProcessor.java @@ -607,7 +607,7 @@ private void mergeUser(final @NotNull SentryBaseEvent event) { private @Nullable String getDeviceId() { try { - return Installation.id(context); + return options.getRuntimeManager().runWithRelaxedPolicy(() -> Installation.id(context)); } catch (Throwable e) { options.getLogger().log(SentryLevel.ERROR, "Error getting installationId.", e); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 1e37916aaee..dd4baed4630 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -166,7 +166,7 @@ private void mergeUser(final @NotNull SentryBaseEvent event) { // userId should be set even if event is Cached as the userId is static and won't change anyway. if (user.getId() == null) { - user.setId(Installation.id(context)); + user.setId(options.getRuntimeManager().runWithRelaxedPolicy(() -> Installation.id(context))); } if (user.getIpAddress() == null && options.isSendDefaultPii()) { user.setIpAddress(IpAddressUtils.DEFAULT_IP_ADDRESS); @@ -336,7 +336,7 @@ private void setAppExtras(final @NotNull App app, final @NotNull Hint hint) { */ public @NotNull User getDefaultUser(final @NotNull Context context) { final @NotNull User user = new User(); - user.setId(Installation.id(context)); + user.setId(options.getRuntimeManager().runWithRelaxedPolicy(() -> Installation.id(context))); return user; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DeviceInfoUtil.java b/sentry-android-core/src/main/java/io/sentry/android/core/DeviceInfoUtil.java index 7ba321426a1..5d590b9bfcd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DeviceInfoUtil.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DeviceInfoUtil.java @@ -227,18 +227,24 @@ private void setDeviceIO(final @NotNull Device device, final boolean includeDyna // this way of getting the size of storage might be problematic for storages bigger than 2GB // check the use of // https://developer.android.com/reference/java/io/File.html#getFreeSpace%28%29 - final @Nullable File internalStorageFile = context.getExternalFilesDir(null); - if (internalStorageFile != null) { - StatFs internalStorageStat = new StatFs(internalStorageFile.getPath()); - device.setStorageSize(getTotalInternalStorage(internalStorageStat)); - device.setFreeStorage(getUnusedInternalStorage(internalStorageStat)); - } - - final @Nullable StatFs externalStorageStat = getExternalStorageStat(internalStorageFile); - if (externalStorageStat != null) { - device.setExternalStorageSize(getTotalExternalStorage(externalStorageStat)); - device.setExternalFreeStorage(getUnusedExternalStorage(externalStorageStat)); - } + options + .getRuntimeManager() + .runWithRelaxedPolicy( + () -> { + final @Nullable File internalStorageFile = context.getExternalFilesDir(null); + if (internalStorageFile != null) { + StatFs internalStorageStat = new StatFs(internalStorageFile.getPath()); + device.setStorageSize(getTotalInternalStorage(internalStorageStat)); + device.setFreeStorage(getUnusedInternalStorage(internalStorageStat)); + } + + final @Nullable StatFs externalStorageStat = + getExternalStorageStat(internalStorageFile); + if (externalStorageStat != null) { + device.setExternalStorageSize(getTotalExternalStorage(externalStorageStat)); + device.setExternalFreeStorage(getUnusedExternalStorage(externalStorageStat)); + } + }); if (device.getConnectionType() == null) { // wifi, ethernet or cellular, null if none @@ -479,7 +485,7 @@ private Long getUnusedExternalStorage(final @NotNull StatFs stat) { @Nullable private String getDeviceId() { try { - return Installation.id(context); + return options.getRuntimeManager().runWithRelaxedPolicy(() -> Installation.id(context)); } catch (Throwable e) { options.getLogger().log(SentryLevel.ERROR, "Error getting installationId.", e); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java b/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java index cae558f0d43..7d0a7f77e58 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/InternalSentrySdk.java @@ -102,7 +102,8 @@ public static Map serializeScope( } if (user.getId() == null) { try { - user.setId(Installation.id(context)); + user.setId( + options.getRuntimeManager().runWithRelaxedPolicy(() -> Installation.id(context))); } catch (RuntimeException e) { logger.log(SentryLevel.ERROR, "Could not retrieve installation ID", e); } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java b/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java index af57727a8d2..6481be70b0e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java @@ -131,9 +131,10 @@ public static boolean hasStartupCrashMarker(final @NotNull SentryOptions options final File crashMarkerFile = new File(outboxPath, STARTUP_CRASH_MARKER_FILE); try { - final boolean exists = crashMarkerFile.exists(); + final boolean exists = + options.getRuntimeManager().runWithRelaxedPolicy(() -> crashMarkerFile.exists()); if (exists) { - if (!crashMarkerFile.delete()) { + if (!options.getRuntimeManager().runWithRelaxedPolicy(() -> crashMarkerFile.delete())) { options .getLogger() .log( diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidRuntimeManager.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidRuntimeManager.java index b24ac70b5b0..bed8d63ec11 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidRuntimeManager.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidRuntimeManager.java @@ -20,4 +20,13 @@ public T runWithRelaxedPolicy(final @NotNull IRuntimeManagerCallback toRu StrictMode.setVmPolicy(oldVmPolicy); } } + + @Override + public void runWithRelaxedPolicy(final @NotNull Runnable toRun) { + runWithRelaxedPolicy( + () -> { + toRun.run(); + return null; + }); + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidRuntimeManagerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidRuntimeManagerTest.kt index 427d1287423..6d4d1fd52de 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidRuntimeManagerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/util/AndroidRuntimeManagerTest.kt @@ -37,7 +37,7 @@ class AndroidRuntimeManagerTest { // Run the function and assert LAX policies called = - sut.runWithRelaxedPolicy { + sut.runWithRelaxedPolicy { assertEquals( StrictMode.ThreadPolicy.LAX.toString(), StrictMode.getThreadPolicy().toString(), @@ -64,6 +64,37 @@ class AndroidRuntimeManagerTest { StrictMode.setThreadPolicy(threadPolicy) StrictMode.setVmPolicy(vmPolicy) + // Run the function and assert LAX policies + try { + sut.runWithRelaxedPolicy { + assertEquals( + StrictMode.ThreadPolicy.LAX.toString(), + StrictMode.getThreadPolicy().toString(), + ) + assertEquals(StrictMode.VmPolicy.LAX.toString(), StrictMode.getVmPolicy().toString()) + called = true + throw Exception("Test exception") + } + } catch (_: Exception) {} + + // Policies should be reverted back + assertEquals(threadPolicy.toString(), StrictMode.getThreadPolicy().toString()) + assertEquals(vmPolicy.toString(), StrictMode.getVmPolicy().toString()) + + // Ensure the code ran + assertTrue(called) + } + + @Test + fun `runWithRelaxedPolicy with Runnable changes policy when running and restores it afterwards even if the code throws`() { + var called = false + val threadPolicy = StrictMode.ThreadPolicy.Builder().detectAll().penaltyDeath().build() + val vmPolicy = StrictMode.VmPolicy.Builder().detectAll().penaltyDeath().build() + + // Set and assert the StrictMode policies + StrictMode.setThreadPolicy(threadPolicy) + StrictMode.setVmPolicy(vmPolicy) + // Run the function and assert LAX policies try { sut.runWithRelaxedPolicy { diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/BaseUiTest.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/BaseUiTest.kt index 03667b537c1..62be835eb2b 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/BaseUiTest.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/BaseUiTest.kt @@ -157,8 +157,7 @@ internal fun SentryEnvelope.describeForTest(): String { val deserialized = JsonSerializer(SentryOptions()) .deserialize(item.data.inputStream().reader(), SentryEvent::class.java)!! - descr += - "Event (${deserialized.eventId}) - message: ${deserialized.message!!.formatted} -- " + descr += "Event (${deserialized.eventId}) - message: ${deserialized.message?.formatted} -- " } SentryItemType.Transaction -> { val deserialized = diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt index 8a6dc50c1fa..0bd5faf8d98 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt @@ -11,6 +11,7 @@ import io.sentry.android.core.AndroidLogger import io.sentry.android.core.CurrentActivityHolder import io.sentry.android.core.NdkIntegration import io.sentry.android.core.SentryAndroidOptions +import io.sentry.assertEnvelopeEvent import io.sentry.assertEnvelopeTransaction import io.sentry.protocol.SentryTransaction import java.util.concurrent.CountDownLatch @@ -256,10 +257,49 @@ class SdkInitTests : BaseUiTest() { @Test fun initNotThrowStrictMode() { StrictMode.setThreadPolicy(StrictMode.ThreadPolicy.Builder().detectAll().penaltyDeath().build()) - StrictMode.setVmPolicy(StrictMode.VmPolicy.Builder().detectAll().penaltyDeath().build()) + StrictMode.setVmPolicy( + StrictMode.VmPolicy.Builder() + .detectActivityLeaks() + // .detectCleartextNetwork() <- mockWebServer is on http, not https + .detectContentUriWithoutPermission() + .detectCredentialProtectedWhileLocked() + .detectFileUriExposure() + .detectImplicitDirectBoot() + .detectIncorrectContextUse() + .detectLeakedRegistrationObjects() + .detectLeakedSqlLiteObjects() + // .detectNonSdkApiUsage() <- thrown by leakCanary + // .detectUnsafeIntentLaunch() <- fails CI with java.lang.NoSuchMethodError + // .detectUntaggedSockets() <- thrown by mockWebServer + .penaltyDeath() + .build() + ) + initSentry(true) { it.tracesSampleRate = 1.0 } val sampleScenario = launchActivity() - initSentry() + relayIdlingResource.increment() + relayIdlingResource.increment() + Sentry.captureException(Exception("test")) sampleScenario.moveToState(Lifecycle.State.DESTROYED) + + // Avoid interferences with other tests and assertion logic + StrictMode.setThreadPolicy(StrictMode.ThreadPolicy.LAX) + StrictMode.setVmPolicy(StrictMode.VmPolicy.LAX) + + relay.assert { + findEnvelope { + assertEnvelopeEvent(it.items.toList()).exceptions!!.any { it.value == "test" } + } + .assert { + it.assertEvent() + it.assertNoOtherItems() + } + findEnvelope { assertEnvelopeTransaction(it.items.toList()).transaction == "EmptyActivity" } + .assert { + it.assertTransaction() + it.assertNoOtherItems() + } + assertNoOtherEnvelopes() + } } private fun assertDefaultIntegrations() { diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/mockservers/EnvelopeAsserter.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/mockservers/EnvelopeAsserter.kt index aa903004a3c..9b0106775ef 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/mockservers/EnvelopeAsserter.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/mockservers/EnvelopeAsserter.kt @@ -2,7 +2,9 @@ package io.sentry.uitest.android.mockservers import io.sentry.ProfilingTraceData import io.sentry.SentryEnvelope +import io.sentry.SentryEvent import io.sentry.android.core.AndroidLogger +import io.sentry.assertEnvelopeEvent import io.sentry.assertEnvelopeItem import io.sentry.assertEnvelopeProfile import io.sentry.assertEnvelopeTransaction @@ -27,6 +29,16 @@ class EnvelopeAsserter(val envelope: SentryEnvelope, val response: MockResponse) return item } + /** + * Asserts a transaction exists and returns the first one. It is then removed from internal list + * of unasserted items. + */ + fun assertEvent(): SentryEvent = + assertEnvelopeEvent(unassertedItems, AndroidLogger()) { index, item -> + unassertedItems.removeAt(index) + return item + } + /** * Asserts a transaction exists and returns the first one. It is then removed from internal list * of unasserted items. diff --git a/sentry-test-support/api/sentry-test-support.api b/sentry-test-support/api/sentry-test-support.api index ce9d0506ea6..8bfaaeefef1 100644 --- a/sentry-test-support/api/sentry-test-support.api +++ b/sentry-test-support/api/sentry-test-support.api @@ -1,4 +1,6 @@ public final class io/sentry/AssertionsKt { + public static final fun assertEnvelopeEvent (Ljava/util/List;Lio/sentry/ILogger;Lkotlin/jvm/functions/Function2;)Lio/sentry/SentryEvent; + public static synthetic fun assertEnvelopeEvent$default (Ljava/util/List;Lio/sentry/ILogger;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)Lio/sentry/SentryEvent; public static final fun assertEnvelopeFeedback (Ljava/util/List;Lio/sentry/ILogger;Lkotlin/jvm/functions/Function2;)Lio/sentry/SentryEvent; public static synthetic fun assertEnvelopeFeedback$default (Ljava/util/List;Lio/sentry/ILogger;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)Lio/sentry/SentryEvent; public static final fun assertEnvelopeProfile (Ljava/util/List;Lio/sentry/ILogger;Lkotlin/jvm/functions/Function2;)Lio/sentry/ProfilingTraceData; diff --git a/sentry-test-support/src/main/kotlin/io/sentry/Assertions.kt b/sentry-test-support/src/main/kotlin/io/sentry/Assertions.kt index 73affefa2fa..8d621adb1a2 100644 --- a/sentry-test-support/src/main/kotlin/io/sentry/Assertions.kt +++ b/sentry-test-support/src/main/kotlin/io/sentry/Assertions.kt @@ -69,6 +69,16 @@ inline fun assertEnvelopeItem( return item.second } +/** + * Asserts a transaction exists in [items] and returns the first one. Otherwise it throws an + * [AssertionError]. + */ +inline fun assertEnvelopeEvent( + items: List, + logger: ILogger = NoOpLogger.getInstance(), + predicate: (index: Int, item: SentryEvent) -> Unit = { _, _ -> }, +): SentryEvent = assertEnvelopeItem(items, SentryItemType.Event, logger, predicate) + /** * Asserts a transaction exists in [items] and returns the first one. Otherwise it throws an * [AssertionError]. diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 3964619e0b9..ed9339708b2 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -7297,6 +7297,7 @@ public final class io/sentry/util/UrlUtils$UrlDetails { public abstract interface class io/sentry/util/runtime/IRuntimeManager { public abstract fun runWithRelaxedPolicy (Lio/sentry/util/runtime/IRuntimeManager$IRuntimeManagerCallback;)Ljava/lang/Object; + public abstract fun runWithRelaxedPolicy (Ljava/lang/Runnable;)V } public abstract interface class io/sentry/util/runtime/IRuntimeManager$IRuntimeManagerCallback { @@ -7306,6 +7307,7 @@ public abstract interface class io/sentry/util/runtime/IRuntimeManager$IRuntimeM public final class io/sentry/util/runtime/NeutralRuntimeManager : io/sentry/util/runtime/IRuntimeManager { public fun ()V public fun runWithRelaxedPolicy (Lio/sentry/util/runtime/IRuntimeManager$IRuntimeManagerCallback;)Ljava/lang/Object; + public fun runWithRelaxedPolicy (Ljava/lang/Runnable;)V } public abstract interface class io/sentry/util/thread/IThreadChecker { diff --git a/sentry/src/main/java/io/sentry/util/runtime/IRuntimeManager.java b/sentry/src/main/java/io/sentry/util/runtime/IRuntimeManager.java index f6e27cce89b..ec0f8b05baf 100644 --- a/sentry/src/main/java/io/sentry/util/runtime/IRuntimeManager.java +++ b/sentry/src/main/java/io/sentry/util/runtime/IRuntimeManager.java @@ -7,6 +7,8 @@ public interface IRuntimeManager { T runWithRelaxedPolicy(final @NotNull IRuntimeManagerCallback toRun); + void runWithRelaxedPolicy(final @NotNull Runnable toRun); + interface IRuntimeManagerCallback { T run(); } diff --git a/sentry/src/main/java/io/sentry/util/runtime/NeutralRuntimeManager.java b/sentry/src/main/java/io/sentry/util/runtime/NeutralRuntimeManager.java index a6112ff60b3..36dd061612c 100644 --- a/sentry/src/main/java/io/sentry/util/runtime/NeutralRuntimeManager.java +++ b/sentry/src/main/java/io/sentry/util/runtime/NeutralRuntimeManager.java @@ -9,4 +9,9 @@ public final class NeutralRuntimeManager implements IRuntimeManager { public T runWithRelaxedPolicy(final @NotNull IRuntimeManagerCallback toRun) { return toRun.run(); } + + @Override + public void runWithRelaxedPolicy(final @NotNull Runnable toRun) { + toRun.run(); + } } diff --git a/sentry/src/test/java/io/sentry/util/runtime/NeutralRuntimeManagerTest.kt b/sentry/src/test/java/io/sentry/util/runtime/NeutralRuntimeManagerTest.kt index a645915d7ab..c8efdb68fec 100644 --- a/sentry/src/test/java/io/sentry/util/runtime/NeutralRuntimeManagerTest.kt +++ b/sentry/src/test/java/io/sentry/util/runtime/NeutralRuntimeManagerTest.kt @@ -11,7 +11,17 @@ class NeutralRuntimeManagerTest { fun `runWithRelaxedPolicy runs the code`() { var called = false - called = sut.runWithRelaxedPolicy { true } + called = sut.runWithRelaxedPolicy { true } + + // Ensure the code ran + assertTrue(called) + } + + @Test + fun `runWithRelaxedPolicy with runnable runs the code`() { + var called = false + + sut.runWithRelaxedPolicy { called = true } // Ensure the code ran assertTrue(called)