diff --git a/CHANGELOG.md b/CHANGELOG.md index 0036c65807e..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. @@ -60,6 +61,16 @@ ### Fixes +- Fix SDK callback error handling ([#6140](https://github.com/getsentry/sentry-java/pull/6140)) + - Add `DiscardReason.CALLBACK_ERROR` and use it for telemetry dropped when a `beforeSend*` callback throws. `OnDiscardCallback` can now receive this value. + - Report attached profiles dropped by transaction callback errors as `callback_error` in client reports and `OnDiscardCallback` ([#6166](https://github.com/getsentry/sentry-java/pull/6166)) + - Drop telemetry and record `callback_error` when a customer event processor throws instead of continuing with a potentially partially processed item. SDK-owned processor failures are logged and processing continues without a `callback_error` client report. + - Drop breadcrumbs when `beforeBreadcrumb` throws instead of storing exception details on the breadcrumb. + - Skip Android screenshot or view hierarchy capture when its capture callback throws, while retaining the error event ([#6167](https://github.com/getsentry/sentry-java/pull/6167)) + - Drop spans when `beforeSpan` throws in OkHttp, OpenFeign, GraphQL, Ktor, or Apollo, without disrupting the request. Report lost sampled spans as `callback_error` in client reports and `OnDiscardCallback` ([#6167](https://github.com/getsentry/sentry-java/pull/6167)) + - Skip replay capture when `beforeErrorSampling` throws, while still sending the error event ([#6165](https://github.com/getsentry/sentry-java/pull/6165)) + - When `tracesSampler` throws, drop the transaction and record `callback_error` instead of inheriting the parent sampling decision or falling back to `tracesSampleRate` ([#6163](https://github.com/getsentry/sentry-java/pull/6163)) + - When `profilesSampler` throws, disable profiling instead of falling back to `profilesSampleRate` or inheriting the parent's profiling decision. Trace sampling is unchanged ([#6164](https://github.com/getsentry/sentry-java/pull/6164)) - Disable URL caching when reading `META-INF/MANIFEST.MF` files during version detection so that the SDK no longer keeps jar file handles open for the life of the process ([#6124](https://github.com/getsentry/sentry-java/pull/6124) - Keep the `EventListener` wrapped by `SentryOkHttpEventListener` per `Call` ([#6003](https://github.com/getsentry/sentry-java/pull/6003)) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index efd776cabb8..e8b95794ea0 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -233,7 +233,7 @@ public final class io/sentry/android/core/AppState$LifecycleObserver : androidx/ public fun onStop (Landroidx/lifecycle/LifecycleOwner;)V } -public final class io/sentry/android/core/ApplicationExitInfoEventProcessor : io/sentry/BackfillingEventProcessor { +public final class io/sentry/android/core/ApplicationExitInfoEventProcessor : io/sentry/BackfillingEventProcessor, io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Landroid/content/Context;Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/BuildInfoProvider;)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; @@ -401,7 +401,7 @@ public class io/sentry/android/core/PerfettoProfiler { public fun endAndCollect (Ljava/util/function/Consumer;)V } -public final class io/sentry/android/core/ScreenshotEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/android/core/ScreenshotEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/BuildInfoProvider;Z)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; @@ -680,7 +680,7 @@ public final class io/sentry/android/core/UserInteractionIntegration : android/a public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V } -public final class io/sentry/android/core/ViewHierarchyEventProcessor : io/sentry/EventProcessor { +public final class io/sentry/android/core/ViewHierarchyEventProcessor : io/sentry/internal/eventprocessor/SentryEventProcessor { public fun (Lio/sentry/android/core/SentryAndroidOptions;)V public fun getOrder ()Ljava/lang/Long; public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java index 400a3293bcb..483259fa8dd 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ApplicationExitInfoEventProcessor.java @@ -52,6 +52,7 @@ import io.sentry.hints.AbnormalExit; import io.sentry.hints.Backfillable; import io.sentry.hints.NativeCrashExit; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.App; import io.sentry.protocol.Contexts; import io.sentry.protocol.DebugImage; @@ -89,7 +90,8 @@ */ @ApiStatus.Internal @WorkerThread -public final class ApplicationExitInfoEventProcessor implements BackfillingEventProcessor { +public final class ApplicationExitInfoEventProcessor + implements BackfillingEventProcessor, SentryEventProcessor { private final @NotNull Context context; diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java index 84e953ee69e..c7ea0e27e4f 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/DefaultAndroidEventProcessor.java @@ -8,6 +8,7 @@ import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.android.core.performance.AppStartMetrics; import io.sentry.android.core.performance.TimeSpan; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.App; import io.sentry.protocol.OperatingSystem; import io.sentry.protocol.SentryException; @@ -32,7 +33,7 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; -final class DefaultAndroidEventProcessor implements EventProcessor { +final class DefaultAndroidEventProcessor implements SentryEventProcessor { @TestOnly final Context context; 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/PerformanceAndroidEventProcessor.java b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java index 7f81ea1b738..142bb1f4752 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/PerformanceAndroidEventProcessor.java @@ -7,7 +7,6 @@ import static io.sentry.android.core.ActivityLifecycleIntegration.STANDALONE_APP_START_OP; import static io.sentry.android.core.ActivityLifecycleIntegration.UI_LOAD_OP; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.ISentryLifecycleToken; import io.sentry.MeasurementUnit; @@ -20,6 +19,7 @@ import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.android.core.performance.AppStartMetrics; import io.sentry.android.core.performance.TimeSpan; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.App; import io.sentry.protocol.MeasurementValue; import io.sentry.protocol.SentryId; @@ -36,7 +36,7 @@ import org.jetbrains.annotations.Nullable; /** Event Processor responsible for adding Android metrics to transactions */ -final class PerformanceAndroidEventProcessor implements EventProcessor { +final class PerformanceAndroidEventProcessor implements SentryEventProcessor { private static final String APP_METRICS_ORIGIN = "auto.ui"; 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 8c4b1feef7c..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 @@ -8,7 +8,6 @@ import android.graphics.Bitmap; import android.view.View; import io.sentry.Attachment; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.SentryEvent; import io.sentry.SentryLevel; @@ -18,6 +17,7 @@ import io.sentry.android.replay.util.MaskRenderer; import io.sentry.android.replay.util.ViewsKt; import io.sentry.android.replay.viewhierarchy.ViewHierarchyNode; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.protocol.SentryTransaction; import io.sentry.util.HintUtils; import io.sentry.util.Objects; @@ -34,7 +34,7 @@ * captured. */ @ApiStatus.Internal -public final class ScreenshotEventProcessor implements EventProcessor { +public final class ScreenshotEventProcessor implements SentryEventProcessor { private final @NotNull SentryAndroidOptions options; private final @NotNull BuildInfoProvider buildInfoProvider; @@ -112,7 +112,21 @@ private boolean isMaskingEnabled() { final @Nullable SentryAndroidOptions.BeforeCaptureCallback beforeCaptureCallback = options.getBeforeScreenshotCaptureCallback(); if (beforeCaptureCallback != null) { - if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) { + try { + if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) { + return event; + } + } catch (Exception | Error e) { + io.sentry.util.CallbackUtils.rethrowIfStrictCallbackMode(options, e); + if (e instanceof Error) { + throw (Error) e; + } + options + .getLogger() + .log( + SentryLevel.ERROR, + "The beforeScreenshotCapture callback threw an exception. Skipping screenshot capture.", + e); return event; } } else if (shouldDebounce) { 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 6d21edb3d0c..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 @@ -7,7 +7,6 @@ import android.view.ViewGroup; import android.view.Window; import io.sentry.Attachment; -import io.sentry.EventProcessor; import io.sentry.Hint; import io.sentry.ILogger; import io.sentry.ISerializer; @@ -18,6 +17,7 @@ import io.sentry.android.core.internal.util.AndroidThreadChecker; import io.sentry.android.core.internal.util.ClassUtil; import io.sentry.android.core.internal.util.Debouncer; +import io.sentry.internal.eventprocessor.SentryEventProcessor; import io.sentry.internal.viewhierarchy.ViewHierarchyExporter; import io.sentry.protocol.SentryTransaction; import io.sentry.protocol.ViewHierarchy; @@ -37,7 +37,7 @@ /** ViewHierarchyEventProcessor responsible for taking a snapshot of the current view hierarchy. */ @ApiStatus.Internal -public final class ViewHierarchyEventProcessor implements EventProcessor { +public final class ViewHierarchyEventProcessor implements SentryEventProcessor { private final @NotNull SentryAndroidOptions options; private final @NotNull Debouncer debouncer; @@ -91,7 +91,21 @@ public ViewHierarchyEventProcessor(final @NotNull SentryAndroidOptions options) final @Nullable SentryAndroidOptions.BeforeCaptureCallback beforeCaptureCallback = options.getBeforeViewHierarchyCaptureCallback(); if (beforeCaptureCallback != null) { - if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) { + try { + if (!beforeCaptureCallback.execute(event, hint, shouldDebounce)) { + return event; + } + } catch (Exception | Error e) { + io.sentry.util.CallbackUtils.rethrowIfStrictCallbackMode(options, e); + if (e instanceof Error) { + throw (Error) e; + } + options + .getLogger() + .log( + SentryLevel.ERROR, + "The beforeViewHierarchyCapture callback threw an exception. Skipping view hierarchy capture.", + e); return event; } } else if (shouldDebounce) { 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 b8e223f08e9..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 @@ -29,11 +29,14 @@ import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.sp import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat import io.sentry.Attachment import io.sentry.Hint +import io.sentry.ILogger import io.sentry.MainEventProcessor import io.sentry.SentryEvent import io.sentry.SentryIntegrationPackageStorage +import io.sentry.SentryLevel import io.sentry.TypeCheckHint.ANDROID_ACTIVITY import io.sentry.protocol.SentryException import io.sentry.util.thread.IThreadChecker @@ -48,6 +51,7 @@ import kotlin.test.assertSame import kotlin.test.assertTrue import org.junit.runner.RunWith import org.mockito.kotlin.mock +import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.robolectric.Robolectric.buildActivity import org.robolectric.Shadows.shadowOf @@ -310,11 +314,54 @@ 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) + val logger = mock() + fixture.options.isDebug = true + fixture.options.setLogger(logger) + val failure = IllegalStateException("callback failed") + fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ -> throw failure } + val processor = fixture.getSut(true) + val event = SentryEvent().apply { exceptions = listOf(SentryException()) } + val hint = Hint() + + assertThat(processor.process(event, hint)).isSameInstanceAs(event) + assertThat(hint.screenshot).isNull() + verify(logger) + .log( + SentryLevel.ERROR, + "The beforeScreenshotCapture callback threw an exception. Skipping screenshot capture.", + failure, + ) + + fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ -> true } + val nextHint = Hint() + assertThat(processor.process(event, nextHint)).isSameInstanceAs(event) + assertThat(nextHint.screenshot).isNotNull() + } + @Test fun `when capture callback returns true, a screenshot should be captured`() { CurrentActivityHolder.getInstance().setActivity(fixture.activity) - fixture.options.setBeforeViewHierarchyCaptureCallback { _, _, _ -> true } + fixture.options.setBeforeScreenshotCaptureCallback { _, _, _ -> true } val processor = fixture.getSut(true) val event = SentryEvent().apply { exceptions = listOf(SentryException()) } 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