fix(replay): Don't capture replays for events dropped in beforeSend

CHANGELOG.md

@@ -2,6 +2,16 @@
 
 ## Unreleased
 
+### Features
+
+- Structured Logs: Flush logs on SDK flush/close (#5834)
+
+### Fixes
+
+- Don't capture replays for events dropped in `beforeSend` (#5916)
+
+## 8.54.1-alpha.1
+
 ### Fixes
 
 - Add padding to tap area of widget button (#5949)

Sources/Sentry/SentryClient.m

@@ -834,22 +834,6 @@ - (SentryEvent *_Nullable)prepareEvent:(SentryEvent *)event
         currentSpanCount = 0;
     }
 
-    event = [self callEventProcessors:event];
-    if (event == nil) {
-        [self recordLost:eventIsNotATransaction reason:kSentryDiscardReasonEventProcessor];
-        if (eventIsATransaction) {
-            // We dropped the whole transaction, the dropped count includes all child spans + 1 root
-            // span
-            [self recordLostSpanWithReason:kSentryDiscardReasonEventProcessor
-                                  quantity:currentSpanCount + 1];
-        }
-    } else {
-        if (eventIsATransactionClass) {
-            [self recordPartiallyDroppedSpans:(SentryTransaction *)event
-                                   withReason:kSentryDiscardReasonEventProcessor
-                         withCurrentSpanCount:&currentSpanCount];
-        }
-    }
     if (event != nil && eventIsATransaction && self.options.beforeSendSpan != nil) {
         SentryTransaction *transaction = (SentryTransaction *)event;
         NSMutableArray<id<SentrySpan>> *processedSpans = [NSMutableArray array];
@@ -887,6 +871,27 @@ - (SentryEvent *_Nullable)prepareEvent:(SentryEvent *)event
         }
     }
 
