Skip to content

Commit a385352

Browse files
marandanetomarandaneto
authored
Feat: OutboxSender supports all envelope item types (#1158)
1 parent ac92322 commit a385352

File tree

8 files changed

+155
-58
lines changed

8 files changed

+155
-58
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
* Fix: Set release and environment on Transactions (#1152)
2626
* Fix: Do not set transaction on the scope automatically
2727
* Enhancement: Automatically assign span context to captured events (#1156)
28+
* Feat: OutboxSender supports all envelope item types #1158
2829
* Enhancement: Improve ITransaction and ISpan null-safety compatibility (#1161)
2930

3031
# 4.0.0-alpha.2

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import io.sentry.hints.ApplyScopeData;
1010
import io.sentry.hints.Cached;
1111
import io.sentry.hints.Flushable;
12+
import io.sentry.hints.Resettable;
1213
import io.sentry.hints.Retryable;
1314
import io.sentry.hints.SubmissionResult;
1415
import io.sentry.util.Objects;
@@ -59,17 +60,17 @@ public void onEvent(int eventType, @Nullable String relativePath) {
5960
}
6061

6162
private static final class CachedEnvelopeHint
62-
implements Cached, Retryable, SubmissionResult, Flushable, ApplyScopeData {
63-
boolean retry = false;
64-
boolean succeeded = false;
63+
implements Cached, Retryable, SubmissionResult, Flushable, ApplyScopeData, Resettable {
64+
boolean retry;
65+
boolean succeeded;
6566

66-
private @NotNull final CountDownLatch latch;
67+
private @NotNull CountDownLatch latch;
6768
private final long flushTimeoutMillis;
6869
private final @NotNull ILogger logger;
6970

7071
public CachedEnvelopeHint(final long flushTimeoutMillis, final @NotNull ILogger logger) {
72+
reset();
7173
this.flushTimeoutMillis = flushTimeoutMillis;
72-
this.latch = new CountDownLatch(1);
7374
this.logger = Objects.requireNonNull(logger, "ILogger is required.");
7475
}
7576

@@ -104,5 +105,12 @@ public void setResult(boolean succeeded) {
104105
public boolean isSuccess() {
105106
return succeeded;
106107
}
108+
109+
@Override
110+
public void reset() {
111+
latch = new CountDownLatch(1);
112+
retry = false;
113+
succeeded = false;
114+
}
107115
}
108116
}

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

Lines changed: 61 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import android.os.FileObserver
44
import androidx.test.ext.junit.runners.AndroidJUnit4
55
import com.nhaarman.mockitokotlin2.any
66
import com.nhaarman.mockitokotlin2.anyOrNull
7-
import com.nhaarman.mockitokotlin2.argWhere
7+
import com.nhaarman.mockitokotlin2.check
88
import com.nhaarman.mockitokotlin2.eq
99
import com.nhaarman.mockitokotlin2.mock
1010
import com.nhaarman.mockitokotlin2.never
@@ -14,67 +14,105 @@ import io.sentry.IEnvelopeSender
1414
import io.sentry.ILogger
1515
import io.sentry.SentryOptions
1616
import io.sentry.hints.ApplyScopeData
17+
import io.sentry.hints.Resettable
18+
import io.sentry.hints.Retryable
19+
import io.sentry.hints.SubmissionResult
1720
import java.io.File
1821
import kotlin.test.Test
1922
import kotlin.test.assertEquals
2023
import kotlin.test.assertFailsWith
24+
import kotlin.test.assertFalse
2125
import org.junit.runner.RunWith
2226

2327
@RunWith(AndroidJUnit4::class)
2428
class EnvelopeFileObserverTest {
2529

2630
private class Fixture {
31+
val fileName = "file-name.txt"
2732
var path: String? = "."
28-
var envelopeSender: IEnvelopeSender = mock()
29-
var logger: ILogger = mock()
30-
var options: SentryOptions = SentryOptions()
31-
32-
init {
33-
options.isDebug = true
34-
options.setLogger(logger)
33+
val envelopeSender = mock<IEnvelopeSender>()
34+
val logger = mock<ILogger>()
35+
val options = SentryOptions().apply {
36+
isDebug = true
37+
setLogger(logger)
3538
}
3639

37-
fun getSut(): EnvelopeFileObserver {
38-
return EnvelopeFileObserver(path, envelopeSender, logger, options.flushTimeoutMillis)
40+
fun getSut(flushTimeoutMillis: Long): EnvelopeFileObserver {
41+
return EnvelopeFileObserver(path, envelopeSender, logger, flushTimeoutMillis)
3942
}
4043
}
4144

4245
private val fixture = Fixture()
4346

4447
@Test
4548
fun `envelope sender is called with fully qualified path`() {
46-
val sut = fixture.getSut()
47-
val param = "file-name.txt"
48-
sut.onEvent(FileObserver.CLOSE_WRITE, param)
49-
verify(fixture.envelopeSender).processEnvelopeFile(eq(fixture.path + File.separator + param), any())
49+
triggerEvent()
50+
51+
verify(fixture.envelopeSender).processEnvelopeFile(eq(fixture.path + File.separator + fixture.fileName), any())
5052
}
5153

5254
@Test
5355
fun `when event type is not close write, envelope sender is not called`() {
54-
val sut = fixture.getSut()
55-
sut.onEvent(FileObserver.CLOSE_WRITE.inv(), "file-name.txt")
56+
triggerEvent(eventType = FileObserver.CLOSE_WRITE.inv())
57+
5658
verifyZeroInteractions(fixture.envelopeSender)
5759
}
5860

5961
@Test
6062
fun `when event is fired with null path, envelope reader is not called`() {
61-
val sut = fixture.getSut()
62-
sut.onEvent(0, null)
63+
triggerEvent(relativePath = null)
64+
6365
verify(fixture.envelopeSender, never()).processEnvelopeFile(anyOrNull(), any())
6466
}
6567

6668
@Test
6769
fun `when null is passed as a path, ctor throws`() {
6870
fixture.path = null
69-
val exception = assertFailsWith<Exception> { fixture.getSut() }
71+
val exception = assertFailsWith<Exception> { fixture.getSut(0) }
7072
assertEquals("File path is required.", exception.message)
7173
}
7274

7375
@Test
7476
fun `envelope sender is called with fully qualified path and ApplyScopeData hint`() {
75-
val sut = fixture.getSut()
76-
val param = "file-name.txt"
77-
sut.onEvent(FileObserver.CLOSE_WRITE, param)
78-
verify(fixture.envelopeSender).processEnvelopeFile(eq(fixture.path + File.separator + param), argWhere { it is ApplyScopeData })
77+
triggerEvent()
78+
79+
verify(fixture.envelopeSender).processEnvelopeFile(
80+
eq(fixture.path + File.separator + fixture.fileName),
81+
check { it is ApplyScopeData })
82+
}
83+
84+
@Test
85+
fun `envelope sender Hint is Resettable`() {
86+
triggerEvent()
87+
88+
verify(fixture.envelopeSender).processEnvelopeFile(
89+
eq(fixture.path + File.separator + fixture.fileName),
90+
check { it is Resettable })
91+
}
92+
93+
@Test
94+
fun `Hint resets its state`() {
95+
triggerEvent(flushTimeoutMillis = 0)
96+
97+
verify(fixture.envelopeSender).processEnvelopeFile(
98+
eq(fixture.path + File.separator + fixture.fileName),
99+
check {
100+
(it as SubmissionResult).setResult(true)
101+
(it as Retryable).isRetry = true
102+
103+
(it as Resettable).reset()
104+
105+
assertFalse(it.isRetry)
106+
assertFalse(it.isSuccess)
107+
})
108+
}
109+
110+
private fun triggerEvent(
111+
flushTimeoutMillis: Long = 15_000,
112+
eventType: Int = FileObserver.CLOSE_WRITE,
113+
relativePath: String? = fixture.fileName
114+
) {
115+
val sut = fixture.getSut(flushTimeoutMillis)
116+
sut.onEvent(eventType, relativePath)
79117
}
80118
}

‎sentry/api/sentry.api‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,6 +1144,10 @@ public abstract interface class io/sentry/hints/Flushable {
11441144
public abstract fun waitFlush ()Z
11451145
}
11461146

1147+
public abstract interface class io/sentry/hints/Resettable {
1148+
public abstract fun reset ()V
1149+
}
1150+
11471151
public abstract interface class io/sentry/hints/Retryable {
11481152
public abstract fun isRetry ()Z
11491153
public abstract fun setRetry (Z)V

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

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import static io.sentry.cache.EnvelopeCache.PREFIX_CURRENT_SESSION_FILE;
55

66
import io.sentry.hints.Flushable;
7+
import io.sentry.hints.Resettable;
78
import io.sentry.hints.Retryable;
89
import io.sentry.hints.SubmissionResult;
910
import io.sentry.util.CollectionUtils;
@@ -107,6 +108,7 @@ private void processEnvelope(final @NotNull SentryEnvelope envelope, final @Null
107108
"Processing Envelope with %d item(s)",
108109
CollectionUtils.size(envelope.getItems()));
109110
int items = 0;
111+
110112
for (final SentryEnvelopeItem item : envelope.getItems()) {
111113
items++;
112114

@@ -138,26 +140,37 @@ private void processEnvelope(final @NotNull SentryEnvelope envelope, final @Null
138140
}
139141
hub.captureEvent(event, hint);
140142
logger.log(SentryLevel.DEBUG, "Item %d is being captured.", items);
141-
if (hint instanceof Flushable) {
142-
if (!((Flushable) hint).waitFlush()) {
143-
logger.log(
144-
SentryLevel.WARNING,
145-
"Timed out waiting for event submission: %s",
146-
event.getEventId());
147-
148-
break;
149-
}
150-
} else {
151-
LogUtils.logIfNotFlushable(logger, hint);
143+
144+
if (!waitFlush(hint)) {
145+
logger.log(
146+
SentryLevel.WARNING,
147+
"Timed out waiting for event submission: %s",
148+
event.getEventId());
149+
break;
152150
}
153151
}
154152
} catch (Exception e) {
155153
logger.log(ERROR, "Item failed to process.", e);
156154
}
157155
} else {
158-
// TODO: Handle attachments and other types
156+
// send unknown item types over the wire
157+
final SentryEnvelope newEnvelope =
158+
new SentryEnvelope(
159+
envelope.getHeader().getEventId(), envelope.getHeader().getSdkVersion(), item);
160+
hub.captureEnvelope(newEnvelope, hint);
159161
logger.log(
160-
SentryLevel.WARNING, "Item %d of type: %s ignored.", items, item.getHeader().getType());
162+
SentryLevel.DEBUG,
163+
"%s item %d is being captured.",
164+
item.getHeader().getType().getItemType(),
165+
items);
166+
167+
if (!waitFlush(hint)) {
168+
logger.log(
169+
SentryLevel.WARNING,
170+
"Timed out waiting for item type submission: %s",
171+
item.getHeader().getType().getItemType());
172+
break;
173+
}
161174
}
162175

163176
if (hint instanceof SubmissionResult) {
@@ -171,6 +184,20 @@ private void processEnvelope(final @NotNull SentryEnvelope envelope, final @Null
171184
break;
172185
}
173186
}
187+
188+
// reset the Hint to its initial state as we use it multiple times.
189+
if (hint instanceof Resettable) {
190+
((Resettable) hint).reset();
191+
}
192+
}
193+
}
194+
195+
private boolean waitFlush(final @Nullable Object hint) {
196+
if (hint instanceof Flushable) {
197+
return ((Flushable) hint).waitFlush();
198+
} else {
199+
LogUtils.logIfNotFlushable(logger, hint);
174200
}
201+
return true;
175202
}
176203
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package io.sentry.hints;
2+
3+
/** Marker interface for a reusable Hint */
4+
public interface Resettable {
5+
/** Reset the Hint to its initial state */
6+
void reset();
7+
}

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

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,10 @@ import kotlin.test.assertTrue
2424
class OutboxSenderTest {
2525
private class Fixture {
2626

27-
var hub: IHub = mock()
28-
var envelopeReader: IEnvelopeReader = mock()
29-
var serializer: ISerializer = mock()
30-
var logger: ILogger = mock()
31-
var options: SentryOptions
32-
33-
init {
34-
options = SentryOptions()
35-
options.isDebug = true
36-
options.setLogger(logger)
37-
}
27+
val hub = mock<IHub>()
28+
var envelopeReader = mock<IEnvelopeReader>()
29+
val serializer = mock<ISerializer>()
30+
val logger = mock<ILogger>()
3831

3932
fun getSut(): OutboxSender {
4033
return OutboxSender(hub, envelopeReader, serializer, logger, 15000)
@@ -43,7 +36,7 @@ class OutboxSenderTest {
4336

4437
private val fixture = Fixture()
4538

46-
private fun getTempEnvelope(fileName: String): String {
39+
private fun getTempEnvelope(fileName: String = "envelope-event-attachment.txt"): String {
4740
val testFile = this::class.java.classLoader.getResource(fileName)
4841
val testFileBytes = testFile!!.readBytes()
4942
val targetFile = File.createTempFile("temp-envelope", ".tmp")
@@ -55,7 +48,7 @@ class OutboxSenderTest {
5548
fun `when envelopeReader returns null, file is deleted `() {
5649
whenever(fixture.envelopeReader.read(any())).thenReturn(null)
5750
val sut = fixture.getSut()
58-
val path = getTempEnvelope("envelope-event-attachment.txt")
51+
val path = getTempEnvelope()
5952
assertTrue(File(path).exists()) // sanity check
6053
sut.processEnvelopeFile(path, mock<Retryable>())
6154
assertFalse(File(path).exists())
@@ -69,7 +62,7 @@ class OutboxSenderTest {
6962
val expected = SentryEvent(SentryId(UUID.fromString("9ec79c33-ec99-42ab-8353-589fcb2e04dc")), Date())
7063
whenever(fixture.serializer.deserialize(any(), eq(SentryEvent::class.java))).thenReturn(expected)
7164
val sut = fixture.getSut()
72-
val path = getTempEnvelope("envelope-event-attachment.txt")
65+
val path = getTempEnvelope()
7366
assertTrue(File(path).exists()) // sanity check
7467
sut.processEnvelopeFile(path, mock<Retryable>())
7568

@@ -89,7 +82,7 @@ class OutboxSenderTest {
8982
whenever(fixture.serializer.deserializeEnvelope(any())).thenReturn(expected)
9083
whenever(fixture.serializer.deserialize(any(), eq(SentryEvent::class.java))).thenReturn(event)
9184
val sut = fixture.getSut()
92-
val path = getTempEnvelope("envelope-event-attachment.txt")
85+
val path = getTempEnvelope()
9386
assertTrue(File(path).exists()) // sanity check
9487
sut.processEnvelopeFile(path, mock<Retryable>())
9588

@@ -100,12 +93,28 @@ class OutboxSenderTest {
10093
verify(fixture.logger, never()).log(eq(SentryLevel.ERROR), any<String>(), any())
10194
}
10295

96+
@Test
97+
fun `when envelope has unknown item type, create and capture an envelope`() {
98+
fixture.envelopeReader = EnvelopeReader()
99+
100+
val sut = fixture.getSut()
101+
val path = getTempEnvelope(fileName = "envelope_attachment.txt")
102+
assertTrue(File(path).exists()) // sanity check
103+
sut.processEnvelopeFile(path, mock<Retryable>())
104+
105+
verify(fixture.hub).captureEnvelope(any(), any())
106+
assertFalse(File(path).exists())
107+
// Additionally make sure we have no errors logged
108+
verify(fixture.logger, never()).log(eq(SentryLevel.ERROR), any(), any<Any>())
109+
verify(fixture.logger, never()).log(eq(SentryLevel.ERROR), any<String>(), any())
110+
}
111+
103112
@Test
104113
fun `when parser is EnvelopeReader and serializer returns a null event, file error logged, no event captured `() {
105114
fixture.envelopeReader = EnvelopeReader()
106115
whenever(fixture.serializer.deserialize(any(), eq(SentryEvent::class.java))).thenReturn(null)
107116
val sut = fixture.getSut()
108-
val path = getTempEnvelope("envelope-event-attachment.txt")
117+
val path = getTempEnvelope()
109118
assertTrue(File(path).exists()) // sanity check
110119
sut.processEnvelopeFile(path, mock<Retryable>())
111120

@@ -121,7 +130,7 @@ class OutboxSenderTest {
121130
whenever(fixture.serializer.deserializeEnvelope(any())).thenReturn(null)
122131
whenever(fixture.serializer.deserialize(any(), eq(SentryEvent::class.java))).thenReturn(null)
123132
val sut = fixture.getSut()
124-
val path = getTempEnvelope("envelope-event-attachment.txt")
133+
val path = getTempEnvelope()
125134
assertTrue(File(path).exists()) // sanity check
126135
sut.processEnvelopeFile(path, mock<Retryable>())
127136

‎sentry/src/test/resources/envelope_attachment.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{}
2+
{"type":"attachment","length":61,"filename":"attachment.txt","content_type":"text/plain"}
3+
some plain text attachment file which include two line breaks

0 commit comments

Comments
 (0)