attach-screenshot set on Manual init. didn't work (#2186)

getsentry · Aug 1, 2022 · f75a0a7 · f75a0a7
1 parent ea821b6
commit f75a0a7
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,7 @@
 
 ### Fixes
 
+- `attach-screenshot` set on Manual init. didn't work ([#2186](https://github.com/getsentry/sentry-java/pull/2186))
 - Remove extra space from `spring.factories` causing issues in old versions of Spring Boot ([#2181](https://github.com/getsentry/sentry-java/pull/2181))
 
 ### Features

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
@@ -36,7 +36,8 @@ public final class ScreenshotEventProcessor
   private final @NotNull Application application;
   private final @NotNull SentryAndroidOptions options;
   private @Nullable WeakReference<Activity> currentActivity;
-  final @NotNull BuildInfoProvider buildInfoProvider;
+  private final @NotNull BuildInfoProvider buildInfoProvider;
+  private boolean lifecycleCallbackInstalled = true;
 
   public ScreenshotEventProcessor(
       final @NotNull Application application,
@@ -47,27 +48,29 @@ public ScreenshotEventProcessor(
     this.buildInfoProvider =
         Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required");
 
-    if (this.options.isAttachScreenshot()) {
-      application.registerActivityLifecycleCallbacks(this);
+    application.registerActivityLifecycleCallbacks(this);
+  }
+
+  @SuppressWarnings("NullAway")
+  @Override
+  public @NotNull SentryEvent process(final @NotNull SentryEvent event, @NotNull Hint hint) {
+    if (!lifecycleCallbackInstalled) {
+      return event;
+    }
+    if (!options.isAttachScreenshot()) {
+      application.unregisterActivityLifecycleCallbacks(this);
+      lifecycleCallbackInstalled = false;
 
-      this.options
-          .getLogger()
-          .log(
-              SentryLevel.DEBUG,
-              "attachScreenshot is enabled, ScreenshotEventProcessor is installed.");
-    } else {
       this.options
           .getLogger()
           .log(
               SentryLevel.DEBUG,
               "attachScreenshot is disabled, ScreenshotEventProcessor isn't installed.");
+
+      return event;
     }
-  }
 
-  @SuppressWarnings("NullAway")
-  @Override
-  public @NotNull SentryEvent process(final @NotNull SentryEvent event, @NotNull Hint hint) {
-    if (options.isAttachScreenshot() && event.isErrored() && currentActivity != null) {
+    if (event.isErrored() && currentActivity != null) {
       final Activity activity = currentActivity.get();
       if (isActivityValid(activity)
           && activity.getWindow() != null

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
@@ -61,19 +61,12 @@ class ScreenshotEventProcessorTest {
     }
 
     @Test
-    fun `when attach screenshot is enabled, registerActivityLifecycleCallbacks`() {
-        fixture.getSut(true)
+    fun `when adding screenshot event processor, registerActivityLifecycleCallbacks`() {
+        fixture.getSut()
 
         verify(fixture.application).registerActivityLifecycleCallbacks(any())
     }
 
-    @Test
-    fun `when attach screenshot is disabled, does not registerActivityLifecycleCallbacks`() {
-        fixture.getSut(false)
-
-        verify(fixture.application, never()).registerActivityLifecycleCallbacks(any())
-    }
-
     @Test
     fun `when close is called and attach screenshot is enabled, unregisterActivityLifecycleCallbacks`() {
         val sut = fixture.getSut(true)
@@ -85,16 +78,27 @@ class ScreenshotEventProcessorTest {
 
     @Test
     fun `when close is called and  attach screenshot is disabled, does not unregisterActivityLifecycleCallbacks`() {
-        val sut = fixture.getSut(false)
+        val sut = fixture.getSut()
 
         sut.close()
 
         verify(fixture.application, never()).unregisterActivityLifecycleCallbacks(any())
     }
 
+    @Test
+    fun `when process is called and attachScreenshot is disabled, unregisterActivityLifecycleCallbacks`() {
+        val sut = fixture.getSut()
+        val hint = Hint()
+
+        val event = fixture.mainProcessor.process(getEvent(), hint)
+        sut.process(event, hint)
+
+        verify(fixture.application).unregisterActivityLifecycleCallbacks(any())
+    }
+
     @Test
     fun `when process is called and attachScreenshot is disabled, does nothing`() {
-        val sut = fixture.getSut(false)
+        val sut = fixture.getSut()
         val hint = Hint()
 
         sut.onActivityCreated(fixture.activity, null)