From bfe02baa7ac9d32bf509aba1e5f8ac0ef0062cce Mon Sep 17 00:00:00 2001 From: markushi Date: Wed, 16 Jul 2025 11:20:49 +0200 Subject: [PATCH 1/4] fix(android): Prevent repeated scroll target logging by updating scrollState.type When ViewUtils.findTarget returns null in SentryGestureListener.onScroll, the code was logging an error but not updating scrollState.type from Unknown. This caused repeated target searches and duplicate log messages on subsequent onScroll calls during the same gesture. The fix sets scrollState.type = GestureType.Scroll even when target is null, preventing repeated search attempts while maintaining existing behavior. Fixes: "Unable to find scroll target. No breadcrumb captured." being logged repeatedly --- .../gestures/SentryGestureListener.java | 1 + .../SentryGestureListenerScrollTest.kt | 47 +++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java index f0183b21b56..7ffc5d2412f 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/internal/gestures/SentryGestureListener.java @@ -139,6 +139,7 @@ public boolean onScroll( options .getLogger() .log(SentryLevel.DEBUG, "Unable to find scroll target. No breadcrumb captured."); + scrollState.type = GestureType.Scroll; return false; } else { options diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt index fcb07735760..9aeedc3ea6d 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt @@ -11,11 +11,13 @@ import android.widget.AbsListView import android.widget.ListAdapter import androidx.core.view.ScrollingView import io.sentry.Breadcrumb +import io.sentry.ILogger import io.sentry.IScope import io.sentry.IScopes import io.sentry.PropagationContext import io.sentry.Scope import io.sentry.ScopeCallback +import io.sentry.SentryLevel import io.sentry.SentryLevel.INFO import io.sentry.android.core.SentryAndroidOptions import kotlin.test.Test @@ -28,6 +30,7 @@ import org.mockito.kotlin.doAnswer import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock import org.mockito.kotlin.never +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.verifyNoMoreInteractions import org.mockito.kotlin.whenever @@ -229,6 +232,50 @@ class SentryGestureListenerScrollTest { verify(fixture.scope).propagationContext = any() } + @Test + fun `logs error message only once per gesture when no scroll target is found`() { + val mockLogger = mock() + fixture.options.setLogger(mockLogger) + + // Create a setup where no scrollable view is found + // Use regular View which doesn't implement ScrollingView/AbsListView/ScrollView + fixture.target = + mockView( + event = fixture.firstEvent, + touchWithinBounds = true, + context = fixture.context, + ) + fixture.window.mockDecorView(event = fixture.firstEvent) { + whenever(it.childCount).thenReturn(1) + whenever(it.getChildAt(0)).thenReturn(fixture.target) + } + + fixture.resources.mockForTarget(fixture.target, "test_view") + whenever(fixture.context.resources).thenReturn(fixture.resources) + whenever(fixture.target.context).thenReturn(fixture.context) + whenever(fixture.activity.window).thenReturn(fixture.window) + doAnswer { (it.arguments[0] as ScopeCallback).run(fixture.scope) } + .whenever(fixture.scopes) + .configureScope(any()) + doAnswer { + (it.arguments[0] as Scope.IWithPropagationContext).accept(fixture.propagationContext) + fixture.propagationContext + } + .whenever(fixture.scope) + .withPropagationContext(any()) + + val sut = SentryGestureListener(fixture.activity, fixture.scopes, fixture.options) + + sut.onDown(fixture.firstEvent) + // Multiple onScroll calls during the same gesture - should only log once + fixture.eventsInBetween.forEach { sut.onScroll(fixture.firstEvent, it, 10f, 0f) } + sut.onUp(fixture.endEvent) + + // Verify that the error message is logged only once during the entire gesture + verify(mockLogger, times(1)) + .log(SentryLevel.DEBUG, "Unable to find scroll target. No breadcrumb captured.") + } + internal class ScrollableView : View(mock()), ScrollingView { override fun computeVerticalScrollOffset(): Int = 0 From fe809d38405144034e61ac76122eaa8e6d4a3ea7 Mon Sep 17 00:00:00 2001 From: markushi Date: Wed, 16 Jul 2025 11:25:51 +0200 Subject: [PATCH 2/4] Update Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a1bb8a6656f..6d2e68f5d20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Allow multiple UncaughtExceptionHandlerIntegrations to be active at the same time ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) +- Prevent repeated scroll target determination during a single scroll gesture ([#4557](https://github.com/getsentry/sentry-java/pull/4557)) ## 8.17.0 From db94d96ab830ef462020fd38b83c4f44ba4d2e69 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 17 Jul 2025 08:41:58 +0200 Subject: [PATCH 3/4] Update CHANGELOG.md Co-authored-by: Roman Zavarnitsyn --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d2e68f5d20..b3a7262f49e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Allow multiple UncaughtExceptionHandlerIntegrations to be active at the same time ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) - Prevent repeated scroll target determination during a single scroll gesture ([#4557](https://github.com/getsentry/sentry-java/pull/4557)) + - This should reduce the number of ANRs seen in `SentryGestureListener` ## 8.17.0 From 2fb367cff133b0507ea3516333cd86b1ef919135 Mon Sep 17 00:00:00 2001 From: markushi Date: Thu, 17 Jul 2025 10:02:47 +0200 Subject: [PATCH 4/4] Fix tests --- .../SentryGestureListenerScrollTest.kt | 42 ++++--------------- .../core/internal/gestures/ViewHelpers.kt | 9 +++- 2 files changed, 14 insertions(+), 37 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt index 9aeedc3ea6d..3dd1f726d7b 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/SentryGestureListenerScrollTest.kt @@ -59,7 +59,7 @@ class SentryGestureListenerScrollTest { val directions = setOf("up", "down", "left", "right") internal inline fun getSut( - resourceName: String = "test_scroll_view", + resourceName: String? = "test_scroll_view", touchWithinBounds: Boolean = true, direction: String = "", ): SentryGestureListener { @@ -234,45 +234,17 @@ class SentryGestureListenerScrollTest { @Test fun `logs error message only once per gesture when no scroll target is found`() { - val mockLogger = mock() - fixture.options.setLogger(mockLogger) - - // Create a setup where no scrollable view is found - // Use regular View which doesn't implement ScrollingView/AbsListView/ScrollView - fixture.target = - mockView( - event = fixture.firstEvent, - touchWithinBounds = true, - context = fixture.context, - ) - fixture.window.mockDecorView(event = fixture.firstEvent) { - whenever(it.childCount).thenReturn(1) - whenever(it.getChildAt(0)).thenReturn(fixture.target) - } - - fixture.resources.mockForTarget(fixture.target, "test_view") - whenever(fixture.context.resources).thenReturn(fixture.resources) - whenever(fixture.target.context).thenReturn(fixture.context) - whenever(fixture.activity.window).thenReturn(fixture.window) - doAnswer { (it.arguments[0] as ScopeCallback).run(fixture.scope) } - .whenever(fixture.scopes) - .configureScope(any()) - doAnswer { - (it.arguments[0] as Scope.IWithPropagationContext).accept(fixture.propagationContext) - fixture.propagationContext - } - .whenever(fixture.scope) - .withPropagationContext(any()) - - val sut = SentryGestureListener(fixture.activity, fixture.scopes, fixture.options) + val logger = mock() + fixture.options.setLogger(logger) + fixture.options.isDebug = true + val sut = fixture.getSut(resourceName = null) sut.onDown(fixture.firstEvent) - // Multiple onScroll calls during the same gesture - should only log once - fixture.eventsInBetween.forEach { sut.onScroll(fixture.firstEvent, it, 10f, 0f) } + fixture.eventsInBetween.forEach { sut.onScroll(fixture.firstEvent, it, 10.0f, 0f) } sut.onUp(fixture.endEvent) // Verify that the error message is logged only once during the entire gesture - verify(mockLogger, times(1)) + verify(logger, times(1)) .log(SentryLevel.DEBUG, "Unable to find scroll target. No breadcrumb captured.") } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/ViewHelpers.kt b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/ViewHelpers.kt index 339357bea39..86123d0a3a2 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/ViewHelpers.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/internal/gestures/ViewHelpers.kt @@ -67,6 +67,11 @@ internal inline fun mockView( return mockView } -internal fun Resources.mockForTarget(target: View, expectedResourceName: String) { - whenever(getResourceEntryName(target.id)).thenReturn(expectedResourceName) +internal fun Resources.mockForTarget(target: View, expectedResourceName: String?) { + if (expectedResourceName == null) { + whenever(getResourceEntryName(target.id)) + .thenThrow(Resources.NotFoundException("res not found")) + } else { + whenever(getResourceEntryName(target.id)).thenReturn(expectedResourceName) + } }