Merge branch 'main' into feat/capture-touch-breadcrumbs

getsentry · Sep 9, 2024 · b58b91d · b58b91d
2 parents 7563246 + 77db8d4
commit b58b91d
Show file tree

Hide file tree

Showing 8 changed files with 111 additions and 146 deletions.
diff --git 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/event_processor/url_filter/html_url_filter_event_processor.dart b/flutter/lib/src/event_processor/url_filter/html_url_filter_event_processor.dart
@@ -15,40 +15,23 @@ class WebUrlFilterEventProcessor implements UrlFilterEventProcessor {
     this._options,
   );
 
+  final html.Window _window = html.window;
   final SentryFlutterOptions _options;
 
   @override
   SentryEvent? apply(SentryEvent event, Hint hint) {
-    final frames = _getStacktraceFrames(event);
-    final lastPath = frames?.first?.absPath;
-
-    if (lastPath == null) {
-      return event;
-    }
+    final url = _window.location.toString();
 
     if (_options.allowUrls.isNotEmpty &&
-        !isMatchingRegexPattern(lastPath, _options.allowUrls)) {
+        !isMatchingRegexPattern(url, _options.allowUrls)) {
       return null;
     }
 
     if (_options.denyUrls.isNotEmpty &&
-        isMatchingRegexPattern(lastPath, _options.denyUrls)) {
+        isMatchingRegexPattern(url, _options.denyUrls)) {
       return null;
     }
 
     return event;
   }
-
-  Iterable<SentryStackFrame?>? _getStacktraceFrames(SentryEvent event) {
-    if (event.exceptions?.isNotEmpty == true) {
-      return event.exceptions?.first.stackTrace?.frames;
-    }
-    if (event.threads?.isNotEmpty == true) {
-      final stacktraces = event.threads?.map((e) => e.stacktrace);
-      return stacktraces
-          ?.where((element) => element != null)
-          .expand((element) => element!.frames);
-    }
-    return null;
-  }
 }
diff --git a/flutter/lib/src/event_processor/url_filter/web_url_filter_event_processor.dart b/flutter/lib/src/event_processor/url_filter/web_url_filter_event_processor.dart
@@ -17,40 +17,23 @@ class WebUrlFilterEventProcessor implements UrlFilterEventProcessor {
     this._options,
   );
 
+  final web.Window _window = web.window;
   final SentryFlutterOptions _options;
 
   @override
   SentryEvent? apply(SentryEvent event, Hint hint) {
-    final frames = _getStacktraceFrames(event);
-    final lastPath = frames?.first?.absPath;
-
-    if (lastPath == null) {
-      return event;
-    }
+    final url = _window.location.toString();
 
     if (_options.allowUrls.isNotEmpty &&
-        !isMatchingRegexPattern(lastPath, _options.allowUrls)) {
+        !isMatchingRegexPattern(url, _options.allowUrls)) {
       return null;
     }
 
     if (_options.denyUrls.isNotEmpty &&
-        isMatchingRegexPattern(lastPath, _options.denyUrls)) {
+        isMatchingRegexPattern(url, _options.denyUrls)) {
       return null;
     }
 
     return event;
   }
-
-  Iterable<SentryStackFrame?>? _getStacktraceFrames(SentryEvent event) {
-    if (event.exceptions?.isNotEmpty == true) {
-      return event.exceptions?.first.stackTrace?.frames;
-    }
-    if (event.threads?.isNotEmpty == true) {
-      final stacktraces = event.threads?.map((e) => e.stacktrace);
-      return stacktraces
-          ?.where((element) => element != null)
-          .expand((element) => element!.frames);
-    }
-    return null;
-  }
 }
diff --git 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/event_processor/url_filter/web_url_filter_event_processor_test.dart b/flutter/test/event_processor/url_filter/web_url_filter_event_processor_test.dart
@@ -7,6 +7,8 @@ import 'package:sentry_flutter/src/event_processor/url_filter/url_filter_event_p
 
 // can be tested on command line with
 // `flutter test --platform=chrome test/event_processor/url_filter/web_url_filter_event_processor_test.dart`
