Merge pull request #6239 from getsentry/fix/callback-error-handling-user-feedback

fix(android): Handle feedback configurator failures

Author: adinauer

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 ### Fixes
 
+- Keep the Android feedback form available when an options configurator throws ([#6239](https://github.com/getsentry/sentry-java/pull/6239))
 - Keep Spring requests running when a `SentryUserProvider` throws and discard the incomplete user identity ([#6240](https://github.com/getsentry/sentry-java/pull/6240))
 - Drop Apollo 5 spans when `beforeSpan` throws without disrupting the GraphQL request ([#6238](https://github.com/getsentry/sentry-java/pull/6238))
 

sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java

@@ -62,14 +62,37 @@ public class SentryUserFeedbackForm extends AlertDialog {
       final @Nullable SentryFeedbackOptions.OptionsConfigurator configurator) {
     super(context, themeResId);
     this.associatedEventId = associatedEventId;
-    this.resolvedFeedbackOptions =
-        new SentryFeedbackOptions(Sentry.getCurrentScopes().getOptions().getFeedbackOptions());
+    final @NotNull SentryOptions options = Sentry.getCurrentScopes().getOptions();
+    @NotNull
+    SentryFeedbackOptions feedbackOptions = new SentryFeedbackOptions(options.getFeedbackOptions());
     if (configuration != null) {
-      configuration.configure(context, resolvedFeedbackOptions);
+      final @NotNull SentryFeedbackOptions configuredOptions =
+          new SentryFeedbackOptions(feedbackOptions);
+      try {
+        configuration.configure(context, configuredOptions);
+        feedbackOptions = configuredOptions;
+      } catch (Throwable e) {
+        ExceptionUtils.rethrowIfFatal(e);
+        options
+            .getLogger()
+            .log(
+                SentryLevel.ERROR, "Feedback OptionsConfiguration callback threw an exception.", e);
+      }
     }
     if (configurator != null) {
-      configurator.configure(resolvedFeedbackOptions);
+      final @NotNull SentryFeedbackOptions configuredOptions =
+          new SentryFeedbackOptions(feedbackOptions);
+      try {
+        configurator.configure(configuredOptions);
+        feedbackOptions = configuredOptions;
+      } catch (Throwable e) {
+        ExceptionUtils.rethrowIfFatal(e);
+        options
+            .getLogger()
+            .log(SentryLevel.ERROR, "Feedback OptionsConfigurator callback threw an exception.", e);
+      }
     }
+    this.resolvedFeedbackOptions = feedbackOptions;
     SentryIntegrationPackageStorage.getInstance().addIntegration("UserFeedbackWidget");
     maybeStartShakeDetection(context);
   }

sentry-android-core/src/test/java/io/sentry/android/core/SentryUserFeedbackFormTest.kt

@@ -164,6 +164,75 @@ class SentryUserFeedbackFormTest {
     assertNotEquals("custom title", fixture.options.feedbackOptions.formTitle)
   }
 
+  @Test
+  fun `when configuration throws, configurator still runs with original options`() {
+    fixture.options.isEnabled = true
+    val originalTitle = fixture.options.feedbackOptions.formTitle
+    val customNameLabel = "custom name label"
+    val failure = IllegalStateException("configuration failed")
+    var titleReceivedByConfigurator: CharSequence? = null
+
+    val sut =
+      fixture.getSut(
+        configuration = { _, options ->
+          options.formTitle = "partial title"
+          throw failure
+        },
+        configurator = { options ->
+          titleReceivedByConfigurator = options.formTitle
+          options.nameLabel = customNameLabel
+        },
+      )
+    sut.show()
+
+    assertThat(titleReceivedByConfigurator).isEqualTo(originalTitle)
+    assertThat(sut.findViewById<TextView>(R.id.sentry_dialog_user_feedback_title).text)
+      .isEqualTo(originalTitle)
+    assertThat(sut.findViewById<TextView>(R.id.sentry_dialog_user_feedback_txt_name).text)
+      .isEqualTo(customNameLabel)
+  }
+
+  @Test
+  fun `when configuration throws a fatal error, it is not swallowed`() {
+    val failure = OutOfMemoryError("configuration failed")
+
+    val thrown =
+      assertFailsWith<OutOfMemoryError> {
+        fixture.getSut(configuration = { _, _ -> throw failure })
+      }
+
+    assertThat(thrown).isSameInstanceAs(failure)
+  }
+
+  @Test
+  fun `when configurator throws, its partial changes are ignored`() {
+    fixture.options.isEnabled = true
+    val failure = IllegalStateException("configurator failed")
+
+    val sut =
+      fixture.getSut(
+        configuration = { _, options -> options.formTitle = "configured title" },
+        configurator = { options ->
+          options.formTitle = "partial title"
+          throw failure
+        },
+      )
+    sut.show()
+
+    assertThat(sut.findViewById<TextView>(R.id.sentry_dialog_user_feedback_title).text)
+      .isEqualTo("configured title")
+  }
+
+  @Test
+  fun `when configurator throws a fatal error, it is not swallowed`() {
+    val failure = OutOfMemoryError("configurator failed")
+
+    val thrown =
+      assertFailsWith<OutOfMemoryError> { fixture.getSut(configurator = { throw failure }) }
+
+    assertThat(thrown).isSameInstanceAs(failure)
+  }
+
   @Test
   fun `dialog window does not have FLAG_ALT_FOCUSABLE_IM so soft keyboard can appear`() {
     fixture.options.isEnabled = true