merged main

stefanosiano · stefanosiano · commit 212ffbf44d74 · 2025-05-09T18:12:39.000+02:00
added feedback message truncation
SentryClient doesn't check for backfillable feedbacks anymore
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -2,6 +2,11 @@
 
 ## Unreleased
 
+### Features
+
+- Add new User Feedback API ([#4286](https://github.com/getsentry/sentry-java/pull/4286))
+    - We now introduced Sentry.captureFeedback, which supersedes Sentry.captureUserFeedback
+
 ### Dependencies
 
 - Bump Gradle from v8.13 to v8.14.0 ([#4360](https://github.com/getsentry/sentry-java/pull/4360))
@@ -50,8 +55,6 @@
 
 ### Features
 
-- Add new User Feedback API ([#4286](https://github.com/getsentry/sentry-java/pull/4286))
-  - We now introduced Sentry.captureFeedback, which supersedes Sentry.captureUserFeedback
 - Add `SentryWrapper.wrapRunnable` to wrap `Runnable` for use with Sentry ([#4332](https://github.com/getsentry/sentry-java/pull/4332))
 
 ### Fixes
diff --git a/sentry/src/main/java/io/sentry/SentryClient.java b/sentry/src/main/java/io/sentry/SentryClient.java
@@ -1048,10 +1048,8 @@ public void captureSession(final @NotNull Session session, final @Nullable Hint
       sentryId = event.getEventId();
     }
 
-    final boolean isBackfillable = HintUtils.hasType(hint, Backfillable.class);
-    // if event is backfillable we don't wanna trigger capture replay, because it's an event from
-    // the past. If feedback already has a replayId, we don't want to overwrite it.
-    if (feedback.getReplayId() == null && !isBackfillable) {
+    // If feedback already has a replayId, we don't want to overwrite it.
+    if (feedback.getReplayId() == null) {
       options.getReplayController().captureReplay(false);
       final @NotNull SentryId replayId = scope.getReplayId();
       if (!replayId.equals(SentryId.EMPTY_ID)) {
diff --git a/sentry/src/main/java/io/sentry/protocol/Feedback.java b/sentry/src/main/java/io/sentry/protocol/Feedback.java
@@ -31,7 +31,7 @@ public final class Feedback implements JsonUnknown, JsonSerializable {
   private @Nullable Map<String, Object> unknown;
 
   public Feedback(final @NotNull String message) {
-    this.message = message;
+    setMessage(message);
   }
 
   public Feedback(final @NotNull Feedback feedback) {
@@ -89,7 +89,12 @@ public void setUrl(final @Nullable String url) {
   }
 
   public void setMessage(final @NotNull String message) {
-    this.message = message;
+    // Sentry limits the message to 4096 characters
+    if (message.length() > 4096) {
+      this.message = message.substring(0, 4096);
+    } else {
+      this.message = message;
+    }
   }
 
   @Override
diff --git a/sentry/src/test/java/io/sentry/SentryClientTest.kt b/sentry/src/test/java/io/sentry/SentryClientTest.kt
@@ -3166,19 +3166,6 @@ class SentryClientTest {
         )
     }
 
-    @Test
-    fun `captureFeedback does not capture replay when backfilled`() {
-        val replayController = mock<ReplayController>()
-        val sut = fixture.getSut { it.setReplayController(replayController) }
-
-        sut.captureFeedback(
-            Feedback("message"),
-            HintUtils.createWithTypeCheckHint(BackfillableHint()),
-            createScope()
-        )
-        verify(replayController, never()).captureReplay(any())
-    }
-
     //endregion
 
     private fun givenScopeWithStartedSession(errored: Boolean = false, crashed: Boolean = false): IScope {
diff --git a/sentry/src/test/java/io/sentry/protocol/FeedbackTest.kt b/sentry/src/test/java/io/sentry/protocol/FeedbackTest.kt
@@ -49,4 +49,13 @@ class FeedbackTest {
             assertEquals("unknown", it["unknown"])
         }
     }
+
+    @Test
+    fun `setting a message longer than 4096 characters truncates the message`() {
+        val feedback = fixture.getSut()
+        feedback.message = "X".repeat(4095) + "Y" + "Z"
+        val expectedMessage = "X".repeat(4095) + "Y"
+        assertEquals(expectedMessage, feedback.message)
+        assertEquals(4096, feedback.message.length)
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ public final class Feedback implements JsonUnknown, JsonSerializable {`
`31`	`31`	`private @Nullable Map<String, Object> unknown;`
`32`	`32`
`33`	`33`	`public Feedback(final @NotNull String message) {`
`34`		`- this.message = message;`
	`34`	`+ setMessage(message);`
`35`	`35`	`}`
`36`	`36`
`37`	`37`	`public Feedback(final @NotNull Feedback feedback) {`
`@@ -89,7 +89,12 @@ public void setUrl(final @Nullable String url) {`
`89`	`89`	`}`
`90`	`90`
`91`	`91`	`public void setMessage(final @NotNull String message) {`
`92`		`- this.message = message;`
	`92`	`+ // Sentry limits the message to 4096 characters`
	`93`	`+ if (message.length() > 4096) {`
	`94`	`+ this.message = message.substring(0, 4096);`
	`95`	`+ } else {`
	`96`	`+ this.message = message;`
	`97`	`+ }`
`93`	`98`	`}`
`94`	`99`
`95`	`100`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -49,4 +49,13 @@ class FeedbackTest {`
`49`	`49`	`assertEquals("unknown", it["unknown"])`
`50`	`50`	`}`
`51`	`51`	`}`
	`52`	`+`
	`53`	`+ @Test`
	`54`	+ fun `setting a message longer than 4096 characters truncates the message`() {
	`55`	`+ val feedback = fixture.getSut()`
	`56`	`+ feedback.message = "X".repeat(4095) + "Y" + "Z"`
	`57`	`+ val expectedMessage = "X".repeat(4095) + "Y"`
	`58`	`+ assertEquals(expectedMessage, feedback.message)`
	`59`	`+ assertEquals(4096, feedback.message.length)`
	`60`	`+ }`
`52`	`61`	`}`