diff --git a/CHANGELOG.md b/CHANGELOG.md index 44026569446..504f5d41a84 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/Sources/Sentry/SentryClient.m b/Sources/Sentry/SentryClient.m index feb5a795059..fe73bb58872 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,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:¤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..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() @@ -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() diff --git a/Tests/SentryTests/SentryClientTests.swift b/Tests/SentryTests/SentryClientTests.swift index 91732c8f890..05b8b53f2ea 100644 --- a/Tests/SentryTests/SentryClientTests.swift +++ b/Tests/SentryTests/SentryClientTests.swift @@ -1367,7 +1367,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) } @@ -1375,7 +1375,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) } @@ -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.")