diff --git a/CHANGELOG.md b/CHANGELOG.md index 35f1f30820b..abee0c33746 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Add `Sentry.feedback().enableFeedbackOnShake()` and `Sentry.feedback().disableFeedbackOnShake()` to toggle shake-to-report at runtime ([#5827](https://github.com/getsentry/sentry-java/pull/5827)) + ### Improvements - Skip building Android manifest metadata debug log messages when debug logging is disabled, reducing allocations during SDK init ([#5790](https://github.com/getsentry/sentry-java/pull/5790)) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index adebedf2700..6f5227ec9ac 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -293,9 +293,12 @@ public abstract class io/sentry/android/core/EnvelopeFileObserverIntegration : i public final fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V } -public final class io/sentry/android/core/FeedbackShakeIntegration : android/app/Application$ActivityLifecycleCallbacks, io/sentry/Integration, java/io/Closeable { +public final class io/sentry/android/core/FeedbackShakeIntegration : android/app/Application$ActivityLifecycleCallbacks, io/sentry/Integration, io/sentry/SentryFeedbackOptions$IShakeController, java/io/Closeable { public fun (Landroid/app/Application;)V public fun close ()V + public fun disable ()V + public fun enable ()V + public fun isEnabled ()Z public fun onActivityCreated (Landroid/app/Activity;Landroid/os/Bundle;)V public fun onActivityDestroyed (Landroid/app/Activity;)V public fun onActivityPaused (Landroid/app/Activity;)V @@ -303,6 +306,7 @@ public final class io/sentry/android/core/FeedbackShakeIntegration : android/app public fun onActivitySaveInstanceState (Landroid/app/Activity;Landroid/os/Bundle;)V public fun onActivityStarted (Landroid/app/Activity;)V public fun onActivityStopped (Landroid/app/Activity;)V + public fun pauseDetection (Z)V public fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V } @@ -556,6 +560,7 @@ public abstract interface class io/sentry/android/core/SentryUserFeedbackDialog$ public class io/sentry/android/core/SentryUserFeedbackForm : android/app/AlertDialog { protected fun onCreate (Landroid/os/Bundle;)V protected fun onStart ()V + protected fun onStop ()V public fun setCancelable (Z)V public fun setOnDismissListener (Landroid/content/DialogInterface$OnDismissListener;)V public fun show ()V diff --git a/sentry-android-core/build.gradle.kts b/sentry-android-core/build.gradle.kts index 23248d6dae4..0e3708a89bf 100644 --- a/sentry-android-core/build.gradle.kts +++ b/sentry-android-core/build.gradle.kts @@ -115,6 +115,7 @@ dependencies { testImplementation(libs.androidx.test.ext.junit) testImplementation(libs.androidx.test.runner) testImplementation(libs.awaitility.kotlin) + testImplementation(libs.google.truth) testImplementation(libs.mockito.kotlin) testImplementation(libs.mockito.inline) testImplementation(projects.sentryTestSupport) 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 b059b0104da..b0ccda495e1 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 @@ -7,6 +7,7 @@ import android.os.Bundle; import io.sentry.IScopes; import io.sentry.Integration; +import io.sentry.SentryFeedbackOptions; import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.util.Objects; @@ -17,18 +18,27 @@ import org.jetbrains.annotations.Nullable; /** - * Detects shake gestures and shows the user feedback dialog when a shake is detected. Only active - * when {@link io.sentry.SentryFeedbackOptions#isUseShakeGesture()} returns {@code true}. + * Detects shake gestures and shows the user feedback dialog when a shake is detected. {@link + * io.sentry.SentryFeedbackOptions#isUseShakeGesture()} determines the initial state; it can be + * toggled at runtime via {@code Sentry.feedback().enableFeedbackOnShake()} and {@code + * Sentry.feedback().disableFeedbackOnShake()}. + * + *

While any feedback dialog is visible it pauses shake handling via {@link + * #pauseDetection(boolean)}, so a shake can never stack a second dialog on top of one that is + * already showing — no matter how the visible dialog was opened. */ public final class FeedbackShakeIntegration - implements Integration, Closeable, Application.ActivityLifecycleCallbacks { + implements Integration, + Closeable, + Application.ActivityLifecycleCallbacks, + SentryFeedbackOptions.IShakeController { private final @NotNull Application application; private final @NotNull SentryShakeDetector shakeDetector; private @Nullable SentryAndroidOptions options; + private volatile boolean enabled = false; + private volatile boolean paused = false; private volatile @Nullable WeakReference currentActivityRef; - private volatile boolean isDialogShowing = false; - private volatile @Nullable Runnable previousOnFormClose; public FeedbackShakeIntegration(final @NotNull Application application) { this.application = Objects.requireNonNull(application, "Application is required"); @@ -46,13 +56,25 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions final @NotNull SentryAndroidOptions options = this.options; - if (!options.getFeedbackOptions().isUseShakeGesture()) { + // Always expose the runtime toggle, even when the option starts out disabled. + options.getFeedbackOptions().setShakeController(this); + + if (options.getFeedbackOptions().isUseShakeGesture()) { + enable(); + } + } + + @Override + public synchronized void enable() { + final @Nullable SentryAndroidOptions options = this.options; + if (enabled || options == null) { return; } + enabled = true; - // Re-arm the detector in case this integration is being re-registered after a previous close() - // (e.g. a second Sentry.init reusing the same options), otherwise the closed latch would keep - // shake detection off permanently. + // Re-arm the detector in case it was closed before, either by disable() or by a previous + // close() (e.g. a second Sentry.init reusing the same options), otherwise the closed latch + // would keep shake detection off permanently. shakeDetector.reopen(); // Resolving the accelerometer is the most expensive part of init (the first SensorManager @@ -72,7 +94,7 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions application.registerActivityLifecycleCallbacks(this); options.getLogger().log(SentryLevel.DEBUG, "FeedbackShakeIntegration installed."); - // In case of a deferred init, hook into any already-resumed activity + // In case of a deferred init or runtime enable, hook into any already-resumed activity final @Nullable Activity activity = CurrentActivityHolder.getInstance().getActivity(); if (activity != null) { currentActivityRef = new WeakReference<>(activity); @@ -81,34 +103,34 @@ public void register(final @NotNull IScopes scopes, final @NotNull SentryOptions } @Override - public void close() throws IOException { + public synchronized void disable() { + if (!enabled) { + return; + } + enabled = false; + application.unregisterActivityLifecycleCallbacks(this); shakeDetector.close(); - // Restore onFormClose if a dialog is still showing, since lifecycle callbacks - // are now unregistered and onActivityDestroyed cleanup won't fire. - if (isDialogShowing) { - isDialogShowing = false; - if (options != null) { - options.getFeedbackOptions().setOnFormClose(previousOnFormClose); - } - previousOnFormClose = null; - } currentActivityRef = null; } + @Override + public boolean isEnabled() { + return enabled; + } + + @Override + public void pauseDetection(final boolean paused) { + this.paused = paused; + } + + @Override + public void close() throws IOException { + disable(); + } + @Override public void onActivityResumed(final @NotNull Activity activity) { - // If a dialog is showing on a different activity (e.g. user navigated via notification), - // clean up since the dialog's host activity is going away and onActivityDestroyed - // won't match currentActivity anymore. - final @Nullable Activity current = currentActivityRef != null ? currentActivityRef.get() : null; - if (isDialogShowing && current != null && current != activity) { - isDialogShowing = false; - if (options != null) { - options.getFeedbackOptions().setOnFormClose(previousOnFormClose); - } - previousOnFormClose = null; - } currentActivityRef = new WeakReference<>(activity); startShakeDetection(activity); } @@ -118,16 +140,11 @@ public void onActivityPaused(final @NotNull Activity activity) { // Only stop if this is the activity we're tracking. When transitioning between // activities, B.onResume may fire before A.onPause — stopping unconditionally // would kill shake detection for the new activity. - final @Nullable Activity current = currentActivityRef != null ? currentActivityRef.get() : null; + final @Nullable WeakReference currentRef = currentActivityRef; + final @Nullable Activity current = currentRef != null ? currentRef.get() : null; if (activity == current) { stopShakeDetection(); - // Keep currentActivityRef set when a dialog is showing so onActivityDestroyed - // can still match and clean up. Otherwise the cleanup condition - // (activity == current) would always be false since onPause fires - // before onDestroy. - if (!isDialogShowing) { - currentActivityRef = null; - } + currentActivityRef = null; } } @@ -146,19 +163,7 @@ public void onActivitySaveInstanceState( final @NotNull Activity activity, final @NotNull Bundle outState) {} @Override - public void onActivityDestroyed(final @NotNull Activity activity) { - // Only reset if this is the activity that hosts the dialog — the dialog cannot - // outlive its host activity being destroyed. - final @Nullable Activity current = currentActivityRef != null ? currentActivityRef.get() : null; - if (isDialogShowing && activity == current) { - isDialogShowing = false; - currentActivityRef = null; - if (options != null) { - options.getFeedbackOptions().setOnFormClose(previousOnFormClose); - } - previousOnFormClose = null; - } - } + public void onActivityDestroyed(final @NotNull Activity activity) {} private void startShakeDetection(final @NotNull Activity activity) { if (options == null) { @@ -172,41 +177,24 @@ private void startShakeDetection(final @NotNull Activity activity) { final @Nullable WeakReference ref = currentActivityRef; final Activity active = ref != null ? ref.get() : null; final Boolean inBackground = AppState.getInstance().isInBackground(); - if (active != null - && options != null - && !isDialogShowing - && !Boolean.TRUE.equals(inBackground)) { - active.runOnUiThread( - () -> { - if (isDialogShowing || active.isFinishing() || active.isDestroyed()) { - return; - } - try { - isDialogShowing = true; - final Runnable captured = options.getFeedbackOptions().getOnFormClose(); - previousOnFormClose = captured; - options - .getFeedbackOptions() - .setOnFormClose( - () -> { - isDialogShowing = false; - options.getFeedbackOptions().setOnFormClose(captured); - if (captured != null) { - captured.run(); - } - previousOnFormClose = null; - }); - new SentryUserFeedbackForm.Builder(active).create().show(); - } catch (Throwable e) { - isDialogShowing = false; - options.getFeedbackOptions().setOnFormClose(previousOnFormClose); - previousOnFormClose = null; - options - .getLogger() - .log(SentryLevel.ERROR, "Failed to show feedback dialog on shake.", e); - } - }); + if (active == null || options == null || paused || Boolean.TRUE.equals(inBackground)) { + return; } + active.runOnUiThread( + () -> { + // Re-check on the main thread: an earlier queued shake may have shown a form + // in the meantime (the form pauses detection synchronously in onStart). + if (paused || active.isFinishing() || active.isDestroyed()) { + return; + } + try { + new SentryUserFeedbackForm.Builder(active).create().show(); + } catch (Throwable 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/SentryUserFeedbackForm.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java index 43500d50ebc..2ad374d5393 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 @@ -60,9 +60,14 @@ public class SentryUserFeedbackForm extends AlertDialog { } private void maybeStartShakeDetection(final @NotNull Context context) { + // Only an explicit per-form opt-in starts a detector for this form. When shake is + // configured globally (via the option or the runtime toggle), FeedbackShakeIntegration + // already shows a form on shake and this form defers to it. final @NotNull SentryFeedbackOptions globalFeedbackOptions = Sentry.getCurrentScopes().getOptions().getFeedbackOptions(); - if (!resolvedFeedbackOptions.isUseShakeGesture() || globalFeedbackOptions.isUseShakeGesture()) { + if (!resolvedFeedbackOptions.isUseShakeGesture() + || globalFeedbackOptions.isUseShakeGesture() + || globalFeedbackOptions.getShakeController().isEnabled()) { return; } final @Nullable Activity activity = getActivity(context); @@ -95,6 +100,15 @@ private void stopShakeDetection() { private @NotNull SentryShakeDetector.Listener shakeListener( final @NotNull WeakReference activityRef) { return () -> { + // If shake-to-report got enabled globally in the meantime, FeedbackShakeIntegration + // reacts to the same shake — don't show a second dialog for it. + if (Sentry.getCurrentScopes() + .getOptions() + .getFeedbackOptions() + .getShakeController() + .isEnabled()) { + return; + } final @Nullable Activity active = activityRef.get(); if (active != null && !active.isFinishing() && !active.isDestroyed()) { active.runOnUiThread( @@ -284,13 +298,27 @@ protected void onCreate(Bundle savedInstanceState) { final @Nullable SentryFeedbackOptions.SentryFeedbackCallback onSubmitSuccess = feedbackOptions.getOnSubmitSuccess(); if (onSubmitSuccess != null) { - onSubmitSuccess.call(feedback); + try { + onSubmitSuccess.call(feedback); + } catch (Throwable 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) { - onSubmitError.call(feedback); + try { + onSubmitError.call(feedback); + } catch (Throwable e) { + Sentry.getCurrentScopes() + .getOptions() + .getLogger() + .log(SentryLevel.ERROR, "onSubmitError callback threw an exception.", e); + } } } cancel(); @@ -310,7 +338,15 @@ public void setOnDismissListener(final @Nullable OnDismissListener listener) { if (onFormClose != null) { super.setOnDismissListener( dialog -> { - onFormClose.run(); + // 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(); + } catch (Throwable e) { + options + .getLogger() + .log(SentryLevel.ERROR, "onFormClose callback threw an exception.", e); + } currentReplayId = null; if (delegate != null) { delegate.onDismiss(dialog); @@ -332,14 +368,44 @@ protected void onStart() { final @NotNull SentryOptions options = Sentry.getCurrentScopes().getOptions(); final @NotNull SentryFeedbackOptions feedbackOptions = options.getFeedbackOptions(); + // Pause shake-to-report while this form is visible, so a shake can't stack a second + // form on top of it + feedbackOptions.getShakeController().pauseDetection(true); final @Nullable Runnable onFormOpen = feedbackOptions.getOnFormOpen(); if (onFormOpen != null) { - onFormOpen.run(); + try { + onFormOpen.run(); + } catch (Throwable e) { + options.getLogger().log(SentryLevel.ERROR, "onFormOpen callback threw an exception.", e); + } } options.getReplayController().captureReplay(false); currentReplayId = options.getReplayController().getReplayId(); } + @Override + protected void onStop() { + super.onStop(); + Sentry.getCurrentScopes() + .getOptions() + .getFeedbackOptions() + .getShakeController() + .pauseDetection(false); + } + + @Override + public void onDetachedFromWindow() { + super.onDetachedFromWindow(); + // Safety net for teardown without a dismiss (e.g. the host activity is destroyed while the + // form is still showing): onStop never fires then, but the window is still detached — + // without this, shake-to-report would stay paused forever. + Sentry.getCurrentScopes() + .getOptions() + .getFeedbackOptions() + .getShakeController() + .pauseDetection(false); + } + @Override public void show() { // If Sentry is disabled, don't show the dialog, but log a warning diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/FeedbackShakeIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/FeedbackShakeIntegrationTest.kt index 170211abf55..2b3b629cdd2 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/FeedbackShakeIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/FeedbackShakeIntegrationTest.kt @@ -4,6 +4,7 @@ import android.app.Activity import android.app.Application import android.content.Context import androidx.test.ext.junit.runners.AndroidJUnit4 +import com.google.common.truth.Truth.assertThat import io.sentry.Scopes import io.sentry.SentryFeedbackOptions import io.sentry.test.DeferredExecutorService @@ -16,6 +17,7 @@ import org.mockito.kotlin.atLeastOnce import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever @@ -165,4 +167,122 @@ class FeedbackShakeIntegrationTest { val sut = fixture.getSut() sut.close() } + + @Test + fun `register sets itself as shake controller even when useShakeGesture is disabled`() { + val sut = fixture.getSut(useShakeGesture = false) + sut.register(fixture.scopes, fixture.options) + + assertThat(fixture.options.feedbackOptions.shakeController).isSameInstanceAs(sut) + assertThat(sut.isEnabled).isFalse() + } + + @Test + fun `enable after register starts shake detection at runtime`() { + CurrentActivityHolder.getInstance().setActivity(fixture.activity) + whenever(fixture.activity.getSystemService(any())).thenReturn(null) + + val sut = fixture.getSut(useShakeGesture = false) + sut.register(fixture.scopes, fixture.options) + verify(fixture.application, never()).registerActivityLifecycleCallbacks(any()) + + sut.enable() + + assertThat(sut.isEnabled).isTrue() + verify(fixture.application).registerActivityLifecycleCallbacks(any()) + // Hooks into the already-resumed activity + verify(fixture.activity).getSystemService(eq(Context.SENSOR_SERVICE)) + } + + @Test + fun `enable is idempotent`() { + val sut = fixture.getSut(useShakeGesture = false) + sut.register(fixture.scopes, fixture.options) + + sut.enable() + sut.enable() + + verify(fixture.application, times(1)).registerActivityLifecycleCallbacks(any()) + } + + @Test + fun `disable stops shake detection at runtime`() { + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + sut.disable() + + assertThat(sut.isEnabled).isFalse() + verify(fixture.application).unregisterActivityLifecycleCallbacks(any()) + } + + @Test + fun `disable is idempotent`() { + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + sut.disable() + sut.disable() + + verify(fixture.application, times(1)).unregisterActivityLifecycleCallbacks(any()) + } + + @Test + fun `disable when never enabled does not unregister callbacks`() { + val sut = fixture.getSut(useShakeGesture = false) + sut.register(fixture.scopes, fixture.options) + + sut.disable() + + verify(fixture.application, never()).unregisterActivityLifecycleCallbacks(any()) + } + + @Test + fun `enable before register is a no-op`() { + val sut = fixture.getSut(useShakeGesture = false) + + sut.enable() + + assertThat(sut.isEnabled).isFalse() + verify(fixture.application, never()).registerActivityLifecycleCallbacks(any()) + } + + @Test + fun `re-enable after disable re-arms shake detection`() { + val deferredExecutor = DeferredExecutorService() + fixture.options.executorService = deferredExecutor + whenever(fixture.application.getSystemService(any())).thenReturn(null) + + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + sut.disable() + sut.enable() + + deferredExecutor.runAll() + + assertThat(sut.isEnabled).isTrue() + verify(fixture.application, atLeastOnce()).getSystemService(eq(Context.SENSOR_SERVICE)) + } + + @Test + fun `close disables shake detection`() { + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + sut.close() + + assertThat(sut.isEnabled).isFalse() + } + + @Test + fun `pauseDetection toggles the paused state`() { + val sut = fixture.getSut(useShakeGesture = true) + sut.register(fixture.scopes, fixture.options) + + sut.pauseDetection(true) + sut.pauseDetection(false) + // Pausing only gates shake handling; detection machinery stays untouched + verify(fixture.application, never()).unregisterActivityLifecycleCallbacks(any()) + assertThat(sut.isEnabled).isTrue() + } } 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 9df2a16d72e..4fb96ab8e90 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 @@ -1,6 +1,7 @@ package io.sentry.android.core import android.content.Context +import android.os.Looper import android.view.WindowManager import android.widget.TextView import androidx.test.core.app.ApplicationProvider @@ -19,13 +20,16 @@ import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertNotEquals import kotlin.test.assertNotNull +import kotlin.test.assertTrue import org.junit.runner.RunWith import org.mockito.Mockito.mockStatic +import org.mockito.kotlin.any import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoInteractions import org.mockito.kotlin.whenever +import org.robolectric.Shadows.shadowOf @RunWith(AndroidJUnit4::class) class SentryUserFeedbackFormTest { @@ -143,4 +147,48 @@ class SentryUserFeedbackFormTest { val flags = window.attributes.flags assertEquals(0, flags and WindowManager.LayoutParams.FLAG_ALT_FOCUSABLE_IM) } + + @Test + fun `a crashing onFormClose callback does not crash the app when the dialog is closed`() { + fixture.options.isEnabled = true + fixture.options.feedbackOptions.onFormClose = Runnable { throw RuntimeException("user bug") } + val sut = fixture.getSut() + sut.show() + + sut.dismiss() + // The dismiss listener is dispatched via a Handler message + shadowOf(Looper.getMainLooper()).idle() + + verify(fixture.mockLogger) + .log(eq(SentryLevel.ERROR), eq("onFormClose callback threw an exception."), any()) + } + + @Test + fun `a crashing onFormClose callback still runs the user's dismiss listener`() { + fixture.options.isEnabled = true + fixture.options.feedbackOptions.onFormClose = Runnable { throw RuntimeException("user bug") } + val sut = fixture.getSut() + var dismissed = false + sut.setOnDismissListener { dismissed = true } + sut.show() + + sut.dismiss() + shadowOf(Looper.getMainLooper()).idle() + + assertTrue(dismissed) + } + + @Test + fun `a crashing onFormOpen callback does not crash the app when the dialog is shown`() { + fixture.options.isEnabled = true + fixture.options.feedbackOptions.onFormOpen = Runnable { throw RuntimeException("user bug") } + val sut = fixture.getSut() + + sut.show() + + verify(fixture.mockLogger) + .log(eq(SentryLevel.ERROR), eq("onFormOpen callback threw an exception."), any()) + // The form open must still complete its own work after the callback crash + verify(fixture.mockReplayController).captureReplay(eq(false)) + } } diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.kt b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.kt index 90e75feee71..6b2147edb19 100644 --- a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.kt +++ b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.kt @@ -805,21 +805,23 @@ fun UserFeedbackScreen() { } } - // Enable shake-to-show for a specific form instance + // Toggle shake-to-show at runtime using the global Sentry.feedback() API item(span = { GridItemSpan(maxLineSpan) }) { + var shakeEnabled by remember { mutableStateOf(Sentry.feedback().isFeedbackOnShakeEnabled) } Button( modifier = Modifier, onClick = { - SentryUserFeedbackForm.Builder(activity) - .configurator { options -> - options.isUseShakeGesture = true - options.formTitle = "Shake Feedback" - } - .create() - Toast.makeText(activity, "Shake your device to open the form!", Toast.LENGTH_SHORT).show() + if (shakeEnabled) { + Sentry.feedback().disableFeedbackOnShake() + } else { + Sentry.feedback().enableFeedbackOnShake() + Toast.makeText(activity, "Shake your device to open the form!", Toast.LENGTH_SHORT) + .show() + } + shakeEnabled = Sentry.feedback().isFeedbackOnShakeEnabled }, ) { - Text(text = "Enable Shake-to-Show") + Text(text = if (shakeEnabled) "Disable Shake-to-Show" else "Enable Shake-to-Show") } } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index c623e71d08f..11c58ddf325 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -853,6 +853,9 @@ public abstract interface class io/sentry/IFeedbackApi { public abstract fun capture (Lio/sentry/protocol/Feedback;)Lio/sentry/protocol/SentryId; public abstract fun capture (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public abstract fun capture (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; + public abstract fun disableFeedbackOnShake ()V + public abstract fun enableFeedbackOnShake ()V + public abstract fun isFeedbackOnShakeEnabled ()Z public abstract fun show ()V public abstract fun show (Lio/sentry/SentryFeedbackOptions$OptionsConfigurator;)V public abstract fun show (Lio/sentry/protocol/SentryId;Lio/sentry/SentryFeedbackOptions$OptionsConfigurator;)V @@ -1604,7 +1607,10 @@ public final class io/sentry/NoOpFeedbackApi : io/sentry/IFeedbackApi { public fun capture (Lio/sentry/protocol/Feedback;)Lio/sentry/protocol/SentryId; public fun capture (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;)Lio/sentry/protocol/SentryId; public fun capture (Lio/sentry/protocol/Feedback;Lio/sentry/Hint;Lio/sentry/ScopeCallback;)Lio/sentry/protocol/SentryId; + public fun disableFeedbackOnShake ()V + public fun enableFeedbackOnShake ()V public static fun getInstance ()Lio/sentry/NoOpFeedbackApi; + public fun isFeedbackOnShakeEnabled ()Z public fun show ()V public fun show (Lio/sentry/SentryFeedbackOptions$OptionsConfigurator;)V public fun show (Lio/sentry/protocol/SentryId;Lio/sentry/SentryFeedbackOptions$OptionsConfigurator;)V @@ -3221,6 +3227,7 @@ public final class io/sentry/SentryFeedbackOptions { public fun getOnFormOpen ()Ljava/lang/Runnable; public fun getOnSubmitError ()Lio/sentry/SentryFeedbackOptions$SentryFeedbackCallback; public fun getOnSubmitSuccess ()Lio/sentry/SentryFeedbackOptions$SentryFeedbackCallback; + public fun getShakeController ()Lio/sentry/SentryFeedbackOptions$IShakeController; public fun getSubmitButtonLabel ()Ljava/lang/CharSequence; public fun getSuccessMessageText ()Ljava/lang/CharSequence; public fun isEmailRequired ()Z @@ -3246,6 +3253,7 @@ public final class io/sentry/SentryFeedbackOptions { public fun setOnFormOpen (Ljava/lang/Runnable;)V public fun setOnSubmitError (Lio/sentry/SentryFeedbackOptions$SentryFeedbackCallback;)V public fun setOnSubmitSuccess (Lio/sentry/SentryFeedbackOptions$SentryFeedbackCallback;)V + public fun setShakeController (Lio/sentry/SentryFeedbackOptions$IShakeController;)V public fun setShowBranding (Z)V public fun setShowEmail (Z)V public fun setShowName (Z)V @@ -3260,6 +3268,13 @@ public abstract interface class io/sentry/SentryFeedbackOptions$IFormHandler { public abstract fun showForm (Lio/sentry/protocol/SentryId;Lio/sentry/SentryFeedbackOptions$OptionsConfigurator;)V } +public abstract interface class io/sentry/SentryFeedbackOptions$IShakeController { + public abstract fun disable ()V + public abstract fun enable ()V + public abstract fun isEnabled ()Z + public abstract fun pauseDetection (Z)V +} + public abstract interface class io/sentry/SentryFeedbackOptions$OptionsConfigurator { public abstract fun configure (Lio/sentry/SentryFeedbackOptions;)V } diff --git a/sentry/src/main/java/io/sentry/FeedbackApi.java b/sentry/src/main/java/io/sentry/FeedbackApi.java index b8b8a3c9b9a..d6b3cc658bd 100644 --- a/sentry/src/main/java/io/sentry/FeedbackApi.java +++ b/sentry/src/main/java/io/sentry/FeedbackApi.java @@ -31,6 +31,21 @@ public void show( options.getFeedbackOptions().getFormHandler().showForm(associatedEventId, configurator); } + @Override + public void enableFeedbackOnShake() { + scopes.getOptions().getFeedbackOptions().getShakeController().enable(); + } + + @Override + public void disableFeedbackOnShake() { + scopes.getOptions().getFeedbackOptions().getShakeController().disable(); + } + + @Override + public boolean isFeedbackOnShakeEnabled() { + return scopes.getOptions().getFeedbackOptions().getShakeController().isEnabled(); + } + @Override public @NotNull SentryId capture(final @NotNull Feedback feedback) { return scopes.captureFeedback(feedback); diff --git a/sentry/src/main/java/io/sentry/IFeedbackApi.java b/sentry/src/main/java/io/sentry/IFeedbackApi.java index 5bab630fa8b..80fd980d3b5 100644 --- a/sentry/src/main/java/io/sentry/IFeedbackApi.java +++ b/sentry/src/main/java/io/sentry/IFeedbackApi.java @@ -2,6 +2,7 @@ import io.sentry.protocol.Feedback; import io.sentry.protocol.SentryId; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -15,6 +16,29 @@ void show( final @Nullable SentryId associatedEventId, final @Nullable SentryFeedbackOptions.OptionsConfigurator configurator); + /** + * Enables showing the feedback form when a shake gesture is detected, overriding {@link + * SentryFeedbackOptions#isUseShakeGesture()}. Only supported on Android; no-op on other + * platforms. + */ + void enableFeedbackOnShake(); + + /** + * Disables showing the feedback form when a shake gesture is detected, overriding {@link + * SentryFeedbackOptions#isUseShakeGesture()}. Only supported on Android; no-op on other + * platforms. + */ + void disableFeedbackOnShake(); + + /** + * Whether showing the feedback form on a shake gesture is currently enabled. Always {@code false} + * on non-Android platforms. + * + * @return true if the feedback form is shown when a shake gesture is detected + */ + @ApiStatus.Internal + boolean isFeedbackOnShakeEnabled(); + @NotNull SentryId capture(final @NotNull Feedback feedback); diff --git a/sentry/src/main/java/io/sentry/NoOpFeedbackApi.java b/sentry/src/main/java/io/sentry/NoOpFeedbackApi.java index bdef5d37590..6c884181a39 100644 --- a/sentry/src/main/java/io/sentry/NoOpFeedbackApi.java +++ b/sentry/src/main/java/io/sentry/NoOpFeedbackApi.java @@ -26,6 +26,17 @@ public void show( final @Nullable SentryId associatedEventId, final @Nullable SentryFeedbackOptions.OptionsConfigurator configurator) {} + @Override + public void enableFeedbackOnShake() {} + + @Override + public void disableFeedbackOnShake() {} + + @Override + public boolean isFeedbackOnShakeEnabled() { + return false; + } + @Override public @NotNull SentryId capture(final @NotNull Feedback feedback) { return SentryId.EMPTY_ID; diff --git a/sentry/src/main/java/io/sentry/SentryFeedbackOptions.java b/sentry/src/main/java/io/sentry/SentryFeedbackOptions.java index a72b352317e..35fa8d0df4b 100644 --- a/sentry/src/main/java/io/sentry/SentryFeedbackOptions.java +++ b/sentry/src/main/java/io/sentry/SentryFeedbackOptions.java @@ -93,8 +93,12 @@ public final class SentryFeedbackOptions { private @NotNull IFormHandler iFormHandler; - SentryFeedbackOptions(@NotNull IFormHandler iFormHandler) { + private @NotNull IShakeController shakeController; + + SentryFeedbackOptions( + final @NotNull IFormHandler iFormHandler, final @NotNull IShakeController shakeController) { this.iFormHandler = iFormHandler; + this.shakeController = shakeController; } /** Creates a copy of the passed {@link SentryFeedbackOptions}. */ @@ -122,6 +126,7 @@ public SentryFeedbackOptions(final @NotNull SentryFeedbackOptions other) { this.onSubmitSuccess = other.onSubmitSuccess; this.onSubmitError = other.onSubmitError; this.iFormHandler = other.iFormHandler; + this.shakeController = other.shakeController; } /** @@ -554,6 +559,26 @@ public void setFormHandler(final @NotNull IFormHandler iFormHandler) { return iFormHandler; } + /** + * Sets the controller to be used to enable/disable shake-to-report at runtime. + * + * @param shakeController the controller to be used to enable/disable shake-to-report at runtime + */ + @ApiStatus.Internal + public void setShakeController(final @NotNull IShakeController shakeController) { + this.shakeController = shakeController; + } + + /** + * Gets the controller to be used to enable/disable shake-to-report at runtime. + * + * @return the controller to be used to enable/disable shake-to-report at runtime + */ + @ApiStatus.Internal + public @NotNull IShakeController getShakeController() { + return shakeController; + } + @Override public String toString() { return "SentryFeedbackOptions{" @@ -615,6 +640,25 @@ void showForm( final @Nullable SentryFeedbackOptions.OptionsConfigurator configurator); } + /** Controls shake-to-report at runtime, overriding {@link #isUseShakeGesture()}. */ + @ApiStatus.Internal + public interface IShakeController { + void enable(); + + void disable(); + + boolean isEnabled(); + + /** + * Pauses or resumes reacting to detected shakes without tearing down shake detection. Feedback + * dialogs pause detection while they are visible, so a shake can never stack a second dialog on + * top of one that is already showing — no matter how the visible dialog was opened. + * + * @param paused true to ignore detected shakes, false to react to them again + */ + void pauseDetection(boolean paused); + } + /** Configuration callback for feedback options. */ public interface OptionsConfigurator { diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index f10f2aede05..9e99473fdb8 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -3496,7 +3496,26 @@ private SentryOptions(final boolean empty) { feedbackOptions = new SentryFeedbackOptions( (associatedEventId, configurator) -> - logger.log(SentryLevel.WARNING, "showForm() can only be called in Android.")); + logger.log(SentryLevel.WARNING, "showForm() can only be called in Android."), + new SentryFeedbackOptions.IShakeController() { + @Override + public void enable() { + logger.log(SentryLevel.WARNING, "Shake to report is only supported on Android."); + } + + @Override + public void disable() { + logger.log(SentryLevel.WARNING, "Shake to report is only supported on Android."); + } + + @Override + public boolean isEnabled() { + return false; + } + + @Override + public void pauseDetection(final boolean paused) {} + }); if (!empty) { setSpanFactory(SpanFactoryFactory.create(new LoadClass(), NoOpLogger.getInstance())); diff --git a/sentry/src/test/java/io/sentry/FeedbackApiTest.kt b/sentry/src/test/java/io/sentry/FeedbackApiTest.kt new file mode 100644 index 00000000000..dd588dfd2ed --- /dev/null +++ b/sentry/src/test/java/io/sentry/FeedbackApiTest.kt @@ -0,0 +1,47 @@ +package io.sentry + +import com.google.common.truth.Truth.assertThat +import kotlin.test.Test +import org.mockito.kotlin.mock +import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever + +class FeedbackApiTest { + + private class Fixture { + val shakeController = mock() + val options = SentryOptions().apply { feedbackOptions.setShakeController(shakeController) } + val scopes = mock().also { whenever(it.options).thenReturn(options) } + + fun getSut(): FeedbackApi = FeedbackApi(scopes) + } + + private val fixture = Fixture() + + @Test + fun `enableFeedbackOnShake delegates to the shake controller`() { + fixture.getSut().enableFeedbackOnShake() + + verify(fixture.shakeController).enable() + } + + @Test + fun `disableFeedbackOnShake delegates to the shake controller`() { + fixture.getSut().disableFeedbackOnShake() + + verify(fixture.shakeController).disable() + } + + @Test + fun `isFeedbackOnShakeEnabled delegates to the shake controller`() { + whenever(fixture.shakeController.isEnabled).thenReturn(true) + + assertThat(fixture.getSut().isFeedbackOnShakeEnabled).isTrue() + verify(fixture.shakeController).isEnabled + } + + @Test + fun `default shake controller is disabled`() { + assertThat(SentryOptions().feedbackOptions.shakeController.isEnabled).isFalse() + } +} diff --git a/sentry/src/test/java/io/sentry/SentryFeedbackOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryFeedbackOptionsTest.kt index e4b96cb17d0..c3e0dbd7303 100644 --- a/sentry/src/test/java/io/sentry/SentryFeedbackOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryFeedbackOptionsTest.kt @@ -1,6 +1,7 @@ package io.sentry import io.sentry.SentryFeedbackOptions.IFormHandler +import io.sentry.SentryFeedbackOptions.IShakeController import kotlin.test.Test import kotlin.test.assertEquals import org.mockito.kotlin.mock @@ -8,7 +9,7 @@ import org.mockito.kotlin.mock class SentryFeedbackOptionsTest { @Test fun `feedback options is initialized with default values`() { - val options = SentryFeedbackOptions(mock()) + val options = SentryFeedbackOptions(mock(), mock()) assertEquals(false, options.isNameRequired) assertEquals(true, options.isShowName) assertEquals(false, options.isEmailRequired) @@ -35,7 +36,7 @@ class SentryFeedbackOptionsTest { @Test fun `feedback options copy constructor`() { val options = - SentryFeedbackOptions(mock()).apply { + SentryFeedbackOptions(mock(), mock()).apply { isNameRequired = true isShowName = false isEmailRequired = true @@ -81,5 +82,6 @@ class SentryFeedbackOptionsTest { assertEquals(options.onSubmitSuccess, optionsCopy.onSubmitSuccess) assertEquals(options.onSubmitError, optionsCopy.onSubmitError) assertEquals(options.formHandler, optionsCopy.formHandler) + assertEquals(options.shakeController, optionsCopy.shakeController) } } diff --git a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt index 9402c6fee9b..0ae479d848a 100644 --- a/sentry/src/test/java/io/sentry/SentryOptionsTest.kt +++ b/sentry/src/test/java/io/sentry/SentryOptionsTest.kt @@ -936,6 +936,18 @@ class SentryOptionsTest { verify(logger).log(eq(SentryLevel.WARNING), eq("showForm() can only be called in Android.")) } + @Test + fun `default shake controller logs a warning`() { + val logger = mock() + val options = + SentryOptions.empty().apply { + setLogger(logger) + isDebug = true + } + options.feedbackOptions.shakeController.enable() + verify(logger).log(eq(SentryLevel.WARNING), eq("Shake to report is only supported on Android.")) + } + @Test fun `autoTransactionDeadlineTimeoutMillis option defaults to 30000`() { val options = SentryOptions.empty()