Skip to content

feat(types): Add custom variant to AttachmentType #916

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Oct 8, 2025
Merged

feat(types): Add custom variant to AttachmentType #916

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cc7b53c
feat: Add custom attachment type
tobias-wilfert Oct 7, 2025
1b42c15
Add changelog message
tobias-wilfert Oct 8, 2025
3ad7329
Add changelog entry in correct section
tobias-wilfert Oct 8, 2025
9122269
Apply suggestion from @Swatinem
tobias-wilfert Oct 8, 2025
092f407
Undo formatting changes
tobias-wilfert Oct 8, 2025
a6888fd
Undo formatting changes
tobias-wilfert Oct 8, 2025
42033ae
Avoid the clone
tobias-wilfert Oct 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions sentry-types/src/protocol/attachment.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::fmt;
use serde::Deserialize;

/// The different types an attachment can have.
#[derive(Debug, Copy, Clone, Eq, PartialEq, Deserialize)]
#[derive(Debug, Clone, Eq, PartialEq, Deserialize)]
Copy link

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

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

Removing Copy trait from the enum will impact performance for small operations. Consider keeping Copy and use Cow<'static, str> for the Custom variant to maintain zero-cost abstractions for predefined types while supporting custom strings.

Copilot uses AI. Check for mistakes.

pub enum AttachmentType {
#[serde(rename = "event.attachment")]
/// (default) A standard attachment without special meaning.
Expand All @@ -23,6 +23,9 @@ pub enum AttachmentType {
/// the last logs are extracted into event breadcrumbs.
#[serde(rename = "unreal.logs")]
UnrealLogs,
/// A custom attachment type with an arbitrary string value.
#[serde(untagged)]
Custom(String),
}

impl Default for AttachmentType {
Expand All @@ -33,13 +36,14 @@ impl Default for AttachmentType {

impl AttachmentType {
/// Gets the string value Sentry expects for the attachment type.
pub fn as_str(self) -> &'static str {
pub fn as_str(&self) -> &str {
match self {
Self::Attachment => "event.attachment",
Self::Minidump => "event.minidump",
Self::AppleCrashReport => "event.applecrashreport",
Self::UnrealContext => "unreal.context",
Self::UnrealLogs => "unreal.logs",
Self::Custom(s) => s,
}
}
}
Expand Down Expand Up @@ -68,7 +72,7 @@ impl Attachment {
r#"{{"type":"attachment","length":{length},"filename":"{filename}","attachment_type":"{at}","content_type":"{ct}"}}"#,
filename = self.filename,
length = self.buffer.len(),
at = self.ty.unwrap_or_default().as_str(),
at = self.ty.clone().unwrap_or_default().as_str(),
Copy link

Copilot AI Oct 7, 2025

Choose a reason for hiding this comment

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

Unnecessary clone operation. Since as_str now takes &self, you can use self.ty.as_ref().unwrap_or(&AttachmentType::default()).as_str() to avoid cloning the attachment type.

Suggested change
at = self.ty.clone().unwrap_or_default().as_str(),
at = self.ty.as_ref().unwrap_or(&AttachmentType::default()).as_str(),

Copilot uses AI. Check for mistakes.

Copy link
Member

Choose a reason for hiding this comment

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

maybe a .as_ref().map(|a| a.as_str()).unwrap_or_default() would be even more concise (and avoid the clone).

Copy link
Member Author

Choose a reason for hiding this comment

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

This .as_ref().map(|a| a.as_str()).unwrap_or_default() will yield and empty str rather than event.attachment so not sure we want to change this (at least in this PR).

Copy link
Member Author

Choose a reason for hiding this comment

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

I can still do the explicit unwrap_or though that one should work.

ct = self
.content_type
.as_ref()
Expand All @@ -92,3 +96,18 @@ impl fmt::Debug for Attachment {
.finish()
}
}

#[cfg(test)]
mod tests {
use super::*;
use serde_json;

#[test]
fn test_attachment_type_deserialize() {
let result: AttachmentType = serde_json::from_str(r#""event.minidump""#).unwrap();
assert_eq!(result, AttachmentType::Minidump);

let result: AttachmentType = serde_json::from_str(r#""my.custom.type""#).unwrap();
assert_eq!(result, AttachmentType::Custom("my.custom.type".to_string()));
}
}
Loading