Skip to content

fix(core): Send outbox attachments with their event or transaction - #6513

Open
mvanhorn wants to merge 2 commits into
getsentry:mainfrom
mvanhorn:fix/6067-outbox-attachment-reassembly
Open

mvanhorn wants to merge 2 commits into
getsentry:mainfrom
mvanhorn:fix/6067-outbox-attachment-reassembly

Conversation

@mvanhorn

@mvanhorn mvanhorn commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

📜 Description

OutboxSender.processEnvelope now sends an outbox envelope's attachments in the same envelope as its event or transaction, instead of one standalone envelope per attachment. If beforeSend, an event processor or sampling drops the parent, its attachments are dropped with it.

For an envelope with exactly one Event or Transaction item, the sender collects every attachment up front (so attachments before or after the parent both work), wraps each item's data in the existing Attachment(Callable<byte[]>, filename, contentType, attachmentType, addToTransactions) constructor, and adds them to the hint for captureEvent / captureTransaction. SentryClient already builds the combined envelope from hint.getAttachments(), and addToTransactions is set for transaction parents so filterForTransaction keeps them. The hint's previous attachments are restored in finally, since Hint.clear() does not clear attachments.

Unchanged paths:

  • attachment-only envelopes, envelopes with more than one parent, and attachments without a filename keep the existing per-item dispatch
  • a parent that fails to deserialize or has a mismatched event ID suppresses its attachments rather than sending them alone
  • grouped attachments skip the per-item flush/result/reset block, so a failed or timed-out parent still stops later items and a retryable failure keeps the file

Grouped attachments now go through SentryEnvelopeItem.fromAttachment, so they follow the configured per-attachment maxAttachmentSize. There is no aggregate envelope cap; the issue raises one as a proposal, and this PR does not add it.

💡 Motivation and Context

Native crash attachments (for example event.minidump) were ingested even when the event that owns them was filtered or sampled out, which produced orphaned attachments.

Fixes #6067

  • resolves: JAVA-724

💚 How did you test it?

New tests in OutboxSenderTest.kt, several of them through a real SentryClient, JsonSerializer and EnvelopeReader with a recording transport:

  • attachments before and after an event and a transaction arrive in one envelope with the parent, keeping bytes, filenames, content types and attachment types
  • a parent dropped by the client sends no attachment and no standalone fallback
  • invalid or mismatched parents suppress their attachments
  • attachment-only and multiple-parent envelopes keep per-item dispatch
  • the hint is restored across mixed items and reused files
  • no extra flushes or resets, and a failed parent keeps the retry file
  • oversized and unreadable attachments are dropped at serialization while valid items remain

Commands (JDK 21):

  • ./gradlew :sentry:test --tests="*OutboxSenderTest*": passes
  • ./gradlew :sentry:test --tests="*SentryClientTest*": passes
  • ./gradlew :sentry:spotlessCheck: passes
  • ./gradlew :sentry:updateKotlinAbi: no API file changes

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.

🔮 Next steps

The changelog entry gets this PR's link once the number exists.

AI was used for assistance.

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 getsentry#6067
@mvanhorn
mvanhorn requested review from a team and adinauer as code owners November 9, 2026 07:33
@mvanhorn
mvanhorn requested review from 0xadam-brown and markushi and removed request for a team November 9, 2026 07:33

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 9097b0b. Configure here.

} finally {
if (!attachments.isEmpty()) {
hint.replaceAttachments(originalAttachments);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rate limits can orphan grouped attachments

Medium Severity

Grouped outbox attachments are added onto the parent and sent as one envelope, but RateLimiter.filter drops items by category. An error or transaction 429 with attachments still allowed strips the parent and forwards the rest, so minidumps can be ingested without their event. Previously a failed parent capture stopped later items, so those attachments never went out alone.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 9097b0b. Configure here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at this and I don't think it's a regression. On main, every outbox attachment already goes out in its own envelope through captureEnvelope, so an error or transaction 429 drops the event and the attachments still go out alone. That orphaning exists today, and the PR doesn't add it.

With this change the attachments ride on the parent's hint, so the envelope is built the same way SentryClient builds it for a normal captureEvent with attachments, and RateLimiter.filter treats it exactly like any other SDK capture. If dropping attachments whenever their parent's category is rate limited is the behavior you want, I think it belongs in RateLimiter for all captures rather than special-cased in OutboxSender. Happy to do that as a follow-up if you'd like.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OutboxSender sends attachments as standalone envelopes

1 participant