From 5ac2fce3a568e03949402d9593813d7d4cb89e8a Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 20 Aug 2024 14:14:05 +0200 Subject: [PATCH 01/14] chore: cleanup user interaction widget code --- .../sentry_user_interaction_widget.dart | 50 +++++++++++-------- 1 file changed, 29 insertions(+), 21 deletions(-) diff --git a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart index 018f750a2e..c162526ca4 100644 --- a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart +++ b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart @@ -317,45 +317,53 @@ class _SentryUserInteractionWidgetState void _onTappedAt(Offset position) { final tappedWidget = _getElementAt(position); - final keyValue = - WidgetUtils.toStringValue(tappedWidget?.element.widget.key); - if (tappedWidget == null || keyValue == null) { + if (tappedWidget == null) { return; } - final element = tappedWidget.element; - Map? data; - // ignore: invalid_use_of_internal_member - if ((_options?.sendDefaultPii ?? false) && - tappedWidget.description.isNotEmpty) { - data = {}; - data['label'] = tappedWidget.description; - } + final widgetKey = + WidgetUtils.toStringValue(tappedWidget.element.widget.key); + + _createBreadcrumbOnTap(tappedWidget, widgetKey); + _startTransactionOnTap(tappedWidget, widgetKey); + } - const category = 'click'; + void _createBreadcrumbOnTap(UserInteractionWidget widget, String? widgetKey) { // ignore: invalid_use_of_internal_member if (_options?.enableUserInteractionBreadcrumbs ?? false) { + Map? data; + // ignore: invalid_use_of_internal_member + if ((_options?.sendDefaultPii ?? false) && + widget.description.isNotEmpty) { + data = {}; + data['label'] = widget.description; + } + final crumb = Breadcrumb.userInteraction( - subCategory: category, - viewId: keyValue, - viewClass: tappedWidget.type, // to avoid minification + subCategory: 'click', + viewId: widgetKey, + viewClass: widget.type, // to avoid minification data: data, ); - final hint = Hint.withMap({TypeCheckHint.widget: element.widget}); + final hint = Hint.withMap({TypeCheckHint.widget: widget.element.widget}); _hub.addBreadcrumb(crumb, hint: hint); } + } + void _startTransactionOnTap(UserInteractionWidget widget, String? widgetKey) { // ignore: invalid_use_of_internal_member - if (!(_options?.isTracingEnabled() ?? false) || + if (widgetKey == null || + !(_options?.isTracingEnabled() ?? false) || !(_options?.enableUserInteractionTracing ?? false)) { return; } + final element = widget.element; // getting the name of the screen using ModalRoute.of(context).settings.name // is expensive, so we expect that the keys are unique across the app final transactionContext = SentryTransactionContext( - keyValue, - 'ui.action.$category', + widgetKey, + 'ui.action.click', transactionNameSource: SentryTransactionNameSource.component, ); @@ -365,7 +373,7 @@ class _SentryUserInteractionWidgetState if (_isElementMounted(lastElement) && _isElementMounted(element) && lastElement?.widget == element.widget && - _lastTappedWidget?.eventType == tappedWidget.eventType && + _lastTappedWidget?.eventType == widget.eventType && !activeTransaction.finished) { // ignore: invalid_use_of_internal_member activeTransaction.scheduleFinish(); @@ -382,7 +390,7 @@ class _SentryUserInteractionWidgetState } } - _lastTappedWidget = tappedWidget; + _lastTappedWidget = widget; bool hasRunningTransaction = false; _hub.configureScope((scope) { From 1445ba4e1576b5a95d163fc3c77e75de26ace069 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 20 Aug 2024 14:18:39 +0200 Subject: [PATCH 02/14] renames & more cleanup --- .../sentry_user_interaction_widget.dart | 55 +++++++++---------- ...widget.dart => user_interaction_info.dart} | 6 +- 2 files changed, 30 insertions(+), 31 deletions(-) rename flutter/lib/src/user_interaction/{user_interaction_widget.dart => user_interaction_info.dart} (72%) diff --git a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart index c162526ca4..909ddef236 100644 --- a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart +++ b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart @@ -208,7 +208,7 @@ import 'package:meta/meta.dart'; import '../../sentry_flutter.dart'; import '../widget_utils.dart'; -import 'user_interaction_widget.dart'; +import 'user_interaction_info.dart'; const _tapDeltaArea = 20 * 20; Element? _clickTrackerElement; @@ -272,7 +272,7 @@ class _SentryUserInteractionWidgetState extends State { int? _lastPointerId; Offset? _lastPointerDownLocation; - UserInteractionWidget? _lastTappedWidget; + UserInteractionInfo? _lastTappedWidget; ISentrySpan? _activeTransaction; Hub get _hub => widget._hub; @@ -316,41 +316,38 @@ class _SentryUserInteractionWidgetState } void _onTappedAt(Offset position) { - final tappedWidget = _getElementAt(position); - if (tappedWidget == null) { + final tapInfo = _getElementAt(position); + if (tapInfo == null) { return; } - final widgetKey = - WidgetUtils.toStringValue(tappedWidget.element.widget.key); - - _createBreadcrumbOnTap(tappedWidget, widgetKey); - _startTransactionOnTap(tappedWidget, widgetKey); + final widgetKey = WidgetUtils.toStringValue(tapInfo.element.widget.key); + _createBreadcrumbOnTap(tapInfo, widgetKey); + _startTransactionOnTap(tapInfo, widgetKey); } - void _createBreadcrumbOnTap(UserInteractionWidget widget, String? widgetKey) { + void _createBreadcrumbOnTap(UserInteractionInfo info, String? widgetKey) { // ignore: invalid_use_of_internal_member if (_options?.enableUserInteractionBreadcrumbs ?? false) { Map? data; // ignore: invalid_use_of_internal_member - if ((_options?.sendDefaultPii ?? false) && - widget.description.isNotEmpty) { + if ((_options?.sendDefaultPii ?? false) && info.description.isNotEmpty) { data = {}; - data['label'] = widget.description; + data['label'] = info.description; } final crumb = Breadcrumb.userInteraction( subCategory: 'click', viewId: widgetKey, - viewClass: widget.type, // to avoid minification + viewClass: info.type, // to avoid minification data: data, ); - final hint = Hint.withMap({TypeCheckHint.widget: widget.element.widget}); + final hint = Hint.withMap({TypeCheckHint.widget: info.element.widget}); _hub.addBreadcrumb(crumb, hint: hint); } } - void _startTransactionOnTap(UserInteractionWidget widget, String? widgetKey) { + void _startTransactionOnTap(UserInteractionInfo info, String? widgetKey) { // ignore: invalid_use_of_internal_member if (widgetKey == null || !(_options?.isTracingEnabled() ?? false) || @@ -358,7 +355,7 @@ class _SentryUserInteractionWidgetState return; } - final element = widget.element; + final element = info.element; // getting the name of the screen using ModalRoute.of(context).settings.name // is expensive, so we expect that the keys are unique across the app final transactionContext = SentryTransactionContext( @@ -373,7 +370,7 @@ class _SentryUserInteractionWidgetState if (_isElementMounted(lastElement) && _isElementMounted(element) && lastElement?.widget == element.widget && - _lastTappedWidget?.eventType == widget.eventType && + _lastTappedWidget?.eventType == info.eventType && !activeTransaction.finished) { // ignore: invalid_use_of_internal_member activeTransaction.scheduleFinish(); @@ -390,7 +387,7 @@ class _SentryUserInteractionWidgetState } } - _lastTappedWidget = widget; + _lastTappedWidget = info; bool hasRunningTransaction = false; _hub.configureScope((scope) { @@ -459,7 +456,7 @@ class _SentryUserInteractionWidgetState return description; } - UserInteractionWidget? _getElementAt(Offset position) { + UserInteractionInfo? _getElementAt(Offset position) { // WidgetsBinding.instance.renderViewElement does not work, so using // the element from createElement final rootElement = _clickTrackerElement; @@ -467,7 +464,7 @@ class _SentryUserInteractionWidgetState return null; } - UserInteractionWidget? tappedWidget; + UserInteractionInfo? tappedWidget; void elementFinder(Element element) { if (tappedWidget != null) { @@ -507,12 +504,12 @@ class _SentryUserInteractionWidgetState return tappedWidget; } - UserInteractionWidget? _getDescriptionFrom(Element element) { + UserInteractionInfo? _getDescriptionFrom(Element element) { final widget = element.widget; // Used by ElevatedButton, TextButton, OutlinedButton. if (widget is ButtonStyleButton) { if (widget.enabled) { - return UserInteractionWidget( + return UserInteractionInfo( element: element, description: _findDescriptionOf(element, true), type: 'ButtonStyleButton', @@ -521,7 +518,7 @@ class _SentryUserInteractionWidgetState } } else if (widget is MaterialButton) { if (widget.enabled) { - return UserInteractionWidget( + return UserInteractionInfo( element: element, description: _findDescriptionOf(element, true), type: 'MaterialButton', @@ -530,7 +527,7 @@ class _SentryUserInteractionWidgetState } } else if (widget is CupertinoButton) { if (widget.enabled) { - return UserInteractionWidget( + return UserInteractionInfo( element: element, description: _findDescriptionOf(element, true), type: 'CupertinoButton', @@ -539,7 +536,7 @@ class _SentryUserInteractionWidgetState } } else if (widget is InkWell) { if (widget.onTap != null) { - return UserInteractionWidget( + return UserInteractionInfo( element: element, description: _findDescriptionOf(element, false), type: 'InkWell', @@ -548,7 +545,7 @@ class _SentryUserInteractionWidgetState } } else if (widget is IconButton) { if (widget.onPressed != null) { - return UserInteractionWidget( + return UserInteractionInfo( element: element, description: _findDescriptionOf(element, false), type: 'IconButton', @@ -557,7 +554,7 @@ class _SentryUserInteractionWidgetState } } else if (widget is PopupMenuButton) { if (widget.enabled) { - return UserInteractionWidget( + return UserInteractionInfo( element: element, description: _findDescriptionOf(element, false), type: 'PopupMenuButton', @@ -566,7 +563,7 @@ class _SentryUserInteractionWidgetState } } else if (widget is PopupMenuItem) { if (widget.enabled) { - return UserInteractionWidget( + return UserInteractionInfo( element: element, description: _findDescriptionOf(element, false), type: 'PopupMenuItem', diff --git a/flutter/lib/src/user_interaction/user_interaction_widget.dart b/flutter/lib/src/user_interaction/user_interaction_info.dart similarity index 72% rename from flutter/lib/src/user_interaction/user_interaction_widget.dart rename to flutter/lib/src/user_interaction/user_interaction_info.dart index 58d7d18e79..f7848f260f 100644 --- a/flutter/lib/src/user_interaction/user_interaction_widget.dart +++ b/flutter/lib/src/user_interaction/user_interaction_info.dart @@ -1,12 +1,14 @@ import 'package:flutter/widgets.dart'; +import 'package:meta/meta.dart'; -class UserInteractionWidget { +@internal +class UserInteractionInfo { final Element element; final String description; final String type; final String eventType; - const UserInteractionWidget({ + const UserInteractionInfo({ required this.element, required this.description, required this.type, From 8deed2efdfe08a598c44f5deca560097e3bfd53c Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 20 Aug 2024 14:54:42 +0200 Subject: [PATCH 03/14] more cleanup --- .../sentry_user_interaction_widget.dart | 37 +++++++++---------- 1 file changed, 17 insertions(+), 20 deletions(-) diff --git a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart index 909ddef236..26b9e717e1 100644 --- a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart +++ b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart @@ -327,28 +327,27 @@ class _SentryUserInteractionWidgetState } void _createBreadcrumbOnTap(UserInteractionInfo info, String? widgetKey) { - // ignore: invalid_use_of_internal_member - if (_options?.enableUserInteractionBreadcrumbs ?? false) { - Map? data; - // ignore: invalid_use_of_internal_member - if ((_options?.sendDefaultPii ?? false) && info.description.isNotEmpty) { - data = {}; - data['label'] = info.description; - } + if (!(_options?.enableUserInteractionBreadcrumbs ?? false)) { + return; + } - final crumb = Breadcrumb.userInteraction( - subCategory: 'click', - viewId: widgetKey, - viewClass: info.type, // to avoid minification - data: data, - ); - final hint = Hint.withMap({TypeCheckHint.widget: info.element.widget}); - _hub.addBreadcrumb(crumb, hint: hint); + Map? data; + if ((_options?.sendDefaultPii ?? false) && info.description.isNotEmpty) { + data = {}; + data['label'] = info.description; } + + final crumb = Breadcrumb.userInteraction( + subCategory: 'click', + viewId: widgetKey, + viewClass: info.type, // to avoid minification + data: data, + ); + final hint = Hint.withMap({TypeCheckHint.widget: info.element.widget}); + _hub.addBreadcrumb(crumb, hint: hint); } void _startTransactionOnTap(UserInteractionInfo info, String? widgetKey) { - // ignore: invalid_use_of_internal_member if (widgetKey == null || !(_options?.isTracingEnabled() ?? false) || !(_options?.enableUserInteractionTracing ?? false)) { @@ -404,9 +403,7 @@ class _SentryUserInteractionWidgetState _activeTransaction = _hub.startTransactionWithContext( transactionContext, waitForChildren: true, - autoFinishAfter: - // ignore: invalid_use_of_internal_member - _options?.idleTimeout, + autoFinishAfter: _options?.idleTimeout, trimEnd: true, ); From 65106242bc8c274b1ea8784f458d0f6eff70a799 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 20 Aug 2024 15:55:08 +0200 Subject: [PATCH 04/14] more refactoring & clenaup before actual functional changes --- .../sentry_user_interaction_widget.dart | 67 ++++++------------- .../user_interaction_info.dart | 2 - 2 files changed, 21 insertions(+), 48 deletions(-) diff --git a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart index 26b9e717e1..ebe95aa6d7 100644 --- a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart +++ b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart @@ -369,7 +369,6 @@ class _SentryUserInteractionWidgetState if (_isElementMounted(lastElement) && _isElementMounted(element) && lastElement?.widget == element.widget && - _lastTappedWidget?.eventType == info.eventType && !activeTransaction.finished) { // ignore: invalid_use_of_internal_member activeTransaction.scheduleFinish(); @@ -417,7 +416,11 @@ class _SentryUserInteractionWidgetState }); } - String _findDescriptionOf(Element element, bool allowText) { + String _findDescriptionOf(Element element) { + final widget = element.widget; + final allowText = widget is ButtonStyleButton || + widget is MaterialButton || + widget is CupertinoButton; var description = ''; // traverse tree to find a suiting element @@ -489,7 +492,14 @@ class _SentryUserInteractionWidgetState return; } - tappedWidget = _getDescriptionFrom(element); + final type = _getElementType(element); + if (type != null) { + tappedWidget = UserInteractionInfo( + element: element, + description: _findDescriptionOf(element), + type: type, + ); + } if (tappedWidget == null || !hitFound) { element.visitChildElements(elementFinder); @@ -501,71 +511,36 @@ class _SentryUserInteractionWidgetState return tappedWidget; } - UserInteractionInfo? _getDescriptionFrom(Element element) { + String? _getElementType(Element element) { final widget = element.widget; // Used by ElevatedButton, TextButton, OutlinedButton. if (widget is ButtonStyleButton) { if (widget.enabled) { - return UserInteractionInfo( - element: element, - description: _findDescriptionOf(element, true), - type: 'ButtonStyleButton', - eventType: 'onClick', - ); + return 'ButtonStyleButton'; } } else if (widget is MaterialButton) { if (widget.enabled) { - return UserInteractionInfo( - element: element, - description: _findDescriptionOf(element, true), - type: 'MaterialButton', - eventType: 'onClick', - ); + return 'MaterialButton'; } } else if (widget is CupertinoButton) { if (widget.enabled) { - return UserInteractionInfo( - element: element, - description: _findDescriptionOf(element, true), - type: 'CupertinoButton', - eventType: 'onPressed', - ); + return 'CupertinoButton'; } } else if (widget is InkWell) { if (widget.onTap != null) { - return UserInteractionInfo( - element: element, - description: _findDescriptionOf(element, false), - type: 'InkWell', - eventType: 'onTap', - ); + return 'InkWell'; } } else if (widget is IconButton) { if (widget.onPressed != null) { - return UserInteractionInfo( - element: element, - description: _findDescriptionOf(element, false), - type: 'IconButton', - eventType: 'onPressed', - ); + return 'IconButton'; } } else if (widget is PopupMenuButton) { if (widget.enabled) { - return UserInteractionInfo( - element: element, - description: _findDescriptionOf(element, false), - type: 'PopupMenuButton', - eventType: 'onTap', - ); + return 'PopupMenuButton'; } } else if (widget is PopupMenuItem) { if (widget.enabled) { - return UserInteractionInfo( - element: element, - description: _findDescriptionOf(element, false), - type: 'PopupMenuItem', - eventType: 'onTap', - ); + return 'PopupMenuItem'; } } diff --git a/flutter/lib/src/user_interaction/user_interaction_info.dart b/flutter/lib/src/user_interaction/user_interaction_info.dart index f7848f260f..153f80998f 100644 --- a/flutter/lib/src/user_interaction/user_interaction_info.dart +++ b/flutter/lib/src/user_interaction/user_interaction_info.dart @@ -6,12 +6,10 @@ class UserInteractionInfo { final Element element; final String description; final String type; - final String eventType; const UserInteractionInfo({ required this.element, required this.description, required this.type, - required this.eventType, }); } From e307397aab1b71d887c91769bd0b0be2f5437017 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 20 Aug 2024 16:06:47 +0200 Subject: [PATCH 05/14] more refactoring --- .../sentry_user_interaction_widget.dart | 66 ++++++++++--------- .../user_interaction_info.dart | 2 - 2 files changed, 34 insertions(+), 34 deletions(-) diff --git a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart index ebe95aa6d7..4bd7fd7961 100644 --- a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart +++ b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart @@ -331,10 +331,10 @@ class _SentryUserInteractionWidgetState return; } - Map? data; - if ((_options?.sendDefaultPii ?? false) && info.description.isNotEmpty) { - data = {}; - data['label'] = info.description; + Map? data = {}; + final description = _findDescriptionOf(info.element); + if (description.isNotEmpty) { + data['label'] = description; } final crumb = Breadcrumb.userInteraction( @@ -417,42 +417,45 @@ class _SentryUserInteractionWidgetState } String _findDescriptionOf(Element element) { - final widget = element.widget; - final allowText = widget is ButtonStyleButton || - widget is MaterialButton || - widget is CupertinoButton; var description = ''; - // traverse tree to find a suiting element - void descriptionFinder(Element element) { - bool foundDescription = false; - + if (_options?.sendDefaultPii ?? false) { final widget = element.widget; - if (allowText && widget is Text) { - final data = widget.data; - if (data != null && data.isNotEmpty) { - description = data; - foundDescription = true; - } - } else if (widget is Semantics) { - if (widget.properties.label?.isNotEmpty ?? false) { - description = widget.properties.label!; - foundDescription = true; + final allowText = widget is ButtonStyleButton || + widget is MaterialButton || + widget is CupertinoButton; + + // traverse tree to find a suiting element + void descriptionFinder(Element element) { + bool foundDescription = false; + + final widget = element.widget; + if (allowText && widget is Text) { + final data = widget.data; + if (data != null && data.isNotEmpty) { + description = data; + foundDescription = true; + } + } else if (widget is Semantics) { + if (widget.properties.label?.isNotEmpty ?? false) { + description = widget.properties.label!; + foundDescription = true; + } + } else if (widget is Icon) { + if (widget.semanticLabel?.isNotEmpty ?? false) { + description = widget.semanticLabel!; + foundDescription = true; + } } - } else if (widget is Icon) { - if (widget.semanticLabel?.isNotEmpty ?? false) { - description = widget.semanticLabel!; - foundDescription = true; + + if (!foundDescription) { + element.visitChildren(descriptionFinder); } } - if (!foundDescription) { - element.visitChildren(descriptionFinder); - } + element.visitChildren(descriptionFinder); } - element.visitChildren(descriptionFinder); - return description; } @@ -496,7 +499,6 @@ class _SentryUserInteractionWidgetState if (type != null) { tappedWidget = UserInteractionInfo( element: element, - description: _findDescriptionOf(element), type: type, ); } diff --git a/flutter/lib/src/user_interaction/user_interaction_info.dart b/flutter/lib/src/user_interaction/user_interaction_info.dart index 153f80998f..38ceb8f6a4 100644 --- a/flutter/lib/src/user_interaction/user_interaction_info.dart +++ b/flutter/lib/src/user_interaction/user_interaction_info.dart @@ -4,12 +4,10 @@ import 'package:meta/meta.dart'; @internal class UserInteractionInfo { final Element element; - final String description; final String type; const UserInteractionInfo({ required this.element, - required this.description, required this.type, }); } From 4e3626010c90e815945a20f3069bc773f0f8e941 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 21 Aug 2024 10:09:27 +0200 Subject: [PATCH 06/14] feat: collect touch element path --- .../sentry_user_interaction_widget.dart | 150 ++++++++++++------ 1 file changed, 105 insertions(+), 45 deletions(-) diff --git a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart index 4bd7fd7961..c995c937d5 100644 --- a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart +++ b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart @@ -294,24 +294,50 @@ class _SentryUserInteractionWidgetState } void _onPointerDown(PointerDownEvent event) { - _lastPointerId = event.pointer; - _lastPointerDownLocation = event.localPosition; + try { + _lastPointerId = event.pointer; + _lastPointerDownLocation = event.localPosition; + } catch (exception, stacktrace) { + _options?.logger( + SentryLevel.error, + 'Error while handling pointer-down event $event in $SentryUserInteractionWidget', + exception: exception, + stackTrace: stacktrace, + ); + // ignore: invalid_use_of_internal_member + if (_options?.automatedTestMode ?? false) { + rethrow; + } + } } void _onPointerUp(PointerUpEvent event) { - // Figure out if something was tapped - final location = _lastPointerDownLocation; - if (location == null || event.pointer != _lastPointerId) { - return; - } - final delta = Offset( - location.dx - event.localPosition.dx, - location.dy - event.localPosition.dy, - ); + try { + // Figure out if something was tapped + final location = _lastPointerDownLocation; + if (location == null || event.pointer != _lastPointerId) { + return; + } + final delta = Offset( + location.dx - event.localPosition.dx, + location.dy - event.localPosition.dy, + ); - if (delta.distanceSquared < _tapDeltaArea) { - // Widget was tapped - _onTappedAt(event.localPosition); + if (delta.distanceSquared < _tapDeltaArea) { + // Widget was tapped + _onTappedAt(event.localPosition); + } + } catch (exception, stacktrace) { + _options?.logger( + SentryLevel.error, + 'Error while handling pointer-up event $event in $SentryUserInteractionWidget', + exception: exception, + stackTrace: stacktrace, + ); + // ignore: invalid_use_of_internal_member + if (_options?.automatedTestMode ?? false) { + rethrow; + } } } @@ -331,11 +357,11 @@ class _SentryUserInteractionWidgetState return; } - Map? data = {}; - final description = _findDescriptionOf(info.element); - if (description.isNotEmpty) { - data['label'] = description; - } + final label = _getLabelRecursively(info.element); + final data = { + 'path': _getTouchPath(info.element), + if (label != null) 'label': label + }; final crumb = Breadcrumb.userInteraction( subCategory: 'click', @@ -347,6 +373,37 @@ class _SentryUserInteractionWidgetState _hub.addBreadcrumb(crumb, hint: hint); } + List> _getTouchPath(Element element) { + final path = >[]; + + bool addToPath(Element element) { + // Break at the boundary (i.e. this [SentryUserInteractionWidget]). + if (element.widget == widget) { + return false; + } + + final widgetName = element.widget.toStringShort(); + if (!widgetName.startsWith('_')) { + final info = { + 'name': WidgetUtils.toStringValue(element.widget.key), + 'element': _getElementType(element) ?? widgetName, + 'label': _getLabel(element, true), + }..removeWhere((key, value) => value == null); + if (info.isNotEmpty) { + path.add(info); + } + } + + return path.length < 10; + } + + if (addToPath(element)) { + element.visitAncestorElements(addToPath); + } + + return path; + } + void _startTransactionOnTap(UserInteractionInfo info, String? widgetKey) { if (widgetKey == null || !(_options?.isTracingEnabled() ?? false) || @@ -416,8 +473,31 @@ class _SentryUserInteractionWidgetState }); } - String _findDescriptionOf(Element element) { - var description = ''; + String? _getLabel(Element element, bool allowText) { + String? label; + + if (_options?.sendDefaultPii ?? false) { + final widget = element.widget; + if (allowText && widget is Text) { + label = widget.data; + } else if (widget is Semantics) { + label = widget.properties.label; + } else if (widget is Icon) { + label = widget.semanticLabel; + } else if (widget is Tooltip) { + label = widget.message; + } + + if (label?.isEmpty ?? true) { + label = null; + } + } + + return label; + } + + String? _getLabelRecursively(Element element) { + String? label; if (_options?.sendDefaultPii ?? false) { final widget = element.widget; @@ -427,36 +507,16 @@ class _SentryUserInteractionWidgetState // traverse tree to find a suiting element void descriptionFinder(Element element) { - bool foundDescription = false; - - final widget = element.widget; - if (allowText && widget is Text) { - final data = widget.data; - if (data != null && data.isNotEmpty) { - description = data; - foundDescription = true; - } - } else if (widget is Semantics) { - if (widget.properties.label?.isNotEmpty ?? false) { - description = widget.properties.label!; - foundDescription = true; - } - } else if (widget is Icon) { - if (widget.semanticLabel?.isNotEmpty ?? false) { - description = widget.semanticLabel!; - foundDescription = true; - } - } - - if (!foundDescription) { + label ??= _getLabel(element, allowText); + if (label == null) { element.visitChildren(descriptionFinder); } } - element.visitChildren(descriptionFinder); + descriptionFinder(element); } - return description; + return label; } UserInteractionInfo? _getElementAt(Offset position) { From 3a3696b4ac6a7f89defe1745f639239c939a3a24 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 21 Aug 2024 11:13:37 +0200 Subject: [PATCH 07/14] update tests --- .../sentry_user_interaction_widget.dart | 2 +- .../sentry_user_interaction_widget_test.dart | 274 ++++++++++++++---- 2 files changed, 216 insertions(+), 60 deletions(-) diff --git a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart index c995c937d5..b737132dea 100644 --- a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart +++ b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart @@ -382,7 +382,7 @@ class _SentryUserInteractionWidgetState return false; } - final widgetName = element.widget.toStringShort(); + final widgetName = element.widget.runtimeType.toString(); if (!widgetName.startsWith('_')) { final info = { 'name': WidgetUtils.toStringValue(element.widget.key), diff --git a/flutter/test/user_interaction/sentry_user_interaction_widget_test.dart b/flutter/test/user_interaction/sentry_user_interaction_widget_test.dart index 8f326579f4..04b5577d3f 100644 --- a/flutter/test/user_interaction/sentry_user_interaction_widget_test.dart +++ b/flutter/test/user_interaction/sentry_user_interaction_widget_test.dart @@ -109,13 +109,24 @@ void main() { await tapMe(tester, sut, 'btn_1'); - Breadcrumb? crumb; - fixture.hub.configureScope((scope) { - crumb = scope.breadcrumbs.last; - }); - expect(crumb?.category, 'ui.click'); - expect(crumb?.data?['view.id'], 'btn_1'); - expect(crumb?.data?['view.class'], 'MaterialButton'); + expect( + fixture.getBreadcrumb().data?.replaceHashCodes(), + equals({ + 'path': [ + {'name': 'btn_1', 'element': 'MaterialButton'}, + {'element': 'Column'}, + {'element': 'Center'}, + {'name': '[GlobalKey#00000]', 'element': 'KeyedSubtree'}, + {'element': 'MediaQuery'}, + {'name': '_ScaffoldSlot.body', 'element': 'LayoutId'}, + {'element': 'CustomMultiChildLayout'}, + {'element': 'Actions'}, + {'element': 'AnimatedBuilder'}, + {'element': 'DefaultTextStyle'} + ], + 'view.id': 'btn_1', + 'view.class': 'MaterialButton', + })); }); }); @@ -125,11 +136,25 @@ void main() { await tapMe(tester, sut, 'btn_1'); - Breadcrumb? crumb; - fixture.hub.configureScope((scope) { - crumb = scope.breadcrumbs.last; - }); - expect(crumb?.data?['label'], 'Button 1'); + expect( + fixture.getBreadcrumb().data?.replaceHashCodes(), + equals({ + 'path': [ + {'name': 'btn_1', 'element': 'MaterialButton'}, + {'element': 'Column'}, + {'element': 'Center'}, + {'name': '[GlobalKey#00000]', 'element': 'KeyedSubtree'}, + {'element': 'MediaQuery'}, + {'name': '_ScaffoldSlot.body', 'element': 'LayoutId'}, + {'element': 'CustomMultiChildLayout'}, + {'element': 'Actions'}, + {'element': 'AnimatedBuilder'}, + {'element': 'DefaultTextStyle'} + ], + 'label': 'Button 1', + 'view.id': 'btn_1', + 'view.class': 'MaterialButton' + })); }); }); @@ -139,11 +164,25 @@ void main() { await tapMe(tester, sut, 'btn_3'); - Breadcrumb? crumb; - fixture.hub.configureScope((scope) { - crumb = scope.breadcrumbs.last; - }); - expect(crumb?.data?['label'], 'My Icon'); + expect( + fixture.getBreadcrumb().data?.replaceHashCodes(), + equals({ + 'path': [ + {'name': 'btn_3', 'element': 'IconButton'}, + {'element': 'Column'}, + {'element': 'Center'}, + {'name': '[GlobalKey#00000]', 'element': 'KeyedSubtree'}, + {'element': 'MediaQuery'}, + {'name': '_ScaffoldSlot.body', 'element': 'LayoutId'}, + {'element': 'CustomMultiChildLayout'}, + {'element': 'Actions'}, + {'element': 'AnimatedBuilder'}, + {'element': 'DefaultTextStyle'} + ], + 'label': 'My Icon', + 'view.id': 'btn_3', + 'view.class': 'IconButton' + })); }); }); @@ -153,11 +192,25 @@ void main() { await tapMe(tester, sut, 'btn_2'); - Breadcrumb? crumb; - fixture.hub.configureScope((scope) { - crumb = scope.breadcrumbs.last; - }); - expect(crumb?.data?['label'], 'Button 2'); + expect( + fixture.getBreadcrumb().data?.replaceHashCodes(), + equals({ + 'path': [ + {'name': 'btn_2', 'element': 'CupertinoButton'}, + {'element': 'Column'}, + {'element': 'Center'}, + {'name': '[GlobalKey#00000]', 'element': 'KeyedSubtree'}, + {'element': 'MediaQuery'}, + {'name': '_ScaffoldSlot.body', 'element': 'LayoutId'}, + {'element': 'CustomMultiChildLayout'}, + {'element': 'Actions'}, + {'element': 'AnimatedBuilder'}, + {'element': 'DefaultTextStyle'} + ], + 'label': 'Button 2', + 'view.id': 'btn_2', + 'view.class': 'CupertinoButton' + })); }); }); @@ -183,11 +236,25 @@ void main() { await tapMe(tester, sut, 'btn_5'); - Breadcrumb? crumb; - fixture.hub.configureScope((scope) { - crumb = scope.breadcrumbs.last; - }); - expect(crumb?.data?['label'], 'Button 5'); + expect( + fixture.getBreadcrumb().data?.replaceHashCodes(), + equals({ + 'path': [ + {'name': 'btn_5', 'element': 'ButtonStyleButton'}, + {'element': 'Stack'}, + {'element': 'Listener'}, + {'element': 'RawGestureDetector'}, + {'name': 'btn_4', 'element': 'GestureDetector'}, + {'element': 'Semantics'}, + {'element': 'DefaultTextStyle'}, + {'element': 'AnimatedDefaultTextStyle'}, + {'element': 'NotificationListener'}, + {'element': 'CustomPaint'} + ], + 'label': 'Button 5', + 'view.id': 'btn_5', + 'view.class': 'ButtonStyleButton' + })); }); }); @@ -197,13 +264,24 @@ void main() { await tapMe(tester, sut, 'popup_menu_button'); - Breadcrumb? crumb; - fixture.hub.configureScope((scope) { - crumb = scope.breadcrumbs.last; - }); - expect(crumb?.category, 'ui.click'); - expect(crumb?.data?['view.id'], 'popup_menu_button'); - expect(crumb?.data?['view.class'], 'PopupMenuButton'); + expect( + fixture.getBreadcrumb().data?.replaceHashCodes(), + equals({ + 'path': [ + {'name': 'popup_menu_button', 'element': 'PopupMenuButton'}, + {'element': 'Column'}, + {'element': 'Center'}, + {'name': '[GlobalKey#00000]', 'element': 'KeyedSubtree'}, + {'element': 'MediaQuery'}, + {'name': '_ScaffoldSlot.body', 'element': 'LayoutId'}, + {'element': 'CustomMultiChildLayout'}, + {'element': 'Actions'}, + {'element': 'AnimatedBuilder'}, + {'element': 'DefaultTextStyle'} + ], + 'view.id': 'popup_menu_button', + 'view.class': 'PopupMenuButton' + })); }); }); @@ -217,13 +295,56 @@ void main() { await tapMe(tester, sut, 'popup_menu_item_1'); - Breadcrumb? crumb; - fixture.hub.configureScope((scope) { - crumb = scope.breadcrumbs.last; - }); - expect(crumb?.category, 'ui.click'); - expect(crumb?.data?['view.id'], 'popup_menu_item_1'); - expect(crumb?.data?['view.class'], 'PopupMenuItem'); + expect( + fixture.getBreadcrumb().data?.replaceHashCodes(), + equals({ + 'path': [ + {'name': 'popup_menu_item_1', 'element': 'PopupMenuItem'}, + {'name': '[GlobalKey#00000]', 'element': 'FadeTransition'}, + {'element': 'ListBody'}, + {'element': 'Padding'}, + {'name': '[GlobalKey#00000]', 'element': 'IgnorePointer'}, + {'element': 'Semantics'}, + {'element': 'Listener'}, + { + 'name': '[LabeledGlobalKey#00000]', + 'element': 'RawGestureDetector' + }, + {'element': 'Listener'}, + {'element': 'NotificationListener'} + ], + 'view.id': 'popup_menu_item_1', + 'view.class': 'PopupMenuItem' + })); + }); + }); + + testWidgets('Add crumb for button with tooltip', (tester) async { + await tester.runAsync(() async { + final sut = fixture.getSut(sendDefaultPii: true); + + // open the popup menu and wait for the animation to complete + await tapMe(tester, sut, 'tooltip_button'); + + expect( + fixture.getBreadcrumb().data?.replaceHashCodes(), + equals({ + 'path': [ + {'name': 'tooltip_button', 'element': 'ButtonStyleButton'}, + {'element': 'Semantics'}, + {'element': 'Listener'}, + {'element': 'OverlayPortal'}, + {'element': 'Tooltip', 'label': 'Tooltip message.'}, + {'element': 'Column'}, + {'element': 'Center'}, + {'name': '[GlobalKey#00000]', 'element': 'KeyedSubtree'}, + {'element': 'MediaQuery'}, + {'name': '_ScaffoldSlot.body', 'element': 'LayoutId'} + ], + 'label': 'Button text', + 'view.id': 'tooltip_button', + 'view.class': 'ButtonStyleButton' + })); }); }); }); @@ -394,6 +515,14 @@ class Fixture { child: MyApp(), ); } + + Breadcrumb getBreadcrumb() { + late final Breadcrumb crumb; + hub.configureScope((scope) { + crumb = scope.breadcrumbs.last; + }); + return crumb; + } } class MyApp extends StatelessWidget { @@ -420,23 +549,17 @@ class Page1 extends StatelessWidget { children: [ MaterialButton( key: Key('btn_1'), - onPressed: () { - // print('button pressed'); - }, + onPressed: () {}, child: const Text('Button 1'), ), CupertinoButton( key: Key('btn_2'), - onPressed: () { - // print('button pressed 2'); - }, + onPressed: () {}, child: const Text('Button 2'), ), IconButton( key: Key('btn_3'), - onPressed: () { - // print('button pressed 3'); - }, + onPressed: () {}, icon: Icon( Icons.dark_mode, semanticLabel: 'My Icon', @@ -445,17 +568,13 @@ class Page1 extends StatelessWidget { Card( child: GestureDetector( key: Key('btn_4'), - onTap: () => { - // print('button pressed 4'), - }, + onTap: () => {}, child: Stack( children: [ //fancy card layout ElevatedButton( key: Key('btn_5'), - onPressed: () => { - // print('button pressed 5'), - }, + onPressed: () => {}, child: const Text('Button 5'), ), ], @@ -478,6 +597,14 @@ class Page1 extends StatelessWidget { ), ], ), + Tooltip( + message: 'Tooltip message.', + child: ElevatedButton( + key: ValueKey('tooltip_button'), + onPressed: () {}, + child: Text('Button text'), + ), + ) ], ), ), @@ -496,9 +623,7 @@ class Page2 extends StatelessWidget { children: [ MaterialButton( key: Key('btn_page_2'), - onPressed: () { - // print('button page 2 pressed'); - }, + onPressed: () {}, child: const Text('Button Page 2'), ), ], @@ -507,3 +632,34 @@ class Page2 extends StatelessWidget { ); } } + +extension on String { + String replaceHashCodes() => replaceAll(RegExp(r'#[\da-fA-F]{5}'), '#00000'); +} + +extension on Map { + Map replaceHashCodes() => map((key, value) { + if (value is String) { + value = value.replaceHashCodes(); + } else if (value is Map) { + value = value.replaceHashCodes(); + } else if (value is List) { + value = value.replaceHashCodes(); + } + return MapEntry(key, value); + }); +} + +extension on List { + Iterable replaceHashCodes() => map((value) { + if (value is String) { + return value.replaceHashCodes(); + } else if (value is Map) { + return value.replaceHashCodes(); + } else if (value is List) { + return value.replaceHashCodes(); + } else { + return value; + } + }); +} From 0102a0f1afd9af7ba03b1737680c1bd730e21ed7 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 21 Aug 2024 12:56:45 +0200 Subject: [PATCH 08/14] add tests for the new support of non-keyed button presses --- .../sentry_user_interaction_widget_test.dart | 41 ++++++++++++++++++- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/flutter/test/user_interaction/sentry_user_interaction_widget_test.dart b/flutter/test/user_interaction/sentry_user_interaction_widget_test.dart index 04b5577d3f..b0288729b2 100644 --- a/flutter/test/user_interaction/sentry_user_interaction_widget_test.dart +++ b/flutter/test/user_interaction/sentry_user_interaction_widget_test.dart @@ -347,6 +347,40 @@ void main() { })); }); }); + + testWidgets('Add crumb for button without key', (tester) async { + await tester.runAsync(() async { + final sut = fixture.getSut(sendDefaultPii: true); + + await tester.pumpWidget(sut); + await tester.tap(find.byElementPredicate((element) { + final widget = element.widget; + if (widget is MaterialButton) { + return (widget.child as Text).data == 'Button 5'; + } + return false; + })); + + expect( + fixture.getBreadcrumb().data?.replaceHashCodes(), + equals({ + 'path': [ + {'element': 'MaterialButton'}, + {'element': 'Column'}, + {'element': 'Center'}, + {'name': '[GlobalKey#00000]', 'element': 'KeyedSubtree'}, + {'element': 'MediaQuery'}, + {'name': '_ScaffoldSlot.body', 'element': 'LayoutId'}, + {'element': 'CustomMultiChildLayout'}, + {'element': 'Actions'}, + {'element': 'AnimatedBuilder'}, + {'element': 'DefaultTextStyle'} + ], + 'label': 'Button 5', + 'view.class': 'MaterialButton' + })); + }); + }); }); group('$SentryUserInteractionWidget performance', () { @@ -486,7 +520,6 @@ Future tapMe( if (pumpWidget) { await tester.pumpWidget(widget); } - await tester.tap(find.byKey(Key(key))); } @@ -604,7 +637,11 @@ class Page1 extends StatelessWidget { onPressed: () {}, child: Text('Button text'), ), - ) + ), + MaterialButton( + onPressed: () {}, + child: const Text('Button 5'), + ), ], ), ), From 06e60cbd614a59431e77c4832c0b7a20b184a2e2 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 21 Aug 2024 13:22:44 +0200 Subject: [PATCH 09/14] cleanup & improve existing code --- dart/lib/src/protocol/breadcrumb.dart | 35 +++---------------- flutter/lib/src/sentry_flutter_options.dart | 4 +-- .../sentry_user_interaction_widget.dart | 2 +- 3 files changed, 8 insertions(+), 33 deletions(-) diff --git a/dart/lib/src/protocol/breadcrumb.dart b/dart/lib/src/protocol/breadcrumb.dart index 9ece68d032..282a5cee8f 100644 --- a/dart/lib/src/protocol/breadcrumb.dart +++ b/dart/lib/src/protocol/breadcrumb.dart @@ -105,42 +105,17 @@ class Breadcrumb { String? viewId, String? viewClass, }) { - final newData = data ?? {}; - var path = ''; - - if (viewId != null) { - newData['view.id'] = viewId; - path = viewId; - } - - if (newData.containsKey('label')) { - if (path.isEmpty) { - path = newData['label']; - } else { - path = "$path, label: ${newData['label']}"; - } - } - - if (viewClass != null) { - newData['view.class'] = viewClass; - if (path.isEmpty) { - path = viewClass; - } else { - path = "$viewClass($path)"; - } - } - - if (path.isNotEmpty && !newData.containsKey('path')) { - newData['path'] = path; - } - return Breadcrumb( message: message, level: level, category: 'ui.$subCategory', type: 'user', timestamp: timestamp, - data: newData, + data: { + if (viewId != null) 'view.id': viewId, + if (viewClass != null) 'view.class': viewClass, + if (data != null) ...data, + }, ); } diff --git a/flutter/lib/src/sentry_flutter_options.dart b/flutter/lib/src/sentry_flutter_options.dart index 8b6f7ed491..79203dd6f5 100644 --- a/flutter/lib/src/sentry_flutter_options.dart +++ b/flutter/lib/src/sentry_flutter_options.dart @@ -187,14 +187,14 @@ class SentryFlutterOptions extends SentryOptions { /// /// Requires adding the [SentryUserInteractionWidget] to the widget tree. /// Example: - /// runApp(SentryUserInteractionWidget(child: App())); + /// runApp(SentryWidget(child: App())); bool enableUserInteractionBreadcrumbs = true; /// Enables the Auto instrumentation for user interaction tracing. /// /// Requires adding the [SentryUserInteractionWidget] to the widget tree. /// Example: - /// runApp(SentryUserInteractionWidget(child: App())); + /// runApp(SentryWidget(child: App())); bool enableUserInteractionTracing = true; /// Enable or disable the tracing of time to full display (TTFD). diff --git a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart index b737132dea..45c3a0921c 100644 --- a/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart +++ b/flutter/lib/src/user_interaction/sentry_user_interaction_widget.dart @@ -223,7 +223,7 @@ Element? _clickTrackerElement; /// Mostly for onPressed, onTap, and onLongPress events /// /// Example on how to set up: -/// runApp(SentryUserInteractionWidget(child: App())); +/// runApp(SentryWidget(child: App())); /// /// For transactions, enable it in the [SentryFlutterOptions.enableUserInteractionTracing]. /// The idle timeout can be configured in the [SentryOptions.idleTimeout]. From d9abf70266cef131d9999e3b3ae614d7e298d848 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Wed, 21 Aug 2024 13:26:08 +0200 Subject: [PATCH 10/14] chore: update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a486e6eb1a..6fb2f0b2a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ ### Improvements - Debouncing of SentryWidgetsBindingObserver.didChangeMetrics with delay of 100ms. ([#2232](https://github.com/getsentry/sentry-dart/pull/2232)) +- Collect touch breadcrumbs for all buttons, not just those with `key` specified. ([#2242](https://github.com/getsentry/sentry-dart/pull/2242)) ### Dependencies From 30cc34b4240964e9f9590fe1fc30d875d0ae13f3 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 2 Sep 2024 19:57:06 +0200 Subject: [PATCH 11/14] update native replay integration with touch breadcrumb path --- .../SentryFlutterReplayBreadcrumbConverter.kt | 36 +++++++++++++++++- .../SentryFlutterReplayBreadcrumbConverter.m | 38 ++++++++++++++++++- 2 files changed, 72 insertions(+), 2 deletions(-) diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayBreadcrumbConverter.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayBreadcrumbConverter.kt index 3dd549802f..4a4d4f7e20 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayBreadcrumbConverter.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayBreadcrumbConverter.kt @@ -30,7 +30,7 @@ class SentryFlutterReplayBreadcrumbConverter : DefaultReplayBreadcrumbConverter( "ui.click" -> newRRWebBreadcrumb(breadcrumb).apply { category = "ui.tap" - message = breadcrumb.data["path"] as String? + message = getTouchPathMessage(breadcrumb.data["path"]) } else -> { @@ -83,4 +83,38 @@ class SentryFlutterReplayBreadcrumbConverter : DefaultReplayBreadcrumbConverter( } return rrWebEvent } + + private fun getTouchPathMessage(maybePath: Any?): String? { + if (maybePath !is List<*>) { + return null + } + + if (maybePath.isEmpty()) { + return null + } + + val message = StringBuilder() + for (i in Math.min(3, maybePath.size - 1) downTo 0) { + val item = maybePath[i] + if (item !is Map<*, *>) { + continue + } + + message.append(item["element"] ?: "?") + + var identifier = item["label"] ?: item["name"] + if (identifier is String && identifier.isNotEmpty()) { + if (identifier.length > 20) { + identifier = identifier.substring(0, 17) + "..." + } + message.append("(").append(identifier).append(")") + } + + if (i > 0) { + message.append(" > ") + } + } + + return message.toString() + } } diff --git a/flutter/ios/Classes/SentryFlutterReplayBreadcrumbConverter.m b/flutter/ios/Classes/SentryFlutterReplayBreadcrumbConverter.m index 75b073de82..bde889b6bf 100644 --- a/flutter/ios/Classes/SentryFlutterReplayBreadcrumbConverter.m +++ b/flutter/ios/Classes/SentryFlutterReplayBreadcrumbConverter.m @@ -38,7 +38,7 @@ - (instancetype _Nonnull)init { if ([breadcrumb.category isEqualToString:@"ui.click"]) { return [self convertFrom:breadcrumb withCategory:@"ui.tap" - andMessage:breadcrumb.data[@"path"]]; + andMessage:[self getTouchPathMessage:breadcrumb.data[@"path"]]]; } SentryRRWebEvent *nativeBreadcrumb = @@ -112,6 +112,42 @@ - (NSDate *_Nonnull)dateFrom:(NSNumber *_Nonnull)timestamp { return [NSDate dateWithTimeIntervalSince1970:(timestamp.doubleValue / 1000)]; } +- (NSString * _Nullable)getTouchPathMessage:(id _Nullable)maybePath { + if (![maybePath isKindOfClass:[NSArray class]]) { + return nil; + } + + NSArray *path = (NSArray *)maybePath; + if (path.count == 0) { + return nil; + } + + NSMutableString *message = [NSMutableString string]; + for (NSInteger i = MIN(3, path.count - 1); i >= 0; i--) { + id item = path[i]; + if (![item isKindOfClass:[NSDictionary class]]) { + continue; + } + + NSDictionary *itemDict = (NSDictionary *)item; + [message appendString:itemDict[@"element"] ?: @"?"]; + + id identifier = itemDict[@"label"] ?: itemDict[@"name"]; + if ([identifier isKindOfClass:[NSString class]] && [(NSString *)identifier length] > 0) { + NSString *identifierStr = (NSString *)identifier; + if (identifierStr.length > 20) { + identifierStr = [[identifierStr substringToIndex:17] stringByAppendingString:@"..."]; + } + [message appendFormat:@"(%@)", identifierStr]; + } + + if (i > 0) { + [message appendString:@" > "]; + } + } + + return message.length > 0 ? message : nil; +} @end #endif From ce4719dc13ca0981b38c5a7d03823d9d4ea1be7a Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Mon, 2 Sep 2024 20:02:09 +0200 Subject: [PATCH 12/14] fix tests --- dart/test/protocol/breadcrumb_test.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/dart/test/protocol/breadcrumb_test.dart b/dart/test/protocol/breadcrumb_test.dart index 2dea300561..24f0a1b408 100644 --- a/dart/test/protocol/breadcrumb_test.dart +++ b/dart/test/protocol/breadcrumb_test.dart @@ -222,7 +222,6 @@ void main() { 'foo': 'bar', 'view.id': 'foo', 'view.class': 'bar', - 'path': 'bar(foo)', }, }); }); From 301b327c355a4b628b0b4c2e14db185b479c3d75 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos <6349682+vaind@users.noreply.github.com> Date: Mon, 2 Sep 2024 20:38:00 +0200 Subject: [PATCH 13/14] Update CHANGELOG.md --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fb2f0b2a0..45b3f9cc36 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ ); ``` +- Collect touch breadcrumbs for all buttons, not just those with `key` specified. ([#2242](https://github.com/getsentry/sentry-dart/pull/2242)) + ### Dependencies - Bump Cocoa SDK from v8.35.1 to v8.36.0 ([#2252](https://github.com/getsentry/sentry-dart/pull/2252)) @@ -42,7 +44,6 @@ ### Improvements - Debouncing of SentryWidgetsBindingObserver.didChangeMetrics with delay of 100ms. ([#2232](https://github.com/getsentry/sentry-dart/pull/2232)) -- Collect touch breadcrumbs for all buttons, not just those with `key` specified. ([#2242](https://github.com/getsentry/sentry-dart/pull/2242)) ### Dependencies From ed4cab29b61f459a17658fe4ac49ba3b0a67ae27 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos Date: Tue, 3 Sep 2024 16:38:12 +0200 Subject: [PATCH 14/14] linter issues --- .../SentryFlutterReplayBreadcrumbConverter.kt | 14 ++++++-------- .../sentry/flutter/SentryFlutterReplayRecorder.kt | 8 ++++---- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayBreadcrumbConverter.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayBreadcrumbConverter.kt index 4a4d4f7e20..a711a36439 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayBreadcrumbConverter.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayBreadcrumbConverter.kt @@ -8,6 +8,8 @@ import io.sentry.rrweb.RRWebSpanEvent import java.util.Date private const val MILLIS_PER_SECOND = 1000.0 +private const val MAX_PATH_ITEMS = 4 +private const val MAX_PATH_IDENTIFIER_LENGTH = 20 class SentryFlutterReplayBreadcrumbConverter : DefaultReplayBreadcrumbConverter() { internal companion object { @@ -85,16 +87,12 @@ class SentryFlutterReplayBreadcrumbConverter : DefaultReplayBreadcrumbConverter( } private fun getTouchPathMessage(maybePath: Any?): String? { - if (maybePath !is List<*>) { - return null - } - - if (maybePath.isEmpty()) { + if (maybePath !is List<*> || maybePath.isEmpty()) { return null } val message = StringBuilder() - for (i in Math.min(3, maybePath.size - 1) downTo 0) { + for (i in Math.min(MAX_PATH_ITEMS, maybePath.size) - 1 downTo 0) { val item = maybePath[i] if (item !is Map<*, *>) { continue @@ -104,8 +102,8 @@ class SentryFlutterReplayBreadcrumbConverter : DefaultReplayBreadcrumbConverter( var identifier = item["label"] ?: item["name"] if (identifier is String && identifier.isNotEmpty()) { - if (identifier.length > 20) { - identifier = identifier.substring(0, 17) + "..." + if (identifier.length > MAX_PATH_IDENTIFIER_LENGTH) { + identifier = identifier.substring(0, MAX_PATH_IDENTIFIER_LENGTH - "...".length) + "..." } message.append("(").append(identifier).append(")") } diff --git a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt index 41209f75b6..ba285a12a0 100644 --- a/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt +++ b/flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterReplayRecorder.kt @@ -12,7 +12,7 @@ internal class SentryFlutterReplayRecorder( private val channel: MethodChannel, private val integration: ReplayIntegration, ) : Recorder { - override fun start(config: ScreenshotRecorderConfig) { + override fun start(recorderConfig: ScreenshotRecorderConfig) { val cacheDirPath = integration.replayCacheDir?.absolutePath if (cacheDirPath == null) { Log.w("Sentry", "Replay cache directory is null, can't start replay recorder.") @@ -24,9 +24,9 @@ internal class SentryFlutterReplayRecorder( "ReplayRecorder.start", mapOf( "directory" to cacheDirPath, - "width" to config.recordingWidth, - "height" to config.recordingHeight, - "frameRate" to config.frameRate, + "width" to recorderConfig.recordingWidth, + "height" to recorderConfig.recordingHeight, + "frameRate" to recorderConfig.frameRate, "replayId" to integration.getReplayId().toString(), ), )