fix(android): Prevent repeated scroll target logging by updating scroll state #4557
Conversation
…llState.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
markushi
requested review from
adinauer,
lcian,
romtsn and
stefanosiano
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where scroll target logging was being repeated during Android gesture handling. When no scroll target is found in SentryGestureListener.onScroll, the code was logging an error but not updating the scroll state, causing duplicate log messages and unnecessary target searches on subsequent scroll events within the same gesture.
- Updates scroll state type to prevent repeated target determination when no scroll target is found
- Adds unit test to verify the fix ensures error logging occurs only once per gesture
- Includes changelog entry documenting the fix
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| SentryGestureListener.java | Sets scroll state type to prevent repeated target searches when no scroll target is found |
| SentryGestureListenerScrollTest.kt | Adds test verifying error message is logged only once per gesture when no scroll target exists |
| CHANGELOG.md | Documents the fix for repeated scroll target determination |
| options | ||
| .getLogger() | ||
| .log(SentryLevel.DEBUG, "Unable to find scroll target. No breadcrumb captured."); | ||
| scrollState.type = GestureType.Scroll; |
There was a problem hiding this comment.
[nitpick] Setting scrollState.type to GestureType.Scroll when no scroll target is found may be misleading. Consider using a more descriptive state like GestureType.NoTarget or adding a comment explaining why Scroll is appropriate when no target exists.
Performance metrics 🚀
|
romtsn
left a comment
romtsn
left a comment
There was a problem hiding this comment.
LGTM, very simple change with a lot of impact 👀
Co-authored-by: Roman Zavarnitsyn <rom4ek93@gmail.com>
📜 Description
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.
💡 Motivation and Context
Less verbose logging, better performance.
💚 How did you test it?
Added unit tests.
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps