diff --git a/CHANGELOG.md b/CHANGELOG.md index 50319e4d8c..31e4b37ae1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ ### Fixes - Prevent the main thread from potentially hanging during app start if Android is slow to register the SDK's network or telephony listeners. `Sentry.init`, Session Replay, and capturing events no longer wait for that registration ([#6537](https://github.com/getsentry/sentry-java/pull/6537)) +- Send outbox attachments together with their event or transaction so parent filtering also drops its attachments. Grouped attachments now respect the configured per-attachment `maxAttachmentSize` limit. ([#6513](https://github.com/getsentry/sentry-java/pull/6513)) - Add opt-in support for matching GraphQL ignored error types against classification specifications via `sentry.graphql.enable-specification-error-type-filtering` ([#6092](https://github.com/getsentry/sentry-java/pull/6092)) - Session Replay: Fix `SentryOkHttpInterceptor` hanging forever on responses whose body has no known length, such as Server-Sent Events or a gzipped response, when network details are enabled for the URL ([#6231](https://github.com/getsentry/sentry-java/pull/6231)) - Session Replay: `SentryOkHttpInterceptor` now captures network request and response details only while Session Replay is recording or buffering ([#6486](https://github.com/getsentry/sentry-java/pull/6486)) diff --git a/sentry/src/main/java/io/sentry/OutboxSender.java b/sentry/src/main/java/io/sentry/OutboxSender.java index cbe4f6ee00..20b52ad753 100644 --- a/sentry/src/main/java/io/sentry/OutboxSender.java +++ b/sentry/src/main/java/io/sentry/OutboxSender.java @@ -26,6 +26,8 @@ import java.io.InputStreamReader; import java.io.Reader; import java.nio.charset.Charset; +import java.util.ArrayList; +import java.util.List; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -121,6 +123,39 @@ private void processEnvelope(final @NotNull SentryEnvelope envelope, final @NotN CollectionUtils.size(envelope.getItems())); int currentItem = 0; + int parentCount = 0; + boolean transactionParent = false; + for (final SentryEnvelopeItem item : envelope.getItems()) { + if (item.getHeader() != null + && (SentryItemType.Event.equals(item.getHeader().getType()) + || SentryItemType.Transaction.equals(item.getHeader().getType()))) { + parentCount++; + transactionParent = SentryItemType.Transaction.equals(item.getHeader().getType()); + } + } + + final List attachments = new ArrayList<>(); + if (parentCount == 1) { + // Collect before dispatch so attachments may precede or follow their parent. Multiple parents + // are ambiguous and retain the legacy per-item path. + for (final SentryEnvelopeItem item : envelope.getItems()) { + final SentryEnvelopeItemHeader header = item.getHeader(); + if (header != null + && SentryItemType.Attachment.equals(header.getType()) + && header.getFileName() != null) { + attachments.add( + new Attachment( + () -> item.getData(), + header.getFileName(), + header.getContentType(), + header.getAttachmentType(), + transactionParent)); + } + } + } + // Hint.clear() leaves attachments intact, so restore this snapshot after each grouped capture. + final List originalAttachments = hint.getAttachments(); + for (final SentryEnvelopeItem item : envelope.getItems()) { currentItem++; @@ -128,6 +163,12 @@ private void processEnvelope(final @NotNull SentryEnvelope envelope, final @NotN logger.log(SentryLevel.ERROR, "Item %d has no header", currentItem); continue; } + if (parentCount == 1 + && SentryItemType.Attachment.equals(item.getHeader().getType()) + && item.getHeader().getFileName() != null) { + // Even an invalid or filtered parent owns these attachments. Never send them on their own. + continue; + } if (SentryItemType.Event.equals(item.getHeader().getType())) { try (final Reader eventReader = new BufferedReader( @@ -144,7 +185,14 @@ private void processEnvelope(final @NotNull SentryEnvelope envelope, final @NotN logUnexpectedEventId(envelope, event.getEventId(), currentItem); continue; } - scopes.captureEvent(event, hint); + try { + hint.addAttachments(attachments); + scopes.captureEvent(event, hint); + } finally { + if (!attachments.isEmpty()) { + hint.replaceAttachments(originalAttachments); + } + } logItemCaptured(currentItem); if (!waitFlush(hint)) { @@ -181,7 +229,14 @@ private void processEnvelope(final @NotNull SentryEnvelope envelope, final @NotN .getTrace() .setSamplingDecision(extractSamplingDecision(traceContext)); } - scopes.captureTransaction(transaction, traceContext, hint); + try { + hint.addAttachments(attachments); + scopes.captureTransaction(transaction, traceContext, hint); + } finally { + if (!attachments.isEmpty()) { + hint.replaceAttachments(originalAttachments); + } + } logItemCaptured(currentItem); if (!waitFlush(hint)) { diff --git a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt index 703f0823c0..a48ffb034c 100644 --- a/sentry/src/test/java/io/sentry/OutboxSenderTest.kt +++ b/sentry/src/test/java/io/sentry/OutboxSenderTest.kt @@ -1,16 +1,24 @@ package io.sentry +import com.google.common.truth.Truth.assertThat import io.sentry.cache.EnvelopeCache +import io.sentry.hints.Flushable +import io.sentry.hints.Resettable import io.sentry.hints.Retryable +import io.sentry.hints.SubmissionResult import io.sentry.protocol.SentryId import io.sentry.protocol.SentryTransaction +import io.sentry.transport.ITransport import io.sentry.util.HintUtils import io.sentry.util.thread.NoOpThreadChecker +import java.io.ByteArrayOutputStream import java.io.File import java.io.FileNotFoundException +import java.io.IOException import java.nio.file.Files import java.nio.file.Paths import java.util.Date +import java.util.concurrent.Callable import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -19,11 +27,13 @@ import kotlin.test.assertTrue import org.mockito.kotlin.any import org.mockito.kotlin.anyVararg import org.mockito.kotlin.argWhere +import org.mockito.kotlin.argumentCaptor import org.mockito.kotlin.check import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.spy +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -51,6 +61,412 @@ class OutboxSenderTest { private val fixture = Fixture() + @Test + fun `attachments on both sides of a parent travel together through the client`() { + for (isTransaction in listOf(false, true)) { + val transport = mock() + val options = + SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + maxAttachmentSize = 1024 + setTransportFactory { _, _ -> transport } + } + val serializer = JsonSerializer(options) + val reader = EnvelopeReader(serializer) + val scopes = mock() + whenever(scopes.options).thenReturn(options) + val traceContext = + TraceContext(SentryId(), "key", null, null, null, "outbox", "0.5", "true", null, "0.25") + val parent = + if (isTransaction) { + SentryTransaction(SentryTracer(TransactionContext("outbox", "test"), scopes)) + } else { + SentryEvent() + } + val attachments = + (1..4).map { + SentryEnvelopeItem( + SentryEnvelopeItemHeader( + SentryItemType.Attachment, + { 3 }, + "application/octet-stream", + "$it.dmp", + "event.minidump", + ), + byteArrayOf(it.toByte(), 0, -1), + ) + } + val envelope = + SentryEnvelope( + SentryEnvelopeHeader(parent.eventId, null, traceContext), + attachments.take(2) + + SentryEnvelopeItem.fromEvent(serializer, parent) + + attachments.drop(2), + ) + val file = File.createTempFile("outbox", ".envelope") + file.outputStream().use { serializer.serialize(envelope, it) } + val hint = HintUtils.createWithTypeCheckHint(mock()) + val snapshots = mutableListOf>() + val client = SentryClient(options) + whenever(scopes.captureEvent(any(), any())).thenAnswer { + val captureHint = it.getArgument(1) + snapshots.add(captureHint.attachments) + client.captureEvent(it.getArgument(0), null, captureHint) + } + whenever(scopes.captureTransaction(any(), any(), any())).thenAnswer { + val transaction = it.getArgument(0) + assertThat(transaction.samplingDecision!!.sampled).isTrue() + assertThat(transaction.samplingDecision!!.sampleRate).isEqualTo(0.5) + assertThat(transaction.samplingDecision!!.sampleRand).isEqualTo(0.25) + val captureHint = it.getArgument(2) + snapshots.add(captureHint.attachments) + client.captureTransaction( + transaction, + it.getArgument(1), + null, + captureHint, + null, + ) + } + + OutboxSender(scopes, reader, serializer, options.logger, 15000, 30) + .processEnvelopeFile(file.absolutePath, hint) + + assertThat(snapshots).hasSize(1) + assertThat(snapshots.single().map { it.filename }) + .containsExactly("1.dmp", "2.dmp", "3.dmp", "4.dmp") + .inOrder() + val sent = argumentCaptor() + verify(transport).send(sent.capture(), eq(hint)) + val bytes = ByteArrayOutputStream() + serializer.serialize(sent.firstValue, bytes) + val parsed = reader.read(bytes.toByteArray().inputStream())!! + assertThat(parsed.header.eventId).isEqualTo(parent.eventId) + val items = parsed.items.toList() + assertThat(items).hasSize(5) + assertThat(items.first().header.type) + .isEqualTo(if (isTransaction) SentryItemType.Transaction else SentryItemType.Event) + assertThat( + serializer + .deserialize( + items.first().data.inputStream().reader(), + if (isTransaction) SentryTransaction::class.java else SentryEvent::class.java, + )!! + .eventId + ) + .isEqualTo(parent.eventId) + if (isTransaction) { + assertThat(parsed.header.traceContext!!.traceId).isEqualTo(traceContext.traceId) + assertThat(parsed.header.traceContext!!.sampleRate).isEqualTo("0.5") + assertThat(parsed.header.traceContext!!.sampleRand).isEqualTo("0.25") + } + items.drop(1).forEachIndexed { index, item -> + assertThat(item.data).isEqualTo(attachments[index].data) + assertThat(item.header.fileName).isEqualTo("${index + 1}.dmp") + assertThat(item.header.contentType).isEqualTo("application/octet-stream") + assertThat(item.header.attachmentType).isEqualTo("event.minidump") + } + verify(scopes, never()).captureEnvelope(any(), any()) + assertThat(hint.attachments).isEmpty() + assertThat(file.exists()).isFalse() + client.close() + } + } + + @Test + fun `serialization drops oversized and unreadable grouped attachments but keeps valid items`() { + val transport = mock() + val options = + SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + maxAttachmentSize = 4 + setTransportFactory { _, _ -> transport } + } + val serializer = JsonSerializer(options) + val event = SentryEvent() + val attachments = + listOf(4, 5, 4).mapIndexed { index, size -> + SentryEnvelopeItem( + SentryEnvelopeItemHeader(SentryItemType.Attachment, { size }, "text/plain", "$index.txt"), + ByteArray(size) { index.toByte() }, + ) + } + val broken = + SentryEnvelopeItem( + SentryEnvelopeItemHeader(SentryItemType.Attachment, { 1 }, null, "broken"), + Callable { throw IOException("unreadable attachment") }, + ) + whenever(fixture.envelopeReader.read(any())) + .thenReturn( + SentryEnvelope( + event.eventId, + null, + attachments + broken + SentryEnvelopeItem.fromEvent(serializer, event), + ) + ) + val client = SentryClient(options) + whenever(fixture.scopes.captureEvent(any(), any())).thenAnswer { + client.captureEvent(it.getArgument(0), null, it.getArgument(1)) + } + OutboxSender(fixture.scopes, fixture.envelopeReader, serializer, fixture.logger, 15000, 30) + .processEnvelopeFile(getTempEnvelope(), HintUtils.createWithTypeCheckHint(mock())) + + val sent = argumentCaptor() + verify(transport).send(sent.capture(), any()) + val bytes = ByteArrayOutputStream() + serializer.serialize(sent.firstValue, bytes) + val parsed = EnvelopeReader(serializer).read(bytes.toByteArray().inputStream())!! + assertThat(parsed.header.eventId).isEqualTo(event.eventId) + assertThat(parsed.items.map { it.header.type }) + .containsExactly(SentryItemType.Event, SentryItemType.Attachment, SentryItemType.Attachment) + .inOrder() + assertThat(parsed.items.drop(1).map { it.header.fileName }) + .containsExactly("0.txt", "2.txt") + .inOrder() + assertThat(parsed.items.drop(1).map { it.data.size }).containsExactly(4, 4) + verify(fixture.scopes, never()).captureEnvelope(any(), any()) + client.close() + } + + @Test + fun `filtered parent does not send standalone attachments`() { + val transport = mock() + val options = + SentryOptions().apply { + dsn = "https://key@sentry.io/proj" + maxAttachmentSize = 1024 + setTransportFactory { _, _ -> transport } + setBeforeSend { _, _ -> null } + } + val serializer = JsonSerializer(options) + val event = SentryEvent() + val envelope = + SentryEnvelope( + event.eventId, + null, + listOf( + SentryEnvelopeItem.fromEvent(serializer, event), + SentryEnvelopeItem.fromAttachment( + serializer, + options.logger, + Attachment(byteArrayOf(1), "crash.dmp"), + options.maxAttachmentSize, + ), + ), + ) + val file = File.createTempFile("outbox", ".envelope") + file.outputStream().use { serializer.serialize(envelope, it) } + val client = SentryClient(options) + whenever(fixture.scopes.captureEvent(any(), any())).thenAnswer { + client.captureEvent(it.getArgument(0), null, it.getArgument(1)) + } + OutboxSender(fixture.scopes, EnvelopeReader(serializer), serializer, fixture.logger, 15000, 30) + .processEnvelopeFile(file.absolutePath, HintUtils.createWithTypeCheckHint(mock())) + + verify(fixture.scopes).captureEvent(any(), any()) + verify(transport, never()).send(any(), any()) + verify(fixture.scopes, never()).captureEnvelope(any(), any()) + client.close() + } + + @Test + fun `invalid event and transaction parents suppress their attachments`() { + for (type in listOf(SentryItemType.Event, SentryItemType.Transaction)) { + for (nullParent in listOf(false, true)) { + val fixture = Fixture() + val event = SentryEvent() + val transaction = + SentryTransaction(SentryTracer(TransactionContext("outbox", "test"), fixture.scopes)) + whenever(fixture.serializer.deserialize(any(), eq(SentryEvent::class.java))) + .thenReturn(if (nullParent) null else event) + whenever(fixture.serializer.deserialize(any(), eq(SentryTransaction::class.java))) + .thenReturn(if (nullParent) null else transaction) + val attachment = + SentryEnvelopeItem( + SentryEnvelopeItemHeader(SentryItemType.Attachment, { 1 }, null, "crash.dmp"), + byteArrayOf(1), + ) + val parent = + SentryEnvelopeItem( + SentryEnvelopeItemHeader(type, { 2 }, "application/json", null), + "{}".toByteArray(), + ) + whenever(fixture.envelopeReader.read(any())) + .thenReturn(SentryEnvelope(SentryId(), null, listOf(attachment, parent, attachment))) + + fixture + .getSut() + .processEnvelopeFile( + getTempEnvelope(), + HintUtils.createWithTypeCheckHint(mock()), + ) + + verify(fixture.scopes, never()).captureEvent(any(), any()) + verify(fixture.scopes, never()).captureTransaction(any(), any(), any()) + verify(fixture.scopes, never()).captureEnvelope(any(), any()) + verify(fixture.logger).log(eq(SentryLevel.ERROR), any(), anyVararg()) + } + } + } + + @Test + fun `attachment only and multiple parent envelopes retain per-item dispatch`() { + for (parentCount in listOf(0, 2)) { + val fixture = Fixture() + val event = SentryEvent() + whenever(fixture.serializer.deserialize(any(), eq(SentryEvent::class.java))).thenReturn(event) + val attachments = + (1..2).map { + SentryEnvelopeItem( + SentryEnvelopeItemHeader(SentryItemType.Attachment, { 1 }, null, "$it.dmp"), + byteArrayOf(it.toByte()), + ) + } + val parents = + (0 until parentCount).map { + SentryEnvelopeItem( + SentryEnvelopeItemHeader(SentryItemType.Event, { 2 }, "application/json", null), + "{}".toByteArray(), + ) + } + whenever(fixture.envelopeReader.read(any())) + .thenReturn(SentryEnvelope(event.eventId, null, attachments + parents)) + val hint = HintUtils.createWithTypeCheckHint(mock()) + val snapshots = mutableListOf>() + whenever(fixture.scopes.captureEvent(any(), any())).thenAnswer { + snapshots.add(it.getArgument(1).attachments) + event.eventId + } + + fixture.getSut().processEnvelopeFile(getTempEnvelope(), hint) + + val captured = argumentCaptor() + verify(fixture.scopes, times(2)).captureEnvelope(captured.capture(), eq(hint)) + assertThat(captured.allValues.flatMap { it.items }) + .containsExactlyElementsIn(attachments) + .inOrder() + verify(fixture.scopes, times(parentCount)).captureEvent(eq(event), eq(hint)) + assertThat(snapshots.flatten()).isEmpty() + } + } + + @Test + fun `grouped attachments restore hint across mixed items and reused files`() { + val event = SentryEvent() + whenever(fixture.serializer.deserialize(any(), eq(SentryEvent::class.java))).thenReturn(event) + val attachment = + SentryEnvelopeItem( + SentryEnvelopeItemHeader(SentryItemType.Attachment, { 1 }, null, "crash.dmp"), + byteArrayOf(1), + ) + val unnamed = + SentryEnvelopeItem( + SentryEnvelopeItemHeader(SentryItemType.Attachment, { 1 }, null, null), + byteArrayOf(2), + ) + val feedback = + SentryEnvelopeItem( + SentryEnvelopeItemHeader(SentryItemType.UserFeedback, { 2 }, "application/json", null), + "{}".toByteArray(), + ) + val parent = + SentryEnvelopeItem( + SentryEnvelopeItemHeader(SentryItemType.Event, { 2 }, "application/json", null), + "{}".toByteArray(), + ) + // An absent envelope ID must still allow a valid parent ID. + whenever(fixture.envelopeReader.read(any())) + .thenReturn( + SentryEnvelope(null, null, listOf(attachment, parent, feedback, unnamed)), + SentryEnvelope(null, null, listOf(parent)), + ) + val hint = HintUtils.createWithTypeCheckHint(mock()) + val original = Attachment(byteArrayOf(3), "original") + hint.addAttachment(original) + val snapshots = mutableListOf>() + whenever(fixture.scopes.captureEvent(any(), any())).thenAnswer { + snapshots.add(it.getArgument(1).attachments) + event.eventId + } + whenever(fixture.scopes.captureEnvelope(any(), any())).thenAnswer { + assertThat(it.getArgument(1).attachments).containsExactly(original) + event.eventId + } + + repeat(2) { fixture.getSut().processEnvelopeFile(getTempEnvelope(), hint) } + + assertThat(snapshots.map { snapshot -> snapshot.map { it.filename } }) + .containsExactly(listOf("original", "crash.dmp"), listOf("original")) + .inOrder() + assertThat(hint.attachments).containsExactly(original) + val captured = argumentCaptor() + verify(fixture.scopes, times(2)).captureEnvelope(captured.capture(), eq(hint)) + assertThat(captured.allValues.flatMap { it.items }).containsExactly(feedback, unnamed).inOrder() + } + + @Test + fun `grouped attachments do not add flushes or resets and parent failure preserves retry`() { + for (outcome in listOf("success", "timeout", "failure", "exception")) { + val fixture = Fixture() + val event = SentryEvent() + whenever(fixture.serializer.deserialize(any(), eq(SentryEvent::class.java))).thenReturn(event) + val attachment = + SentryEnvelopeItem( + SentryEnvelopeItemHeader(SentryItemType.Attachment, { 1 }, null, "crash.dmp"), + byteArrayOf(1), + ) + val parent = + SentryEnvelopeItem( + SentryEnvelopeItemHeader(SentryItemType.Event, { 2 }, "application/json", null), + "{}".toByteArray(), + ) + val feedback = + SentryEnvelopeItem( + SentryEnvelopeItemHeader(SentryItemType.UserFeedback, { 2 }, "application/json", null), + "{}".toByteArray(), + ) + whenever(fixture.envelopeReader.read(any())) + .thenReturn( + SentryEnvelope(event.eventId, null, listOf(attachment, parent, attachment, feedback)) + ) + val sdkHint = + mock( + extraInterfaces = arrayOf(Flushable::class, SubmissionResult::class, Resettable::class) + ) + whenever(sdkHint.isRetry).thenReturn(outcome != "success") + whenever((sdkHint as Flushable).waitFlush()).thenReturn(outcome != "timeout") + whenever((sdkHint as SubmissionResult).isSuccess).thenReturn(outcome == "success") + val hint = HintUtils.createWithTypeCheckHint(sdkHint) + val original = Attachment(byteArrayOf(2), "original") + hint.addAttachment(original) + val snapshots = mutableListOf>() + whenever(fixture.scopes.captureEvent(any(), any())).thenAnswer { + snapshots.add(it.getArgument(1).attachments) + if (outcome == "exception") throw IllegalStateException("capture failed") + event.eventId + } + val path = getTempEnvelope() + + fixture.getSut().processEnvelopeFile(path, hint) + + assertThat(snapshots.single().map { it.filename }) + .containsExactly("original", "crash.dmp", "crash.dmp") + .inOrder() + assertThat(hint.attachments).containsExactly(original) + assertThat(HintUtils.getSentrySdkHint(hint)).isSameInstanceAs(sdkHint) + verify( + sdkHint as Flushable, + times(if (outcome == "success") 2 else if (outcome == "exception") 0 else 1), + ) + .waitFlush() + verify(sdkHint as Resettable, times(if (outcome == "success") 2 else 0)).reset() + verify(fixture.scopes, times(if (outcome == "success") 1 else 0)) + .captureEnvelope(any(), eq(hint)) + assertThat(File(path).exists()).isEqualTo(outcome != "success") + File(path).delete() + } + } + private fun getTempEnvelope(fileName: String = "envelope-event-attachment.txt"): String { val testFile = this::class.java.classLoader.getResource(fileName) val testFileBytes = testFile!!.readBytes()