Feat: OutboxSender supports all envelope item types (#1158)

CHANGELOG.md

@@ -25,6 +25,7 @@
 * Fix: Set release and environment on Transactions (#1152)
 * Fix: Do not set transaction on the scope automatically   
 * Enhancement: Automatically assign span context to captured events (#1156)
+* Feat: OutboxSender supports all envelope item types #1158
 * Enhancement: Improve ITransaction and ISpan null-safety compatibility (#1161)
 
 # 4.0.0-alpha.2

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

@@ -9,6 +9,7 @@
 import io.sentry.hints.ApplyScopeData;
 import io.sentry.hints.Cached;
 import io.sentry.hints.Flushable;
+import io.sentry.hints.Resettable;
 import io.sentry.hints.Retryable;
 import io.sentry.hints.SubmissionResult;
 import io.sentry.util.Objects;
@@ -59,17 +60,17 @@ public void onEvent(int eventType, @Nullable String relativePath) {
   }
 
   private static final class CachedEnvelopeHint
-      implements Cached, Retryable, SubmissionResult, Flushable, ApplyScopeData {
-    boolean retry = false;
-    boolean succeeded = false;
+      implements Cached, Retryable, SubmissionResult, Flushable, ApplyScopeData, Resettable {
+    boolean retry;
+    boolean succeeded;
 
-    private @NotNull final CountDownLatch latch;
+    private @NotNull CountDownLatch latch;
     private final long flushTimeoutMillis;
     private final @NotNull ILogger logger;
 
     public CachedEnvelopeHint(final long flushTimeoutMillis, final @NotNull ILogger logger) {
+      reset();
       this.flushTimeoutMillis = flushTimeoutMillis;
-      this.latch = new CountDownLatch(1);
       this.logger = Objects.requireNonNull(logger, "ILogger is required.");
     }
 
@@ -104,5 +105,12 @@ public void setResult(boolean succeeded) {
     public boolean isSuccess() {
       return succeeded;
     }
+
+    @Override
+    public void reset() {
+      latch = new CountDownLatch(1);
+      retry = false;
+      succeeded = false;
+    }
   }
 }

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

@@ -4,7 +4,7 @@ import android.os.FileObserver
 import androidx.test.ext.junit.runners.AndroidJUnit4
 import com.nhaarman.mockitokotlin2.any
 import com.nhaarman.mockitokotlin2.anyOrNull
-import com.nhaarman.mockitokotlin2.argWhere
+import com.nhaarman.mockitokotlin2.check
 import com.nhaarman.mockitokotlin2.eq
 import com.nhaarman.mockitokotlin2.mock
 import com.nhaarman.mockitokotlin2.never
@@ -14,67 +14,105 @@ import io.sentry.IEnvelopeSender
 import io.sentry.ILogger
 import io.sentry.SentryOptions
 import io.sentry.hints.ApplyScopeData
+import io.sentry.hints.Resettable
+import io.sentry.hints.Retryable
+import io.sentry.hints.SubmissionResult
 import java.io.File
 import kotlin.test.Test
 import kotlin.test.assertEquals
 import kotlin.test.assertFailsWith
+import kotlin.test.assertFalse
 import org.junit.runner.RunWith
 
 @RunWith(AndroidJUnit4::class)
 class EnvelopeFileObserverTest {
 
     private class Fixture {
+        val fileName = "file-name.txt"
         var path: String? = "."
-        var envelopeSender: IEnvelopeSender = mock()
-        var logger: ILogger = mock()
-        var options: SentryOptions = SentryOptions()
-
-        init {
-            options.isDebug = true
-            options.setLogger(logger)
+        val envelopeSender = mock<IEnvelopeSender>()
+        val logger = mock<ILogger>()
+        val options = SentryOptions().apply {
+            isDebug = true
+            setLogger(logger)
         }
 
-        fun getSut(): EnvelopeFileObserver {
-            return EnvelopeFileObserver(path, envelopeSender, logger, options.flushTimeoutMillis)
+        fun getSut(flushTimeoutMillis: Long): EnvelopeFileObserver {
+            return EnvelopeFileObserver(path, envelopeSender, logger, flushTimeoutMillis)
         }
     }
 
     private val fixture = Fixture()
 
     @Test
     fun `envelope sender is called with fully qualified path`() {
-        val sut = fixture.getSut()
-        val param = "file-name.txt"
-        sut.onEvent(FileObserver.CLOSE_WRITE, param)
-        verify(fixture.envelopeSender).processEnvelopeFile(eq(fixture.path + File.separator + param), any())
+        triggerEvent()
+
+        verify(fixture.envelopeSender).processEnvelopeFile(eq(fixture.path + File.separator + fixture.fileName), any())
     }
 
     @Test
     fun `when event type is not close write, envelope sender is not called`() {
-        val sut = fixture.getSut()
-        sut.onEvent(FileObserver.CLOSE_WRITE.inv(), "file-name.txt")
+        triggerEvent(eventType = FileObserver.CLOSE_WRITE.inv())
+
         verifyZeroInteractions(fixture.envelopeSender)
     }
 
     @Test
     fun `when event is fired with null path, envelope reader is not called`() {
-        val sut = fixture.getSut()
-        sut.onEvent(0, null)
+        triggerEvent(relativePath = null)
+
         verify(fixture.envelopeSender, never()).processEnvelopeFile(anyOrNull(), any())
     }
 
     @Test
     fun `when null is passed as a path, ctor throws`() {
         fixture.path = null
-        val exception = assertFailsWith<Exception> { fixture.getSut() }
+        val exception = assertFailsWith<Exception> { fixture.getSut(0) }
         assertEquals("File path is required.", exception.message)
     }
 
     @Test
     fun `envelope sender is called with fully qualified path and ApplyScopeData hint`() {
-        val sut = fixture.getSut()
-        val param = "file-name.txt"
-        sut.onEvent(FileObserver.CLOSE_WRITE, param)
-        verify(fixture.envelopeSender).processEnvelopeFile(eq(fixture.path + File.separator + param), argWhere { it is ApplyScopeData })
+        triggerEvent()
+
+        verify(fixture.envelopeSender).processEnvelopeFile(
+                eq(fixture.path + File.separator + fixture.fileName),
+                check { it is ApplyScopeData })
+    }
+
+    @Test
+    fun `envelope sender Hint is Resettable`() {
+        triggerEvent()
+
+        verify(fixture.envelopeSender).processEnvelopeFile(
+                eq(fixture.path + File.separator + fixture.fileName),
+                check { it is Resettable })
+    }
+
+    @Test
+    fun `Hint resets its state`() {
+        triggerEvent(flushTimeoutMillis = 0)
+
+        verify(fixture.envelopeSender).processEnvelopeFile(
+                eq(fixture.path + File.separator + fixture.fileName),
+                check {
+                    (it as SubmissionResult).setResult(true)
+                    (it as Retryable).isRetry = true
+
+                    (it as Resettable).reset()
+
+                    assertFalse(it.isRetry)
+                    assertFalse(it.isSuccess)
+                })
+    }
+
+    private fun triggerEvent(
+        flushTimeoutMillis: Long = 15_000,
+        eventType: Int = FileObserver.CLOSE_WRITE,
+        relativePath: String? = fixture.fileName
+    ) {
+        val sut = fixture.getSut(flushTimeoutMillis)
+        sut.onEvent(eventType, relativePath)
     }
 }

sentry/api/sentry.api

@@ -1144,6 +1144,10 @@ public abstract interface class io/sentry/hints/Flushable {
 	public abstract fun waitFlush ()Z
 }
 
+public abstract interface class io/sentry/hints/Resettable {
+	public abstract fun reset ()V
+}
+
 public abstract interface class io/sentry/hints/Retryable {
 	public abstract fun isRetry ()Z
 	public abstract fun setRetry (Z)V

sentry/src/main/java/io/sentry/OutboxSender.java

@@ -4,6 +4,7 @@
 import static io.sentry.cache.EnvelopeCache.PREFIX_CURRENT_SESSION_FILE;
 
 import io.sentry.hints.Flushable;
+import io.sentry.hints.Resettable;
 import io.sentry.hints.Retryable;
 import io.sentry.hints.SubmissionResult;
 import io.sentry.util.CollectionUtils;
@@ -107,6 +108,7 @@ private void processEnvelope(final @NotNull SentryEnvelope envelope, final @Null
         "Processing Envelope with %d item(s)",
         CollectionUtils.size(envelope.getItems()));
     int items = 0;
+
     for (final SentryEnvelopeItem item : envelope.getItems()) {
       items++;
 
@@ -138,26 +140,37 @@ private void processEnvelope(final @NotNull SentryEnvelope envelope, final @Null
             }
             hub.captureEvent(event, hint);
             logger.log(SentryLevel.DEBUG, "Item %d is being captured.", items);
-            if (hint instanceof Flushable) {
-              if (!((Flushable) hint).waitFlush()) {
-                logger.log(
-                    SentryLevel.WARNING,
-                    "Timed out waiting for event submission: %s",
-                    event.getEventId());
-
-                break;
-              }
-            } else {
-              LogUtils.logIfNotFlushable(logger, hint);
+
+            if (!waitFlush(hint)) {
+              logger.log(
+                  SentryLevel.WARNING,
+                  "Timed out waiting for event submission: %s",
+                  event.getEventId());
+              break;
             }
           }
         } catch (Exception e) {
           logger.log(ERROR, "Item failed to process.", e);
         }
       } else {
-        // TODO: Handle attachments and other types
+        // send unknown item types over the wire
+        final SentryEnvelope newEnvelope =
+            new SentryEnvelope(
+                envelope.getHeader().getEventId(), envelope.getHeader().getSdkVersion(), item);
+        hub.captureEnvelope(newEnvelope, hint);
         logger.log(
-            SentryLevel.WARNING, "Item %d of type: %s ignored.", items, item.getHeader().getType());
+            SentryLevel.DEBUG,
+            "%s item %d is being captured.",
+            item.getHeader().getType().getItemType(),
+            items);
+
+        if (!waitFlush(hint)) {
+          logger.log(
+              SentryLevel.WARNING,
+              "Timed out waiting for item type submission: %s",
+              item.getHeader().getType().getItemType());
+          break;
+        }
       }
 
       if (hint instanceof SubmissionResult) {
@@ -171,6 +184,20 @@ private void processEnvelope(final @NotNull SentryEnvelope envelope, final @Null
           break;
         }
       }
+
+      // reset the Hint to its initial state as we use it multiple times.
+      if (hint instanceof Resettable) {
+        ((Resettable) hint).reset();
+      }
+    }
+  }
+
+  private boolean waitFlush(final @Nullable Object hint) {
+    if (hint instanceof Flushable) {
+      return ((Flushable) hint).waitFlush();
+    } else {
+      LogUtils.logIfNotFlushable(logger, hint);
     }
+    return true;
   }
 }

sentry/src/main/java/io/sentry/hints/Resettable.java

@@ -0,0 +1,7 @@
+package io.sentry.hints;
+
+/** Marker interface for a reusable Hint */
+public interface Resettable {
+  /** Reset the Hint to its initial state */
+  void reset();
+}

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

@@ -24,17 +24,10 @@ import kotlin.test.assertTrue
 class OutboxSenderTest {
     private class Fixture {
 
-        var hub: IHub = mock()
-        var envelopeReader: IEnvelopeReader = mock()
-        var serializer: ISerializer = mock()
-        var logger: ILogger = mock()
-        var options: SentryOptions
-
-        init {
-            options = SentryOptions()
-            options.isDebug = true
-            options.setLogger(logger)
-        }
+        val hub = mock<IHub>()
+        var envelopeReader = mock<IEnvelopeReader>()
+        val serializer = mock<ISerializer>()
+        val logger = mock<ILogger>()
 
         fun getSut(): OutboxSender {
             return OutboxSender(hub, envelopeReader, serializer, logger, 15000)
@@ -43,7 +36,7 @@ class OutboxSenderTest {
 
     private val fixture = Fixture()
 
-    private fun getTempEnvelope(fileName: String): String {
+    private fun getTempEnvelope(fileName: String = "envelope-event-attachment.txt"): String {
         val testFile = this::class.java.classLoader.getResource(fileName)
         val testFileBytes = testFile!!.readBytes()
         val targetFile = File.createTempFile("temp-envelope", ".tmp")
@@ -55,7 +48,7 @@ class OutboxSenderTest {
     fun `when envelopeReader returns null, file is deleted `() {
         whenever(fixture.envelopeReader.read(any())).thenReturn(null)
         val sut = fixture.getSut()
-        val path = getTempEnvelope("envelope-event-attachment.txt")
+        val path = getTempEnvelope()
         assertTrue(File(path).exists()) // sanity check
         sut.processEnvelopeFile(path, mock<Retryable>())
         assertFalse(File(path).exists())
@@ -69,7 +62,7 @@ class OutboxSenderTest {
         val expected = SentryEvent(SentryId(UUID.fromString("9ec79c33-ec99-42ab-8353-589fcb2e04dc")), Date())
         whenever(fixture.serializer.deserialize(any(), eq(SentryEvent::class.java))).thenReturn(expected)
         val sut = fixture.getSut()
-        val path = getTempEnvelope("envelope-event-attachment.txt")
+        val path = getTempEnvelope()
         assertTrue(File(path).exists()) // sanity check
         sut.processEnvelopeFile(path, mock<Retryable>())
 
@@ -89,7 +82,7 @@ class OutboxSenderTest {
         whenever(fixture.serializer.deserializeEnvelope(any())).thenReturn(expected)
         whenever(fixture.serializer.deserialize(any(), eq(SentryEvent::class.java))).thenReturn(event)
         val sut = fixture.getSut()
-        val path = getTempEnvelope("envelope-event-attachment.txt")
+        val path = getTempEnvelope()
         assertTrue(File(path).exists()) // sanity check
         sut.processEnvelopeFile(path, mock<Retryable>())
 
@@ -100,12 +93,28 @@ class OutboxSenderTest {
         verify(fixture.logger, never()).log(eq(SentryLevel.ERROR), any<String>(), any())
     }
 
+    @Test
+    fun `when envelope has unknown item type, create and capture an envelope`() {
+        fixture.envelopeReader = EnvelopeReader()
+
+        val sut = fixture.getSut()
+        val path = getTempEnvelope(fileName = "envelope_attachment.txt")
+        assertTrue(File(path).exists()) // sanity check
+        sut.processEnvelopeFile(path, mock<Retryable>())
+
+        verify(fixture.hub).captureEnvelope(any(), any())
+        assertFalse(File(path).exists())
+        // Additionally make sure we have no errors logged
+        verify(fixture.logger, never()).log(eq(SentryLevel.ERROR), any(), any<Any>())
+        verify(fixture.logger, never()).log(eq(SentryLevel.ERROR), any<String>(), any())
+    }
+
     @Test
     fun `when parser is EnvelopeReader and serializer returns a null event, file error logged, no event captured `() {
         fixture.envelopeReader = EnvelopeReader()
         whenever(fixture.serializer.deserialize(any(), eq(SentryEvent::class.java))).thenReturn(null)
         val sut = fixture.getSut()
-        val path = getTempEnvelope("envelope-event-attachment.txt")
+        val path = getTempEnvelope()
         assertTrue(File(path).exists()) // sanity check
         sut.processEnvelopeFile(path, mock<Retryable>())
 
@@ -121,7 +130,7 @@ class OutboxSenderTest {
         whenever(fixture.serializer.deserializeEnvelope(any())).thenReturn(null)
         whenever(fixture.serializer.deserialize(any(), eq(SentryEvent::class.java))).thenReturn(null)
         val sut = fixture.getSut()
-        val path = getTempEnvelope("envelope-event-attachment.txt")
+        val path = getTempEnvelope()
         assertTrue(File(path).exists()) // sanity check
         sut.processEnvelopeFile(path, mock<Retryable>())
 

sentry/src/test/resources/envelope_attachment.txt

@@ -0,0 +1,3 @@
+{}
+{"type":"attachment","length":61,"filename":"attachment.txt","content_type":"text/plain"}
+some plain text attachment file which include two line breaks