diff --git a/CHANGELOG.md b/CHANGELOG.md index f00c2d97b6..896e619709 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ - Add support for using Encoder with logback.SentryAppender ([#2246](https://github.com/getsentry/sentry-java/pull/2246)) - Add captureProfile method to hub and client ([#2290](https://github.com/getsentry/sentry-java/pull/2290)) +- Report Startup Crashes ([#2277](https://github.com/getsentry/sentry-java/pull/2277)) ## 6.5.0 diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index c124f8ee4c..11e5f679ce 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -45,9 +45,12 @@ public final class io/sentry/android/core/AppLifecycleIntegration : io/sentry/In public final class io/sentry/android/core/AppStartState { public fun getAppStartInterval ()Ljava/lang/Long; + public fun getAppStartMillis ()Ljava/lang/Long; public fun getAppStartTime ()Ljava/util/Date; public static fun getInstance ()Lio/sentry/android/core/AppStartState; public fun isColdStart ()Ljava/lang/Boolean; + public fun reset ()V + public fun setAppStartMillis (J)V } public final class io/sentry/android/core/BuildConfig { @@ -128,6 +131,7 @@ public final class io/sentry/android/core/SentryAndroidOptions : io/sentry/Sentr public fun getDebugImagesLoader ()Lio/sentry/android/core/IDebugImagesLoader; public fun getProfilingTracesHz ()I public fun getProfilingTracesIntervalMillis ()I + public fun getStartupCrashDurationThresholdMillis ()J public fun isAnrEnabled ()Z public fun isAnrReportInDebug ()Z public fun isAttachScreenshot ()Z @@ -216,3 +220,9 @@ public final class io/sentry/android/core/UserInteractionIntegration : android/a public fun register (Lio/sentry/IHub;Lio/sentry/SentryOptions;)V } +public final class io/sentry/android/core/cache/AndroidEnvelopeCache : io/sentry/cache/EnvelopeCache { + public fun (Lio/sentry/android/core/SentryAndroidOptions;)V + public static fun hasStartupCrashMarker (Lio/sentry/SentryOptions;)Z + public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V +} + 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 6eb31de464..8a6c244a4b 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 @@ -8,10 +8,10 @@ import android.content.res.AssetManager; import android.os.Build; import io.sentry.ILogger; -import io.sentry.SendCachedEnvelopeFireAndForgetIntegration; import io.sentry.SendFireAndForgetEnvelopeSender; import io.sentry.SendFireAndForgetOutboxSender; import io.sentry.SentryLevel; +import io.sentry.android.core.cache.AndroidEnvelopeCache; import io.sentry.android.fragment.FragmentLifecycleIntegration; import io.sentry.android.timber.SentryTimberIntegration; import io.sentry.util.Objects; @@ -132,6 +132,7 @@ static void init( ManifestMetadataReader.applyMetadata(context, options, buildInfoProvider); initializeCacheDirs(context, options); + options.setEnvelopeDiskCache(new AndroidEnvelopeCache(options)); final ActivityFramesTracker activityFramesTracker = new ActivityFramesTracker(loadClass, options.getLogger()); @@ -164,9 +165,14 @@ private static void installDefaultIntegrations( final boolean isFragmentAvailable, final boolean isTimberAvailable) { + // read the startup crash marker here to avoid doing double-IO for the SendCachedEnvelope + // integrations below + final boolean hasStartupCrashMarker = AndroidEnvelopeCache.hasStartupCrashMarker(options); + options.addIntegration( - new SendCachedEnvelopeFireAndForgetIntegration( - new SendFireAndForgetEnvelopeSender(() -> options.getCacheDirPath()))); + new SendCachedEnvelopeIntegration( + new SendFireAndForgetEnvelopeSender(() -> options.getCacheDirPath()), + hasStartupCrashMarker)); // Integrations are registered in the same order. NDK before adding Watch outbox, // because sentry-native move files around and we don't want to watch that. @@ -184,8 +190,9 @@ private static void installDefaultIntegrations( // this should be executed after NdkIntegration because sentry-native move files on init. // and we'd like to send them right away options.addIntegration( - new SendCachedEnvelopeFireAndForgetIntegration( - new SendFireAndForgetOutboxSender(() -> options.getOutboxPath()))); + new SendCachedEnvelopeIntegration( + new SendFireAndForgetOutboxSender(() -> options.getOutboxPath()), + hasStartupCrashMarker)); options.addIntegration(new AnrIntegration(context)); options.addIntegration(new AppLifecycleIntegration()); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java index 7ad96f9ea0..263f6e10d3 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AppStartState.java @@ -84,6 +84,11 @@ public Date getAppStartTime() { return appStartTime; } + @Nullable + public Long getAppStartMillis() { + return appStartMillis; + } + synchronized void setAppStartTime(final long appStartMillis, final @NotNull Date appStartTime) { // method is synchronized because the SDK may by init. on a background thread. if (this.appStartTime != null && this.appStartMillis != null) { @@ -92,4 +97,16 @@ synchronized void setAppStartTime(final long appStartMillis, final @NotNull Date this.appStartTime = appStartTime; this.appStartMillis = appStartMillis; } + + @TestOnly + public synchronized void setAppStartMillis(final long appStartMillis) { + this.appStartMillis = appStartMillis; + } + + @TestOnly + public synchronized void reset() { + appStartTime = null; + appStartMillis = null; + appStartEndMillis = null; + } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java new file mode 100644 index 0000000000..61ec47618d --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java @@ -0,0 +1,84 @@ +package io.sentry.android.core; + +import io.sentry.IHub; +import io.sentry.Integration; +import io.sentry.SendCachedEnvelopeFireAndForgetIntegration; +import io.sentry.SentryLevel; +import io.sentry.SentryOptions; +import io.sentry.util.Objects; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import org.jetbrains.annotations.NotNull; + +final class SendCachedEnvelopeIntegration implements Integration { + + private final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory + factory; + private final boolean hasStartupCrashMarker; + + public SendCachedEnvelopeIntegration( + final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory, + final boolean hasStartupCrashMarker) { + this.factory = Objects.requireNonNull(factory, "SendFireAndForgetFactory is required"); + this.hasStartupCrashMarker = hasStartupCrashMarker; + } + + @Override + public void register(@NotNull IHub hub, @NotNull SentryOptions options) { + Objects.requireNonNull(hub, "Hub is required"); + final SentryAndroidOptions androidOptions = + Objects.requireNonNull( + (options instanceof SentryAndroidOptions) ? (SentryAndroidOptions) options : null, + "SentryAndroidOptions is required"); + + final String cachedDir = options.getCacheDirPath(); + if (!factory.hasValidPath(cachedDir, options.getLogger())) { + options.getLogger().log(SentryLevel.ERROR, "No cache dir path is defined in options."); + return; + } + + final SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget sender = + factory.create(hub, androidOptions); + + if (sender == null) { + androidOptions.getLogger().log(SentryLevel.ERROR, "SendFireAndForget factory is null."); + return; + } + + try { + Future future = + androidOptions + .getExecutorService() + .submit( + () -> { + try { + sender.send(); + } catch (Throwable e) { + androidOptions + .getLogger() + .log(SentryLevel.ERROR, "Failed trying to send cached events.", e); + } + }); + + if (hasStartupCrashMarker) { + androidOptions + .getLogger() + .log(SentryLevel.DEBUG, "Startup Crash marker exists, blocking flush."); + try { + future.get(androidOptions.getStartupCrashFlushTimeoutMillis(), TimeUnit.MILLISECONDS); + } catch (TimeoutException e) { + androidOptions + .getLogger() + .log(SentryLevel.DEBUG, "Synchronous send timed out, continuing in the background."); + } + } + + androidOptions.getLogger().log(SentryLevel.DEBUG, "SendCachedEnvelopeIntegration installed."); + } catch (Throwable e) { + androidOptions + .getLogger() + .log(SentryLevel.ERROR, "Failed to call the executor. Cached events will not be sent", e); + } + } +} diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java index e3a83964f4..9bcc107d21 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java @@ -9,8 +9,10 @@ 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; @@ -104,6 +106,7 @@ public static synchronized void init( options, context, logger, isFragmentAvailable, isTimberAvailable); configuration.configure(options); deduplicateIntegrations(options, isFragmentAvailable, isTimberAvailable); + resetEnvelopeCacheIfNeeded(options); }, true); } catch (IllegalAccessException e) { @@ -168,4 +171,18 @@ private static void deduplicateIntegrations( } } } + + /** + * 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()); + } + } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java index 32eb2e01ed..e456228537 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java @@ -8,6 +8,7 @@ import io.sentry.protocol.SdkVersion; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.TestOnly; /** Sentry SDK options for Android */ public final class SentryAndroidOptions extends SentryOptions { @@ -107,6 +108,29 @@ public final class SentryAndroidOptions extends SentryOptions { */ private boolean collectAdditionalContext = true; + /** + * Controls how many seconds to wait for sending events in case there were Startup Crashes in the + * previous run. Sentry SDKs normally send events from a background queue, but in the case of + * Startup Crashes, it blocks the execution of the {@link Sentry#init()} function for the amount + * of startupCrashFlushTimeoutMillis to make sure the events make it to Sentry. + * + *

When the timeout is reached, the execution will continue on background. + * + *

Default is 5000 = 5s. + */ + private long startupCrashFlushTimeoutMillis = 5000; // 5s + + /** + * Controls the threshold after the application startup time, within which a crash should happen + * to be considered a Startup Crash. + * + *

Startup Crashes are sent on {@link Sentry#init()} in a blocking way, controlled by {@link + * SentryAndroidOptions#startupCrashFlushTimeoutMillis}. + * + *

Default is 2000 = 2s. + */ + private final long startupCrashDurationThresholdMillis = 2000; // 2s + public SentryAndroidOptions() { setSentryClientName(BuildConfig.SENTRY_ANDROID_SDK_NAME + "/" + BuildConfig.VERSION_NAME); setSdkVersion(createSdkVersion()); @@ -332,4 +356,34 @@ public boolean isCollectAdditionalContext() { public void setCollectAdditionalContext(boolean collectAdditionalContext) { this.collectAdditionalContext = collectAdditionalContext; } + + /** + * Returns the Startup Crash flush timeout in Millis + * + * @return the timeout in Millis + */ + @ApiStatus.Internal + long getStartupCrashFlushTimeoutMillis() { + return startupCrashFlushTimeoutMillis; + } + + /** + * Sets the Startup Crash flush timeout in Millis + * + * @param startupCrashFlushTimeoutMillis the timeout in Millis + */ + @TestOnly + void setStartupCrashFlushTimeoutMillis(long startupCrashFlushTimeoutMillis) { + this.startupCrashFlushTimeoutMillis = startupCrashFlushTimeoutMillis; + } + + /** + * Returns the Startup Crash duration threshold in Millis + * + * @return the threshold in Millis + */ + @ApiStatus.Internal + public long getStartupCrashDurationThresholdMillis() { + return startupCrashDurationThresholdMillis; + } } 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 new file mode 100644 index 0000000000..e08ef8b128 --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java @@ -0,0 +1,109 @@ +package io.sentry.android.core.cache; + +import static io.sentry.SentryLevel.DEBUG; +import static io.sentry.SentryLevel.ERROR; + +import io.sentry.Hint; +import io.sentry.SentryEnvelope; +import io.sentry.SentryOptions; +import io.sentry.android.core.AppStartState; +import io.sentry.android.core.SentryAndroidOptions; +import io.sentry.android.core.internal.util.AndroidCurrentDateProvider; +import io.sentry.cache.EnvelopeCache; +import io.sentry.hints.DiskFlushNotification; +import io.sentry.transport.ICurrentDateProvider; +import io.sentry.util.HintUtils; +import io.sentry.util.Objects; +import java.io.File; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; + +@ApiStatus.Internal +public final class AndroidEnvelopeCache extends EnvelopeCache { + + private final @NotNull ICurrentDateProvider currentDateProvider; + + public AndroidEnvelopeCache(final @NotNull SentryAndroidOptions options) { + this(options, AndroidCurrentDateProvider.getInstance()); + } + + AndroidEnvelopeCache( + final @NotNull SentryAndroidOptions options, + final @NotNull ICurrentDateProvider currentDateProvider) { + super( + options, + Objects.requireNonNull(options.getCacheDirPath(), "cacheDirPath must not be null"), + options.getMaxCacheItems()); + this.currentDateProvider = currentDateProvider; + } + + @Override + public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) { + super.store(envelope, hint); + + final SentryAndroidOptions options = (SentryAndroidOptions) this.options; + + final Long appStartTime = AppStartState.getInstance().getAppStartMillis(); + if (HintUtils.hasType(hint, DiskFlushNotification.class) && appStartTime != null) { + long timeSinceSdkInit = currentDateProvider.getCurrentTimeMillis() - appStartTime; + if (timeSinceSdkInit <= options.getStartupCrashDurationThresholdMillis()) { + options + .getLogger() + .log( + DEBUG, + "Startup Crash detected %d milliseconds after SDK init. Writing a startup crash marker file to disk.", + timeSinceSdkInit); + writeStartupCrashMarkerFile(); + } + } + } + + 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 + final String outboxPath = options.getOutboxPath(); + if (outboxPath == null) { + options + .getLogger() + .log(DEBUG, "Outbox path is null, the startup crash marker file will not be written"); + return; + } + final File crashMarkerFile = new File(options.getOutboxPath(), STARTUP_CRASH_MARKER_FILE); + try { + crashMarkerFile.createNewFile(); + } catch (Throwable e) { + options.getLogger().log(ERROR, "Error writing the startup crash marker file to the disk", e); + } + } + + public static boolean hasStartupCrashMarker(final @NotNull SentryOptions options) { + final String outboxPath = options.getOutboxPath(); + if (outboxPath == null) { + options + .getLogger() + .log(DEBUG, "Outbox path is null, the startup crash marker file does not exist"); + return false; + } + + final File crashMarkerFile = new File(options.getOutboxPath(), STARTUP_CRASH_MARKER_FILE); + try { + final boolean exists = crashMarkerFile.exists(); + if (exists) { + if (!crashMarkerFile.delete()) { + options + .getLogger() + .log( + ERROR, + "Failed to delete the startup crash marker file. %s.", + crashMarkerFile.getAbsolutePath()); + } + } + return exists; + } catch (Throwable e) { + options + .getLogger() + .log(ERROR, "Error reading/deleting the startup crash marker file on the disk", e); + } + return false; + } +} diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidCurrentDateProvider.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidCurrentDateProvider.java new file mode 100644 index 0000000000..cdf66c319a --- /dev/null +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/util/AndroidCurrentDateProvider.java @@ -0,0 +1,22 @@ +package io.sentry.android.core.internal.util; + +import android.os.SystemClock; +import io.sentry.transport.ICurrentDateProvider; +import org.jetbrains.annotations.ApiStatus; + +@ApiStatus.Internal +public final class AndroidCurrentDateProvider implements ICurrentDateProvider { + + private static final ICurrentDateProvider instance = new AndroidCurrentDateProvider(); + + public static ICurrentDateProvider getInstance() { + return instance; + } + + private AndroidCurrentDateProvider() {} + + @Override + public long getCurrentTimeMillis() { + return SystemClock.uptimeMillis(); + } +} diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index 3f1ff1a586..2f26c4317a 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -9,8 +9,8 @@ import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.whenever import io.sentry.ILogger import io.sentry.MainEventProcessor -import io.sentry.SendCachedEnvelopeFireAndForgetIntegration import io.sentry.SentryOptions +import io.sentry.android.core.cache.AndroidEnvelopeCache import io.sentry.android.fragment.FragmentLifecycleIntegration import io.sentry.android.timber.SentryTimberIntegration import org.junit.runner.RunWith @@ -288,12 +288,12 @@ class AndroidOptionsInitializerTest { } @Test - fun `SendCachedEnvelopeFireAndForgetIntegration added to integration list`() { + fun `SendCachedEnvelopeIntegration added to integration list`() { fixture.initSut() val actual = fixture.sentryOptions.integrations - .firstOrNull { it is SendCachedEnvelopeFireAndForgetIntegration } + .firstOrNull { it is SendCachedEnvelopeIntegration } assertNotNull(actual) } @@ -364,4 +364,11 @@ class AndroidOptionsInitializerTest { fixture.sentryOptions.integrations.firstOrNull { it is SentryTimberIntegration } assertNull(actual) } + + @Test + fun `AndroidEnvelopeCache is set to options`() { + fixture.initSut() + + assertTrue { fixture.sentryOptions.envelopeDiskCache is AndroidEnvelopeCache } + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt new file mode 100644 index 0000000000..d0dedbf823 --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SendCachedEnvelopeIntegrationTest.kt @@ -0,0 +1,119 @@ +package io.sentry.android.core + +import com.nhaarman.mockitokotlin2.any +import com.nhaarman.mockitokotlin2.eq +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.never +import com.nhaarman.mockitokotlin2.verify +import com.nhaarman.mockitokotlin2.whenever +import io.sentry.IHub +import io.sentry.ILogger +import io.sentry.SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget +import io.sentry.SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory +import io.sentry.SentryLevel.DEBUG +import org.awaitility.kotlin.await +import java.util.concurrent.ExecutionException +import java.util.concurrent.TimeUnit.MILLISECONDS +import java.util.concurrent.atomic.AtomicBoolean +import kotlin.test.Test + +class SendCachedEnvelopeIntegrationTest { + private class Fixture { + val hub: IHub = mock() + val options = SentryAndroidOptions() + val logger = mock() + val factory = mock() + val flag = AtomicBoolean(true) + val sender = mock() + + fun getSut( + cacheDirPath: String? = "abc", + hasStartupCrashMarker: Boolean = false, + hasSender: Boolean = true, + delaySend: Long = 0L, + taskFails: Boolean = false + ): SendCachedEnvelopeIntegration { + options.cacheDirPath = cacheDirPath + options.setLogger(logger) + options.isDebug = true + + whenever(sender.send()).then { + Thread.sleep(delaySend) + if (taskFails) { + throw ExecutionException(RuntimeException("Something went wrong")) + } + flag.set(false) + } + whenever(factory.hasValidPath(any(), any())).thenCallRealMethod() + whenever(factory.create(any(), any())).thenReturn( + if (hasSender) { + sender + } else { + null + } + ) + + return SendCachedEnvelopeIntegration(factory, hasStartupCrashMarker) + } + } + + private val fixture = Fixture() + + @Test + fun `when cacheDirPath is not set, does nothing`() { + val sut = fixture.getSut(cacheDirPath = null) + + sut.register(fixture.hub, fixture.options) + + verify(fixture.factory, never()).create(any(), any()) + } + + @Test + fun `when factory returns null, does nothing`() { + val sut = fixture.getSut(hasSender = false) + + sut.register(fixture.hub, fixture.options) + + verify(fixture.factory).create(any(), any()) + verify(fixture.sender, never()).send() + } + + @Test + fun `when has factory and cacheDirPath set, submits task into queue`() { + val sut = fixture.getSut() + + sut.register(fixture.hub, fixture.options) + + await.untilFalse(fixture.flag) + verify(fixture.sender).send() + } + + @Test + fun `when has startup crash marker, awaits the task on the calling thread`() { + val sut = fixture.getSut(hasStartupCrashMarker = true) + + sut.register(fixture.hub, fixture.options) + + // we do not need to await here, because it's executed synchronously + verify(fixture.sender).send() + } + + @Test + fun `when synchronous send times out, continues the task on a background thread`() { + val sut = fixture.getSut(hasStartupCrashMarker = true, delaySend = 50) + fixture.options.startupCrashFlushTimeoutMillis = 10 + + sut.register(fixture.hub, fixture.options) + + // first wait until synchronous send times out and check that the logger was hit in the catch block + await.atLeast(11, MILLISECONDS) + verify(fixture.logger).log( + eq(DEBUG), + eq("Synchronous send timed out, continuing in the background.") + ) + + // then wait until the async send finishes in background + await.untilFalse(fixture.flag) + verify(fixture.sender).send() + } +} diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt index 3da78a530c..68178671cd 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt @@ -8,13 +8,17 @@ import com.nhaarman.mockitokotlin2.eq import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.never import com.nhaarman.mockitokotlin2.verify +import io.sentry.Hint import io.sentry.ILogger import io.sentry.Sentry +import io.sentry.SentryEnvelope import io.sentry.SentryLevel import io.sentry.SentryLevel.DEBUG import io.sentry.SentryLevel.FATAL import io.sentry.android.fragment.FragmentLifecycleIntegration import io.sentry.android.timber.SentryTimberIntegration +import io.sentry.cache.IEnvelopeCache +import io.sentry.transport.NoOpEnvelopeCache import org.junit.runner.RunWith import kotlin.test.BeforeTest import kotlin.test.Test @@ -131,4 +135,37 @@ class SentryAndroidTest { // but we just verify here that the single integration is preserved assertEquals(refOptions!!.integrations.filterIsInstance().size, 1) } + + @Test + fun `AndroidEnvelopeCache is reset if the user disabled caching via cacheDirPath`() { + var refOptions: SentryAndroidOptions? = null + + fixture.initSut { + it.cacheDirPath = null + + refOptions = it + } + + assertTrue { refOptions!!.envelopeDiskCache is NoOpEnvelopeCache } + } + + @Test + fun `envelopeCache remains unchanged if the user set their own IEnvelopCache impl`() { + var refOptions: SentryAndroidOptions? = null + + fixture.initSut { + it.cacheDirPath = null + it.setEnvelopeDiskCache(CustomEnvelopCache()) + + refOptions = it + } + + assertTrue { refOptions!!.envelopeDiskCache is CustomEnvelopCache } + } + + private class CustomEnvelopCache : IEnvelopeCache { + override fun iterator(): MutableIterator = TODO() + override fun store(envelope: SentryEnvelope, hint: Hint) = Unit + override fun discard(envelope: SentryEnvelope) = Unit + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt new file mode 100644 index 0000000000..1528730312 --- /dev/null +++ b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt @@ -0,0 +1,112 @@ +package io.sentry.android.core.cache + +import com.nhaarman.mockitokotlin2.mock +import com.nhaarman.mockitokotlin2.whenever +import io.sentry.SentryEnvelope +import io.sentry.android.core.AppStartState +import io.sentry.android.core.SentryAndroidOptions +import io.sentry.cache.EnvelopeCache +import io.sentry.hints.DiskFlushNotification +import io.sentry.transport.ICurrentDateProvider +import io.sentry.util.HintUtils +import java.io.File +import java.nio.file.Files +import java.nio.file.Path +import kotlin.test.BeforeTest +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class AndroidEnvelopeCacheTest { + private class Fixture { + private val dir: Path = Files.createTempDirectory("sentry-cache") + val envelope = mock { + whenever(it.header).thenReturn(mock()) + } + val options = SentryAndroidOptions() + val dateProvider = mock() + lateinit var markerFile: File + + fun getSut( + appStartMillis: Long? = null, + currentTimeMillis: Long? = null + ): AndroidEnvelopeCache { + options.cacheDirPath = dir.toAbsolutePath().toFile().absolutePath + val outboxDir = File(options.outboxPath!!) + outboxDir.mkdirs() + + markerFile = File(outboxDir, EnvelopeCache.STARTUP_CRASH_MARKER_FILE) + + if (appStartMillis != null) { + AppStartState.getInstance().setAppStartMillis(appStartMillis) + } + if (currentTimeMillis != null) { + whenever(dateProvider.currentTimeMillis).thenReturn(currentTimeMillis) + } + + return AndroidEnvelopeCache(options, dateProvider) + } + } + + private val fixture = Fixture() + + @BeforeTest + fun `set up`() { + AppStartState.getInstance().reset() + } + + @Test + fun `when no flush hint exists, does not write startup crash file`() { + val cache = fixture.getSut() + + cache.store(fixture.envelope) + + assertFalse(fixture.markerFile.exists()) + } + + @Test + fun `when startup time is null, does not write startup crash file`() { + val cache = fixture.getSut() + + val hints = HintUtils.createWithTypeCheckHint(DiskFlushHint()) + cache.store(fixture.envelope, hints) + + assertFalse(fixture.markerFile.exists()) + } + + @Test + fun `when time since sdk init is more than duration threshold, does not write startup crash file`() { + val cache = fixture.getSut(appStartMillis = 1000L, currentTimeMillis = 5000L) + + val hints = HintUtils.createWithTypeCheckHint(DiskFlushHint()) + cache.store(fixture.envelope, hints) + + assertFalse(fixture.markerFile.exists()) + } + + @Test + fun `when outbox dir is not set, does not write startup crash file`() { + val cache = fixture.getSut(appStartMillis = 1000L, currentTimeMillis = 2000L) + + fixture.options.cacheDirPath = null + + val hints = HintUtils.createWithTypeCheckHint(DiskFlushHint()) + cache.store(fixture.envelope, hints) + + assertFalse(fixture.markerFile.exists()) + } + + @Test + fun `when time since sdk init is less than duration threshold, writes startup crash file`() { + val cache = fixture.getSut(appStartMillis = 1000L, currentTimeMillis = 2000L) + + val hints = HintUtils.createWithTypeCheckHint(DiskFlushHint()) + cache.store(fixture.envelope, hints) + + assertTrue(fixture.markerFile.exists()) + } + + internal class DiskFlushHint : DiskFlushNotification { + override fun markFlushed() {} + } +} diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 6fdb89ff61..0ebf31c178 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -1441,6 +1441,7 @@ public class io/sentry/SentryOptions { public fun setEnvelopeDiskCache (Lio/sentry/cache/IEnvelopeCache;)V public fun setEnvelopeReader (Lio/sentry/IEnvelopeReader;)V public fun setEnvironment (Ljava/lang/String;)V + public fun setExecutorService (Lio/sentry/ISentryExecutorService;)V public fun setFlushTimeoutMillis (J)V public fun setHostnameVerifier (Ljavax/net/ssl/HostnameVerifier;)V public fun setIdleTimeout (Ljava/lang/Long;)V @@ -1953,12 +1954,14 @@ public final class io/sentry/UserFeedback$JsonKeys { public fun ()V } -public final class io/sentry/cache/EnvelopeCache : io/sentry/cache/IEnvelopeCache { +public class io/sentry/cache/EnvelopeCache : io/sentry/cache/IEnvelopeCache { public static final field CRASH_MARKER_FILE Ljava/lang/String; public static final field NATIVE_CRASH_MARKER_FILE Ljava/lang/String; public static final field PREFIX_CURRENT_SESSION_FILE Ljava/lang/String; + public static final field STARTUP_CRASH_MARKER_FILE Ljava/lang/String; public static final field SUFFIX_ENVELOPE_FILE Ljava/lang/String; protected static final field UTF_8 Ljava/nio/charset/Charset; + public fun (Lio/sentry/SentryOptions;Ljava/lang/String;I)V public static fun create (Lio/sentry/SentryOptions;)Lio/sentry/cache/IEnvelopeCache; public fun discard (Lio/sentry/SentryEnvelope;)V public fun iterator ()Ljava/util/Iterator; diff --git a/sentry/src/main/java/io/sentry/OutboxSender.java b/sentry/src/main/java/io/sentry/OutboxSender.java index 902e76bfb0..db7efb5812 100644 --- a/sentry/src/main/java/io/sentry/OutboxSender.java +++ b/sentry/src/main/java/io/sentry/OutboxSender.java @@ -3,6 +3,7 @@ import static io.sentry.SentryLevel.ERROR; import static io.sentry.cache.EnvelopeCache.PREFIX_CURRENT_SESSION_FILE; +import io.sentry.cache.EnvelopeCache; import io.sentry.hints.Flushable; import io.sentry.hints.Resettable; import io.sentry.hints.Retryable; @@ -96,7 +97,9 @@ protected void processFile(final @NotNull File file, @NotNull Hint hint) { @Override protected boolean isRelevantFileName(final @Nullable String fileName) { // ignore current.envelope - return fileName != null && !fileName.startsWith(PREFIX_CURRENT_SESSION_FILE); + return fileName != null + && !fileName.startsWith(PREFIX_CURRENT_SESSION_FILE) + && !fileName.startsWith(EnvelopeCache.STARTUP_CRASH_MARKER_FILE); // TODO: Use an extension to filter out relevant files } diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index e8f7d91751..d616aa03b5 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -1,9 +1,11 @@ package io.sentry; import io.sentry.cache.EnvelopeCache; +import io.sentry.cache.IEnvelopeCache; import io.sentry.config.PropertiesProviderFactory; import io.sentry.protocol.SentryId; import io.sentry.protocol.User; +import io.sentry.transport.NoOpEnvelopeCache; import io.sentry.util.FileUtils; import java.io.File; import java.lang.reflect.InvocationTargetException; @@ -241,7 +243,11 @@ private static boolean initConfigurations(final @NotNull SentryOptions options) if (cacheDirPath != null) { final File cacheDir = new File(cacheDirPath); cacheDir.mkdirs(); - options.setEnvelopeDiskCache(EnvelopeCache.create(options)); + final IEnvelopeCache envelopeCache = options.getEnvelopeDiskCache(); + // only overwrite the cache impl if it's not already set + if (envelopeCache instanceof NoOpEnvelopeCache) { + options.setEnvelopeDiskCache(EnvelopeCache.create(options)); + } } final String profilingTracesDirPath = options.getProfilingTracesDirPath(); diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index e7db4c41fa..50090385be 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -28,6 +28,7 @@ import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; /** Sentry SDK options */ @Open @@ -1085,7 +1086,9 @@ public ISentryExecutorService getExecutorService() { * * @param executorService the SentryExecutorService */ - void setExecutorService(final @NotNull ISentryExecutorService executorService) { + @ApiStatus.Internal + @TestOnly + public void setExecutorService(final @NotNull ISentryExecutorService executorService) { if (executorService != null) { this.executorService = executorService; } diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index e700e07d02..973a978fcc 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -6,6 +6,7 @@ import static io.sentry.SentryLevel.WARNING; import static java.lang.String.format; +import com.jakewharton.nopen.annotation.Open; import io.sentry.DateUtils; import io.sentry.Hint; import io.sentry.SentryCrashLastRunState; @@ -47,8 +48,9 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +@Open @ApiStatus.Internal -public final class EnvelopeCache extends CacheStrategy implements IEnvelopeCache { +public class EnvelopeCache extends CacheStrategy implements IEnvelopeCache { /** File suffix added to all serialized envelopes files. */ public static final String SUFFIX_ENVELOPE_FILE = ".envelope"; @@ -58,20 +60,22 @@ public final class EnvelopeCache extends CacheStrategy implements IEnvelopeCache public static final String CRASH_MARKER_FILE = "last_crash"; public static final String NATIVE_CRASH_MARKER_FILE = ".sentry-native/" + CRASH_MARKER_FILE; + public static final String STARTUP_CRASH_MARKER_FILE = "startup_crash"; + private final @NotNull Map fileNameMap = new WeakHashMap<>(); public static @NotNull IEnvelopeCache create(final @NotNull SentryOptions options) { final String cacheDirPath = options.getCacheDirPath(); final int maxCacheItems = options.getMaxCacheItems(); if (cacheDirPath == null) { - options.getLogger().log(WARNING, "maxCacheItems is null, returning NoOpEnvelopeCache"); + options.getLogger().log(WARNING, "cacheDirPath is null, returning NoOpEnvelopeCache"); return NoOpEnvelopeCache.getInstance(); } else { return new EnvelopeCache(options, cacheDirPath, maxCacheItems); } } - private EnvelopeCache( + public EnvelopeCache( final @NotNull SentryOptions options, final @NotNull String cacheDirPath, final int maxCacheItems) { diff --git a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt index 4b23fdea90..dc91a76c0b 100644 --- a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt +++ b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt @@ -315,6 +315,11 @@ class OutboxSenderTest { assertFalse(fixture.getSut().isRelevantFileName(EnvelopeCache.PREFIX_CURRENT_SESSION_FILE)) } + @Test + fun `when file name is startup crash marker, should be ignored`() { + assertFalse(fixture.getSut().isRelevantFileName(EnvelopeCache.STARTUP_CRASH_MARKER_FILE)) + } + @Test fun `when file name is relevant, should return true`() { assertTrue(fixture.getSut().isRelevantFileName("123.envelope")) diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 5717f40046..75658d06e6 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -5,6 +5,8 @@ import com.nhaarman.mockitokotlin2.argThat import com.nhaarman.mockitokotlin2.eq import com.nhaarman.mockitokotlin2.mock import com.nhaarman.mockitokotlin2.verify +import io.sentry.cache.EnvelopeCache +import io.sentry.cache.IEnvelopeCache import io.sentry.protocol.SentryId import org.junit.rules.TemporaryFolder import java.io.File @@ -266,6 +268,39 @@ class SentryTest { verify(logger).log(eq(SentryLevel.ERROR), any(), eq(initException)) } + @Test + fun `overrides envelope cache if it's not set`() { + var sentryOptions: SentryOptions? = null + + Sentry.init { + it.dsn = dsn + it.cacheDirPath = getTempPath() + sentryOptions = it + } + + assertTrue { sentryOptions!!.envelopeDiskCache is EnvelopeCache } + } + + @Test + fun `does not override envelope cache if it's already set`() { + var sentryOptions: SentryOptions? = null + + Sentry.init { + it.dsn = dsn + it.cacheDirPath = getTempPath() + it.setEnvelopeDiskCache(CustomEnvelopCache()) + sentryOptions = it + } + + assertTrue { sentryOptions!!.envelopeDiskCache is CustomEnvelopCache } + } + + private class CustomEnvelopCache : IEnvelopeCache { + override fun iterator(): MutableIterator = TODO() + override fun store(envelope: SentryEnvelope, hint: Hint) = Unit + override fun discard(envelope: SentryEnvelope) = Unit + } + private fun getTempPath(): String { val tempFile = Files.createTempDirectory("cache").toFile() tempFile.delete()