Fix ensure all options are processed before integrations are loaded (#2377)

Author: markushi

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Fixes
 
 - Remove profiler main thread io ([#2348](https://github.com/getsentry/sentry-java/pull/2348))
+- Fix ensure all options are processed before integrations are loaded ([#2377](https://github.com/getsentry/sentry-java/pull/2377))
 
 ### Features
 

sentry-android-core/api/sentry-android-core.api

@@ -224,6 +224,7 @@ public final class io/sentry/android/core/UserInteractionIntegration : android/a
 
 public final class io/sentry/android/core/cache/AndroidEnvelopeCache : io/sentry/cache/EnvelopeCache {
 	public fun <init> (Lio/sentry/android/core/SentryAndroidOptions;)V
+	public fun getDirectory ()Ljava/io/File;
 	public static fun hasStartupCrashMarker (Lio/sentry/SentryOptions;)Z
 	public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V
 }

sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java

@@ -16,6 +16,7 @@
 import io.sentry.android.core.internal.util.SentryFrameMetricsCollector;
 import io.sentry.android.fragment.FragmentLifecycleIntegration;
 import io.sentry.android.timber.SentryTimberIntegration;
+import io.sentry.transport.NoOpEnvelopeCache;
 import io.sentry.util.Objects;
 import java.io.BufferedInputStream;
 import java.io.File;
@@ -25,6 +26,7 @@
 import java.util.Properties;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
+import org.jetbrains.annotations.TestOnly;
 
 /**
  * Android Options initializer, it reads configurations from AndroidManifest and sets to the
@@ -42,62 +44,11 @@ private AndroidOptionsInitializer() {}
    * @param options the SentryAndroidOptions
    * @param context the Application context
    */
-  static void init(final @NotNull SentryAndroidOptions options, final @NotNull Context context) {
-    Objects.requireNonNull(context, "The application context is required.");
-    Objects.requireNonNull(options, "The options object is required.");
-
-    init(options, context, new AndroidLogger(), false, false);
-  }
-
-  /**
-   * Init method of the Android Options initializer
-   *
-   * @param options the SentryAndroidOptions
-   * @param context the Application context
-   * @param logger the ILogger interface
-   * @param isFragmentAvailable whether the Fragment integration is available on the classpath
-   * @param isTimberAvailable whether the Timber integration is available on the classpath
-   */
-  static void init(
-      final @NotNull SentryAndroidOptions options,
-      @NotNull Context context,
-      final @NotNull ILogger logger,
-      final boolean isFragmentAvailable,
-      final boolean isTimberAvailable) {
-    init(
-        options,
-        context,
-        logger,
-        new BuildInfoProvider(logger),
-        isFragmentAvailable,
-        isTimberAvailable);
-  }
-
-  /**
-   * Init method of the Android Options initializer
-   *
-   * @param options the SentryAndroidOptions
-   * @param context the Application context
-   * @param logger the ILogger interface
-   * @param buildInfoProvider the BuildInfoProvider interface
-   * @param isFragmentAvailable whether the Fragment integration is available on the classpath
-   * @param isTimberAvailable whether the Timber integration is available on the classpath
-   */
-  static void init(
-      final @NotNull SentryAndroidOptions options,
-      @NotNull Context context,
-      final @NotNull ILogger logger,
-      final @NotNull BuildInfoProvider buildInfoProvider,
-      final boolean isFragmentAvailable,
-      final boolean isTimberAvailable) {
-    init(
-        options,
-        context,
-        logger,
-        buildInfoProvider,
-        new LoadClass(),
-        isFragmentAvailable,
-        isTimberAvailable);
+  @TestOnly
+  static void loadDefaultAndMetadataOptions(
+      final @NotNull SentryAndroidOptions options, final @NotNull Context context) {
+    final ILogger logger = new AndroidLogger();
+    loadDefaultAndMetadataOptions(options, context, logger, new BuildInfoProvider(logger));
   }
 
   /**
@@ -107,18 +58,12 @@ static void init(
    * @param context the Application context
    * @param logger the ILogger interface
    * @param buildInfoProvider the BuildInfoProvider interface
-   * @param loadClass the LoadClass wrapper
-   * @param isFragmentAvailable whether the Fragment integration is available on the classpath
-   * @param isTimberAvailable whether the Timber integration is available on the classpath
    */
-  static void init(
+  static void loadDefaultAndMetadataOptions(
       final @NotNull SentryAndroidOptions options,
       @NotNull Context context,
       final @NotNull ILogger logger,
-      final @NotNull BuildInfoProvider buildInfoProvider,
-      final @NotNull LoadClass loadClass,
-      final boolean isFragmentAvailable,
-      final boolean isTimberAvailable) {
+      final @NotNull BuildInfoProvider buildInfoProvider) {
     Objects.requireNonNull(context, "The context is required.");
 
     // it returns null if ContextImpl, so let's check for nullability
@@ -134,7 +79,34 @@ static void init(
 
     ManifestMetadataReader.applyMetadata(context, options, buildInfoProvider);
     initializeCacheDirs(context, options);
-    options.setEnvelopeDiskCache(new AndroidEnvelopeCache(options));
+
+    readDefaultOptionValues(options, context, buildInfoProvider);
+  }
+
+  @TestOnly
+  static void initializeIntegrationsAndProcessors(
+      final @NotNull SentryAndroidOptions options, final @NotNull Context context) {
+    initializeIntegrationsAndProcessors(
+        options,
+        context,
+        new BuildInfoProvider(new AndroidLogger()),
+        new LoadClass(),
+        false,
+        false);
+  }
+
+  static void initializeIntegrationsAndProcessors(
+      final @NotNull SentryAndroidOptions options,
+      final @NotNull Context context,
+      final @NotNull BuildInfoProvider buildInfoProvider,
+      final @NotNull LoadClass loadClass,
+      final boolean isFragmentAvailable,
+      final boolean isTimberAvailable) {
+
+    if (options.getCacheDirPath() != null
+        && options.getEnvelopeDiskCache() instanceof NoOpEnvelopeCache) {
+      options.setEnvelopeDiskCache(new AndroidEnvelopeCache(options));
+    }
 
     final ActivityFramesTracker activityFramesTracker =
         new ActivityFramesTracker(loadClass, options);
@@ -148,8 +120,6 @@ static void init(
         isFragmentAvailable,
         isTimberAvailable);
 
-    readDefaultOptionValues(options, context, buildInfoProvider);
-
     options.addEventProcessor(
         new DefaultAndroidEventProcessor(context, buildInfoProvider, options));
     options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker));

sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java

@@ -9,10 +9,8 @@
 import io.sentry.Sentry;
 import io.sentry.SentryLevel;
 import io.sentry.SentryOptions;
-import io.sentry.android.core.cache.AndroidEnvelopeCache;
 import io.sentry.android.fragment.FragmentLifecycleIntegration;
 import io.sentry.android.timber.SentryTimberIntegration;
-import io.sentry.transport.NoOpEnvelopeCache;
 import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
 import java.util.Date;
@@ -102,11 +100,23 @@ public static synchronized void init(
                 (isTimberUpstreamAvailable
                     && classLoader.isClassAvailable(SENTRY_TIMBER_INTEGRATION_CLASS_NAME, options));
 
-            AndroidOptionsInitializer.init(
-                options, context, logger, isFragmentAvailable, isTimberAvailable);
+            final BuildInfoProvider buildInfoProvider = new BuildInfoProvider(logger);
+            final LoadClass loadClass = new LoadClass();
+
+            AndroidOptionsInitializer.loadDefaultAndMetadataOptions(
+                options, context, logger, buildInfoProvider);
+
             configuration.configure(options);
+
+            AndroidOptionsInitializer.initializeIntegrationsAndProcessors(
+                options,
+                context,
+                buildInfoProvider,
+                loadClass,
+                isFragmentAvailable,
+                isTimberAvailable);
+
             deduplicateIntegrations(options, isFragmentAvailable, isTimberAvailable);
-            resetEnvelopeCacheIfNeeded(options);
           },
           true);
     } catch (IllegalAccessException e) {
@@ -132,7 +142,8 @@ public static synchronized void init(
 
   /**
    * Deduplicate potentially duplicated Fragment and Timber integrations, which can be added
-   * automatically by our SDK as well as by the user. The user's ones win over ours.
+   * automatically by our SDK as well as by the user. The user's ones (provided first in the
+   * options.integrations list) win over ours.
    *
    * @param options SentryOptions to retrieve integrations from
    */
@@ -158,31 +169,17 @@ private static void deduplicateIntegrations(
     }
 
     if (fragmentIntegrations.size() > 1) {
-      for (int i = 0; i < fragmentIntegrations.size() - 1; i++) {
+      for (int i = 1; i < fragmentIntegrations.size(); i++) {
         final Integration integration = fragmentIntegrations.get(i);
         options.getIntegrations().remove(integration);
       }
     }
 
     if (timberIntegrations.size() > 1) {
-      for (int i = 0; i < timberIntegrations.size() - 1; i++) {
+      for (int i = 1; i < timberIntegrations.size(); i++) {
         final Integration integration = timberIntegrations.get(i);
         options.getIntegrations().remove(integration);
       }
     }
   }
-
-  /**
-   * Resets envelope cache if {@link SentryOptions#getCacheDirPath()} was set to null by the user
-   * and the IEnvelopCache implementation remained ours (AndroidEnvelopeCache), which relies on
-   * cacheDirPath set.
-   *
-   * @param options SentryOptions to retrieve cacheDirPath from
-   */
-  private static void resetEnvelopeCacheIfNeeded(final @NotNull SentryAndroidOptions options) {
-    if (options.getCacheDirPath() == null
-        && options.getEnvelopeDiskCache() instanceof AndroidEnvelopeCache) {
-      options.setEnvelopeDiskCache(NoOpEnvelopeCache.getInstance());
-    }
-  }
 }

sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java

@@ -17,6 +17,7 @@
 import java.io.File;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.TestOnly;
 
 @ApiStatus.Internal
 public final class AndroidEnvelopeCache extends EnvelopeCache {
@@ -58,6 +59,11 @@ public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) {
     }
   }
 
+  @TestOnly
+  public @NotNull File getDirectory() {
+    return directory;
+  }
+
   private void writeStartupCrashMarkerFile() {
     // we use outbox path always, as it's the one that will also contain markers if hybrid sdks
     // decide to write it, which will trigger the blocking init

sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt

@@ -52,8 +52,12 @@ class AndroidOptionsInitializerTest {
                 whenever(mockContext.applicationContext.cacheDir).thenReturn(file)
             }
             mockContext.configureContext()
+            AndroidOptionsInitializer.loadDefaultAndMetadataOptions(
+                sentryOptions,
+                if (useRealContext) context else mockContext
+            )
             sentryOptions.configureOptions()
-            AndroidOptionsInitializer.init(
+            AndroidOptionsInitializer.initializeIntegrationsAndProcessors(
                 sentryOptions,
                 if (useRealContext) context else mockContext
             )
@@ -71,12 +75,19 @@ class AndroidOptionsInitializerTest {
                     putString(ManifestMetadataReader.DSN, "https://[email protected]/123")
                 }
             )
-            sentryOptions.setDebug(true)
-            AndroidOptionsInitializer.init(
+            sentryOptions.isDebug = true
+            val buildInfo = createBuildInfo(minApi)
+
+            AndroidOptionsInitializer.loadDefaultAndMetadataOptions(
                 sentryOptions,
-                mockContext,
+                context,
                 logger,
-                createBuildInfo(minApi),
+                buildInfo
+            )
+            AndroidOptionsInitializer.initializeIntegrationsAndProcessors(
+                sentryOptions,
+                context,
+                buildInfo,
                 createClassMock(classToLoad),
                 isFragmentAvailable,
                 isTimberAvailable
@@ -381,9 +392,13 @@ class AndroidOptionsInitializerTest {
 
     @Test
     fun `When Activity Frames Tracking is enabled, the Activity Frames Tracker should be available`() {
-        fixture.initSut(hasAppContext = true, configureOptions = {
-            isEnableFramesTracking = true
-        })
+        fixture.initSut(
+            hasAppContext = true,
+            useRealContext = true,
+            configureOptions = {
+                isEnableFramesTracking = true
+            }
+        )
 
         val activityLifeCycleIntegration = fixture.sentryOptions.integrations
             .first { it is ActivityLifecycleIntegration }
@@ -395,7 +410,7 @@ class AndroidOptionsInitializerTest {
 
     @Test
     fun `When Frames Tracking is disabled, the Activity Frames Tracker should not be available`() {
-        fixture.initSut(hasAppContext = true, configureOptions = {
+        fixture.initSut(hasAppContext = true, useRealContext = true, configureOptions = {
             isEnableFramesTracking = false
         })
 
@@ -410,9 +425,13 @@ class AndroidOptionsInitializerTest {
     @Test
     fun `When Frames Tracking is initially disabled, but enabled via configureOptions it should be available`() {
         fixture.sentryOptions.isEnableFramesTracking = false
-        fixture.initSut(hasAppContext = true, configureOptions = {
-            isEnableFramesTracking = true
-        })
+        fixture.initSut(
+            hasAppContext = true,
+            useRealContext = true,
+            configureOptions = {
+                isEnableFramesTracking = true
+            }
+        )
 
         val activityLifeCycleIntegration = fixture.sentryOptions.integrations
             .first { it is ActivityLifecycleIntegration }

sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt

@@ -88,7 +88,21 @@ class AndroidTransactionProfilerTest {
     @BeforeTest
     fun `set up`() {
         context = ApplicationProvider.getApplicationContext()
-        AndroidOptionsInitializer.init(fixture.options, context, fixture.mockLogger, false, false)
+        val buildInfoProvider = BuildInfoProvider(fixture.mockLogger)
+        AndroidOptionsInitializer.loadDefaultAndMetadataOptions(
+            fixture.options,
+            context,
+            fixture.mockLogger,
+            buildInfoProvider
+        )
+        AndroidOptionsInitializer.initializeIntegrationsAndProcessors(
+            fixture.options,
+            context,
+            buildInfoProvider,
+            LoadClass(),
+            false,
+            false
+        )
         // Profiler doesn't start if the folder doesn't exists.
         // Usually it's generated when calling Sentry.init, but for tests we can create it manually.
         File(fixture.options.profilingTracesDirPath!!).mkdirs()