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

CHANGELOG.md

@@ -6,6 +6,10 @@
 
 - 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
 
 - No documented changes.

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,25 @@ - (SentryEvent *_Nullable)prepareEvent:(SentryEvent *)event
         }
     }
 
+    if (event != nil) {
+        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

@@ -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 capture")
+        expectation.isInverted = true
+        hub.onReplayCapture = {
+            expectation.fulfill()
+        }
+
+        let crash = Event(error: NSError(domain: "Error", code: 1))
+        crash.context = [:]
+        crash.isFatalEvent = true
+        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

@@ -1363,15 +1363,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)
     }
@@ -1534,20 +1534,20 @@ 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)?.quantity, 1)
+        XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(2)?.reason, SentryDiscardReason.eventProcessor)
+        XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(0)?.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.")
     func testNoDsn_UserFeedbackNotSent() {