feat: implement the new User Feedback API (#1304)

* feat: implement the new User Feedback API

https://develop.sentry.dev/sdk/data-model/envelope-items/#user-feedback

* Fix heap-use-after-free

* Update CHANGELOG.md

* remove unused line3_end

* what if we reused the old API?

* use internal id to fix event association in Sentry Web UI

* restore integration test for legacy user report

* convert a deprecated "user report" to a new "user feedback"

* conversion: note in docs & log info message

* let sentry__envelope_add_user_feedback take ownership

* revert back to option 1

* deprecate sentry_value_new_user_feedback() & sentry_capture_user_feedback()

* fix remaining deprecation warnings

* msvc

Author: jpnurmi

CHANGELOG.md

@@ -7,6 +7,8 @@
 - Add `sentry_clear_attachments()` to allow clearing all previously added attachments in the global scope. ([#1290](https://github.com/getsentry/sentry-native/pull/1290))
 - Compiles also on Xbox One ([#1294](https://github.com/getsentry/sentry-native/pull/1294))
 - Provide `sentry_regenerate_trace()` to allow users to set manual trace boundaries. ([#1293](https://github.com/getsentry/sentry-native/pull/1293))
+- Add `sentry_value_new_feedback` and `sentry_capture_feedback` to allow capturing [User Feedback](https://develop.sentry.dev/sdk/data-model/envelope-items/#user-feedback). ([#1304](https://github.com/getsentry/sentry-native/pull/1304))
+  - Deprecate `sentry_value_new_user_feedback` and `sentry_capture_user_feedback` in favor of the new API.
 
 **Fixes**:
 

CONTRIBUTING.md

@@ -189,6 +189,7 @@ The example currently supports the following commands:
 - `capture-with-scope`: Captures an event with a local scope.
 - `attach-to-scope`: Same as `attachment` but attaches the file to the local scope.
 - `clear-attachments`: Clears all attachments from the global scope.
+- `capture-user-feedback`: Captures a user feedback event.
 
 Only on Linux using crashpad:
 - `crashpad-wait-for-upload`: Couples application shutdown to complete the upload in the `crashpad_handler`.

examples/example.c

@@ -556,14 +556,22 @@ main(int argc, char **argv)
         sentry_capture_event(event);
     }
     if (has_arg(argc, argv, "capture-user-feedback")) {
+        sentry_value_t user_feedback = sentry_value_new_feedback(
+            "some-message", "some-email", "some-name", NULL);
+
+        sentry_capture_feedback(user_feedback);
+    }
+    if (has_arg(argc, argv, "capture-user-report")) {
         sentry_value_t event = sentry_value_new_message_event(
             SENTRY_LEVEL_INFO, "my-logger", "Hello user feedback!");
         sentry_uuid_t event_id = sentry_capture_event(event);
 
+        SENTRY_SUPPRESS_DEPRECATED
         sentry_value_t user_feedback = sentry_value_new_user_feedback(
             &event_id, "some-name", "some-email", "some-comment");
 
         sentry_capture_user_feedback(user_feedback);
+        SENTRY_RESTORE_DEPRECATED
     }
 
     if (has_arg(argc, argv, "capture-transaction")) {

include/sentry.h

@@ -119,6 +119,21 @@ extern "C" {
 #    endif
 #endif
 
+#if defined(__GNUC__) || defined(__clang__)
+#    define SENTRY_SUPPRESS_DEPRECATED                                         \
+        _Pragma("GCC diagnostic push");                                        \
+        _Pragma("GCC diagnostic ignored \"-Wdeprecated-declarations\"")
+#    define SENTRY_RESTORE_DEPRECATED _Pragma("GCC diagnostic pop")
+#elif defined(_MSC_VER)
+#    define SENTRY_SUPPRESS_DEPRECATED                                         \
+        __pragma(warning(push));                                               \
+        __pragma(warning(disable : 4996))
+#    define SENTRY_RESTORE_DEPRECATED __pragma(warning(pop))
+#else
+#    define SENTRY_SUPPRESS_DEPRECATED
+#    define SENTRY_RESTORE_DEPRECATED
+#endif
+
 /* marks a function as experimental api */
 #ifndef SENTRY_EXPERIMENTAL_API
 #    define SENTRY_EXPERIMENTAL_API SENTRY_API
@@ -2434,25 +2449,48 @@ SENTRY_EXPERIMENTAL_API void sentry_transaction_set_name_n(
     sentry_transaction_t *transaction, const char *name, size_t name_len);
 
 /**
- * Creates a new User Feedback with a specific name, email and comments.
- *
- * See https://develop.sentry.dev/sdk/envelopes/#user-feedback
+ * Creates a deprecated User Report with a specific name, email and comments.
  *
- * User Feedback has to be associated with a specific event that has been
- * sent to Sentry earlier.
+ * See
+ * https://develop.sentry.dev/sdk/data-model/envelope-items/#user-report---deprecated
  */
+SENTRY_DEPRECATED("Use `sentry_value_new_feedback` instead")
 SENTRY_API sentry_value_t sentry_value_new_user_feedback(
     const sentry_uuid_t *uuid, const char *name, const char *email,
     const char *comments);
+SENTRY_DEPRECATED("Use `sentry_value_new_feedback_n` instead")
 SENTRY_API sentry_value_t sentry_value_new_user_feedback_n(
     const sentry_uuid_t *uuid, const char *name, size_t name_len,
     const char *email, size_t email_len, const char *comments,
     size_t comments_len);
 
+/**
+ * Captures a deprecated User Report and sends it to Sentry.
+ */
+SENTRY_DEPRECATED("Use `sentry_capture_feedback` instead")
+SENTRY_API void sentry_capture_user_feedback(sentry_value_t user_report);
+
+/**
+ * Creates a new User Feedback with a specific message (required), and optional
+ * contact_email, name, message, and associated_event_id.
+ *
+ * See https://develop.sentry.dev/sdk/data-model/envelope-items/#user-feedback
+ *
+ * User Feedback can be associated with a specific event that has been
+ * sent to Sentry earlier.
+ */
+SENTRY_API sentry_value_t sentry_value_new_feedback(const char *message,
+    const char *contact_email, const char *name,
+    const sentry_uuid_t *associated_event_id);
+SENTRY_API sentry_value_t sentry_value_new_feedback_n(const char *message,
+    size_t message_len, const char *contact_email, size_t contact_email_len,
+    const char *name, size_t name_len,
+    const sentry_uuid_t *associated_event_id);
+
 /**
  * Captures a manually created User Feedback and sends it to Sentry.
  */
-SENTRY_API void sentry_capture_user_feedback(sentry_value_t user_feedback);
+SENTRY_API void sentry_capture_feedback(sentry_value_t user_feedback);
 
 /**
  * The status of a Span or Transaction.

src/sentry_core.c

@@ -658,6 +658,25 @@ sentry__prepare_transaction(const sentry_options_t *options,
     return NULL;
 }
 
+static sentry_envelope_t *
+prepare_user_report(sentry_value_t user_report)
+{
+    sentry_envelope_t *envelope = NULL;
+
+    envelope = sentry__envelope_new();
+    if (!envelope || !sentry__envelope_add_user_report(envelope, user_report)) {
+        goto fail;
+    }
+
+    return envelope;
+
+fail:
+    SENTRY_WARN("dropping user report");
+    sentry_envelope_free(envelope);
+    sentry_value_decref(user_report);
+    return NULL;
+}
+
 static sentry_envelope_t *
 prepare_user_feedback(sentry_value_t user_feedback)
 {
@@ -1335,17 +1354,32 @@ sentry_span_finish_ts(sentry_span_t *opaque_span, uint64_t timestamp)
 }
 
 void
-sentry_capture_user_feedback(sentry_value_t user_feedback)
+sentry_capture_user_feedback(sentry_value_t user_report)
+{
+    sentry_envelope_t *envelope = NULL;
+
+    SENTRY_WITH_OPTIONS (options) {
+        envelope = prepare_user_report(user_report);
+        if (envelope) {
+            sentry__capture_envelope(options->transport, envelope);
+        }
+    }
+    sentry_value_decref(user_report);
+}
+
+void
+sentry_capture_feedback(sentry_value_t user_feedback)
 {
     sentry_envelope_t *envelope = NULL;
 
     SENTRY_WITH_OPTIONS (options) {
         envelope = prepare_user_feedback(user_feedback);
         if (envelope) {
             sentry__capture_envelope(options->transport, envelope);
+        } else {
+            sentry_value_decref(user_feedback);
         }
     }
-    sentry_value_decref(user_feedback);
 }
 
 int

src/sentry_envelope.c

@@ -297,8 +297,8 @@ sentry__envelope_add_transaction(
 }
 
 sentry_envelope_item_t *
-sentry__envelope_add_user_feedback(
-    sentry_envelope_t *envelope, sentry_value_t user_feedback)
+sentry__envelope_add_user_report(
+    sentry_envelope_t *envelope, sentry_value_t user_report)
 {
     sentry_envelope_item_t *item = envelope_add_item(envelope);
     if (!item) {
@@ -310,9 +310,9 @@ sentry__envelope_add_user_feedback(
         return NULL;
     }
 
-    sentry_value_t event_id = sentry__ensure_event_id(user_feedback, NULL);
+    sentry_value_t event_id = sentry__ensure_event_id(user_report, NULL);
 
-    sentry__jsonwriter_write_value(jw, user_feedback);
+    sentry__jsonwriter_write_value(jw, user_report);
     item->payload = sentry__jsonwriter_into_string(jw, &item->payload_len);
 
     sentry__envelope_item_set_header(
@@ -326,6 +326,30 @@ sentry__envelope_add_user_feedback(
     return item;
 }
 
+sentry_envelope_item_t *
+sentry__envelope_add_user_feedback(
+    sentry_envelope_t *envelope, sentry_value_t user_feedback)
+{
+    sentry_value_t event = sentry_value_new_event();
+    sentry_value_t contexts = sentry_value_get_by_key(event, "contexts");
+    if (sentry_value_is_null(contexts)) {
+        contexts = sentry_value_new_object();
+    }
+    sentry_value_set_by_key(contexts, "feedback", user_feedback);
+    sentry_value_set_by_key(event, "contexts", contexts);
+
+    sentry_envelope_item_t *item = sentry__envelope_add_event(envelope, event);
+    if (!item) {
+        sentry_value_decref(event);
+        return NULL;
+    }
+
+    sentry__envelope_item_set_header(
+        item, "type", sentry_value_new_string("feedback"));
+
+    return item;
+}
+
 sentry_envelope_item_t *
 sentry__envelope_add_session(
     sentry_envelope_t *envelope, const sentry_session_t *session)

src/sentry_envelope.h

@@ -43,6 +43,12 @@ sentry_envelope_item_t *sentry__envelope_add_event(
 sentry_envelope_item_t *sentry__envelope_add_transaction(
     sentry_envelope_t *envelope, sentry_value_t transaction);
 
+/**
+ * Add a deprecated user report to this envelope.
+ */
+sentry_envelope_item_t *sentry__envelope_add_user_report(
+    sentry_envelope_t *envelope, sentry_value_t user_report);
+
 /**
  * Add a user feedback to this envelope.
  */

src/sentry_value.c

@@ -1350,9 +1350,11 @@ sentry_value_t
 sentry_value_new_user_feedback(const sentry_uuid_t *uuid, const char *name,
     const char *email, const char *comments)
 {
+    SENTRY_SUPPRESS_DEPRECATED
     return sentry_value_new_user_feedback_n(uuid, name,
         sentry__guarded_strlen(name), email, sentry__guarded_strlen(email),
         comments, sentry__guarded_strlen(comments));
+    SENTRY_RESTORE_DEPRECATED
 }
 
 sentry_value_t
@@ -1380,6 +1382,42 @@ sentry_value_new_user_feedback_n(const sentry_uuid_t *uuid, const char *name,
     return rv;
 }
 
+sentry_value_t
+sentry_value_new_feedback(const char *message, const char *contact_email,
+    const char *name, const sentry_uuid_t *associated_event_id)
+{
+    return sentry_value_new_feedback_n(message, sentry__guarded_strlen(message),
+        contact_email, sentry__guarded_strlen(contact_email), name,
+        sentry__guarded_strlen(name), associated_event_id);
+}
+
+sentry_value_t
+sentry_value_new_feedback_n(const char *message, size_t message_len,
+    const char *contact_email, size_t contact_email_len, const char *name,
+    size_t name_len, const sentry_uuid_t *associated_event_id)
+{
+    sentry_value_t rv = sentry_value_new_object();
+
+    if (message) {
+        sentry_value_set_by_key(
+            rv, "message", sentry_value_new_string_n(message, message_len));
+    }
+    if (contact_email) {
+        sentry_value_set_by_key(rv, "contact_email",
+            sentry_value_new_string_n(contact_email, contact_email_len));
+    }
+    if (name) {
+        sentry_value_set_by_key(
+            rv, "name", sentry_value_new_string_n(name, name_len));
+    }
+    if (associated_event_id) {
+        sentry_value_set_by_key(rv, "associated_event_id",
+            sentry__value_new_internal_uuid(associated_event_id));
+    }
+
+    return rv;
+}
+
 static sentry_value_t
 sentry__get_or_insert_values_list(sentry_value_t parent, const char *key)
 {

tests/__init__.py

@@ -278,7 +278,13 @@ def deserialize_from(
         headers = json.loads(line)
         length = headers["length"]
         payload = f.read(length)
-        if headers.get("type") in ["event", "session", "transaction", "user_report"]:
+        if headers.get("type") in [
+            "event",
+            "feedback",
+            "session",
+            "transaction",
+            "user_report",
+        ]:
             rv = cls(headers=headers, payload=PayloadRef(json=json.loads(payload)))
         else:
             rv = cls(headers=headers, payload=payload)

tests/assertions.py

@@ -42,13 +42,25 @@ def assert_session(envelope, extra_assertion=None):
 def assert_user_feedback(envelope):
     user_feedback = None
     for item in envelope:
-        if item.headers.get("type") == "user_report" and item.payload.json is not None:
-            user_feedback = item.payload.json
+        if item.headers.get("type") == "feedback" and item.payload.json is not None:
+            user_feedback = item.payload.json["contexts"]["feedback"]
 
     assert user_feedback is not None
     assert user_feedback["name"] == "some-name"
-    assert user_feedback["email"] == "some-email"
-    assert user_feedback["comments"] == "some-comment"
+    assert user_feedback["contact_email"] == "some-email"
+    assert user_feedback["message"] == "some-message"
+
+
+def assert_user_report(envelope):
+    user_report = None
+    for item in envelope:
+        if item.headers.get("type") == "user_report" and item.payload.json is not None:
+            user_report = item.payload.json
+
+    assert user_report is not None
+    assert user_report["name"] == "some-name"
+    assert user_report["email"] == "some-email"
+    assert user_report["comments"] == "some-comment"
 
 
 def assert_meta(