diff --git a/CHANGELOG.md b/CHANGELOG.md index fc054e0be18..2642833569d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Features +- Add opt-in `strictCallbackMode` to propagate user callback failures as SDK exception or error wrappers. Events containing these failures are silently excluded from capture, including when nested in another exception, to avoid sending data without callback filtering. Configure it through SDK options, `strict-callback-mode` in external configuration, or `io.sentry.strict-callback-mode` in the Android manifest. Disabled by default ([#6173](https://github.com/getsentry/sentry-java/pull/6173)) - Add support for Android Navigation 3 through the new `sentry-android-navigation3` library ([#6233](https://github.com/getsentry/sentry-java/pull/6233)) - Use `SentryNavEffect` to record navigation transactions, breadcrumbs, screen names, and additional context as your nav back stack changes. - See the [Navigation for Android docs](https://docs.sentry.io/platforms/android/integrations/navigation/) for additional details. diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java index 4405cd19309..456706724e7 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/FeedbackShakeIntegration.java @@ -300,6 +300,7 @@ private void startShakeDetection(final @NotNull Activity activity) { if (dialog != null) { onDialogGone(dialog); } + io.sentry.util.ExceptionUtils.maybeRethrow(e); options .getLogger() .log(SentryLevel.ERROR, "Failed to show feedback dialog on shake.", e); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java index 7de5a0c2716..fc7119cec04 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ManifestMetadataReader.java @@ -36,6 +36,7 @@ final class ManifestMetadataReader { static final String DSN = "io.sentry.dsn"; static final String DEBUG = "io.sentry.debug"; + static final String STRICT_CALLBACK_MODE = "io.sentry.strict-callback-mode"; static final String DEBUG_LEVEL = "io.sentry.debug.level"; static final String SAMPLE_RATE = "io.sentry.sample-rate"; static final String ANR_ENABLE = "io.sentry.anr.enable"; @@ -256,6 +257,8 @@ static void applyMetadata( if (metadata != null) { options.setDebug(readBool(metadata, logger, DEBUG, options.isDebug())); + options.setStrictCallbackMode( + readBool(metadata, logger, STRICT_CALLBACK_MODE, options.isStrictCallbackMode())); if (options.isDebug()) { final @Nullable String level = diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java index 6932b6be3f6..5eb73ec8a35 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ScreenshotEventProcessor.java @@ -116,7 +116,11 @@ private boolean isMaskingEnabled() { if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) { return event; } - } catch (Exception e) { + } catch (Exception | Error e) { + io.sentry.util.CallbackUtils.rethrowIfStrictCallbackMode(options, e); + if (e instanceof Error) { + throw (Error) e; + } options .getLogger() .log( diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java index 82916a248e6..e689442e129 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java @@ -145,6 +145,7 @@ public static void init( try { configuration.configure(options); } catch (Throwable t) { + io.sentry.util.CallbackUtils.rethrowIfStrictCallbackMode(options, t); // let it slip, but log it options .getLogger() 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 611db3c1345..04832f955c2 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 @@ -30,6 +30,7 @@ import io.sentry.protocol.Feedback; import io.sentry.protocol.SentryId; import io.sentry.protocol.User; +import io.sentry.util.CallbackUtils; import io.sentry.util.ExceptionUtils; import io.sentry.util.FileUtils; import io.sentry.util.LoadClass; @@ -65,10 +66,14 @@ public class SentryUserFeedbackForm extends AlertDialog { this.resolvedFeedbackOptions = new SentryFeedbackOptions(Sentry.getCurrentScopes().getOptions().getFeedbackOptions()); if (configuration != null) { - configuration.configure(context, resolvedFeedbackOptions); + CallbackUtils.run( + Sentry.getCurrentScopes().getOptions(), + () -> configuration.configure(context, resolvedFeedbackOptions)); } if (configurator != null) { - configurator.configure(resolvedFeedbackOptions); + CallbackUtils.run( + Sentry.getCurrentScopes().getOptions(), + () -> configurator.configure(resolvedFeedbackOptions)); } SentryIntegrationPackageStorage.getInstance().addIntegration("UserFeedbackWidget"); maybeStartShakeDetection(context); @@ -338,37 +343,44 @@ protected void onCreate(Bundle savedInstanceState) { final @NotNull Hint hint = new Hint(); maybeAddImageAttachment(hint); final @NotNull SentryId id = Sentry.feedback().capture(feedback, hint); - if (!id.equals(SentryId.EMPTY_ID)) { - Toast.makeText( - getContext(), feedbackOptions.getSuccessMessageText(), Toast.LENGTH_SHORT) - .show(); - final @Nullable SentryFeedbackOptions.SentryFeedbackCallback onSubmitSuccess = - feedbackOptions.getOnSubmitSuccess(); - if (onSubmitSuccess != null) { - try { - onSubmitSuccess.call(feedback); - } catch (Exception e) { - Sentry.getCurrentScopes() - .getOptions() - .getLogger() - .log(SentryLevel.ERROR, "onSubmitSuccess callback threw an exception.", e); + try { + if (!id.equals(SentryId.EMPTY_ID)) { + Toast.makeText( + getContext(), feedbackOptions.getSuccessMessageText(), Toast.LENGTH_SHORT) + .show(); + final @Nullable SentryFeedbackOptions.SentryFeedbackCallback onSubmitSuccess = + feedbackOptions.getOnSubmitSuccess(); + if (onSubmitSuccess != null) { + try { + CallbackUtils.run( + Sentry.getCurrentScopes().getOptions(), () -> onSubmitSuccess.call(feedback)); + } catch (Exception e) { + ExceptionUtils.maybeRethrow(e); + Sentry.getCurrentScopes() + .getOptions() + .getLogger() + .log(SentryLevel.ERROR, "onSubmitSuccess callback threw an exception.", e); + } } - } - } else { - final @Nullable SentryFeedbackOptions.SentryFeedbackCallback onSubmitError = - feedbackOptions.getOnSubmitError(); - if (onSubmitError != null) { - try { - onSubmitError.call(feedback); - } catch (Exception e) { - Sentry.getCurrentScopes() - .getOptions() - .getLogger() - .log(SentryLevel.ERROR, "onSubmitError callback threw an exception.", e); + } else { + final @Nullable SentryFeedbackOptions.SentryFeedbackCallback onSubmitError = + feedbackOptions.getOnSubmitError(); + if (onSubmitError != null) { + try { + CallbackUtils.run( + Sentry.getCurrentScopes().getOptions(), () -> onSubmitError.call(feedback)); + } catch (Exception e) { + ExceptionUtils.maybeRethrow(e); + Sentry.getCurrentScopes() + .getOptions() + .getLogger() + .log(SentryLevel.ERROR, "onSubmitError callback threw an exception.", e); + } } } + } finally { + cancel(); } - cancel(); }); btnCancel.setText(feedbackOptions.getCancelButtonLabel()); @@ -388,13 +400,15 @@ public void setOnDismissListener(final @Nullable OnDismissListener listener) { // User-provided callback: a crash in it must not take down the app or skip the // cleanup and the user's own dismiss listener below try { - onFormClose.run(); + CallbackUtils.run(options, onFormClose); } catch (Exception e) { + ExceptionUtils.maybeRethrow(e); options .getLogger() .log(SentryLevel.ERROR, "onFormClose callback threw an exception.", e); + } finally { + currentReplayId = null; } - currentReplayId = null; if (delegate != null) { delegate.onDismiss(dialog); } @@ -426,8 +440,9 @@ protected void onStart() { final @Nullable Runnable onFormOpen = feedbackOptions.getOnFormOpen(); if (onFormOpen != null) { try { - onFormOpen.run(); + CallbackUtils.run(options, onFormOpen); } catch (Exception e) { + ExceptionUtils.maybeRethrow(e); options.getLogger().log(SentryLevel.ERROR, "onFormOpen callback threw an exception.", e); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java index 00753ee4c90..a6b365f1e01 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ViewHierarchyEventProcessor.java @@ -95,7 +95,11 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) { return event; } - } catch (Exception e) { + } catch (Exception | Error e) { + io.sentry.util.CallbackUtils.rethrowIfStrictCallbackMode(options, e); + if (e instanceof Error) { + throw (Error) e; + } options .getLogger() .log( diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryWindowCallback.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryWindowCallback.java index 612eb97946e..d1ecd9ba36e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryWindowCallback.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryWindowCallback.java @@ -57,6 +57,7 @@ public boolean dispatchTouchEvent(final @Nullable MotionEvent motionEvent) { try { handleTouchEvent(copy); } catch (Throwable e) { + io.sentry.util.ExceptionUtils.maybeRethrow(e); if (options != null) { options.getLogger().log(SentryLevel.ERROR, "Error dispatching touch event", e); } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt index 463b5d82ddb..ece09e0db81 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ManifestMetadataReaderTest.kt @@ -42,6 +42,39 @@ class ManifestMetadataReaderTest { private val fixture = Fixture() + @Test + fun `strict callback mode defaults to false when metadata is absent`() { + ManifestMetadataReader.applyMetadata( + fixture.getContext(), + fixture.options, + fixture.buildInfoProvider, + ) + assertThat(fixture.options.isStrictCallbackMode).isFalse() + } + + @Test + fun `strict callback mode preserves programmatic value when metadata is absent`() { + fixture.options.isStrictCallbackMode = true + ManifestMetadataReader.applyMetadata( + fixture.getContext(), + fixture.options, + fixture.buildInfoProvider, + ) + assertThat(fixture.options.isStrictCallbackMode).isTrue() + } + + @Test + fun `strict callback mode reads true and false from manifest`() { + for (value in listOf(true, false)) { + fixture.options.isStrictCallbackMode = !value + ContextUtils.resetInstance() + val context = + fixture.getContext(bundleOf(ManifestMetadataReader.STRICT_CALLBACK_MODE to value)) + ManifestMetadataReader.applyMetadata(context, fixture.options, fixture.buildInfoProvider) + assertThat(fixture.options.isStrictCallbackMode).isEqualTo(value) + } + } + @BeforeTest fun `set up`() { ContextUtils.resetInstance() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt index 635d7a308c5..bdf99de117f 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ScreenshotEventProcessorTest.kt @@ -314,6 +314,22 @@ class ScreenshotEventProcessorTest { assertNull(hint.screenshot) } + @Test + fun `strict capture failures propagate without attaching screenshot`() { + CurrentActivityHolder.getInstance().setActivity(fixture.activity) + fixture.options.isStrictCallbackMode = true + val processor = fixture.getSut(true) + for (failure in listOf(IllegalStateException("private"), LinkageError("private"))) { + fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ -> throw failure } + val event = SentryEvent().apply { exceptions = listOf(SentryException()) } + val hint = Hint() + val thrown = kotlin.test.assertFails { processor.process(event, hint) } + assertThat(io.sentry.util.CallbackUtils.isCallbackException(thrown)).isTrue() + assertThat(thrown.cause).isSameInstanceAs(failure) + assertThat(hint.screenshot).isNull() + } + } + @Test fun `when capture callback throws, skips screenshot and retains event`() { CurrentActivityHolder.getInstance().setActivity(fixture.activity) 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 95e50a85b14..18f54a6af07 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 @@ -106,6 +106,82 @@ class SentryUserFeedbackFormTest { fixture.mockedSentry.close() } + @Test + fun `strict submit callbacks propagate failures and close the form`() { + fixture.options.isStrictCallbackMode = true + for (success in listOf(false, true)) { + for (failure in listOf(IllegalStateException("private"), LinkageError("private"))) { + fixture.options.feedbackOptions.setOnSubmitSuccess { throw failure } + fixture.options.feedbackOptions.setOnSubmitError { throw failure } + whenever(fixture.mockFeedbackApi.capture(any(), anyOrNull())) + .thenReturn(if (success) SentryId() else SentryId.EMPTY_ID) + val sut = fixture.getSut() + sut.show() + sut + .findViewById(R.id.sentry_dialog_user_feedback_edt_description) + .setText("message") + val thrown = + kotlin.test.assertFails { + sut.findViewById