Add debounce to ScreenshotWidget

CHANGELOG.md

@@ -10,7 +10,9 @@
     - SDK creates a synthetic trace using `StackTrace.current`
     - Internal SDK frames are removed to reduce noise
   - Original stack traces (when provided) are left unchanged
-
+- Add debounce to `ScreenshotWidget` ([#2368](https://github.com/getsentry/sentry-dart/pull/2368))
+  - This is a breaking change. Replace `BeforeScreenshotCallback` with new `BeforeCaptureCallback`.
+
 ### Features
 
 - Improve frame tracking accuracy ([#2372](https://github.com/getsentry/sentry-dart/pull/2372))

flutter/lib/src/event_processor/screenshot_event_processor.dart

@@ -11,10 +11,19 @@ import '../sentry_flutter_options.dart';
 import '../renderer/renderer.dart';
 import 'package:flutter/widgets.dart' as widget;
 
+import '../utils/debouncer.dart';
+
 class ScreenshotEventProcessor implements EventProcessor {
   final SentryFlutterOptions _options;
-
-  ScreenshotEventProcessor(this._options);
+  final Debouncer _debouncer;
+
+  ScreenshotEventProcessor(this._options)
+      : _debouncer = Debouncer(
+          // ignore: invalid_use_of_internal_member
+          _options.clock,
+          waitTimeMs: 2000,
+          debounceOnFirstTry: false,
+        );
 
   @override
   Future<SentryEvent?> apply(SentryEvent event, Hint hint) async {
@@ -32,10 +41,13 @@ class ScreenshotEventProcessor implements EventProcessor {
       return event; // No need to attach screenshot of feedback form.
     }
 
+    // skip capturing in case of debouncing (=too many frequent capture requests)
+    // the BeforeCaptureCallback may overrules the debouncing decision
+    final shouldDebounce = _debouncer.shouldDebounce();
     final beforeScreenshot = _options.beforeScreenshot;
     if (beforeScreenshot != null) {
       try {
-        final result = beforeScreenshot(event, hint: hint);
+        final result = beforeScreenshot(event, hint, shouldDebounce);
         bool takeScreenshot;
         if (result is Future<bool>) {
           takeScreenshot = await result;
@@ -56,6 +68,8 @@ class ScreenshotEventProcessor implements EventProcessor {
           rethrow;
         }
       }
+    } else if (shouldDebounce) {
+      return event;
     }
 
     final renderer = _options.rendererWrapper.getRenderer();

flutter/lib/src/sentry_flutter_options.dart

@@ -198,7 +198,7 @@ class SentryFlutterOptions extends SentryOptions {
   /// Sets a callback which is executed before capturing screenshots. Only
   /// relevant if `attachScreenshot` is set to true. When false is returned
   /// from the function, no screenshot will be attached.
-  BeforeScreenshotCallback? beforeScreenshot;
+  BeforeCaptureCallback? beforeScreenshot;
 
   /// Enable or disable automatic breadcrumbs for User interactions Using [Listener]
   ///
@@ -387,9 +387,29 @@ class _SentryFlutterExperimentalOptions {
   final replay = SentryReplayOptions();
 }
 
-/// Callback being executed in [ScreenshotEventProcessor], deciding if a
-/// screenshot should be recorded and attached.
-typedef BeforeScreenshotCallback = FutureOr<bool> Function(
-  SentryEvent event, {
-  Hint? hint,
-});
+/// A callback which can be used to suppress capturing of screenshots.
+/// It's called in [ScreenshotEventProcessor] if screenshots are enabled.
+/// This gives more fine-grained control over when capturing should be performed,
+/// e.g., only capture screenshots for fatal events or override any debouncing for important events.
+///
+/// Since capturing can be resource-intensive, the debounce parameter should be respected if possible.
+///
+/// Example:
+/// ```dart
+/// if (debounce) {
+///   return false;
+/// } else {
+///   // check event and hint
+/// }
+/// ```
+///
+/// [event] is the event to be checked.
+/// [hint] provides additional hints.
+/// [debounce] indicates if capturing is marked for being debounced.
+///
+/// Returns `true` if capturing should be performed, otherwise `false`.
+typedef BeforeCaptureCallback = FutureOr<bool> Function(
+  SentryEvent event,
+  Hint hint,
+  bool debounce,
+);

flutter/lib/src/utils/debouncer.dart

@@ -1,20 +1,30 @@
-import 'dart:async';
-import 'package:flutter/foundation.dart';
 import 'package:meta/meta.dart';
+import 'package:sentry/sentry.dart';
 
 @internal
 class Debouncer {
-  final int milliseconds;
-  Timer? _timer;
+  final ClockProvider clockProvider;
+  final int waitTimeMs;
+  final bool debounceOnFirstTry;
+  DateTime? _lastExecutionTime;
 
-  Debouncer({required this.milliseconds});
+  Debouncer(this.clockProvider,
+      {this.waitTimeMs = 2000, this.debounceOnFirstTry = false});
 
-  void run(VoidCallback action) {
-    _timer?.cancel();
-    _timer = Timer(Duration(milliseconds: milliseconds), action);
-  }
+  bool shouldDebounce() {
+    final currentTime = clockProvider();
+    final lastExecutionTime = _lastExecutionTime;
+    _lastExecutionTime = currentTime;
+
+    if (lastExecutionTime == null && debounceOnFirstTry) {
+      return true;
+    }
+
+    if (lastExecutionTime != null &&
+        currentTime.difference(lastExecutionTime).inMilliseconds < waitTimeMs) {
+      return true;
+    }
 
-  void dispose() {
-    _timer?.cancel();
+    return false;
   }
 }

flutter/lib/src/widgets_binding_observer.dart

@@ -25,7 +25,13 @@
     required SentryFlutterOptions options,
   })  : _hub = hub ?? HubAdapter(),
         _options = options,
-        _screenSizeStreamController = StreamController(sync: true) {
+        _screenSizeStreamController = StreamController(sync: true),
+        _didChangeMetricsDebouncer = Debouncer(
+          // ignore: invalid_use_of_internal_member
+          options.clock,
+          waitTimeMs: 100,
+          debounceOnFirstTry: true,
+        ) {
     if (_options.enableWindowMetricBreadcrumbs) {
       _screenSizeStreamController.stream
           .map(
@@ -47,12 +53,11 @@
 
   final Hub _hub;
   final SentryFlutterOptions _options;
+  final Debouncer _didChangeMetricsDebouncer;
 
   // ignore: deprecated_member_use
   final StreamController<SingletonFlutterWindow?> _screenSizeStreamController;
 
-  final _didChangeMetricsDebouncer = Debouncer(milliseconds: 100);
-
   /// This method records lifecycle events.
   /// It tries to mimic the behavior of ActivityBreadcrumbsIntegration of Sentry
   /// Android for lifecycle events.
@@ -92,11 +97,11 @@
       return;
     }
 
-    _didChangeMetricsDebouncer.run(() {
-      // ignore: deprecated_member_use
+    final shouldDebounce = _didChangeMetricsDebouncer.shouldDebounce();
+    if (!shouldDebounce) {
       final window = _options.bindingUtils.instance?.window;
       _screenSizeStreamController.add(window);
-    });
+    }
   }
 
   void _onScreenSizeChanged(Map<String, dynamic> data) {

flutter/test/event_processor/screenshot_event_processor_test.dart

@@ -129,7 +129,8 @@ void main() {
   group('beforeScreenshot', () {
     testWidgets('does add screenshot if beforeScreenshot returns true',
         (tester) async {
-      fixture.options.beforeScreenshot = (SentryEvent event, {Hint? hint}) {
+      fixture.options.beforeScreenshot =
+          (SentryEvent event, Hint hint, bool shouldDebounce) {
         return true;
       };
       await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit,
@@ -139,7 +140,7 @@ void main() {
     testWidgets('does add screenshot if async beforeScreenshot returns true',
         (tester) async {
       fixture.options.beforeScreenshot =
-          (SentryEvent event, {Hint? hint}) async {
+          (SentryEvent event, Hint hint, bool shouldDebounce) async {
         await Future<void>.delayed(Duration(milliseconds: 1));
         return true;
       };
@@ -149,7 +150,8 @@ void main() {
 
     testWidgets('does not add screenshot if beforeScreenshot returns false',
         (tester) async {
-      fixture.options.beforeScreenshot = (SentryEvent event, {Hint? hint}) {
+      fixture.options.beforeScreenshot =
+          (SentryEvent event, Hint hint, bool shouldDebounce) {
         return false;
       };
       await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit,
@@ -160,7 +162,7 @@ void main() {
         'does not add screenshot if async beforeScreenshot returns false',
         (tester) async {
       fixture.options.beforeScreenshot =
-          (SentryEvent event, {Hint? hint}) async {
+          (SentryEvent event, Hint hint, bool shouldDebounce) async {
         await Future<void>.delayed(Duration(milliseconds: 1));
         return false;
       };
@@ -171,7 +173,8 @@ void main() {
     testWidgets('does add screenshot if beforeScreenshot throws',
         (tester) async {
       fixture.options.automatedTestMode = false;
-      fixture.options.beforeScreenshot = (SentryEvent event, {Hint? hint}) {
+      fixture.options.beforeScreenshot =
+          (SentryEvent event, Hint hint, bool shouldDebounce) {
         throw Error();
       };
       await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit,
@@ -182,20 +185,58 @@ void main() {
         (tester) async {
       fixture.options.automatedTestMode = false;
       fixture.options.beforeScreenshot =
-          (SentryEvent event, {Hint? hint}) async {
+          (SentryEvent event, Hint hint, bool shouldDebounce) async {
         await Future<void>.delayed(Duration(milliseconds: 1));
         throw Error();
       };
       await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit,
           added: true, isWeb: false);
     });
 
+    testWidgets('does add screenshot event if shouldDebounce true',
+        (tester) async {
+      await tester.runAsync(() async {
+        var shouldDebounceValues = <bool>[];
+
+        fixture.options.beforeScreenshot =
+            (SentryEvent event, Hint hint, bool shouldDebounce) {
+          shouldDebounceValues.add(shouldDebounce);
+          return true;
+        };
+
+        final sut = fixture.getSut(FlutterRenderer.canvasKit, false);
+
+        await tester.pumpWidget(
+          SentryScreenshotWidget(
+            child: Text(
+              'Catching Pokémon is a snap!',
+              textDirection: TextDirection.ltr,
+            ),
+          ),
+        );
+
+        final event = SentryEvent(throwable: Exception());
+        final hintOne = Hint();
+        final hintTwo = Hint();
+
+        await sut.apply(event, hintOne);
+        await sut.apply(event, hintTwo);
+
+        expect(hintOne.screenshot, isNotNull);
+        expect(hintTwo.screenshot, isNotNull);
+
+        expect(shouldDebounceValues[0], false);
+        expect(shouldDebounceValues[1], true);
+      });
+    });
+
     testWidgets('passes event & hint to beforeScreenshot callback',
         (tester) async {
       SentryEvent? beforeScreenshotEvent;
       Hint? beforeScreenshotHint;
 
-      fixture.options.beforeScreenshot = (SentryEvent event, {Hint? hint}) {
+      fixture.options.beforeScreenshot =
+          (SentryEvent event, Hint hint, bool shouldDebounce) {
         beforeScreenshotEvent = event;
         beforeScreenshotHint = hint;
         return true;
@@ -208,6 +249,86 @@ void main() {
       expect(beforeScreenshotHint, hint);
     });
   });
+
+  group("debounce", () {
+    testWidgets("limits added screenshots within debounce timeframe",
+        (tester) async {
+      // Run with real async https://stackoverflow.com/a/54021863
+      await tester.runAsync(() async {
+        var firstCall = true;
+        // ignore: invalid_use_of_internal_member
+        fixture.options.clock = () {
+          if (firstCall) {
+            firstCall = false;
+            return DateTime.fromMillisecondsSinceEpoch(0);
+          } else {
+            return DateTime.fromMillisecondsSinceEpoch(2000 - 1);
+          }
+        };
+
+        final sut = fixture.getSut(FlutterRenderer.canvasKit, false);
+
+        await tester.pumpWidget(SentryScreenshotWidget(
+            child: Text('Catching Pokémon is a snap!',
+                textDirection: TextDirection.ltr)));
+
+        final throwable = Exception();
+
+        final firstEvent = SentryEvent(throwable: throwable);
+        final firstHint = Hint();
+
+        final secondEvent = SentryEvent(throwable: throwable);
+        final secondHint = Hint();
+
+        await sut.apply(firstEvent, firstHint);
+        await sut.apply(secondEvent, secondHint);
+
+        expect(firstHint.screenshot, isNotNull);
+        expect(secondHint.screenshot, isNull);
+      });
+    });
+
+    testWidgets("adds screenshots after debounce timeframe", (tester) async {
+      // Run with real async https://stackoverflow.com/a/54021863
+      await tester.runAsync(() async {
+        var firstCall = true;
+        // ignore: invalid_use_of_internal_member
+        fixture.options.clock = () {
+          if (firstCall) {
+            firstCall = false;
+            return DateTime.fromMillisecondsSinceEpoch(0);
+          } else {
+            return DateTime.fromMillisecondsSinceEpoch(2001);
+          }
+        };
+
+        final sut = fixture.getSut(FlutterRenderer.canvasKit, false);
+
+        await tester.pumpWidget(
+          SentryScreenshotWidget(
+            child: Text(
+              'Catching Pokémon is a snap!',
+              textDirection: TextDirection.ltr,
+            ),
+          ),
+        );
+
+        final throwable = Exception();
+
+        final firstEvent = SentryEvent(throwable: throwable);
+        final firstHint = Hint();
+
+        final secondEvent = SentryEvent(throwable: throwable);
+        final secondHint = Hint();
+
+        await sut.apply(firstEvent, firstHint);
+        await sut.apply(secondEvent, secondHint);
+
+        expect(firstHint.screenshot, isNotNull);
+        expect(secondHint.screenshot, isNotNull);
+      });
+    });
+  });
 }
 
 class Fixture {