From a40bb7c712c6ca285e8712782b99c6ec1e1d6d03 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos <6349682+vaind@users.noreply.github.com> Date: Wed, 4 Sep 2024 19:56:24 +0200 Subject: [PATCH 1/2] feat: asset images don't need to be obscured in replay (#2269) * feat: asset images don't need to be obscured * chore: update changelog --- CHANGELOG.md | 16 ++--- flutter/example/lib/main.dart | 10 +++ flutter/lib/sentry_flutter.dart | 2 +- flutter/lib/src/replay/widget_filter.dart | 32 +++++++++- flutter/lib/src/sentry_asset_bundle.dart | 7 ++ flutter/test/replay/test_widget.dart | 71 ++++++++++----------- flutter/test/replay/widget_filter_test.dart | 41 +++++++++++- 7 files changed, 131 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b915e3dcf6..b12c189997 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,29 +4,29 @@ ### Features -- Support allowUrls and denyUrls for Flutter Web ([#2227](https://github.com/getsentry/sentry-dart/pull/2227)) +- Session replay Alpha for Android and iOS ([#2208](https://github.com/getsentry/sentry-dart/pull/2208), [#2269](https://github.com/getsentry/sentry-dart/pull/2269)). + + To try out replay, you can set following options (access is limited to early access orgs on Sentry. If you're interested, [sign up for the waitlist](https://sentry.io/lp/mobile-replay-beta/)): ```dart await SentryFlutter.init( (options) { ... - options.allowUrls = ["^https://sentry.com.*\$", "my-custom-domain"]; - options.denyUrls = ["^.*ends-with-this\$", "denied-url"]; + options.experimental.replay.sessionSampleRate = 1.0; + options.experimental.replay.errorSampleRate = 1.0; }, appRunner: () => runApp(MyApp()), ); ``` -- Session replay Alpha for Android and iOS ([#2208](https://github.com/getsentry/sentry-dart/pull/2208)). - - To try out replay, you can set following options (access is limited to early access orgs on Sentry. If you're interested, [sign up for the waitlist](https://sentry.io/lp/mobile-replay-beta/)): +- Support allowUrls and denyUrls for Flutter Web ([#2227](https://github.com/getsentry/sentry-dart/pull/2227)) ```dart await SentryFlutter.init( (options) { ... - options.experimental.replay.sessionSampleRate = 1.0; - options.experimental.replay.errorSampleRate = 1.0; + options.allowUrls = ["^https://sentry.com.*\$", "my-custom-domain"]; + options.denyUrls = ["^.*ends-with-this\$", "denied-url"]; }, appRunner: () => runApp(MyApp()), ); diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index ed8961741c..0f1cd9279d 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -1043,6 +1043,8 @@ Future showDialogWithTextAndImage(BuildContext context) async { await DefaultAssetBundle.of(context).loadString('assets/lorem-ipsum.txt'); if (!context.mounted) return; + final imageBytes = + await DefaultAssetBundle.of(context).load('assets/sentry-wordmark.png'); await showDialog( context: context, // gets tracked if using SentryNavigatorObserver @@ -1056,7 +1058,15 @@ Future showDialogWithTextAndImage(BuildContext context) async { child: Column( mainAxisSize: MainAxisSize.min, children: [ + // Use various ways an image is included in the app. + // Local asset images are not obscured in replay recording. Image.asset('assets/sentry-wordmark.png'), + Image.asset('assets/sentry-wordmark.png', bundle: rootBundle), + Image.asset('assets/sentry-wordmark.png', + bundle: DefaultAssetBundle.of(context)), + Image.network( + 'https://www.gstatic.com/recaptcha/api2/logo_48.png'), + Image.memory(imageBytes.buffer.asUint8List()), Text(text), ], ), diff --git a/flutter/lib/sentry_flutter.dart b/flutter/lib/sentry_flutter.dart index bea9016630..c74013e81e 100644 --- a/flutter/lib/sentry_flutter.dart +++ b/flutter/lib/sentry_flutter.dart @@ -10,7 +10,7 @@ export 'src/sentry_flutter.dart'; export 'src/sentry_flutter_options.dart'; export 'src/sentry_replay_options.dart'; export 'src/flutter_sentry_attachment.dart'; -export 'src/sentry_asset_bundle.dart'; +export 'src/sentry_asset_bundle.dart' show SentryAssetBundle; export 'src/integrations/on_error_integration.dart'; export 'src/screenshot/sentry_screenshot_widget.dart'; export 'src/screenshot/sentry_screenshot_quality.dart'; diff --git a/flutter/lib/src/replay/widget_filter.dart b/flutter/lib/src/replay/widget_filter.dart index 83e069cb97..f269dd041f 100644 --- a/flutter/lib/src/replay/widget_filter.dart +++ b/flutter/lib/src/replay/widget_filter.dart @@ -1,8 +1,9 @@ +import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; import 'package:meta/meta.dart'; -import 'package:sentry/sentry.dart'; import '../../sentry_flutter.dart'; +import '../sentry_asset_bundle.dart'; @internal class WidgetFilter { @@ -14,11 +15,14 @@ class WidgetFilter { late double _pixelRatio; late Rect _bounds; final _warnedWidgets = {}; + final AssetBundle _rootAssetBundle; WidgetFilter( {required this.redactText, required this.redactImages, - required this.logger}); + required this.logger, + @visibleForTesting AssetBundle? rootAssetBundle}) + : _rootAssetBundle = rootAssetBundle ?? rootBundle; void obscure(BuildContext context, double pixelRatio, Rect bounds) { _pixelRatio = pixelRatio; @@ -57,6 +61,14 @@ class WidgetFilter { } else if (redactText && widget is EditableText) { color = widget.style.color; } else if (redactImages && widget is Image) { + if (widget.image is AssetBundleImageProvider) { + final image = widget.image as AssetBundleImageProvider; + if (isBuiltInAssetImage(image)) { + logger(SentryLevel.debug, + "WidgetFilter skipping asset: $widget ($image)."); + return false; + } + } color = widget.color; } else { // No other type is currently obscured. @@ -115,6 +127,22 @@ class WidgetFilter { return true; } + @visibleForTesting + @pragma('vm:prefer-inline') + bool isBuiltInAssetImage(AssetBundleImageProvider image) { + late final AssetBundle? bundle; + if (image is AssetImage) { + bundle = image.bundle; + } else if (image is ExactAssetImage) { + bundle = image.bundle; + } else { + return false; + } + return (bundle == null || + bundle == _rootAssetBundle || + (bundle is SentryAssetBundle && bundle.bundle == _rootAssetBundle)); + } + @pragma('vm:prefer-inline') void _cantObscure(Widget widget, String message) { if (!_warnedWidgets.contains(widget.hashCode)) { diff --git a/flutter/lib/src/sentry_asset_bundle.dart b/flutter/lib/src/sentry_asset_bundle.dart index 52d1a2da0c..a41288209f 100644 --- a/flutter/lib/src/sentry_asset_bundle.dart +++ b/flutter/lib/src/sentry_asset_bundle.dart @@ -7,6 +7,7 @@ import 'dart:ui'; import 'package:flutter/services.dart'; import 'package:flutter/material.dart'; +import 'package:meta/meta.dart'; import 'package:sentry/sentry.dart'; typedef _StringParser = Future Function(String value); @@ -375,3 +376,9 @@ class SentryAssetBundle implements AssetBundle { as Future; } } + +@internal +extension SentryAssetBundleInternal on SentryAssetBundle { + /// Returns the wrapped [AssetBundle]. + AssetBundle get bundle => _bundle; +} diff --git a/flutter/test/replay/test_widget.dart b/flutter/test/replay/test_widget.dart index e85dfacaf8..c5321ce750 100644 --- a/flutter/test/replay/test_widget.dart +++ b/flutter/test/replay/test_widget.dart @@ -1,10 +1,10 @@ -import 'dart:typed_data'; - import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:sentry_flutter/sentry_flutter.dart'; -Future pumpTestElement(WidgetTester tester) async { +Future pumpTestElement(WidgetTester tester, + {List? children}) async { await tester.pumpWidget( MaterialApp( home: SentryWidget( @@ -14,25 +14,26 @@ Future pumpTestElement(WidgetTester tester) async { child: Opacity( opacity: 0.5, child: Column( - children: [ - newImage(), - const Padding( - padding: EdgeInsets.all(15), - child: Center(child: Text('Centered text')), - ), - ElevatedButton( - onPressed: () {}, - child: Text('Button title'), - ), - newImage(), - // Invisible widgets won't be obscured. - Visibility(visible: false, child: Text('Invisible text')), - Visibility(visible: false, child: newImage()), - Opacity(opacity: 0, child: Text('Invisible text')), - Opacity(opacity: 0, child: newImage()), - Offstage(offstage: true, child: Text('Offstage text')), - Offstage(offstage: true, child: newImage()), - ], + children: children ?? + [ + newImage(), + const Padding( + padding: EdgeInsets.all(15), + child: Center(child: Text('Centered text')), + ), + ElevatedButton( + onPressed: () {}, + child: Text('Button title'), + ), + newImage(), + // Invisible widgets won't be obscured. + Visibility(visible: false, child: Text('Invisible text')), + Visibility(visible: false, child: newImage()), + Opacity(opacity: 0, child: Text('Invisible text')), + Opacity(opacity: 0, child: newImage()), + Offstage(offstage: true, child: Text('Offstage text')), + Offstage(offstage: true, child: newImage()), + ], ), ), ), @@ -43,17 +44,15 @@ Future pumpTestElement(WidgetTester tester) async { return TestWidgetsFlutterBinding.instance.rootElement!; } -Image newImage() => Image.memory( - Uint8List.fromList([ - 66, 77, 142, 0, 0, 0, 0, 0, 0, 0, 138, 0, 0, 0, 124, 0, 0, 0, 1, 0, - 0, 0, 255, 255, 255, 255, 1, 0, 32, 0, 3, 0, 0, 0, 4, 0, 0, 0, 19, - 11, 0, 0, 19, 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 0, 0, - 255, 0, 0, 255, 0, 0, 0, 0, 0, 0, 255, 66, 71, 82, 115, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 135, 135, 135, 255, - // This comment prevents dartfmt reformatting this to single-item lines. - ]), - width: 1, - height: 1, - ); +final testImageData = Uint8List.fromList([ + 66, 77, 142, 0, 0, 0, 0, 0, 0, 0, 138, 0, 0, 0, 124, 0, 0, 0, 1, 0, + 0, 0, 255, 255, 255, 255, 1, 0, 32, 0, 3, 0, 0, 0, 4, 0, 0, 0, 19, + 11, 0, 0, 19, 11, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 0, 0, + 255, 0, 0, 255, 0, 0, 0, 0, 0, 0, 255, 66, 71, 82, 115, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 135, 135, 135, 255, + // This comment prevents dartfmt reformatting this to single-item lines. +]); + +Image newImage() => Image.memory(testImageData, width: 1, height: 1); diff --git a/flutter/test/replay/widget_filter_test.dart b/flutter/test/replay/widget_filter_test.dart index 3e17f2b5b6..3f4136b90a 100644 --- a/flutter/test/replay/widget_filter_test.dart +++ b/flutter/test/replay/widget_filter_test.dart @@ -1,5 +1,8 @@ -import 'package:flutter/material.dart'; +import 'package:flutter/rendering.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter/widgets.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:sentry_flutter/sentry_flutter.dart'; import 'package:sentry_flutter/src/replay/widget_filter.dart'; import 'test_widget.dart'; @@ -7,12 +10,15 @@ import 'test_widget.dart'; void main() async { TestWidgetsFlutterBinding.ensureInitialized(); const defaultBounds = Rect.fromLTRB(0, 0, 1000, 1000); + final rootBundle = TestAssetBundle(); + final otherBundle = TestAssetBundle(); final createSut = ({bool redactImages = false, bool redactText = false}) => WidgetFilter( logger: (level, message, {exception, logger, stackTrace}) {}, redactImages: redactImages, redactText: redactText, + rootAssetBundle: rootBundle, ); group('redact text', () { @@ -47,6 +53,32 @@ void main() async { expect(sut.items.length, 2); }); + // Note: we cannot currently test actual asset images without either: + // - introducing assets to the package because those wouldn't get tree-shaken in final user apps (https://github.com/flutter/flutter/issues/64106) + // - using a mock asset bundle implementation, because the image widget loads AssetManifest.bin first and we don't have a way to mock that (https://github.com/flutter/flutter/issues/126860) + // Therefore we only check the function that actually decides whether the image is a built-in asset image. + for (var newAssetImage in [AssetImage.new, ExactAssetImage.new]) { + testWidgets( + 'recognizes ${newAssetImage('').runtimeType} from the root bundle', + (tester) async { + final sut = createSut(redactImages: true); + + expect(sut.isBuiltInAssetImage(newAssetImage('')), isTrue); + expect(sut.isBuiltInAssetImage(newAssetImage('', bundle: rootBundle)), + isTrue); + expect(sut.isBuiltInAssetImage(newAssetImage('', bundle: otherBundle)), + isFalse); + expect( + sut.isBuiltInAssetImage(newAssetImage('', + bundle: SentryAssetBundle(bundle: rootBundle))), + isTrue); + expect( + sut.isBuiltInAssetImage(newAssetImage('', + bundle: SentryAssetBundle(bundle: otherBundle))), + isFalse); + }); + } + testWidgets('does not redact text when disabled', (tester) async { final sut = createSut(redactImages: false); final element = await pumpTestElement(tester); @@ -63,3 +95,10 @@ void main() async { }); }); } + +class TestAssetBundle extends CachingAssetBundle { + @override + Future load(String key) async { + return ByteData(0); + } +} From 3adbea907f8c71f0046e66eed0d4206d1d2a8043 Mon Sep 17 00:00:00 2001 From: Ivan Dlugos <6349682+vaind@users.noreply.github.com> Date: Wed, 4 Sep 2024 23:32:00 +0200 Subject: [PATCH 2/2] feat: improve obscure rectangle fit/size (#2236) --- CHANGELOG.md | 3 +- flutter/lib/src/replay/widget_filter.dart | 40 +++++++++++++-------- flutter/test/replay/test_widget.dart | 24 ++++++++++++- flutter/test/replay/widget_filter_test.dart | 28 +++++++++++++-- 4 files changed, 77 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b12c189997..1d2fb4e2b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Features -- Session replay Alpha for Android and iOS ([#2208](https://github.com/getsentry/sentry-dart/pull/2208), [#2269](https://github.com/getsentry/sentry-dart/pull/2269)). +- Session replay Alpha for Android and iOS ([#2208](https://github.com/getsentry/sentry-dart/pull/2208), [#2269](https://github.com/getsentry/sentry-dart/pull/2269), [#2236](https://github.com/getsentry/sentry-dart/pull/2236)). To try out replay, you can set following options (access is limited to early access orgs on Sentry. If you're interested, [sign up for the waitlist](https://sentry.io/lp/mobile-replay-beta/)): @@ -27,6 +27,7 @@ ... options.allowUrls = ["^https://sentry.com.*\$", "my-custom-domain"]; options.denyUrls = ["^.*ends-with-this\$", "denied-url"]; + options.denyUrls = ["^.*ends-with-this\$", "denied-url"]; }, appRunner: () => runApp(MyApp()), ); diff --git a/flutter/lib/src/replay/widget_filter.dart b/flutter/lib/src/replay/widget_filter.dart index f269dd041f..1f66a42f1a 100644 --- a/flutter/lib/src/replay/widget_filter.dart +++ b/flutter/lib/src/replay/widget_filter.dart @@ -1,3 +1,4 @@ +import 'package:flutter/rendering.dart'; import 'package:flutter/services.dart'; import 'package:flutter/widgets.dart'; import 'package:meta/meta.dart'; @@ -77,25 +78,25 @@ class WidgetFilter { final renderObject = element.renderObject; if (renderObject is! RenderBox) { - _cantObscure(widget, "it's renderObject is not a RenderBox"); + _cantObscure(widget, "its renderObject is not a RenderBox"); return false; } - final size = element.size; - if (size == null) { - _cantObscure(widget, "it's renderObject has a null size"); - return false; + var rect = _boundingBox(renderObject); + + // If it's a clipped render object, use parent's offset and size. + // This helps with text fields which often have oversized render objects. + if (renderObject.parent is RenderStack) { + final renderStack = (renderObject.parent as RenderStack); + final clipBehavior = renderStack.clipBehavior; + if (clipBehavior == Clip.hardEdge || + clipBehavior == Clip.antiAlias || + clipBehavior == Clip.antiAliasWithSaveLayer) { + final clipRect = _boundingBox(renderStack); + rect = rect.intersect(clipRect); + } } - final offset = renderObject.localToGlobal(Offset.zero); - - final rect = Rect.fromLTWH( - offset.dx * _pixelRatio, - offset.dy * _pixelRatio, - size.width * _pixelRatio, - size.height * _pixelRatio, - ); - if (!rect.overlaps(_bounds)) { assert(() { logger(SentryLevel.debug, "WidgetFilter skipping offscreen: $widget"); @@ -151,6 +152,17 @@ class WidgetFilter { "WidgetFilter cannot obscure widget $widget: $message"); } } + + @pragma('vm:prefer-inline') + Rect _boundingBox(RenderBox box) { + final offset = box.localToGlobal(Offset.zero); + return Rect.fromLTWH( + offset.dx * _pixelRatio, + offset.dy * _pixelRatio, + box.size.width * _pixelRatio, + box.size.height * _pixelRatio, + ); + } } class WidgetFilterItem { diff --git a/flutter/test/replay/test_widget.dart b/flutter/test/replay/test_widget.dart index c5321ce750..d800a3ef12 100644 --- a/flutter/test/replay/test_widget.dart +++ b/flutter/test/replay/test_widget.dart @@ -33,6 +33,22 @@ Future pumpTestElement(WidgetTester tester, Opacity(opacity: 0, child: newImage()), Offstage(offstage: true, child: Text('Offstage text')), Offstage(offstage: true, child: newImage()), + Text(dummyText), + SizedBox( + width: 100, + height: 20, + child: Stack(children: [ + Positioned( + top: 0, + left: 0, + width: 50, + child: Text(dummyText)), + Positioned( + top: 0, + left: 0, + width: 50, + child: newImage(width: 500, height: 500)), + ])) ], ), ), @@ -55,4 +71,10 @@ final testImageData = Uint8List.fromList([ // This comment prevents dartfmt reformatting this to single-item lines. ]); -Image newImage() => Image.memory(testImageData, width: 1, height: 1); +Image newImage({double width = 1, double height = 1}) => Image.memory( + testImageData, + width: width, + height: height, + ); + +const dummyText = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit.'; diff --git a/flutter/test/replay/widget_filter_test.dart b/flutter/test/replay/widget_filter_test.dart index 3f4136b90a..e5787431bd 100644 --- a/flutter/test/replay/widget_filter_test.dart +++ b/flutter/test/replay/widget_filter_test.dart @@ -21,12 +21,15 @@ void main() async { rootAssetBundle: rootBundle, ); + boundsRect(WidgetFilterItem item) => + '${item.bounds.width.floor()}x${item.bounds.height.floor()}'; + group('redact text', () { testWidgets('redacts the correct number of elements', (tester) async { final sut = createSut(redactText: true); final element = await pumpTestElement(tester); sut.obscure(element, 1.0, defaultBounds); - expect(sut.items.length, 2); + expect(sut.items.length, 4); }); testWidgets('does not redact text when disabled', (tester) async { @@ -43,6 +46,17 @@ void main() async { sut.obscure(element, 1.0, Rect.fromLTRB(0, 0, 100, 100)); expect(sut.items.length, 1); }); + + testWidgets('correctly determines sizes', (tester) async { + final sut = createSut(redactText: true); + final element = await pumpTestElement(tester); + sut.obscure(element, 1.0, defaultBounds); + expect(sut.items.length, 4); + expect(boundsRect(sut.items[0]), '624x48'); + expect(boundsRect(sut.items[1]), '169x20'); + expect(boundsRect(sut.items[2]), '800x192'); + expect(boundsRect(sut.items[3]), '50x20'); + }); }); group('redact images', () { @@ -50,7 +64,7 @@ void main() async { final sut = createSut(redactImages: true); final element = await pumpTestElement(tester); sut.obscure(element, 1.0, defaultBounds); - expect(sut.items.length, 2); + expect(sut.items.length, 3); }); // Note: we cannot currently test actual asset images without either: @@ -93,6 +107,16 @@ void main() async { sut.obscure(element, 1.0, Rect.fromLTRB(0, 0, 500, 100)); expect(sut.items.length, 1); }); + + testWidgets('correctly determines sizes', (tester) async { + final sut = createSut(redactImages: true); + final element = await pumpTestElement(tester); + sut.obscure(element, 1.0, defaultBounds); + expect(sut.items.length, 3); + expect(boundsRect(sut.items[0]), '1x1'); + expect(boundsRect(sut.items[1]), '1x1'); + expect(boundsRect(sut.items[2]), '50x20'); + }); }); }