Fix: Set current thread only if theres no exceptions (#1064)

CHANGELOG.md

@@ -1,5 +1,7 @@
 # vNext
 
+* Fix: Set current thread only if theres no exceptions
+
 # 4.0.0-alpha.1
 
 * Enhancement: Load `sentry.properties` from the application's current working directory (#1046)

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

@@ -10,6 +10,7 @@
 import java.util.Locale;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 /** Class responsible for reading values from manifest and setting them to the options */
 final class ManifestMetadataReader {
@@ -44,6 +45,8 @@ final class ManifestMetadataReader {
   static final String UNCAUGHT_EXCEPTION_HANDLER_ENABLE =
       "io.sentry.uncaught-exception-handler.enable";
 
+  static final String ATTACH_THREADS = "io.sentry.attach-threads";
+
   /** ManifestMetadataReader ctor */
   private ManifestMetadataReader() {}
 
@@ -185,6 +188,11 @@ static void applyMetadata(
                 UNCAUGHT_EXCEPTION_HANDLER_ENABLE, options.isEnableUncaughtExceptionHandler());
         options.getLogger().log(SentryLevel.DEBUG, "enableUncaughtExceptionHandler read: %s", ndk);
         options.setEnableUncaughtExceptionHandler(enableUncaughtExceptionHandler);
+
+        final boolean attachThreads =
+            metadata.getBoolean(ATTACH_THREADS, options.isAttachThreads());
+        options.getLogger().log(SentryLevel.DEBUG, "attachThreads read: %s", attachThreads);
+        options.setAttachThreads(attachThreads);
       }
       options
           .getLogger()
@@ -228,7 +236,7 @@ static boolean isAutoInit(final @NotNull Context context, final @NotNull ILogger
    * @return the Bundle attached to the PackageManager
    * @throws PackageManager.NameNotFoundException if the package name is non-existent
    */
-  private static Bundle getMetadata(final @NotNull Context context)
+  private static @Nullable Bundle getMetadata(final @NotNull Context context)
       throws PackageManager.NameNotFoundException {
     final ApplicationInfo app =
         context

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

@@ -381,4 +381,33 @@ class ManifestMetadataReaderTest {
         // Assert
         assertTrue(options.isEnableUncaughtExceptionHandler)
     }
+
+    @Test
+    fun `applyMetadata reads attachThreads to options`() {
+        // Arrange
+        val options = SentryAndroidOptions()
+        val bundle = Bundle()
+        val mockContext = ContextUtilsTest.mockMetaData(metaData = bundle)
+        bundle.putBoolean(ManifestMetadataReader.ATTACH_THREADS, true)
+
+        // Act
+        ManifestMetadataReader.applyMetadata(mockContext, options)
+
+        // Assert
+        assertTrue(options.isAttachThreads)
+    }
+
+    @Test
+    fun `applyMetadata reads attachThreads and keep default value if not found`() {
+        // Arrange
+        val options = SentryAndroidOptions()
+        val bundle = Bundle()
+        val mockContext = ContextUtilsTest.mockMetaData(metaData = bundle)
+
+        // Act
+        ManifestMetadataReader.applyMetadata(mockContext, options)
+
+        // Assert
+        assertFalse(options.isAttachThreads)
+    }
 }

sentry/src/main/java/io/sentry/MainEventProcessor.java

@@ -18,24 +18,25 @@ public final class MainEventProcessor implements EventProcessor {
    */
   private static final String DEFAULT_ENVIRONMENT = "production";
 
-  private final SentryOptions options;
-  private final SentryThreadFactory sentryThreadFactory;
-  private final SentryExceptionFactory sentryExceptionFactory;
+  private final @NotNull SentryOptions options;
+  private final @NotNull SentryThreadFactory sentryThreadFactory;
+  private final @NotNull SentryExceptionFactory sentryExceptionFactory;
 
-  MainEventProcessor(final SentryOptions options) {
+  MainEventProcessor(final @NotNull SentryOptions options) {
     this.options = Objects.requireNonNull(options, "The SentryOptions is required.");
 
-    SentryStackTraceFactory sentryStackTraceFactory =
-        new SentryStackTraceFactory(options.getInAppExcludes(), options.getInAppIncludes());
+    final SentryStackTraceFactory sentryStackTraceFactory =
+        new SentryStackTraceFactory(
+            this.options.getInAppExcludes(), this.options.getInAppIncludes());
 
     sentryExceptionFactory = new SentryExceptionFactory(sentryStackTraceFactory);
     sentryThreadFactory = new SentryThreadFactory(sentryStackTraceFactory, this.options);
   }
 
   MainEventProcessor(
-      final SentryOptions options,
-      final SentryThreadFactory sentryThreadFactory,
-      final SentryExceptionFactory sentryExceptionFactory) {
+      final @NotNull SentryOptions options,
+      final @NotNull SentryThreadFactory sentryThreadFactory,
+      final @NotNull SentryExceptionFactory sentryExceptionFactory) {
     this.options = Objects.requireNonNull(options, "The SentryOptions is required.");
     this.sentryThreadFactory =
         Objects.requireNonNull(sentryThreadFactory, "The SentryThreadFactory is required.");
@@ -44,13 +45,14 @@ public final class MainEventProcessor implements EventProcessor {
   }
 
   @Override
-  public @NotNull SentryEvent process(SentryEvent event, @Nullable Object hint) {
+  public @NotNull SentryEvent process(
+      final @NotNull SentryEvent event, final @Nullable Object hint) {
     if (event.getPlatform() == null) {
       // this actually means JVM related.
       event.setPlatform("java");
     }
 
-    Throwable throwable = event.getThrowable();
+    final Throwable throwable = event.getThrowable();
     if (throwable != null) {
       event.setExceptions(sentryExceptionFactory.getSentryExceptions(throwable));
     }
@@ -69,7 +71,7 @@ public final class MainEventProcessor implements EventProcessor {
     return event;
   }
 
-  private void processNonCachedEvent(SentryEvent event) {
+  private void processNonCachedEvent(final @NotNull SentryEvent event) {
     if (event.getRelease() == null) {
       event.setRelease(options.getRelease());
     }
@@ -91,8 +93,12 @@ private void processNonCachedEvent(SentryEvent event) {
       // collecting threadIds that came from the exception mechanism, so we can mark threads as
       // crashed properly
       List<Long> mechanismThreadIds = null;
-      if (event.getExceptions() != null) {
-        for (SentryException item : event.getExceptions()) {
+
+      final boolean hasExceptions =
+          event.getExceptions() != null && !event.getExceptions().isEmpty();
+
+      if (hasExceptions) {
+        for (final SentryException item : event.getExceptions()) {
           if (item.getMechanism() != null && item.getThreadId() != null) {
             if (mechanismThreadIds == null) {
               mechanismThreadIds = new ArrayList<>();
@@ -104,9 +110,10 @@ private void processNonCachedEvent(SentryEvent event) {
 
       if (options.isAttachThreads()) {
         event.setThreads(sentryThreadFactory.getCurrentThreads(mechanismThreadIds));
-      } else if (options.isAttachStacktrace()) {
-        // when attachStacktrace is enabled, we attach only the current thread and its stack traces
-        event.setThreads(sentryThreadFactory.getCurrentThread(mechanismThreadIds));
+      } else if (options.isAttachStacktrace() && !hasExceptions) {
+        // when attachStacktrace is enabled, we attach only the current thread and its stack traces,
+        // if there are no exceptions, exceptions have its own stack traces.
+        event.setThreads(sentryThreadFactory.getCurrentThread());
       }
     }
   }

sentry/src/main/java/io/sentry/SentryThreadFactory.java

@@ -39,16 +39,15 @@ public SentryThreadFactory(
    * Converts the current thread to a SentryThread, it assumes its being called from the captured
    * thread.
    *
-   * @param mechanismThreadIds list of threadIds that came from exception mechanism
    * @return a list of SentryThread with a single item or null
    */
   @Nullable
-  List<SentryThread> getCurrentThread(final @Nullable List<Long> mechanismThreadIds) {
+  List<SentryThread> getCurrentThread() {
     final Map<Thread, StackTraceElement[]> threads = new HashMap<>();
     final Thread currentThread = Thread.currentThread();
     threads.put(currentThread, currentThread.getStackTrace());
 
-    return getCurrentThreads(threads, mechanismThreadIds);
+    return getCurrentThreads(threads, null);
   }
 
   /**

sentry/src/test/java/io/sentry/MainEventProcessorTest.kt

@@ -4,6 +4,7 @@ import com.nhaarman.mockitokotlin2.mock
 import io.sentry.hints.ApplyScopeData
 import io.sentry.hints.Cached
 import io.sentry.protocol.SdkVersion
+import java.lang.RuntimeException
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFalse
@@ -132,7 +133,7 @@ class MainEventProcessorTest {
     }
 
     @Test
-    fun `when processing an event and attach threads is disabled, threads should not be set`() {
+    fun `when attach threads is disabled, threads should not be set`() {
         val sut = fixture.getSut(attachThreads = false, attachStackTrace = false)
 
         var event = SentryEvent()
@@ -142,7 +143,7 @@ class MainEventProcessorTest {
     }
 
     @Test
-    fun `when processing an event and attach threads is enabled, threads should be set`() {
+    fun `when attach threads is enabled, threads should be set`() {
         val sut = fixture.getSut()
 
         var event = SentryEvent()
@@ -152,7 +153,7 @@ class MainEventProcessorTest {
     }
 
     @Test
-    fun `when processing an event and attach threads is disabled, but attach stacktrace is enabled, current thread should be set`() {
+    fun `when attach threads is disabled, but attach stacktrace is enabled, current thread should be set`() {
         val sut = fixture.getSut(attachThreads = false, attachStackTrace = true)
 
         var event = SentryEvent()
@@ -161,6 +162,16 @@ class MainEventProcessorTest {
         assertEquals(1, event.threads.count())
     }
 
+    @Test
+    fun `when attach threads is disabled, but attach stacktrace is enabled and has exceptions, threads should not be set`() {
+        val sut = fixture.getSut(attachThreads = false, attachStackTrace = true)
+
+        var event = SentryEvent(RuntimeException("error"))
+        event = sut.process(event, null)
+
+        assertNull(event.threads)
+    }
+
     @Test
     fun `sets sdkVersion in the event`() {
         val sut = fixture.getSut()

sentry/src/test/java/io/sentry/SentryThreadFactoryTest.kt

@@ -86,7 +86,7 @@ class SentryThreadFactoryTest {
     @Test
     fun `when getCurrentThread is called, returns current thread`() {
         val sut = fixture.getSut()
-        val threads = sut.getCurrentThread(null)
+        val threads = sut.currentThread
         assertEquals(1, threads!!.count())
     }
 }