Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 17 additions & 2 deletions sentry-types/src/protocol/envelope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,10 @@ use serde::{Deserialize, Serialize};
use thiserror::Error;
use uuid::Uuid;

use super::{feedback::Feedback, v7 as protocol};
use crate::utils::ts_rfc3339_opt;
use crate::Dsn;

use super::v7 as protocol;

use protocol::{
Attachment, AttachmentType, ClientSdkInfo, DynamicSamplingContext, Event, Log, MonitorCheckIn,
SessionAggregates, SessionUpdate, Transaction,
Expand Down Expand Up @@ -127,6 +126,9 @@ enum EnvelopeItemType {
/// A container of Log items.
#[serde(rename = "log")]
LogsContainer,
/// A User Feedback Item type.
#[serde(rename = "feedback")]
Feedback,
}

/// An Envelope Item Header.
Expand Down Expand Up @@ -178,6 +180,8 @@ pub enum EnvelopeItem {
MonitorCheckIn(MonitorCheckIn),
/// A container for a list of multiple items.
ItemContainer(ItemContainer),
/// A User Feedback item.
Feedback(Event<'static>),
/// This is a sentinel item used to `filter` raw envelopes.
Raw,
// TODO:
Expand Down Expand Up @@ -283,6 +287,12 @@ impl From<Vec<Log>> for EnvelopeItem {
}
}

impl From<Feedback> for EnvelopeItem {
fn from(feedback: Feedback) -> Self {
EnvelopeItem::Feedback(feedback.to_new_event())
}
}

/// An Iterator over the items of an Envelope.
#[derive(Clone)]
pub struct EnvelopeItemIter<'s> {
Expand Down Expand Up @@ -507,6 +517,7 @@ impl Envelope {
serde_json::to_writer(&mut item_buf, &wrapper)?
}
},
EnvelopeItem::Feedback(feedback) => serde_json::to_writer(&mut item_buf, feedback)?,
EnvelopeItem::Raw => {
continue;
}
Expand All @@ -518,6 +529,7 @@ impl Envelope {
EnvelopeItem::Transaction(_) => "transaction",
EnvelopeItem::MonitorCheckIn(_) => "check_in",
EnvelopeItem::ItemContainer(container) => container.ty(),
EnvelopeItem::Feedback(_) => "feedback",
EnvelopeItem::Attachment(_) | EnvelopeItem::Raw => unreachable!(),
};

Expand Down Expand Up @@ -677,6 +689,9 @@ impl Envelope {
serde_json::from_slice::<ItemsSerdeWrapper<_>>(payload)
.map(|x| EnvelopeItem::ItemContainer(ItemContainer::Logs(x.items.into())))
}
EnvelopeItemType::Feedback => {
serde_json::from_slice(payload).map(EnvelopeItem::Feedback)
}
}
.map_err(EnvelopeError::InvalidItemPayload)?;

Expand Down
39 changes: 39 additions & 0 deletions sentry-types/src/protocol/feedback.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
use std::{collections::BTreeMap, time::SystemTime};

use serde::{Deserialize, Serialize};

use crate::random_uuid;

use super::v7::{Context, Event, Level};

/// Represents feedback from a user.
#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq)]
pub struct Feedback {
/// The user's contact email
pub contact_email: Option<String>,
/// The user's name
pub name: Option<String>,
/// The feedback from the user
pub message: String,
}

impl Feedback {
pub(crate) fn to_context(&self) -> Context {
Context::Feedback(Box::new(self.clone()))
}

pub(crate) fn to_new_event(&self) -> Event<'static> {
let map = {
let mut map = BTreeMap::new();
map.insert("feedback".to_string(), self.to_context());
map
};
Event {
event_id: random_uuid(),
level: Level::Info,
timestamp: SystemTime::now(),
contexts: map,
..Default::default()
}
Copy link

Choose a reason for hiding this comment

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

Feedback event missing required type field

High Severity

The to_new_event method constructs a feedback Event but doesn't set r#type to Some("feedback".to_string()). The PR specifically added a type field to the Event struct for this purpose, but the ..Default::default() fallback leaves it as None, so it won't be serialized. Per the Sentry protocol, feedback event payloads require "type": "feedback" for the backend to correctly identify and process them as feedback rather than regular error events.

Additional Locations (1)
Fix in Cursor Fix in Web

}
Comment on lines +28 to +38
Copy link

Choose a reason for hiding this comment

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

Bug: The to_new_event() function for feedback events fails to set the r#type field, causing them to be processed incorrectly by Sentry.
Severity: HIGH

Suggested Fix

In the to_new_event function, explicitly set the r#type field on the Event struct to Some("feedback".to_string()) to ensure the event is correctly categorized by the Sentry backend.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: sentry-types/src/protocol/feedback.rs#L25-L38

Potential issue: The `to_new_event()` method in `sentry-types/src/protocol/feedback.rs`
creates an `Event` from user feedback but fails to set the `r#type` field. The event is
initialized with `..Default::default()`, which sets the `r#type` field to `None`. Due to
the `skip_serializing_if = "Option::is_none"` attribute, this field is omitted from the
serialized JSON payload sent to Sentry. As a result, the Sentry backend will not
correctly identify the event as a feedback event, causing it to appear as an "unlabeled
event" and not be displayed in the User Feedback section, which defeats the purpose of
the feature.

Did we get this right? 👍 / 👎 to inform future reviews.

}
1 change: 1 addition & 0 deletions sentry-types/src/protocol/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ pub use v7 as latest;

mod attachment;
mod envelope;
mod feedback;
mod monitor;
mod session;
9 changes: 9 additions & 0 deletions sentry-types/src/protocol/v7.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ use crate::utils::{display_from_str_opt, ts_rfc3339_opt, ts_seconds_float};

pub use super::attachment::*;
pub use super::envelope::*;
pub use super::feedback::*;
pub use super::monitor::*;
pub use super::session::*;

Expand Down Expand Up @@ -1102,6 +1103,8 @@ pub enum Context {
Otel(Box<OtelContext>),
/// HTTP response data.
Response(Box<ResponseContext>),
/// User feedback
Feedback(Box<Feedback>),
/// Generic other context data.
#[serde(rename = "unknown")]
Other(Map<String, Value>),
Expand All @@ -1120,6 +1123,7 @@ impl Context {
Context::Gpu(..) => "gpu",
Context::Otel(..) => "otel",
Context::Response(..) => "response",
Context::Feedback(..) => "feedback",
Context::Other(..) => "unknown",
}
}
Expand Down Expand Up @@ -1721,6 +1725,9 @@ pub struct Event<'a> {
/// SDK metadata
#[serde(default, skip_serializing_if = "Option::is_none")]
pub sdk: Option<Cow<'a, ClientSdkInfo>>,
/// The event type
#[serde(default, skip_serializing_if = "Option::is_none")]
pub r#type: Option<String>,
}

impl Default for Event<'_> {
Expand Down Expand Up @@ -1753,6 +1760,7 @@ impl Default for Event<'_> {
extra: Default::default(),
debug_meta: Default::default(),
sdk: Default::default(),
r#type: Default::default(),
}
}
}
Expand Down Expand Up @@ -1798,6 +1806,7 @@ impl<'a> Event<'a> {
extra: self.extra,
debug_meta: Cow::Owned(self.debug_meta.into_owned()),
sdk: self.sdk.map(|x| Cow::Owned(x.into_owned())),
r#type: self.r#type,
}
}
}
Expand Down
Loading