From 1191389c612b97f136e8222a240d794de8ac55bc Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 13 Aug 2025 16:27:44 +0200 Subject: [PATCH 1/5] fix(replay): Don't capture replays for events dropped in beforeSend --- Sources/Sentry/SentryClient.m | 35 ++++++++++--------- .../SentrySessionReplayIntegrationTests.swift | 29 ++++++++++++++- Tests/SentryTests/SentryClientTests.swift | 16 ++++----- 3 files changed, 55 insertions(+), 25 deletions(-) diff --git a/Sources/Sentry/SentryClient.m b/Sources/Sentry/SentryClient.m index 37ac25c4956..3d80a7b83f8 100644 --- a/Sources/Sentry/SentryClient.m +++ b/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:¤tSpanCount]; - } - } if (event != nil && eventIsATransaction && self.options.beforeSendSpan != nil) { SentryTransaction *transaction = (SentryTransaction *)event; NSMutableArray> *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:¤tSpanCount]; + } + } + } + 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 diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift index 6b66508e409..8ddef0e255e 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift @@ -284,7 +284,6 @@ class SentrySessionReplayIntegrationTests: XCTestCase { expectation.fulfill() } - try createLastSessionReplay(writeSessionInfo: false, errorSampleRate: 0) let crash = Event(error: NSError(domain: "Error", code: 1)) crash.context = [:] crash.isFatalEvent = true @@ -294,6 +293,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() diff --git a/Tests/SentryTests/SentryClientTests.swift b/Tests/SentryTests/SentryClientTests.swift index 0251ff4aef6..efca0008b4b 100644 --- a/Tests/SentryTests/SentryClientTests.swift +++ b/Tests/SentryTests/SentryClientTests.swift @@ -1363,7 +1363,7 @@ 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) } @@ -1371,7 +1371,7 @@ class SentryClientTest: XCTestCase { 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() { From 53058ba7e77ec2d134798901013797cca485ee23 Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 13 Aug 2025 17:23:45 +0200 Subject: [PATCH 2/5] Revert test --- .../SessionReplay/SentrySessionReplayIntegrationTests.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift index 8ddef0e255e..740eb47f225 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift @@ -284,6 +284,7 @@ class SentrySessionReplayIntegrationTests: XCTestCase { expectation.fulfill() } + try createLastSessionReplay(writeSessionInfo: false, errorSampleRate: 0) let crash = Event(error: NSError(domain: "Error", code: 1)) crash.context = [:] crash.isFatalEvent = true From d6827d729e1fe4a4f8aa3f8014091039ee3f710b Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Wed, 13 Aug 2025 17:26:02 +0200 Subject: [PATCH 3/5] Changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc53197421a..8e0bf6bd791 100644 --- a/CHANGELOG.md +++ b/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. From 7da2a5a5d1f690b50a3576fe3f18ad2f2100e4dd Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 14 Aug 2025 14:10:11 +0200 Subject: [PATCH 4/5] Address PR review --- Sources/Sentry/SentryClient.m | 2 ++ .../SentrySessionReplayIntegrationTests.swift | 10 +++++----- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Sources/Sentry/SentryClient.m b/Sources/Sentry/SentryClient.m index 3d80a7b83f8..9194c43c999 100644 --- a/Sources/Sentry/SentryClient.m +++ b/Sources/Sentry/SentryClient.m @@ -872,6 +872,8 @@ - (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]; diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift index 740eb47f225..f8385981194 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift +++ b/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() @@ -307,7 +307,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() @@ -316,7 +316,7 @@ class SentrySessionReplayIntegrationTests: XCTestCase { let crash = Event(error: NSError(domain: "Error", code: 1)) crash.context = [:] crash.isFatalEvent = true - client?.capture(event: crash) + try XCTUnwrap(client).capture(event: crash) wait(for: [expectation], timeout: 1) XCTAssertEqual(hub.capturedReplayRecordingVideo.count, 0) From 55bf08e9744db917002f2366f76bf2a1e99bd47f Mon Sep 17 00:00:00 2001 From: Roman Zavarnitsyn Date: Thu, 14 Aug 2025 14:18:22 +0200 Subject: [PATCH 5/5] Fix test assertion --- Tests/SentryTests/SentryClientTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/SentryTests/SentryClientTests.swift b/Tests/SentryTests/SentryClientTests.swift index efca0008b4b..cc706bb6756 100644 --- a/Tests/SentryTests/SentryClientTests.swift +++ b/Tests/SentryTests/SentryClientTests.swift @@ -1547,7 +1547,7 @@ class SentryClientTest: XCTestCase { // span dropped by event processor XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(2)?.category, SentryDataCategory.span) XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(2)?.reason, SentryDiscardReason.eventProcessor) - XCTAssertEqual(fixture.transport.recordLostEventsWithCount.get(0)?.quantity, 1) + 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.") func testNoDsn_UserFeedbackNotSent() {