diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f6c22e1d5f..cc34fce0a8e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ ### Behavioral Changes +- Android: Raise the default `maxCacheItems` from 30 to 100, so offline devices keep more envelopes. `maxQueueSize` also defaults to 100 on Android to match it ([#6608](https://github.com/getsentry/sentry-java/pull/6608)) +- Android: After a startup crash, the crash envelope is sent before the rest of the offline cache, and `Sentry.init` only blocks until that envelope is sent ([#6608](https://github.com/getsentry/sentry-java/pull/6608)) - Nav3: Returning to an Activity or Fragment with its own `SentryNavEffect` within the app (e.g., closing the Activity that covered it, swiping back to its ViewPager2 page, or dismissing a dialog over it) now starts a new navigation transaction for the current destination if the previous one finished, so spans produced upon return can now be consistently captured ([#6482](https://github.com/getsentry/sentry-java/pull/6482)) - Nav3: `ui.load` transactions are bound to `IScope` in preference to Nav3 transactions when transitioning between Activities, matching Nav2 behavior (API 29+) ([#6484](https://github.com/getsentry/sentry-java/pull/6484)) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 712f981ecd2..99adc75ba03 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -783,17 +783,24 @@ public final class io/sentry/android/core/cache/AndroidEnvelopeCache : io/sentry public static final field LAST_TOMBSTONE_REPORT Ljava/lang/String; public fun (Lio/sentry/android/core/SentryAndroidOptions;)V public fun getDirectory ()Ljava/io/File; - public static fun hasStartupCrashMarker (Lio/sentry/SentryOptions;)Z public static fun lastReportedAnr (Lio/sentry/SentryOptions;)Ljava/lang/Long; public static fun lastReportedMemoryLimiter (Lio/sentry/SentryOptions;)Ljava/lang/Long; public static fun lastReportedTombstone (Lio/sentry/SentryOptions;)Ljava/lang/Long; public static fun markAnrReported (Lio/sentry/SentryOptions;J)V public static fun markMemoryLimiterReported (Lio/sentry/SentryOptions;J)V public static fun markTombstoneReported (Lio/sentry/SentryOptions;J)V + public static fun readStartupCrashMarker (Lio/sentry/SentryOptions;)Lio/sentry/android/core/cache/AndroidEnvelopeCache$StartupCrashMarker; public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V public fun storeEnvelope (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)Z } +public final class io/sentry/android/core/cache/AndroidEnvelopeCache$StartupCrashMarker { + public static final field NONE Lio/sentry/android/core/cache/AndroidEnvelopeCache$StartupCrashMarker; + public fun (Ljava/lang/String;)V + public fun exists ()Z + public fun getEnvelopeFileName ()Ljava/lang/String; +} + public abstract class io/sentry/android/core/performance/ActivityLifecycleCallbacksAdapter : android/app/Application$ActivityLifecycleCallbacks { public fun ()V public fun onActivityCreated (Landroid/app/Activity;Landroid/os/Bundle;)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 52a54b339dc..76ca33ab9af 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 @@ -429,8 +429,8 @@ static void installDefaultIntegrations( // read the startup crash marker here to avoid doing double-IO for the SendCachedEnvelope // integrations below - LazyEvaluator startupCrashMarkerEvaluator = - new LazyEvaluator<>(() -> AndroidEnvelopeCache.hasStartupCrashMarker(options)); + LazyEvaluator startupCrashMarkerEvaluator = + new LazyEvaluator<>(() -> AndroidEnvelopeCache.readStartupCrashMarker(options)); options.addIntegration( new SendCachedEnvelopeIntegration( 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 index 05352be394c..11b49d41f29 100644 --- 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 @@ -10,16 +10,17 @@ import io.sentry.SendCachedEnvelopeFireAndForgetIntegration; import io.sentry.SentryLevel; import io.sentry.SentryOptions; +import io.sentry.android.core.cache.AndroidEnvelopeCache.StartupCrashMarker; import io.sentry.transport.RateLimiter; import io.sentry.util.AutoClosableReentrantLock; import io.sentry.util.LazyEvaluator; import io.sentry.util.Objects; import java.io.Closeable; import java.io.IOException; +import java.util.concurrent.CountDownLatch; import java.util.concurrent.Future; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; import java.util.concurrent.atomic.AtomicBoolean; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -29,7 +30,7 @@ final class SendCachedEnvelopeIntegration private final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory; - private final @NotNull LazyEvaluator startupCrashMarkerEvaluator; + private final @NotNull LazyEvaluator startupCrashMarkerEvaluator; private final AtomicBoolean startupCrashHandled = new AtomicBoolean(false); private @Nullable IConnectionStatusProvider connectionStatusProvider; private @Nullable IScopes scopes; @@ -41,7 +42,7 @@ final class SendCachedEnvelopeIntegration public SendCachedEnvelopeIntegration( final @NotNull SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory factory, - final @NotNull LazyEvaluator startupCrashMarkerEvaluator) { + final @NotNull LazyEvaluator startupCrashMarkerEvaluator) { this.factory = Objects.requireNonNull(factory, "SendFireAndForgetFactory is required"); this.startupCrashMarkerEvaluator = startupCrashMarkerEvaluator; } @@ -86,6 +87,17 @@ public void onConnectionStatusChanged( private void sendCachedEnvelopes( final @NotNull IScopes scopes, final @NotNull SentryAndroidOptions options) { try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + // startupCrashMarkerEvaluator remains the same on subsequent runs, let's ensure we only block + // on the very first execution (=app start) + final @NotNull StartupCrashMarker startupCrashMarker = startupCrashMarkerEvaluator.getValue(); + final boolean blockOnStartupCrash = + startupCrashMarker.exists() && startupCrashHandled.compareAndSet(false, true); + final @Nullable String startupCrashFileName = + blockOnStartupCrash ? startupCrashMarker.getEnvelopeFileName() : null; + // released once the startup crash envelope is sent, or once the whole directory is sent if + // this sender can't find it + final CountDownLatch startupCrashSent = new CountDownLatch(1); + final Future future = options .getExecutorService() @@ -138,25 +150,30 @@ private void sendCachedEnvelopes( return; } + if (startupCrashFileName != null && sender.sendFile(startupCrashFileName)) { + startupCrashSent.countDown(); + } sender.send(); } catch (Throwable e) { options .getLogger() .log(SentryLevel.ERROR, "Failed trying to send cached events.", e); + } finally { + startupCrashSent.countDown(); } }); - // startupCrashMarkerEvaluator remains true on subsequent runs, let's ensure we only block on - // the very first execution (=app start) - if (startupCrashMarkerEvaluator.getValue() - && startupCrashHandled.compareAndSet(false, true)) { + // a cancelled task never runs, so there's nothing to wait for + if (blockOnStartupCrash && !future.isCancelled()) { options.getLogger().log(SentryLevel.DEBUG, "Startup Crash marker exists, blocking flush."); try { - future.get(options.getStartupCrashFlushTimeoutMillis(), TimeUnit.MILLISECONDS); - } catch (TimeoutException e) { - options - .getLogger() - .log(SentryLevel.DEBUG, "Synchronous send timed out, continuing in the background."); + if (!startupCrashSent.await( + options.getStartupCrashFlushTimeoutMillis(), TimeUnit.MILLISECONDS)) { + options + .getLogger() + .log( + SentryLevel.DEBUG, "Synchronous send timed out, continuing in the background."); + } } catch (InterruptedException e) { // We don't own the calling thread, so keep its interrupt for the code that does Thread.currentThread().interrupt(); 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 2afb245fe38..1d485774843 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 @@ -30,6 +30,8 @@ /** Sentry SDK options for Android */ public final class SentryAndroidOptions extends SentryOptions { + private static final int DEFAULT_MAX_CACHE_ITEMS = 100; + /** * Enable or disable ANR (Application Not Responding) Default is enabled Used by AnrIntegration */ @@ -337,6 +339,10 @@ public SentryAndroidOptions() { setSentryClientName(BuildConfig.SENTRY_ANDROID_SDK_NAME + "/" + BuildConfig.VERSION_NAME); setSdkVersion(createSdkVersion()); setAttachServerName(false); + // devices are offline more often than servers, so keep more envelopes on disk. The queue size + // matches it, so the transport queue and the dedupe of in-flight cached files fit the cache + setMaxCacheItems(DEFAULT_MAX_CACHE_ITEMS); + setMaxQueueSize(DEFAULT_MAX_CACHE_ITEMS); } private @NotNull SdkVersion createSdkVersion() { 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 index 13148097289..7bd4cec5188 100644 --- 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 @@ -23,6 +23,7 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.FileOutputStream; +import java.io.IOException; import java.io.OutputStream; import java.util.Arrays; import java.util.List; @@ -84,7 +85,7 @@ private boolean storeInternalAndroid(@NotNull SentryEnvelope envelope, @NotNull DEBUG, "Startup Crash detected %d milliseconds after SDK init. Writing a startup crash marker file to disk.", timeSinceSdkInit); - writeStartupCrashMarkerFile(); + writeStartupCrashMarkerFile(getEnvelopeFile(envelope).getName()); } } @@ -100,7 +101,11 @@ private boolean storeInternalAndroid(@NotNull SentryEnvelope envelope, @NotNull return directory.getFile(); } - private void writeStartupCrashMarkerFile() { + /** + * Writes the startup crash marker, which names the cached crash envelope so the next app start + * can send it ahead of the rest of the cache. + */ + private void writeStartupCrashMarkerFile(final @NotNull String envelopeFileName) { // 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(); @@ -118,42 +123,86 @@ private void writeStartupCrashMarkerFile() { return; } final File crashMarkerFile = new File(outboxDir, STARTUP_CRASH_MARKER_FILE); - try { - crashMarkerFile.createNewFile(); + try (final OutputStream outputStream = new FileOutputStream(crashMarkerFile)) { + outputStream.write(envelopeFileName.getBytes(UTF_8)); } 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) { + /** + * Reads and deletes the startup crash marker. + * + * @return the marker, {@link StartupCrashMarker#NONE} if there is none + */ + public static @NotNull StartupCrashMarker readStartupCrashMarker( + 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; + return StartupCrashMarker.NONE; } final File crashMarkerFile = new File(outboxPath, 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()); - } + if (!crashMarkerFile.exists()) { + return StartupCrashMarker.NONE; } - return exists; + @Nullable String content = null; + try { + content = FileUtils.readText(crashMarkerFile); + } catch (IOException e) { + options.getLogger().log(ERROR, "Error reading the startup crash marker file", e); + } + if (!crashMarkerFile.delete()) { + options + .getLogger() + .log( + ERROR, + "Failed to delete the startup crash marker file. %s.", + crashMarkerFile.getAbsolutePath()); + } + // older SDK versions, and possibly hybrid SDKs, write an empty marker + final @Nullable String envelopeFileName = content == null ? null : content.trim(); + return new StartupCrashMarker( + envelopeFileName == null || envelopeFileName.isEmpty() ? null : envelopeFileName); } catch (Throwable e) { options .getLogger() .log(ERROR, "Error reading/deleting the startup crash marker file on the disk", e); } - return false; + return StartupCrashMarker.NONE; + } + + /** The startup crash marker of the previous app run, read once on app start. */ + public static final class StartupCrashMarker { + public static final @NotNull StartupCrashMarker NONE = new StartupCrashMarker(false, null); + + private final boolean exists; + private final @Nullable String envelopeFileName; + + /** + * @param envelopeFileName the file name of the crash envelope in the cache dir, or null if the + * marker doesn't name one + */ + public StartupCrashMarker(final @Nullable String envelopeFileName) { + this(true, envelopeFileName); + } + + private StartupCrashMarker(final boolean exists, final @Nullable String envelopeFileName) { + this.exists = exists; + this.envelopeFileName = envelopeFileName; + } + + public boolean exists() { + return exists; + } + + public @Nullable String getEnvelopeFileName() { + return envelopeFileName; + } } private static @Nullable Long lastReportedMarker( 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 index 32b38764626..61a0373e5f2 100644 --- 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 @@ -10,6 +10,7 @@ import io.sentry.SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForget import io.sentry.SendCachedEnvelopeFireAndForgetIntegration.SendFireAndForgetFactory import io.sentry.SentryExecutorService import io.sentry.SentryLevel.DEBUG +import io.sentry.android.core.cache.AndroidEnvelopeCache.StartupCrashMarker import io.sentry.test.DeferredExecutorService import io.sentry.test.ImmediateExecutorService import io.sentry.transport.RateLimiter @@ -21,6 +22,7 @@ import kotlin.test.Test import org.awaitility.kotlin.await import org.mockito.kotlin.any import org.mockito.kotlin.eq +import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.times @@ -39,6 +41,8 @@ class SendCachedEnvelopeIntegrationTest { fun getSut( cacheDirPath: String? = "abc", hasStartupCrashMarker: Boolean = false, + startupCrashFileName: String? = null, + hasStartupCrashFile: Boolean = true, hasSender: Boolean = true, delaySend: Long = 0L, taskFails: Boolean = false, @@ -56,6 +60,7 @@ class SendCachedEnvelopeIntegrationTest { } flag.set(false) } + whenever(sender.sendFile(any())).thenReturn(hasStartupCrashFile) whenever(factory.hasValidPath(any(), any())).thenCallRealMethod() whenever(factory.create(any(), any())) .thenReturn( @@ -66,7 +71,10 @@ class SendCachedEnvelopeIntegrationTest { } ) - return SendCachedEnvelopeIntegration(factory, LazyEvaluator { hasStartupCrashMarker }) + val startupCrashMarker = + if (hasStartupCrashMarker) StartupCrashMarker(startupCrashFileName) + else StartupCrashMarker.NONE + return SendCachedEnvelopeIntegration(factory, LazyEvaluator { startupCrashMarker }) } } @@ -120,6 +128,72 @@ class SendCachedEnvelopeIntegrationTest { verify(fixture.sender).send() } + @Test + fun `when startup crash marker names a file, sends it first and only awaits it`() { + val sut = + fixture.getSut( + hasStartupCrashMarker = true, + startupCrashFileName = "crash.envelope", + delaySend = 1000, + ) + + sut.register(fixture.scopes, fixture.options) + + // the crash file was sent synchronously, the rest of the cache is still being sent + verify(fixture.sender).sendFile("crash.envelope") + assertThat(fixture.flag.get()).isTrue() + verify(fixture.logger, never()) + .log(eq(DEBUG), eq("Synchronous send timed out, continuing in the background.")) + + await.untilFalse(fixture.flag) + val inOrder = inOrder(fixture.sender) + inOrder.verify(fixture.sender).sendFile("crash.envelope") + inOrder.verify(fixture.sender).send() + } + + @Test + fun `when startup crash marker names a missing file, awaits the whole task`() { + val sut = + fixture.getSut( + hasStartupCrashMarker = true, + startupCrashFileName = "crash.envelope", + hasStartupCrashFile = false, + ) + + sut.register(fixture.scopes, fixture.options) + + // we do not need to await here, because it's executed synchronously + verify(fixture.sender).sendFile("crash.envelope") + verify(fixture.sender).send() + } + + @Test + fun `when startup crash marker is empty, does not send a single file and awaits the whole task`() { + val sut = fixture.getSut(hasStartupCrashMarker = true) + + sut.register(fixture.scopes, fixture.options) + + // we do not need to await here, because it's executed synchronously + verify(fixture.sender, never()).sendFile(any()) + verify(fixture.sender).send() + } + + @Test + fun `when startup crash marker exists, only sends the crash file first on the first run`() { + val sut = + fixture.getSut( + hasStartupCrashMarker = true, + startupCrashFileName = "crash.envelope", + mockExecutorService = ImmediateExecutorService(), + ) + + sut.register(fixture.scopes, fixture.options) + sut.onConnectionStatusChanged(ConnectionStatus.CONNECTED) + + verify(fixture.sender).sendFile("crash.envelope") + verify(fixture.sender, times(2)).send() + } + @Test fun `when synchronous send times out, continues the task on a background thread`() { val sut = fixture.getSut(hasStartupCrashMarker = true, delaySend = 1000) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt index a223358c1d1..0923354d1e4 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt @@ -12,6 +12,14 @@ import kotlin.test.assertTrue import org.mockito.kotlin.mock class SentryAndroidOptionsTest { + @Test + fun `init should raise maxCacheItems and maxQueueSize to 100`() { + val sentryOptions = SentryAndroidOptions() + + assertEquals(100, sentryOptions.maxCacheItems) + assertEquals(100, sentryOptions.maxQueueSize) + } + @Test fun `init should set clientName`() { val sentryOptions = SentryAndroidOptions() 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 index 47163c0e841..95f30ef4b97 100644 --- 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 @@ -137,6 +137,55 @@ class AndroidEnvelopeCacheTest { assertTrue(fixture.startupCrashMarkerFile.exists()) } + @Test + fun `startup crash marker names the cached crash envelope file`() { + val cache = fixture.getSut(dir = tmpDir, appStartMillis = 1000L, currentTimeMillis = 2000L) + + val hints = HintUtils.createWithTypeCheckHint(UncaughtHint()) + cache.storeEnvelope(fixture.envelope, hints) + + val envelopeFiles = + File(fixture.options.cacheDirPath!!).listFiles { _, name -> + name.endsWith(EnvelopeCache.SUFFIX_ENVELOPE_FILE) + }!! + assertThat(envelopeFiles).hasLength(1) + assertThat(fixture.startupCrashMarkerFile.readText()).isEqualTo(envelopeFiles[0].name) + } + + @Test + fun `readStartupCrashMarker returns NONE when there is no marker`() { + fixture.getSut(tmpDir) + + val marker = AndroidEnvelopeCache.readStartupCrashMarker(fixture.options) + + assertThat(marker.exists()).isFalse() + assertThat(marker.envelopeFileName).isNull() + } + + @Test + fun `readStartupCrashMarker returns the envelope file name and deletes the marker`() { + fixture.getSut(tmpDir) + fixture.startupCrashMarkerFile.writeText("crash.envelope") + + val marker = AndroidEnvelopeCache.readStartupCrashMarker(fixture.options) + + assertThat(marker.exists()).isTrue() + assertThat(marker.envelopeFileName).isEqualTo("crash.envelope") + assertThat(fixture.startupCrashMarkerFile.exists()).isFalse() + } + + @Test + fun `readStartupCrashMarker returns no envelope file name for an empty marker`() { + fixture.getSut(tmpDir) + fixture.startupCrashMarkerFile.createNewFile() + + val marker = AndroidEnvelopeCache.readStartupCrashMarker(fixture.options) + + assertThat(marker.exists()).isTrue() + assertThat(marker.envelopeFileName).isNull() + assertThat(fixture.startupCrashMarkerFile.exists()).isFalse() + } + @Test fun `creates outbox dir when writing startup crash file and dir does not exist yet`() { val cache = fixture.getSut(dir = tmpDir, appStartMillis = 1000L, currentTimeMillis = 2000L) diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 17ca4e62279..655303fb221 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -2866,6 +2866,7 @@ public final class io/sentry/SendCachedEnvelopeFireAndForgetIntegration : io/sen public abstract interface class io/sentry/SendCachedEnvelopeFireAndForgetIntegration$SendFireAndForget { public abstract fun send ()V + public fun sendFile (Ljava/lang/String;)Z } public abstract interface class io/sentry/SendCachedEnvelopeFireAndForgetIntegration$SendFireAndForgetDirPath { @@ -5030,6 +5031,7 @@ public class io/sentry/cache/EnvelopeCache : io/sentry/cache/IEnvelopeCache { public fun discard (Lio/sentry/SentryEnvelope;)V public fun flushPreviousSession ()V public static fun getCurrentSessionFile (Ljava/lang/String;)Ljava/io/File; + protected fun getEnvelopeFile (Lio/sentry/SentryEnvelope;)Ljava/io/File; public static fun getPreviousSessionFile (Ljava/lang/String;)Ljava/io/File; public fun iterator ()Ljava/util/Iterator; public fun movePreviousSession (Ljava/io/File;Ljava/io/File;)V diff --git a/sentry/src/main/java/io/sentry/DirectoryProcessor.java b/sentry/src/main/java/io/sentry/DirectoryProcessor.java index d9f0f32c40c..5496440854c 100644 --- a/sentry/src/main/java/io/sentry/DirectoryProcessor.java +++ b/sentry/src/main/java/io/sentry/DirectoryProcessor.java @@ -82,11 +82,7 @@ public void processDirectory(final @NotNull File directory) { logger.log(SentryLevel.DEBUG, "Processing file: %s", filePath); - final SendCachedEnvelopeHint cachedHint = - new SendCachedEnvelopeHint(flushTimeoutMillis, logger, filePath, processedEnvelopes); - - final Hint hint = HintUtils.createWithTypeCheckHint(cachedHint); - processFile(file, hint); + processFile(file, createCachedHint(filePath)); // a short delay between processing envelopes to avoid bursting our server and hitting // another rate limit https://develop.sentry.dev/sdk/features/#additional-capabilities @@ -105,6 +101,29 @@ public void processDirectory(final @NotNull File directory) { } } + /** + * Processes a single file of the directory, e.g. to send it ahead of the others. Once enqueued, + * {@link #processDirectory(File)} skips it like any other processed file. + * + * @param file the file to process + * @return false if the file doesn't exist or isn't relevant for this processor + */ + boolean processSingleFile(final @NotNull File file) { + if (!file.isFile() || !isRelevantFileName(file.getName())) { + logger.log(SentryLevel.DEBUG, "File %s is not a relevant file.", file.getAbsolutePath()); + return false; + } + final String filePath = file.getAbsolutePath(); + logger.log(SentryLevel.DEBUG, "Processing single file: %s", filePath); + processFile(file, createCachedHint(filePath)); + return true; + } + + private @NotNull Hint createCachedHint(final @NotNull String filePath) { + return HintUtils.createWithTypeCheckHint( + new SendCachedEnvelopeHint(flushTimeoutMillis, logger, filePath, processedEnvelopes)); + } + protected abstract void processFile(final @NotNull File file, final @NotNull Hint hint); protected abstract boolean isRelevantFileName(String fileName); diff --git a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java index b08e140392c..dcaf7e82505 100644 --- a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java +++ b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java @@ -28,6 +28,16 @@ public final class SendCachedEnvelopeFireAndForgetIntegration public interface SendFireAndForget { void send(); + + /** + * Sends the file with the given name from this sender's directory ahead of {@link #send()}. + * + * @param fileName the name of the file in this sender's directory + * @return false if this sender has no such file + */ + default boolean sendFile(final @NotNull String fileName) { + return false; + } } public interface SendFireAndForgetDirPath { @@ -52,12 +62,20 @@ default boolean hasValidPath(final @Nullable String dirPath, final @NotNull ILog final @NotNull String dirPath, final @NotNull ILogger logger) { final File dirFile = new File(dirPath); - return () -> { - logger.log(SentryLevel.DEBUG, "Started processing cached files from %s", dirPath); + return new SendFireAndForget() { + @Override + public void send() { + logger.log(SentryLevel.DEBUG, "Started processing cached files from %s", dirPath); + + directoryProcessor.processDirectory(dirFile); - directoryProcessor.processDirectory(dirFile); + logger.log(SentryLevel.DEBUG, "Finished processing cached files from %s", dirPath); + } - logger.log(SentryLevel.DEBUG, "Finished processing cached files from %s", dirPath); + @Override + public boolean sendFile(final @NotNull String fileName) { + return directoryProcessor.processSingleFile(new File(dirFile, fileName)); + } }; } } diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index 4a056c7609c..0ce90922ed9 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -2119,7 +2119,7 @@ public void setEnableShutdownHook(boolean enableShutdownHook) { } /** - * The max cache items for capping the number of events Default is 30 + * The max cache items for capping the number of events Default is 30, and 100 on Android * * @return the maxCacheItems */ diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index d3f473cfbc9..ebbb7ff70b1 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -426,7 +426,7 @@ public void discard(final @NotNull SentryEnvelope envelope) { * @param envelope the SentryEnvelope object * @return the file */ - private @NotNull File getEnvelopeFile(final @NotNull SentryEnvelope envelope) { + protected @NotNull File getEnvelopeFile(final @NotNull SentryEnvelope envelope) { try (final @NotNull ISentryLifecycleToken ignored = cacheLock.acquire()) { final @NotNull String fileName; if (fileNameMap.containsKey(envelope)) { diff --git a/sentry/src/test/java/io/sentry/DirectoryProcessorTest.kt b/sentry/src/test/java/io/sentry/DirectoryProcessorTest.kt index 970a1a99bf6..eeb81c6d2e0 100644 --- a/sentry/src/test/java/io/sentry/DirectoryProcessorTest.kt +++ b/sentry/src/test/java/io/sentry/DirectoryProcessorTest.kt @@ -118,6 +118,49 @@ class DirectoryProcessorTest { verify(fixture.scopes).captureEvent(any(), anyOrNull()) } + @Test + fun `processSingleFile processes the file and returns true`() { + val path = getTempEnvelope("envelope-event-attachment.txt") + + val event = SentryEvent() + val envelope = SentryEnvelope.from(fixture.serializer, event, null) + + whenever(fixture.envelopeReader.read(any())).thenReturn(envelope) + whenever(fixture.serializer.deserialize(any(), eq(SentryEvent::class.java))).thenReturn(event) + + val processed = fixture.getSut().processSingleFile(File(path)) + + assertThat(processed).isTrue() + verify(fixture.scopes).captureEvent(any(), anyOrNull()) + } + + @Test + fun `processSingleFile returns false when the file does not exist`() { + val processed = fixture.getSut().processSingleFile(File(file, "missing.tmp")) + + assertThat(processed).isFalse() + verify(fixture.scopes, never()).captureEvent(any(), anyOrNull()) + } + + @Test + fun `when envelope has been submitted by processSingleFile, processDirectory does not process it again`() { + val path = getTempEnvelope("envelope-event-attachment.txt") + + val event = SentryEvent() + val envelope = SentryEnvelope.from(fixture.serializer, event, null) + + whenever(fixture.envelopeReader.read(any())).thenReturn(envelope) + whenever(fixture.serializer.deserialize(any(), eq(SentryEvent::class.java))).thenReturn(event) + + // make it retryable so it doesn't get deleted + val sut = fixture.getSut(isRetryable = true) + sut.processSingleFile(File(path)) + sut.processDirectory(file) + + // should only capture once + verify(fixture.scopes).captureEvent(any(), anyOrNull()) + } + @Test fun `when rate limiting gets active in the middle of processing, stops processing`() { getTempEnvelope("envelope-event-attachment.txt")