Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Allow Envelopes to contain raw binary data #549

Merged
merged 4 commits into from Feb 7, 2023

Conversation

loewenheim
Copy link
Contributor

This turns the type of Envelope::items into an enum that may be a vector of EnvelopeItems (the status quo) or a blob of binary data. In the latter case, operations on items become no-ops because nothing can be done if there's no structure to the items.

It also adds a function from_path_raw that reads an Envelope from disk without attempting to parse any items. The motivation for this is getsentry/sentry-cli#1458.

@loewenheim loewenheim self-assigned this Feb 6, 2023
@codecov
Copy link

codecov bot commented Feb 6, 2023

Codecov Report

Merging #549 (e06aa64) into master (bc06e5f) will decrease coverage by 0.20%.
The diff coverage is 41.86%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #549      +/-   ##
==========================================
- Coverage   70.87%   70.68%   -0.20%     
==========================================
  Files          66       66              
  Lines        6624     6649      +25     
==========================================
+ Hits         4695     4700       +5     
- Misses       1929     1949      +20     

sentry-types/src/protocol/envelope.rs Outdated Show resolved Hide resolved
@@ -188,6 +213,11 @@ impl Envelope {
where
I: Into<EnvelopeItem>,
{
let Items::EnvelopeItems(ref mut items) = self.items else {
eprintln!("WARNING: This envelope contains raw items. Adding an item is not supported.");
Copy link
Member

Choose a reason for hiding this comment

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

oh, we don’t have sentry_debug here. :-(

sentry-types/src/protocol/envelope.rs Outdated Show resolved Hide resolved
/// The resulting Envelope's `items` will just be a binary blob.
pub fn from_path_raw<P: AsRef<Path>>(path: P) -> Result<Self, EnvelopeError> {
let mut bytes = std::fs::read(path).map_err(|_| EnvelopeError::UnexpectedEof)?;
let (header, offset) = Self::parse_header(&bytes)?;
Copy link
Member

Choose a reason for hiding this comment

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

I would have to check what native is doing, but I believe it does not even parse the header. The header can potentially contain more than just the event id, so its best to keep that opaque as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So should I just manually parse the event id?

Copy link
Member

Choose a reason for hiding this comment

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

do we need it? its optional anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point. So should we just shove the whole file contents into the items in the raw case?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I would say so.

@loewenheim loewenheim merged commit 456ef23 into master Feb 7, 2023
@loewenheim loewenheim deleted the feat/raw-envelope-items branch February 7, 2023 16:12
loewenheim added a commit that referenced this pull request Feb 7, 2023
loewenheim added a commit that referenced this pull request Feb 7, 2023
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.

None yet

2 participants