Send Less Client Reports When Rate Limited

CHANGELOG.md

@@ -24,6 +24,8 @@
   );
   ```
 
+- Send Less Client Reports When Rate Limited ([#2380](https://github.com/getsentry/sentry-dart/pull/2380))
+
 ### Enhancements
 
 - Cache parsed DSN ([#2365](https://github.com/getsentry/sentry-dart/pull/2365))

dart/lib/src/sentry_client.dart

@@ -20,6 +20,7 @@ import 'sentry_options.dart';
 import 'sentry_stack_trace_factory.dart';
 import 'sentry_trace_context_header.dart';
 import 'sentry_user_feedback.dart';
+import 'transport/client_report_transport.dart';
 import 'transport/data_category.dart';
 import 'transport/http_transport.dart';
 import 'transport/noop_transport.dart';
@@ -54,10 +55,17 @@ class SentryClient {
     if (options.sendClientReports) {
       options.recorder = ClientReportRecorder(options.clock);
     }
+    RateLimiter? rateLimiter;
     if (options.transport is NoOpTransport) {
-      final rateLimiter = RateLimiter(options);
+      rateLimiter = RateLimiter(options);
       options.transport = HttpTransport(options, rateLimiter);
     }
+    // rateLimiter is null if FileSystemTransport is active since Native SDKs take care of rate limiting
+    options.transport = ClientReportTransport(
+      rateLimiter,
+      options,
+      options.transport,
+    );
     // TODO: Web might change soon to use the JS SDK so we can remove it here later on
     final enableFlutterSpotlight = (options.spotlight.enabled &&
         (options.platformChecker.isWeb ||
@@ -427,7 +435,7 @@ class SentryClient {
 
   /// Reports the [envelope] to Sentry.io.
   Future<SentryId?> captureEnvelope(SentryEnvelope envelope) {
-    return _attachClientReportsAndSend(envelope);
+    return _options.transport.send(envelope);
   }
 
   /// Reports the [userFeedback] to Sentry.io.
@@ -439,7 +447,7 @@ class SentryClient {
       _options.sdk,
       dsn: _options.dsn,
     );
-    return _attachClientReportsAndSend(envelope);
+    return _options.transport.send(envelope);
   }
 
   /// Reports the [feedback] to Sentry.io.
@@ -469,7 +477,7 @@ class SentryClient {
       _options.sdk,
       dsn: _options.dsn,
     );
-    final id = await _attachClientReportsAndSend(envelope);
+    final id = await _options.transport.send(envelope);
     return id ?? SentryId.empty();
   }
 
@@ -621,10 +629,4 @@ class SentryClient {
     }
     return DataCategory.error;
   }
-
-  Future<SentryId?> _attachClientReportsAndSend(SentryEnvelope envelope) {
-    final clientReport = _options.recorder.flush();
-    envelope.addClientReport(clientReport);
-    return _options.transport.send(envelope);
-  }
 }

dart/lib/src/transport/client_report_transport.dart

@@ -0,0 +1,56 @@
+import 'package:meta/meta.dart';
+import '../../sentry.dart';
+import '../sentry_envelope_header.dart';
+import 'rate_limiter.dart';
+
+/// Decorator that handles attaching of client reports in tandem with rate
+/// limiting. The rate limiter is optional.
+@internal
+class ClientReportTransport implements Transport {
+  final RateLimiter? _rateLimiter;
+  final SentryOptions _options;
+  final Transport _transport;
+
+  ClientReportTransport(this._rateLimiter, this._options, this._transport);
+
+  @visibleForTesting
+  get rateLimiter => _rateLimiter;
+
+  int _numberOfDroppedEnvelopes = 0;
+
+  @visibleForTesting
+  get numberOfDroppedEvents => _numberOfDroppedEnvelopes;
+
+  @override
+  Future<SentryId?> send(SentryEnvelope envelope) async {
+    final rateLimiter = _rateLimiter;
+
+    SentryEnvelope? filteredEnvelope = envelope;
+    if (rateLimiter != null) {
+      filteredEnvelope = rateLimiter.filter(envelope);
+    }
+    if (filteredEnvelope == null) {
+      _numberOfDroppedEnvelopes += 1;
+    }
+    if (_numberOfDroppedEnvelopes >= 10) {
+      // Create new envelope that could only contain client reports
+      filteredEnvelope = SentryEnvelope(
+        SentryEnvelopeHeader(SentryId.newId(), _options.sdk),
+        [],
+      );
+    }
+    if (filteredEnvelope == null) {
+      return SentryId.empty();
+    }
+    _numberOfDroppedEnvelopes = 0;
+
+    final clientReport = _options.recorder.flush();
+    filteredEnvelope.addClientReport(clientReport);
+
+    if (filteredEnvelope.items.isNotEmpty) {
+      return _transport.send(filteredEnvelope);
+    } else {
+      return SentryId.empty();
+    }
+  }
+}

dart/lib/src/transport/http_transport.dart

@@ -35,14 +35,9 @@ class HttpTransport implements Transport {
 
   @override
   Future<SentryId?> send(SentryEnvelope envelope) async {
-    final filteredEnvelope = _rateLimiter.filter(envelope);
-    if (filteredEnvelope == null) {
-      return SentryId.empty();
-    }
-    filteredEnvelope.header.sentAt = _options.clock();
+    envelope.header.sentAt = _options.clock();
 
-    final streamedRequest =
-        await _requestHandler.createRequest(filteredEnvelope);
+    final streamedRequest = await _requestHandler.createRequest(envelope);
 
     final response = await _options.httpClient
         .send(streamedRequest)

dart/test/exception_identifier_test.dart

@@ -126,7 +126,7 @@ void main() {
 
   group('Integration test', () {
     setUp(() {
-      fixture.options.transport = MockTransport();
+      fixture.options.transport = fixture.mockTransport;
     });
 
     test(
@@ -139,7 +139,7 @@ void main() {
 
       await client.captureException(ObfuscatedException());
 
-      final transport = fixture.options.transport as MockTransport;
+      final transport = fixture.mockTransport;
       final capturedEnvelope = transport.envelopes.first;
       final capturedEvent = await eventFromEnvelope(capturedEnvelope);
 
@@ -154,7 +154,7 @@ void main() {
 
       await client.captureException(ObfuscatedException());
 
-      final transport = fixture.options.transport as MockTransport;
+      final transport = fixture.mockTransport;
       final capturedEnvelope = transport.envelopes.first;
       final capturedEvent = await eventFromEnvelope(capturedEnvelope);
 
@@ -165,6 +165,7 @@ void main() {
 }
 
 class Fixture {
+  final mockTransport = MockTransport();
   SentryOptions options = defaultTestOptions();
 }
 

dart/test/mocks/mock_transport.dart

@@ -16,6 +16,8 @@ class MockTransport implements Transport {
     return _calls;
   }
 
+  bool parseFromEnvelope = true;
+
   bool called(int calls) {
     return _calls == calls;
   }
@@ -28,7 +30,9 @@ class MockTransport implements Transport {
     // failure causes. Instead, we log them and check on access to [calls].
     try {
       envelopes.add(envelope);
-      await _eventFromEnvelope(envelope);
+      if (parseFromEnvelope) {
+        await _eventFromEnvelope(envelope);
+      }
     } catch (e, stack) {
       _exceptions += '$e\n$stack\n\n';
       rethrow;