Remove breadcrumbs from transaction to avoid duplication (#1366)

Author: denrase

CHANGELOG.md

@@ -6,6 +6,7 @@
 
 - Fix Dart web builds breaking due to `dart:io` imports when using `SentryIsolate` or `SentryIsolateExtension` ([#1371](https://github.com/getsentry/sentry-dart/pull/1371))
   - When using `SentryIsolate` or `SentryIsolateExtension`, import `sentry_io.dart`.
+- Remove breadcrumbs from transaction to avoid duplication ([#1366](https://github.com/getsentry/sentry-dart/pull/1366))
 
 ## 7.4.0
 

dart/lib/src/sentry_client.dart

@@ -108,18 +108,7 @@ class SentryClient {
       return _sentryId;
     }
 
-    if (_options.platformChecker.platform.isAndroid &&
-        _options.enableScopeSync) {
-      /*
-      We do this to avoid duplicate breadcrumbs on Android as sentry-android applies the breadcrumbs
-      from the native scope onto every envelope sent through it. This scope will contain the breadcrumbs
-      sent through the scope sync feature. This causes duplicate breadcrumbs.
-      We then remove the breadcrumbs in all cases but if it is handled == false,
-      this is a signal that the app would crash and android would lose the breadcrumbs by the time the app is restarted to read
-      the envelope.
-      */
-      preparedEvent = _eventWithRemovedBreadcrumbsIfHandled(preparedEvent);
-    }
+    preparedEvent = _eventWithoutBreadcrumbsIfNeeded(preparedEvent);
 
     var attachments = List<SentryAttachment>.from(scope?.attachments ?? []);
     var screenshot = hint.screenshot;
@@ -329,6 +318,8 @@ class SentryClient {
       return _sentryId;
     }
 
+    preparedTransaction = _eventWithoutBreadcrumbsIfNeeded(preparedTransaction);
+
     final attachments = scope?.attachments
         .where((element) => element.addToTransactions)
         .toList();
@@ -462,18 +453,34 @@ class SentryClient {
     _options.recorder.recordLostEvent(reason, category);
   }
 
-  SentryEvent _eventWithRemovedBreadcrumbsIfHandled(SentryEvent event) {
+  T _eventWithoutBreadcrumbsIfNeeded<T extends SentryEvent>(T event) {
+    if (_shouldRemoveBreadcrumbs(event)) {
+      return event.copyWith(breadcrumbs: []) as T;
+    } else {
+      return event;
+    }
+  }
+
+  /// We do this to avoid duplicate breadcrumbs on Android as sentry-android applies the breadcrumbs
+  /// from the native scope onto every envelope sent through it. This scope will contain the breadcrumbs
+  /// sent through the scope sync feature. This causes duplicate breadcrumbs.
+  /// We then remove the breadcrumbs in all cases but if it is handled == false,
+  /// this is a signal that the app would crash and android would lose the breadcrumbs by the time the app is restarted to read
+  /// the envelope.
+  bool _shouldRemoveBreadcrumbs(SentryEvent event) {
+    final isAndroid = _options.platformChecker.platform.isAndroid;
+    final enableScopeSync = _options.enableScopeSync;
+
+    if (!isAndroid || !enableScopeSync) {
+      return false;
+    }
+
     final mechanisms =
         (event.exceptions ?? []).map((e) => e.mechanism).whereType<Mechanism>();
     final hasNoMechanism = mechanisms.isEmpty;
     final hasOnlyHandledMechanism =
         mechanisms.every((e) => (e.handled ?? true));
-
-    if (hasNoMechanism || hasOnlyHandledMechanism) {
-      return event.copyWith(breadcrumbs: []);
-    } else {
-      return event;
-    }
+    return hasNoMechanism || hasOnlyHandledMechanism;
   }
 
   Future<SentryId?> _attachClientReportsAndSend(SentryEnvelope envelope) {

dart/test/sentry_client_test.dart

@@ -1154,7 +1154,28 @@ void main() {
       fixture = Fixture();
     });
 
-    test('Clears breadcrumbs on Android if mechanism.handled is true',
+    test('Clears breadcrumbs on Android for transaction', () async {
+      fixture.options.enableScopeSync = true;
+      fixture.options.platformChecker =
+          MockPlatformChecker(platform: MockPlatform.android());
+
+      final client = fixture.getSut();
+      final transaction = SentryTransaction(
+        fixture.tracer,
+        breadcrumbs: [
+          Breadcrumb(),
+        ],
+      );
+      await client.captureTransaction(transaction);
+
+      final capturedEnvelope = (fixture.transport).envelopes.first;
+      final capturedTransaction =
+          await transactionFromEnvelope(capturedEnvelope);
+
+      expect((capturedTransaction['breadcrumbs'] ?? []).isEmpty, true);
+    });
+
+    test('Clears breadcrumbs on Android if mechanism.handled is true for event',
         () async {
       fixture.options.enableScopeSync = true;
       fixture.options.platformChecker =
@@ -1181,7 +1202,7 @@ void main() {
       expect((capturedEvent.breadcrumbs ?? []).isEmpty, true);
     });
 
-    test('Clears breadcrumbs on Android if mechanism.handled is null',
+    test('Clears breadcrumbs on Android if mechanism.handled is null for event',
         () async {
       fixture.options.enableScopeSync = true;
       fixture.options.platformChecker =
@@ -1205,7 +1226,8 @@ void main() {
       expect((capturedEvent.breadcrumbs ?? []).isEmpty, true);
     });
 
-    test('Clears breadcrumbs on Android if theres no mechanism', () async {
+    test('Clears breadcrumbs on Android if theres no mechanism for event',
+        () async {
       fixture.options.enableScopeSync = true;
       fixture.options.platformChecker =
           MockPlatformChecker(platform: MockPlatform.android());
@@ -1227,7 +1249,8 @@ void main() {
       expect((capturedEvent.breadcrumbs ?? []).isEmpty, true);
     });
 
-    test('Does not clear breadcrumbs on Android if mechanism.handled is false',
+    test(
+        'Does not clear breadcrumbs on Android if mechanism.handled is false for event',
         () async {
       fixture.options.enableScopeSync = true;
       fixture.options.platformChecker =
@@ -1255,7 +1278,7 @@ void main() {
     });
 
     test(
-        'Does not clear breadcrumbs on Android if any mechanism.handled is false',
+        'Does not clear breadcrumbs on Android if any mechanism.handled is false for event',
         () async {
       fixture.options.enableScopeSync = true;
       fixture.options.platformChecker =