Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

feat: Options for uncaught exception and make SentryOptions list Thread-Safe #384

Merged
merged 3 commits into from
May 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}

Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
33 changes: 28 additions & 5 deletions sentry-core/src/main/java/io/sentry/core/SentryOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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<>();
marandaneto marked this conversation as resolved.
Show resolved Hide resolved

/** The transport is an internal construct of the client that abstracts away the event sending. */
private @Nullable ITransport transport;
Expand Down Expand Up @@ -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
*
Expand Down Expand Up @@ -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 {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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()
Expand All @@ -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);
Expand All @@ -112,6 +130,10 @@ public void close() {
if (defaultExceptionHandler != null
&& this == threadAdapter.getDefaultUncaughtExceptionHandler()) {
threadAdapter.setDefaultUncaughtExceptionHandler(defaultExceptionHandler);

if (options != null) {
marandaneto marked this conversation as resolved.
Show resolved Hide resolved
options.getLogger().log(SentryLevel.DEBUG, "UncaughtExceptionHandlerIntegration removed.");
}
}
}

Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 }
}
}
3 changes: 3 additions & 0 deletions sentry-sample/src/main/AndroidManifest.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>