feat(core): Add rage tap detection with ui.frustration breadcrumbs#5992
feat(core): Add rage tap detection with ui.frustration breadcrumbs#5992
Conversation
Detect rapid consecutive taps on the same UI element and surface them as frustration signals across the SDK: - New RageTapDetector class tracks recent taps in a circular buffer and matches them by component identity (label or name+file). When N taps on the same target occur within a configurable time window, a ui.frustration breadcrumb is emitted automatically. - TouchEventBoundary gains three new props: enableRageTapDetection (default: true), rageTapThreshold (default: 3), and rageTapTimeWindow (default: 1000ms). - Native replay breadcrumb converters on both Android (Java) and iOS (Objective-C) now handle the ui.frustration category, converting it to an RRWeb breadcrumb event so rage taps appear on the session replay timeline with the same touch-path message format as regular ui.tap events. - 7 new JS tests cover detection, threshold configuration, time window expiry, buffer reset, disabled mode, and component-name fallback. Android and iOS converter tests verify the new category is handled correctly.
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
|
alwx
changed the title
alwx
force-pushed
the
feat/rage-tap-detection
branch
from
4cfa1af to
7d06010
Compare
- New ragetap.test.ts with 10 unit tests for RageTapDetector: threshold detection, different targets, time window expiry, buffer reset, disabled mode, custom threshold/timeWindow, component name+file identity, empty path, and consecutive rage tap triggers. - 3 integration tests in touchevents.test.tsx verifying TouchEventBoundary wires the detector correctly: end-to-end detection, disabled prop, and custom threshold/timeWindow props. - Android converter test (Kotlin) and iOS converter test (Swift) for the ui.frustration breadcrumb category in RNSentryReplayBreadcrumbConverter.
alwx
changed the title
|
@cursor review |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
- Fix false-positive detection: reset tap buffer when target changes instead of relying on time-window pruning, which could make non-consecutive taps appear consecutive after interleaved taps aged out (Medium severity, reported by Sentry bugbot). - Add null check for breadcrumb data in Android convertFrustrationBreadcrumb, matching the iOS implementation that already guards against nil data (Low severity). - Remove hardcoded MAX_RECENT_TAPS buffer limit that would silently break detection for thresholds > 10. The buffer is now naturally bounded by target-change resets and time-window pruning. - Deduplicate TouchedComponentInfo: export from ragetap.ts and import in touchevents.tsx instead of maintaining identical interfaces in both files. - Read rage tap props at event time via updateOptions() instead of freezing them in the constructor, consistent with how all other TouchEventBoundary props are consumed.
Rename breadcrumb category from ui.frustration to ui.multiClick and reshape the data payload to match the web JS SDK's rage click format, so the Sentry replay timeline renders rage taps with the fire icon and 'Rage Click' label automatically. Changes to the breadcrumb shape: - category: ui.frustration → ui.multiClick - type: user → default - data.tapCount → data.clickCount - data.type (rage_tap) removed - data.metric: true added (marks as metric event) - data.route added (current screen from navigation tracing) - data.node added with DOM-compatible shape: tagName, textContent, attributes (data-sentry-component, data-sentry-source-file, sentry-label) — this allows the existing stringifyNodeAttributes in the Sentry frontend to render component names for mobile taps. Native replay converters updated on both Android and iOS to handle ui.multiClick instead of ui.frustration.
…sitives When distinct child elements share a labeled ancestor, the tap identity was based solely on the parent label, causing false rage tap detection when tapping different controls in quick succession. Now the identity always includes the root component name and file, even when a label is present (e.g. label:form|name:SubmitButton|file:form.tsx).
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
- iOS: Add NSArray type check on path data in convertMultiClick to prevent runtime crash from unrecognized selector on non-array values (HIGH, Sentry bot). - Clear tap buffer when detection is disabled via updateOptions to prevent stale taps from causing false positives on re-enable (LOW, Sentry bot). - Move changelog entry from released 8.8.0 section to Unreleased (danger bot). - Add time window integration test to touchevents that varies timestamps between taps, verifying rageTapTimeWindow actually excludes old taps (sentry-warden).
| import { getCurrentReactNativeTracingIntegration } from './tracing/reactnativetracing'; | ||
|
|
||
| const DEFAULT_RAGE_TAP_THRESHOLD = 3; | ||
| const DEFAULT_RAGE_TAP_TIME_WINDOW = 1000; |
There was a problem hiding this comment.
i'm curious how this works for the case of app hanging (i.e. dead clicks) -- I'm guessing we won't be able to detect the following touches after the first one that triggered a hang -- the main thread would be occupied/congested, right?
There was a problem hiding this comment.
@romtsn that's the correct assumption — rage taps (rapid taps that register) and dead taps (taps during a hang) are different signals. Dead tap detection would need native-side work.
There was a problem hiding this comment.
@antonis the web SDK's 7s is more like "how long to wait before emitting the breadcrumb", not "how close clicks need to be". So the direct comparison of numbers doesn't seem applicable — in our case it's like a "rolling" time window.
There was a problem hiding this comment.
Sounds good 👍 Let's add a comment explaining the reasoning behind the 1s window if possible 🙇
| } | ||
|
|
||
| return { | ||
| id: 0, |
There was a problem hiding this comment.
Web SDK uses actual rrweb node IDs for DOM element mapping. On mobile we don't have rrweb node IDs so 0 is no more than just a placeholder (and ReplayBaseDomFrameData type requires id: number)
There was a problem hiding this comment.
Sounds good 👍 We should double check that this get's through on the backend
|
No additional comments from my side than the one pointed by the other reviewers. |
- Drop level: 'warning' from ui.multiClick breadcrumb to match the web JS SDK which defaults to info. - Export DEFAULT_RAGE_TAP_THRESHOLD and DEFAULT_RAGE_TAP_TIME_WINDOW from ragetap.ts and import them in touchevents.tsx defaultProps for a single source of truth. - Initialize RageTapDetector with props in the constructor and sync via componentDidUpdate, instead of calling updateOptions on every tap event. - Remove incorrect @testonly annotation from Android convertMultiClickBreadcrumb since it is called from convert() in production code. - Add comment explaining id: 0 placeholder in the node object (mobile replays don't have rrweb node IDs). - Add tests for updateOptions: buffer cleared on disable, and threshold change applies immediately. - Run yarn fix for import ordering lint.
| } | ||
|
|
||
| public @Nullable RRWebEvent convertMultiClickBreadcrumb(final @NotNull Breadcrumb breadcrumb) { | ||
| if (breadcrumb.getData("path") == null) { |
There was a problem hiding this comment.
l: Should we also check the type similar to iOS?
antonis
left a comment
antonis
left a comment
There was a problem hiding this comment.
LGTM!
Iterated on the comments and added one more q but nothing blocking on my side.
Align with the iOS converter which validates the path type before use. Prevents potential ClassCastException if a non-list value is passed.
| const identity = getTapIdentity(root, label); | ||
| const now = Date.now(); | ||
| const tapCount = this._detect(identity, now); | ||
|
|
There was a problem hiding this comment.
Inconsistent breadcrumb category: code emits 'ui.multiClick' but PR description and type say 'ui.frustration'
The PR title and description state that this feature emits ui.frustration breadcrumbs, but the actual implementation emits ui.multiClick. The JSDoc on the class also references ui.multiClick to match the web SDK. This is a backwards-compatibility/API contract concern: downstream consumers (Sentry frontend, alerting rules, docs) need a single, agreed-upon category. The discrepancy will cause user-visible confusion about what category to filter on.
Verification
Compared the PR title/description ('ui.frustration breadcrumbs') against the literal category strings in ragetap.ts at lines 30 (JSDoc), 60 (JSDoc), and 81 (addBreadcrumb call). All three use 'ui.multiClick'. No other category is emitted in this file.
Identified by Warden code-review · PHV-YSD
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit dd2b845. Configure here.
| node, | ||
| path: touchPath, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Rage tap breadcrumb missing level: 'warning'
Medium Severity
The addBreadcrumb call in ragetap.ts omits the level property. Regular touch breadcrumbs in _logTouchEvent explicitly set level: 'info', and both Android and iOS native converter tests set up multi-click breadcrumbs with level = SentryLevel.WARNING / .warning and assert this level is preserved. Without setting level: 'warning' on the JS breadcrumb, the replay breadcrumb event will carry the wrong severity, potentially affecting how the Sentry UI displays this frustration signal.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit dd2b845. Configure here.
4 participants
📢 Type of change
📜 Description
Detects rage taps (rapid consecutive taps on the same UI element) and surfaces them as first-class frustration signals across the SDK.
Design decisions
TouchEventBoundary.ui.frustrationbreadcrumbs.📝 Checklist
sendDefaultPIIis enabled