Skip to content

Navigation Menu

Sign in
Sign up

feat: add feedback API - #1259

Open
xremming wants to merge 1 commit into
getsentry:master from
xremming:zmitchell/add-feedback
Open

feat: add feedback API #1259
xremming wants to merge 1 commit into
getsentry:master from
xremming:zmitchell/add-feedback

Conversation

@xremming

@xremming xremming commented Jul 22, 2026 •
edited by szokeasaurusrex
Loading

Copy link
Copy Markdown

Continues the work from #1033.

I personally need this feature so though I might as well finish the partial PR. Largely done together with Claude.

~~Splitted the change of the ~~~~\~\~type\~\~~~ field from a ~~\~\~String\~\~~~ to en enum into its own commit. Based on my investigations, the whole type field is not actually needed.

Issues

sentry[bot] reacted with hooray emoji

Copy link
Copy Markdown

👋 Thanks for sending this our way! Before a maintainer reviews the code, we ask community contributors to align with us on the approach first — it keeps your time pointed at changes we can land.

The easiest way is to open or find a GitHub issue and discuss the approach with a maintainer there, then link that issue from this PR. If the issue is already assigned to someone else, please check in with them (or with us) before continuing — otherwise two people may end up working on the same task.

See our contributing guidelines for the full picture.

xremming force-pushed the zmitchell/add-feedback branch 3 times, most recently from 46b9076 to a7f744b Compare July 22, 2026 19:59
xremming marked this pull request as ready for review July 22, 2026 20:03
xremming requested a review from a team as a code owner July 22, 2026 20:03
Comment thread sentry-types/src/protocol/v7.rs
Comment thread sentry-types/src/protocol/envelope.rs
Comment thread sentry-types/src/protocol/v7.rs
xremming force-pushed the zmitchell/add-feedback branch from a7f744b to 74e25e6 Compare July 22, 2026 20:28
Comment thread sentry-types/src/protocol/feedback.rs
xremming force-pushed the zmitchell/add-feedback branch from 74e25e6 to ccdb79f Compare July 22, 2026 20:49
Comment thread sentry-types/src/protocol/v7.rs Outdated

@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.

Reviewed by Cursor Bugbot for commit ccdb79f. Configure here.

Comment thread sentry-types/src/protocol/client_report/mod.rs
xremming force-pushed the zmitchell/add-feedback branch from ccdb79f to 651735a Compare July 22, 2026 21:03

Copy link
Copy Markdown
Author

Tested to be working.

image

Copy link
Copy Markdown
Author

@szokeasaurusrex Since you had reviewed the previous PR would you be the correct person to ping?

@szokeasaurusrex szokeasaurusrex left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, thanks for the PR!

I admittedly have only had time to skim this briefly so far; to do a proper review, I would first need to familiarize myself a bit more with the spec, which would take some time.

In the meantime, I have given some suggestions for simplifying and improving the code. Would appreciate if you can address these 🙏

xremming reacted with heart emoji
Comment thread sentry-types/src/protocol/envelope.rs Outdated
Comment on lines +193 to +198
/// A User Feedback item.
///
/// Feedback is transmitted as an [`Event`] carrying a `feedback` context, so this variant
/// wraps an `Event` rather than a [`Feedback`]. Construct it via `EnvelopeItem::from(feedback)`
/// rather than the raw variant, and use [`EnvelopeItem::as_feedback`] to recover the feedback.
Feedback(Event<'static>),

@szokeasaurusrex szokeasaurusrex Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given that Feedback events are meant to have a "feedback" context, I would be in favor of introducing a special FeedbackEvent type which forces the "feedback" context to be included.

This would eliminate the code you have which checks for the presence of a "feedback" context at runtime

xremming reacted with thumbs up emoji
Comment thread sentry-types/src/protocol/v7.rs Outdated
pub use self::client_report::Report as ClientReport;
pub use super::attachment::*;
pub use super::envelope::*;
pub use super::feedback::*;

@szokeasaurusrex szokeasaurusrex Jul 27, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I know we have a lot of existing * imports throughout the codebase, but this is really something I would like us to avoid going forward, as it makes it too easy to accidentally introduce unintended public API changes.

Please therefore replace this with an explicit export of only the types which need to be re-exported.

xremming reacted with thumbs up emoji

Copy link
Copy Markdown
Member

Please also check and address the CI failures before the next review round

codecov Bot commented Jul 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.84946% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.94%. Comparing base (a57b91c) to head (651735a).
⚠️ Report is 146 commits behind head on master.

Additional details and impacted files 
@@ Coverage Diff @@
## master #1259 +/- ##
==========================================
+ Coverage 73.81% 73.94% +0.13% 
==========================================
 Files 64 77 +13 
 Lines 7538 9454 +1916 
==========================================
+ Hits 5564 6991 +1427 
- Misses 1974 2463 +489 

Co-authored-by: Zach Mitchell <zmitchell@fastmail.com>

xremming commented Aug 4, 2026

Copy link
Copy Markdown
Author

Forgot to re-request the review. 😅

I think the issues should now be fixed.

Copy link
Copy Markdown
Member

Hey @xremming, thank you! As I mentioned before, doing a proper review would take some time. It is on my to-do list, but it may take several weeks until I have time to do a full review.
xremming reacted with thumbs up emoji

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sentry sentry[bot] sentry[bot] left review comments
@cursor cursor[bot] cursor[bot] left review comments
@szokeasaurusrex szokeasaurusrex Awaiting requested review from szokeasaurusrex

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implementing a capture_feedback function

AltStyle によって変換されたページ (->オリジナル) /