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: Basic attachment support #466

Merged
merged 3 commits into from
May 25, 2022
Merged

Conversation

timfish
Copy link
Contributor

@timfish timfish commented May 24, 2022

  • Tested working against sentry.io
  • Follows the other sdks by adding attachments to the scope

You can capture an event with a minidump attachment like:

    sentry::with_scope(
        |scope| {
            scope.add_attachment(Attachment {
                filename: "minidump.dmp".to_owned(),
                buffer: Vec::new(), // <-- minidump bytes go here
                ty: Some(AttachmentType::Minidump),
                content_type: None,
            })
        },
        || capture_event(Default::default()),
    );

@jan-auer jan-auer requested a review from Swatinem May 25, 2022 06:37
Copy link
Member

@Swatinem Swatinem left a comment

Choose a reason for hiding this comment

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

lgtm. Too bad adding the content_type is technically a breaking change :-(

An idea that is also implemented in other SDKs is to have "lazy" attachments, where you just give a PathBuf and it will read the attachment lazily only when you really send it out. Not sure its worth changing the implementation right away though.

Some tests for this would be nice as well ;-)

@timfish timfish marked this pull request as ready for review May 25, 2022 11:35
@timfish timfish changed the title Add basic attachment support feat: Basic attachment support May 25, 2022
sentry/tests/test_basic.rs Outdated Show resolved Hide resolved
@Swatinem Swatinem enabled auto-merge (squash) May 25, 2022 14:09
@Swatinem Swatinem merged commit 995a0a8 into getsentry:master May 25, 2022
@timfish timfish deleted the feat/attachments branch May 25, 2022 14:27
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.

2 participants