fix: make attachment payloads replayable - #5511
Conversation
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
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ 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.
Codecov Report❌ Patch coverage is
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. 🚀 New features to boost your workflow:
|
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>
Co-authored-by: James Crosswell <jamescrosswell@users.noreply.github.com>
jamescrosswell
left a comment
There was a problem hiding this comment.
Thanks @XAN9xXx - I added a couple of commits to address the disposal/ownership issues but otherwise looks great. Thanks again for the contribution!
|
Appreciate you catching the oversight, and thanks for the review! Glad to contribute. |

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:
serialization.
serialization.
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:
Changelog Entry
fix: Attachments not being sent properly when Spotlight is enabled