From c7538be1ed4a12aa1e15914430a6e69fb9b85dcb Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sun, 8 Nov 2026 23:21:43 -0800 Subject: [PATCH] fix(core): Send outbox attachments with their event or transaction OutboxSender captured the event or transaction through the SDK and then sent each attachment in its own envelope, so filtering or sampling out the parent left orphaned attachments. Collect the attachments of a single-parent envelope up front and pass them on the capture hint, so the client sends them in the same envelope and drops them with the parent. Fixes #6067 --- CHANGELOG.md | 1 + .../src/main/java/io/sentry/OutboxSender.java | 59 ++- .../test/java/io/sentry/OutboxSenderTest.kt | 416 ++++++++++++++++++ 3 files changed, 474 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a08fb1ae6c..2c4ef74ffc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ ### Fixes +- 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. - 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 cbe4f6ee007..20b52ad7538 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 703f0823c00..a48ffb034c8 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()