diff --git a/CHANGELOG.md b/CHANGELOG.md index 1589a2f6f0..23711a8bbc 100644 --- a/CHANGELOG.md +++ b/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)) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java index 611db3c134..7da33575de 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java +++ b/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); } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryUserFeedbackFormTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryUserFeedbackFormTest.kt index 95e50a85b1..19414515b8 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryUserFeedbackFormTest.kt +++ b/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(R.id.sentry_dialog_user_feedback_title).text) + .isEqualTo(originalTitle) + assertThat(sut.findViewById(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 { + 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(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 { 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