Skip to content

fix: make attachment payloads replayable - #5511

Merged
jamescrosswell merged 5 commits into
getsentry:mainfrom
XAN9xXx:fix/5499-stream-replayability
Aug 31, 2026
Merged

fix: make attachment payloads replayable#5511
jamescrosswell merged 5 commits into
getsentry:mainfrom
XAN9xXx:fix/5499-stream-replayability

Conversation

@XAN9xXx

@XAN9xXx XAN9xXx commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #5499.

When Spotlight is enabled, the same envelope can be serialized by both the
primary transport and the Spotlight transport. Attachment payloads previously
shared a single stream, so one serialization could consume the content before
the other transport read it.

This change makes attachment payloads replayable:

  • Built-in byte attachments obtain an independent stream for each
    serialization.
  • Built-in reusable file attachments open an independent file stream for each
    serialization.
  • Stream-backed, custom or derived attachment content, and delete-on-close
    files are buffered in memory on first serialization and reuse that buffer
    for subsequent serializations.

Implementation notes

The buffering path obtains the source stream while creating the envelope item
for length validation, but does not read its content until the first
serialization. The resulting buffer is shared by repeated and concurrent
serializations.

Exact type checks are used for the built-in byte and file attachment types
because derived or custom implementations may return streams with different
lifetime or replayability behavior.

Envelope and transport ownership semantics are unchanged.

Testing

Regression coverage includes:

  • Repeated byte, file, and stream attachment serialization
  • Concurrent serialization
  • Synchronous and asynchronous serialization
  • Non-seekable and asynchronous-only streams
  • Custom and derived attachment content
  • Delete-on-close file attachments
  • Lazy buffering and cancellation behavior

Changelog Entry

fix: Attachments not being sent properly when Spotlight is enabled

Create independent streams for byte and reusable file attachments.
Lazily buffer one-shot streams so multiple transports can serialize the
same envelope without consuming a shared stream.

Fixes getsentry#5499
@github-actions github-actions Bot added the risk: high PR risk score: high label Aug 26, 2026

@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 reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6f1ac20. Configure here.

Comment thread src/Sentry/Protocol/Envelopes/Envelope.cs Outdated
@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.06349% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.76%. Comparing base (5cd6ad6) to head (35609ec).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/Sentry/Protocol/Envelopes/Envelope.cs 75.00% 2 Missing and 2 partials ⚠️
src/Sentry/Protocol/Envelopes/EnvelopeItem.cs 93.75% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5511      +/-   ##
==========================================
+ Coverage   74.74%   74.76%   +0.01%     
==========================================
  Files         513      515       +2     
  Lines       18829    18884      +55     
  Branches     3682     3688       +6     
==========================================
+ Hits        14074    14118      +44     
- Misses       3875     3883       +8     
- Partials      880      883       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

XAN9xXx and others added 2 commits August 27, 2026 15:48
ProcessEnvelope returns a new Envelope wrapping the caller's EnvelopeItem
instances, so both transports were disposing items they don't own. With
Spotlight enabled the two sends share those items, so whichever finished
first closed the attachment streams out from under the other request.

The caller owns them - BackgroundWorker disposes the envelope once the send
completes - so drop the `using` in both transports.

SendEnvelopeAsync_HeapDumpAttachmentSentSuccessfully_FileIsDeleted asserted
the old ownership: it expected the heap dump file to be gone the moment the
send returned. It now asserts the file outlives the send and is deleted when
the owning envelope is disposed, matching its TooLarge sibling.

This covers the second half of getsentry#5499.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread src/Sentry/Http/SpotlightHttpTransport.cs Outdated
Comment thread src/Sentry/Internal/Http/HttpTransport.cs Outdated
Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com>
@jamescrosswell
jamescrosswell self-requested a review August 31, 2026 01:21

@jamescrosswell jamescrosswell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @XAN9xXx - I added a couple of commits to address the disposal/ownership issues but otherwise looks great. Thanks again for the contribution!

@jamescrosswell
jamescrosswell merged commit 5da3920 into getsentry:main Aug 31, 2026
46 checks passed
@XAN9xXx

XAN9xXx commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Appreciate you catching the oversight, and thanks for the review! Glad to contribute.

@XAN9xXx
XAN9xXx deleted the fix/5499-stream-replayability branch August 31, 2026 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: high PR risk score: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spotlight transport shares and disposes envelope item streams with the inner transport

2 participants