feat: Options for uncaught exception and make SentryOptions list Thre…

…ad-Safe (#384)

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

@@ -40,6 +40,9 @@ final class ManifestMetadataReader {
   static final String BREADCRUMBS_SYSTEM_EVENTS_ENABLE = "io.sentry.breadcrumbs.system-events";
   static final String BREADCRUMBS_APP_COMPONENTS_ENABLE = "io.sentry.breadcrumbs.app-components";
 
+  static final String UNCAUGHT_EXCEPTION_HANDLER_ENABLE =
+      "io.sentry.uncaught-exception-handler.enable";
+
   /** ManifestMetadataReader ctor */
   private ManifestMetadataReader() {}
 
@@ -169,6 +172,12 @@ static void applyMetadata(
                 BREADCRUMBS_APP_COMPONENTS_ENABLE, options.isEnableAppComponentBreadcrumbs());
         options.getLogger().log(SentryLevel.DEBUG, "enableAppComponentBreadcrumbs read: %s", ndk);
         options.setEnableAppComponentBreadcrumbs(enableAppComponentBreadcrumbs);
+
+        final boolean enableUncaughtExceptionHandler =
+            metadata.getBoolean(
+                UNCAUGHT_EXCEPTION_HANDLER_ENABLE, options.isEnableUncaughtExceptionHandler());
+        options.getLogger().log(SentryLevel.DEBUG, "enableUncaughtExceptionHandler read: %s", ndk);
+        options.setEnableUncaughtExceptionHandler(enableUncaughtExceptionHandler);
       }
       options
           .getLogger()

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

@@ -350,4 +350,33 @@ class ManifestMetadataReaderTest {
         // Assert
         assertTrue(options.isEnableAppComponentBreadcrumbs)
     }
+
+    @Test
+    fun `applyMetadata reads enableUncaughtExceptionHandler to options`() {
+        // Arrange
+        val options = SentryAndroidOptions()
+        val bundle = Bundle()
+        val mockContext = ContextUtilsTest.mockMetaData(metaData = bundle)
+        bundle.putBoolean(ManifestMetadataReader.UNCAUGHT_EXCEPTION_HANDLER_ENABLE, false)
+
+        // Act
+        ManifestMetadataReader.applyMetadata(mockContext, options)
+
+        // Assert
+        assertFalse(options.isEnableUncaughtExceptionHandler)
+    }
+
+    @Test
+    fun `applyMetadata reads enableUncaughtExceptionHandler 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
+        assertTrue(options.isEnableUncaughtExceptionHandler)
+    }
 }

sentry-core/src/main/java/io/sentry/core/SentryOptions.java

@@ -5,8 +5,8 @@
 import io.sentry.core.transport.ITransportGate;
 import java.io.File;
 import java.net.Proxy;
-import java.util.ArrayList;
 import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -22,13 +22,13 @@ public class SentryOptions {
    * Are callbacks that run for every event. They can either return a new event which in most cases
    * means just adding data OR return null in case the event will be dropped and not sent.
    */
-  private final @NotNull List<EventProcessor> eventProcessors = new ArrayList<>();
+  private final @NotNull List<EventProcessor> eventProcessors = new CopyOnWriteArrayList<>();
 
   /**
    * Code that provides middlewares, bindings or hooks into certain frameworks or environments,
    * along with code that inserts those bindings and activates them.
    */
-  private final @NotNull List<Integration> integrations = new ArrayList<>();
+  private final @NotNull List<Integration> integrations = new CopyOnWriteArrayList<>();
 
   /**
    * The DSN tells the SDK where to send the events to. If this value is not provided, the SDK will
@@ -128,13 +128,13 @@ public class SentryOptions {
    * packages. Modules considered not to be part of the app will be hidden from stack traces by
    * default.
    */
-  private final @NotNull List<String> inAppExcludes = new ArrayList<>();
+  private final @NotNull List<String> inAppExcludes = new CopyOnWriteArrayList<>();
 
   /**
    * A list of string prefixes of module names that belong to the app. This option takes precedence
    * over inAppExcludes.
    */
-  private final @NotNull List<String> inAppIncludes = new ArrayList<>();
+  private final @NotNull List<String> inAppIncludes = new CopyOnWriteArrayList<>();
 
   /** The transport is an internal construct of the client that abstracts away the event sending. */
   private @Nullable ITransport transport;
@@ -172,6 +172,11 @@ public class SentryOptions {
   /** The server name used in the Sentry messages. */
   private String serverName;
 
+  /*
+  When enabled, Sentry installs UncaughtExceptionHandlerIntegration.
+   */
+  private boolean enableUncaughtExceptionHandler = true;
+
   /**
    * Adds an event processor
    *
@@ -780,6 +785,24 @@ public void setFlushTimeoutMillis(long flushTimeoutMillis) {
     this.flushTimeoutMillis = flushTimeoutMillis;
   }
 
+  /**
+   * Checks if the default UncaughtExceptionHandlerIntegration is enabled or not.
+   *
+   * @return true if enabled or false otherwise.
+   */
+  public boolean isEnableUncaughtExceptionHandler() {
+    return enableUncaughtExceptionHandler;
+  }
+
+  /**
+   * Enable or disable the default UncaughtExceptionHandlerIntegration.
+   *
+   * @param enableUncaughtExceptionHandler true if enabled or false otherwise.
+   */
+  public void setEnableUncaughtExceptionHandler(boolean enableUncaughtExceptionHandler) {
+    this.enableUncaughtExceptionHandler = enableUncaughtExceptionHandler;
+  }
+
   /** The BeforeSend callback */
   public interface BeforeSendCallback {
 

sentry-core/src/main/java/io/sentry/core/UncaughtExceptionHandlerIntegration.java

@@ -11,6 +11,7 @@
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 import org.jetbrains.annotations.TestOnly;
 
 /**
@@ -20,24 +21,24 @@
 public final class UncaughtExceptionHandlerIntegration
     implements Integration, Thread.UncaughtExceptionHandler, Closeable {
   /** Reference to the pre-existing uncaught exception handler. */
-  private Thread.UncaughtExceptionHandler defaultExceptionHandler;
+  private @Nullable Thread.UncaughtExceptionHandler defaultExceptionHandler;
 
-  private IHub hub;
-  private SentryOptions options;
+  private @Nullable IHub hub;
+  private @Nullable SentryOptions options;
 
   private boolean registered = false;
-  private final UncaughtExceptionHandler threadAdapter;
+  private final @NotNull UncaughtExceptionHandler threadAdapter;
 
-  UncaughtExceptionHandlerIntegration() {
+  public UncaughtExceptionHandlerIntegration() {
     this(UncaughtExceptionHandler.Adapter.getInstance());
   }
 
-  UncaughtExceptionHandlerIntegration(UncaughtExceptionHandler threadAdapter) {
+  UncaughtExceptionHandlerIntegration(final @NotNull UncaughtExceptionHandler threadAdapter) {
     this.threadAdapter = Objects.requireNonNull(threadAdapter, "threadAdapter is required.");
   }
 
   @Override
-  public final void register(IHub hub, SentryOptions options) {
+  public final void register(final @NotNull IHub hub, final @NotNull SentryOptions options) {
     if (registered) {
       options
           .getLogger()
@@ -48,60 +49,77 @@ public final void register(IHub hub, SentryOptions options) {
     }
     registered = true;
 
-    this.hub = hub;
-    this.options = options;
-    Thread.UncaughtExceptionHandler currentHandler =
-        threadAdapter.getDefaultUncaughtExceptionHandler();
-    if (currentHandler != null) {
-      options
-          .getLogger()
-          .log(
-              SentryLevel.DEBUG,
-              "default UncaughtExceptionHandler class='"
-                  + currentHandler.getClass().getName()
-                  + "'");
-      defaultExceptionHandler = currentHandler;
-    }
+    this.hub = Objects.requireNonNull(hub, "Hub is required");
+    this.options = Objects.requireNonNull(options, "SentryOptions is required");
+
+    this.options
+        .getLogger()
+        .log(
+            SentryLevel.DEBUG,
+            "UncaughtExceptionHandlerIntegration enabled: %s",
+            this.options.isEnableUncaughtExceptionHandler());
+
+    if (this.options.isEnableUncaughtExceptionHandler()) {
+      final Thread.UncaughtExceptionHandler currentHandler =
+          threadAdapter.getDefaultUncaughtExceptionHandler();
+      if (currentHandler != null) {
+        this.options
+            .getLogger()
+            .log(
+                SentryLevel.DEBUG,
+                "default UncaughtExceptionHandler class='"
+                    + currentHandler.getClass().getName()
+                    + "'");
+        defaultExceptionHandler = currentHandler;
+      }
 
-    threadAdapter.setDefaultUncaughtExceptionHandler(this);
+      threadAdapter.setDefaultUncaughtExceptionHandler(this);
 
-    options.getLogger().log(SentryLevel.DEBUG, "UncaughtExceptionHandlerIntegration installed.");
+      this.options
+          .getLogger()
+          .log(SentryLevel.DEBUG, "UncaughtExceptionHandlerIntegration installed.");
+    }
   }
 
   @Override
   public void uncaughtException(Thread thread, Throwable thrown) {
-    options.getLogger().log(SentryLevel.INFO, "Uncaught exception received.");
-
-    try {
-      UncaughtExceptionHint hint =
-          new UncaughtExceptionHint(options.getFlushTimeoutMillis(), options.getLogger());
-      Throwable throwable = getUnhandledThrowable(thread, thrown);
-      SentryEvent event = new SentryEvent(throwable);
-      event.setLevel(SentryLevel.FATAL);
-      this.hub.captureEvent(event, hint);
-      // Block until the event is flushed to disk
-      if (!hint.waitFlush()) {
+    if (options != null && hub != null) {
+      options.getLogger().log(SentryLevel.INFO, "Uncaught exception received.");
+
+      try {
+        final UncaughtExceptionHint hint =
+            new UncaughtExceptionHint(options.getFlushTimeoutMillis(), options.getLogger());
+        final Throwable throwable = getUnhandledThrowable(thread, thrown);
+        final SentryEvent event = new SentryEvent(throwable);
+        event.setLevel(SentryLevel.FATAL);
+        hub.captureEvent(event, hint);
+        // Block until the event is flushed to disk
+        if (!hint.waitFlush()) {
+          options
+              .getLogger()
+              .log(
+                  SentryLevel.WARNING,
+                  "Timed out waiting to flush event to disk before crashing. Event: %s",
+                  event.getEventId());
+        }
+      } catch (Exception e) {
         options
             .getLogger()
-            .log(
-                SentryLevel.WARNING,
-                "Timed out waiting to flush event to disk before crashing. Event: %s",
-                event.getEventId());
+            .log(SentryLevel.ERROR, "Error sending uncaught exception to Sentry.", e);
       }
-    } catch (Exception e) {
-      options.getLogger().log(SentryLevel.ERROR, "Error sending uncaught exception to Sentry.", e);
-    }
 
-    if (defaultExceptionHandler != null) {
-      options.getLogger().log(SentryLevel.INFO, "Invoking inner uncaught exception handler.");
-      defaultExceptionHandler.uncaughtException(thread, thrown);
+      if (defaultExceptionHandler != null) {
+        options.getLogger().log(SentryLevel.INFO, "Invoking inner uncaught exception handler.");
+        defaultExceptionHandler.uncaughtException(thread, thrown);
+      }
     }
   }
 
   @TestOnly
   @NotNull
-  static Throwable getUnhandledThrowable(Thread thread, Throwable thrown) {
-    Mechanism mechanism = new Mechanism();
+  static Throwable getUnhandledThrowable(
+      final @NotNull Thread thread, final @NotNull Throwable thrown) {
+    final Mechanism mechanism = new Mechanism();
     mechanism.setHandled(false);
     mechanism.setType("UncaughtExceptionHandler");
     return new ExceptionMechanismException(mechanism, thrown, thread);
@@ -112,6 +130,10 @@ public void close() {
     if (defaultExceptionHandler != null
         && this == threadAdapter.getDefaultUncaughtExceptionHandler()) {
       threadAdapter.setDefaultUncaughtExceptionHandler(defaultExceptionHandler);
+
+      if (options != null) {
+        options.getLogger().log(SentryLevel.DEBUG, "UncaughtExceptionHandlerIntegration removed.");
+      }
     }
   }
 
@@ -123,7 +145,7 @@ private static final class UncaughtExceptionHint implements DiskFlushNotificatio
 
     UncaughtExceptionHint(final long flushTimeoutMillis, final @NotNull ILogger logger) {
       this.flushTimeoutMillis = flushTimeoutMillis;
-      this.latch = new CountDownLatch(1);
+      latch = new CountDownLatch(1);
       this.logger = logger;
     }
 

sentry-core/src/test/java/io/sentry/core/UncaughtExceptionHandlerIntegrationTest.kt

@@ -1,7 +1,9 @@
 package io.sentry.core
 
 import com.nhaarman.mockitokotlin2.any
+import com.nhaarman.mockitokotlin2.argWhere
 import com.nhaarman.mockitokotlin2.mock
+import com.nhaarman.mockitokotlin2.never
 import com.nhaarman.mockitokotlin2.verify
 import com.nhaarman.mockitokotlin2.verifyZeroInteractions
 import com.nhaarman.mockitokotlin2.whenever
@@ -98,4 +100,25 @@ class UncaughtExceptionHandlerIntegrationTest {
         hub.close()
         verify(integrationMock).close()
     }
+
+    @Test
+    fun `When defaultUncaughtExceptionHandler is disabled, should not install Sentry UncaughtExceptionHandler`() {
+        val options = SentryOptions()
+        options.isEnableUncaughtExceptionHandler = false
+        val hub = mock<IHub>()
+        val handlerMock = mock<UncaughtExceptionHandler>()
+        val integration = UncaughtExceptionHandlerIntegration(handlerMock)
+        integration.register(hub, options)
+        verify(handlerMock, never()).defaultUncaughtExceptionHandler = any()
+    }
+
+    @Test
+    fun `When defaultUncaughtExceptionHandler is enabled, should install Sentry UncaughtExceptionHandler`() {
+        val options = SentryOptions()
+        val hub = mock<IHub>()
+        val handlerMock = mock<UncaughtExceptionHandler>()
+        val integration = UncaughtExceptionHandlerIntegration(handlerMock)
+        integration.register(hub, options)
+        verify(handlerMock).defaultUncaughtExceptionHandler = argWhere { it is UncaughtExceptionHandlerIntegration }
+    }
 }

sentry-sample/src/main/AndroidManifest.xml

@@ -67,5 +67,8 @@
 <!--    how to enable and set a sampleRate (anything between 0.01 and 1.0), it's disabled by default-->
 <!--    <meta-data android:name="io.sentry.sample-rate" android:value="0.5" />-->
 
+    <!--    how to disable default sentry uncaught exception-->
+<!--    <meta-data android:name="io.sentry.uncaught-exception-handler.enable" android:value="false" />-->
+
   </application>
 </manifest>