+// The URL looks something like this: http://localhost:58551/event_processor/url_filter/web_url_filter_event_processor_test.html
+
 void main() {
   group(UrlFilterEventProcessor, () {
     late Fixture fixture;
@@ -16,57 +18,53 @@ void main() {
     });
 
     test('returns event if no allowUrl and no denyUrl is set', () async {
-      SentryEvent? event = SentryEvent(
-        request: SentryRequest(
-          url: 'foo.bar',
-        ),
-      );
+      final event = SentryEvent();
+      final eventProcessor = fixture.getSut();
 
-      var eventProcessor = fixture.getSut();
-      event = await eventProcessor.apply(event, Hint());
+      final processedEvent = await eventProcessor.apply(event, Hint());
 
-      expect(event, isNotNull);
+      expect(processedEvent, isNotNull);
     });
 
     test('returns null if allowUrl is set and does not match with url',
         () async {
-      final event = _createEventWithException("foo.bar");
+      final event = SentryEvent();
       fixture.options.allowUrls = ["another.url"];
+      final eventProcessor = fixture.getSut();
 
-      var eventProcessor = fixture.getSut();
       final processedEvent = await eventProcessor.apply(event, Hint());
 
       expect(processedEvent, isNull);
     });
 
     test('returns event if allowUrl is set and does partially match with url',
         () async {
-      final event = _createEventWithException("foo.bar");
-      fixture.options.allowUrls = ["bar"];
+      final event = SentryEvent();
+      fixture.options.allowUrls = ["event_processor_test"];
+      final eventProcessor = fixture.getSut();
 
-      var eventProcessor = fixture.getSut();
       final processedEvent = await eventProcessor.apply(event, Hint());
 
       expect(processedEvent, isNotNull);
     });
 
     test('returns event if denyUrl is set and does not match with url',
         () async {
-      final event = _createEventWithException("foo.bar");
+      final event = SentryEvent();
       fixture.options.denyUrls = ["another.url"];
+      final eventProcessor = fixture.getSut();
 
-      var eventProcessor = fixture.getSut();
       final processedEvent = await eventProcessor.apply(event, Hint());
 
       expect(processedEvent, isNotNull);
     });
 
     test('returns null if denyUrl is set and partially matches with url',
         () async {
-      final event = _createEventWithException("foo.bar");
-      fixture.options.denyUrls = ["bar"];
+      final event = SentryEvent();
+      fixture.options.denyUrls = ["event_processor_test"];
+      final eventProcessor = fixture.getSut();
 
-      var eventProcessor = fixture.getSut();
       final processedEvent = await eventProcessor.apply(event, Hint());
 
       expect(processedEvent, isNull);
@@ -75,13 +73,11 @@ void main() {
     test(
         'returns null if it is part of the allowed domain, but blocked for subdomain',
         () async {
-      final event = _createEventWithException(
-          "this.is/a/special/url/for-testing/this-feature");
-
-      fixture.options.allowUrls = ["^this.is/.*\$"];
-      fixture.options.denyUrls = ["special"];
+      final event = SentryEvent();
+      fixture.options.allowUrls = [".*localhost.*\$"];
+      fixture.options.denyUrls = ["event"];
+      final eventProcessor = fixture.getSut();
 
-      var eventProcessor = fixture.getSut();
       final processedEvent = await eventProcessor.apply(event, Hint());
 
       expect(processedEvent, isNull);
@@ -90,12 +86,11 @@ void main() {
     test(
         'returns event if it is part of the allowed domain, and not of the blocked for subdomain',
         () async {
-      final event = _createEventWithException(
-          "this.is/a/test/url/for-testing/this-feature");
-      fixture.options.allowUrls = ["^this.is/.*\$"];
+      final event = SentryEvent();
+      fixture.options.allowUrls = [".*localhost.*\$"];
       fixture.options.denyUrls = ["special"];
+      final eventProcessor = fixture.getSut();
 
-      var eventProcessor = fixture.getSut();
       final processedEvent = await eventProcessor.apply(event, Hint());
 
       expect(processedEvent, isNotNull);
@@ -104,60 +99,15 @@ void main() {
     test(
         'returns null if it is not part of the allowed domain, and not of the blocked for subdomain',
         () async {
-      final event = _createEventWithException(
-          "another.url/for/a/test/testing/this-feature");
+      final event = SentryEvent();
       fixture.options.allowUrls = ["^this.is/.*\$"];
       fixture.options.denyUrls = ["special"];
+      final eventProcessor = fixture.getSut();
 
-      var eventProcessor = fixture.getSut();
       final processedEvent = await eventProcessor.apply(event, Hint());
 
       expect(processedEvent, isNull);
     });
-
-    test(
-        'returns event if denyUrl is set and not matching with url of first exception',
-        () async {
-      final frame1 = SentryStackFrame(absPath: "test.url");
-      final st1 = SentryStackTrace(frames: [frame1]);
-      final exception1 = SentryException(
-          type: "test-type", value: "test-value", stackTrace: st1);
-
-      final frame2 = SentryStackFrame(absPath: "foo.bar");
-      final st2 = SentryStackTrace(frames: [frame2]);
-      final exception2 = SentryException(
-          type: "test-type", value: "test-value", stackTrace: st2);
-
-      SentryEvent event = SentryEvent(exceptions: [exception1, exception2]);
-
-      fixture.options.denyUrls = ["bar"];
-
-      var eventProcessor = fixture.getSut();
-      final processedEvent = await eventProcessor.apply(event, Hint());
-
-      expect(processedEvent, isNotNull);
-    });
-
-    test(
-        'returns event if denyUrl is set and not matching with url of first stacktraceframe',
-        () async {
-      final frame1 = SentryStackFrame(absPath: "test.url");
-      final st1 = SentryStackTrace(frames: [frame1]);
-      final thread1 = SentryThread(stacktrace: st1);
-
-      final frame2 = SentryStackFrame(absPath: "foo.bar");
-      final st2 = SentryStackTrace(frames: [frame2]);
-      final thread2 = SentryThread(stacktrace: st2);
-
-      SentryEvent event = SentryEvent(threads: [thread1, thread2]);
-
-      fixture.options.denyUrls = ["bar"];
-
-      var eventProcessor = fixture.getSut();
-      final processedEvent = await eventProcessor.apply(event, Hint());
-
-      expect(processedEvent, isNotNull);
-    });
   });
 }
 
@@ -167,13 +117,3 @@ class Fixture {
     return UrlFilterEventProcessor(options);
   }
 }
-
-SentryEvent _createEventWithException(String url) {
-  final frame = SentryStackFrame(absPath: url);
-  final st = SentryStackTrace(frames: [frame]);
-  final exception =
-      SentryException(type: "test-type", value: "test-value", stackTrace: st);
-  SentryEvent event = SentryEvent(exceptions: [exception]);
-
-  return event;
-}
diff --git a/flutter/test/replay/test_widget.dart b/flutter/test/replay/test_widget.dart
@@ -33,6 +33,22 @@ Future<Element> 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.';