+    if (event != nil) {
+        // if the event is dropped by beforeSend we should not execute event processors as they
+        // might trigger e.g. unnecessary replay capture
+        event = [self callEventProcessors:event];
+        if (event == nil) {
+            [self recordLost:eventIsNotATransaction reason:kSentryDiscardReasonEventProcessor];
+            if (eventIsATransaction) {
+                // We dropped the whole transaction, the dropped count includes all child spans + 1
+                // root span
+                [self recordLostSpanWithReason:kSentryDiscardReasonEventProcessor
+                                      quantity:currentSpanCount + 1];
+            }
+        } else {
+            if (eventIsATransactionClass) {
+                [self recordPartiallyDroppedSpans:(SentryTransaction *)event
+                                       withReason:kSentryDiscardReasonEventProcessor
+                             withCurrentSpanCount:&currentSpanCount];
+            }
+        }
+    }
+
     if (event != nil && isFatalEvent && nil != self.options.onCrashedLastRun
         && !SentrySDKInternal.crashedLastRunCalled) {
         // We only want to call the callback once. It can occur that multiple crash events are

Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift

@@ -224,7 +224,7 @@ class SentrySessionReplayIntegrationTests: XCTestCase {
         let scope = Scope()
         let hub = TestHub(client: client, andScope: scope)
         SentrySDKInternal.setCurrentHub(hub)
-        let expectation = expectation(description: "Replay to be capture")
+        let expectation = expectation(description: "Replay to be captured")
         hub.onReplayCapture = {
             expectation.fulfill()
         }
@@ -252,7 +252,7 @@ class SentrySessionReplayIntegrationTests: XCTestCase {
         let scope = Scope()
         let hub = TestHub(client: client, andScope: scope)
         SentrySDKInternal.setCurrentHub(hub)
-        let expectation = expectation(description: "Replay to be capture")
+        let expectation = expectation(description: "Replay to be captured")
         hub.onReplayCapture = {
             expectation.fulfill()
         }
@@ -278,7 +278,7 @@ class SentrySessionReplayIntegrationTests: XCTestCase {
         let scope = Scope()
         let hub = TestHub(client: client, andScope: scope)
         SentrySDKInternal.setCurrentHub(hub)
-        let expectation = expectation(description: "Replay to be capture")
+        let expectation = expectation(description: "Replay to be captured")
         expectation.isInverted = true
         hub.onReplayCapture = {
             expectation.fulfill()
@@ -294,6 +294,34 @@ class SentrySessionReplayIntegrationTests: XCTestCase {
         XCTAssertEqual(hub.capturedReplayRecordingVideo.count, 0)
     }
 
+    func testBufferReplayIgnoredBecauseEventDroppedInBeforeSend() throws {
+        try createLastSessionReplay(writeSessionInfo: false)
+
+        startSDK(sessionSampleRate: 1, errorSampleRate: 1, configure: { options in
+            options.beforeSend = { _ in
+                return nil
+            }
+        })
+
+        let client = SentryClient(options: try XCTUnwrap(SentrySDKInternal.options))
+        let scope = Scope()
+        let hub = TestHub(client: client, andScope: scope)
+        SentrySDKInternal.setCurrentHub(hub)
+        let expectation = expectation(description: "Replay to be captured")
+        expectation.isInverted = true
+        hub.onReplayCapture = {
+            expectation.fulfill()
+        }
+
+        let crash = Event(error: NSError(domain: "Error", code: 1))
+        crash.context = [:]
+        crash.isFatalEvent = true
+        try XCTUnwrap(client).capture(event: crash)
+
+        wait(for: [expectation], timeout: 1)
+        XCTAssertEqual(hub.capturedReplayRecordingVideo.count, 0)
+    }
+
     func testPauseSessionReplayWithReacheability() throws {
         startSDK(sessionSampleRate: 1, errorSampleRate: 0)
         let sut = try getSut()

Tests/SentryTests/SentryClientTests.swift

@@ -1367,15 +1367,15 @@ class SentryClientTest: XCTestCase {
     func testEventDroppedByEventProcessor_RecordsLostEvent() {
         SentryDependencyContainer.sharedInstance().globalEventProcessor.add { _ in return nil }
 
-        beforeSendReturnsNil { $0.capture(message: fixture.messageAsString) }
+        fixture.getSut().capture(message: fixture.messageAsString)
 
         assertLostEventRecorded(category: .error, reason: .eventProcessor)
     }
 
     func testTransactionDroppedByEventProcessor_RecordsLostEvent() {
         SentryDependencyContainer.sharedInstance().globalEventProcessor.add { _ in return nil }
 
-        beforeSendReturnsNil { $0.capture(event: fixture.transaction) }
+        fixture.getSut().capture(event: fixture.transaction)
 
         assertLostEventRecorded(category: .transaction, reason: .eventProcessor)
     }
@@ -1538,19 +1538,19 @@ class SentryClientTest: XCTestCase {
 
         XCTAssertEqual(3, fixture.transport.recordLostEventsWithCount.count)
 
-        // span dropped by event processor
+        // span dropped by beforeSendSpan
         XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(0)?.category, SentryDataCategory.span)
-        XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(0)?.reason, SentryDiscardReason.eventProcessor)
+        XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(0)?.reason, SentryDiscardReason.beforeSend)
         XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(0)?.quantity, 1)
 
-        // span dropped by beforeSendSpan
+        // span dropped by beforeSend
         XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(1)?.category, SentryDataCategory.span)
         XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(1)?.reason, SentryDiscardReason.beforeSend)
         XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(1)?.quantity, 1)
 
-        // span dropped by beforeSend
+        // span dropped by event processor
         XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(2)?.category, SentryDataCategory.span)
-        XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(2)?.reason, SentryDiscardReason.beforeSend)
+        XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(2)?.reason, SentryDiscardReason.eventProcessor)
         XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(2)?.quantity, 1)
     }
     @available(*, deprecated, message: "-[SentryClient captureUserFeedback:] is deprecated. -[SentryClient captureFeedback:withScope:] is the new way. This test case can be removed in favor of testNoDsn_FeedbackNotSent when -[SentryClient captureUserFeedback:] is removed.")