Merge b124961 into 72df100

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Don't increase session's error count for dropped events (#2374)
+
 ## 7.31.0
 
 ### Features

Sources/Sentry/SentryClient.m

@@ -184,11 +184,14 @@ - (SentryId *)captureException:(NSException *)exception withScope:(SentryScope *
 }
 
 - (SentryId *)captureException:(NSException *)exception
-                   withSession:(SentrySession *)session
                      withScope:(SentryScope *)scope
+                   withSession:(SentrySession * (^)(BOOL withErrorIncremented))sessionBlock
 {
     SentryEvent *event = [self buildExceptionEvent:exception];
     event = [self prepareEvent:event withScope:scope alwaysAttachStacktrace:YES];
+
+    SentrySession *session = sessionBlock(event != nil);
+
     return [self sendEvent:event withSession:session withScope:scope];
 }
 
@@ -215,11 +218,14 @@ - (SentryId *)captureError:(NSError *)error withScope:(SentryScope *)scope
 }
 
 - (SentryId *)captureError:(NSError *)error
-               withSession:(SentrySession *)session
                  withScope:(SentryScope *)scope
+               withSession:(SentrySession * (^)(BOOL withErrorIncremented))sessionBlock
 {
     SentryEvent *event = [self buildErrorEvent:error];
     event = [self prepareEvent:event withScope:scope alwaysAttachStacktrace:YES];
+
+    SentrySession *session = sessionBlock(event != nil);
+
     return [self sendEvent:event withSession:session withScope:scope];
 }
 
@@ -399,10 +405,9 @@ - (SentryId *)sendEvent:(SentryEvent *)event
         [self.transportAdapter sendEvent:event session:session attachments:attachments];
 
         return event.eventId;
-    } else {
-        [self captureSession:session];
-        return SentryId.empty;
     }
+
+    return SentryId.empty;
 }
 
 - (void)captureSession:(SentrySession *)session

Sources/Sentry/SentryHub.m

@@ -438,11 +438,19 @@ - (SentryId *)captureError:(NSError *)error
 
 - (SentryId *)captureError:(NSError *)error withScope:(SentryScope *)scope
 {
-    SentrySession *currentSession = [self incrementSessionErrors];
+    SentrySession *currentSession = _session;
     SentryClient *client = _client;
     if (nil != client) {
         if (nil != currentSession) {
-            return [client captureError:error withSession:currentSession withScope:scope];
+            return [client captureError:error
+                              withScope:scope
+                            withSession:^(BOOL withErrorIncremented) {
+                                if (withErrorIncremented) {
+                                    return [self incrementSessionErrors];
+                                } else {
+                                    return currentSession;
+                                }
+                            }];
         } else {
             return [client captureError:error withScope:scope];
         }
@@ -457,12 +465,19 @@ - (SentryId *)captureException:(NSException *)exception
 
 - (SentryId *)captureException:(NSException *)exception withScope:(SentryScope *)scope
 {
-    SentrySession *currentSession = [self incrementSessionErrors];
-
+    SentrySession *currentSession = _session;
     SentryClient *client = _client;
     if (nil != client) {
         if (nil != currentSession) {
-            return [client captureException:exception withSession:currentSession withScope:scope];
+            return [client captureException:exception
+                                  withScope:scope
+                                withSession:^(BOOL withErrorIncremented) {
+                                    if (withErrorIncremented) {
+                                        return [self incrementSessionErrors];
+                                    } else {
+                                        return currentSession;
+                                    }
+                                }];
         } else {
             return [client captureException:exception withScope:scope];
         }

Sources/Sentry/include/SentryClient+Private.h

@@ -26,12 +26,12 @@ SentryClient (Private)
 - (SentryFileManager *)fileManager;
 
 - (SentryId *)captureError:(NSError *)error
-               withSession:(SentrySession *)session
-                 withScope:(SentryScope *)scope;
+                 withScope:(SentryScope *)scope
+               withSession:(SentrySession * (^)(BOOL withErrorIncremented))sessionBlock;
 
 - (SentryId *)captureException:(NSException *)exception
-                   withSession:(SentrySession *)session
-                     withScope:(SentryScope *)scope;
+                     withScope:(SentryScope *)scope
+                   withSession:(SentrySession * (^)(BOOL withErrorIncremented))sessionBlock;
 
 - (SentryId *)captureCrashEvent:(SentryEvent *)event withScope:(SentryScope *)scope;
 

Sources/Sentry/include/SentryHub+Private.h

@@ -53,6 +53,8 @@ SentryHub (Private)
                        withScope:(SentryScope *)scope
          additionalEnvelopeItems:(NSArray<SentryEnvelopeItem *> *)additionalEnvelopeItems;
 
+- (nullable SentrySession *)incrementSessionErrors;
+
 @end
 
 NS_ASSUME_NONNULL_END

Tests/SentryTests/SentryClientTests.swift

@@ -269,29 +269,31 @@ class SentryClientTest: XCTestCase {
         sut.add(processor)
         sut.capture(event: event)
 
-        let sendedAttachments = fixture.transportAdapter.sendEventWithTraceStateInvocations.first?.attachments ?? []
+        let sentAttachments = fixture.transportAdapter.sendEventWithTraceStateInvocations.first?.attachments ?? []
 
         wait(for: [expectProcessorCall], timeout: 1)
-        XCTAssertEqual(sendedAttachments.count, 1)
-        XCTAssertEqual(extraAttachment, sendedAttachments.first)
+        XCTAssertEqual(sentAttachments.count, 1)
+        XCTAssertEqual(extraAttachment, sentAttachments.first)
     }
 
     func test_AttachmentProcessor_CaptureError_WithSession() {
         let sut = fixture.getSut()
         let error = NSError(domain: "test", code: -1)
         let extraAttachment = Attachment(data: Data(), filename: "ExtraAttachment")
-        
+
         let processor = TestAttachmentProcessor { atts, _ in
             var result = atts ?? []
             result.append(extraAttachment)
             return result
         }
-        
+
         sut.add(processor)
-        sut.captureError(error, with: fixture.session, with: Scope())
-
+        sut.captureError(error, with: Scope()) { _ in
+            self.fixture.session
+        }
+
         let sentAttachments = fixture.transportAdapter.sentEventsWithSessionTraceState.first?.attachments ?? []
-        
+
         XCTAssertEqual(sentAttachments.count, 1)
         XCTAssertEqual(extraAttachment, sentAttachments.first)
     }
@@ -308,12 +310,14 @@ class SentryClientTest: XCTestCase {
         }
 
         sut.add(processor)
-        sut.captureError(error, with: SentrySession(releaseName: ""), with: Scope())
+        sut.captureError(error, with: Scope()) { _ in
+            return SentrySession(releaseName: "")
+        }
 
-        let sendedAttachments = fixture.transportAdapter.sendEventWithTraceStateInvocations.first?.attachments ?? []
+        let sentAttachments = fixture.transportAdapter.sendEventWithTraceStateInvocations.first?.attachments ?? []
 
-        XCTAssertEqual(sendedAttachments.count, 1)
-        XCTAssertEqual(extraAttachment, sendedAttachments.first)
+        XCTAssertEqual(sentAttachments.count, 1)
+        XCTAssertEqual(extraAttachment, sentAttachments.first)
     }
 
     func testCaptureEventWithDsnSetAfterwards() {
@@ -445,8 +449,11 @@ class SentryClientTest: XCTestCase {
     }
 
     func testCaptureErrorWithSession() {
-        let eventId = fixture.getSut().captureError(error, with: fixture.session, with: Scope())
-
+        let eventId = fixture.getSut().captureError(error, with: Scope()) { increaseErrorCount in
+            XCTAssertTrue(increaseErrorCount)
+            return self.fixture.session
+        }
+
         eventId.assertIsNotEmpty()
         XCTAssertNotNil(fixture.transportAdapter.sentEventsWithSessionTraceState.last)
         if let eventWithSessionArguments = fixture.transportAdapter.sentEventsWithSessionTraceState.last {
@@ -458,7 +465,10 @@ class SentryClientTest: XCTestCase {
     func testCaptureErrorWithSession_WithBeforeSendReturnsNil() {
         let eventId = fixture.getSut(configureOptions: { options in
             options.beforeSend = { _ in return nil }
-        }).captureError(error, with: fixture.session, with: Scope())
+        }).captureError(error, with: Scope()) { increaseErrorCount in
+            XCTAssertFalse(increaseErrorCount)
+            return self.fixture.session
+        }
 
         eventId.assertIsEmpty()
         assertLastSentEnvelopeIsASession()
@@ -680,8 +690,10 @@ class SentryClientTest: XCTestCase {
     }
 
     func testCaptureExceptionWithSession() {
-        let eventId = fixture.getSut().capture(exception, with: fixture.session, with: fixture.scope)
-
+        let eventId = fixture.getSut().capture(exception, with: fixture.scope) { _ in
+            self.fixture.session
+        }
+
         eventId.assertIsNotEmpty()
         XCTAssertNotNil(fixture.transportAdapter.sentEventsWithSessionTraceState.last)
         if let eventWithSessionArguments = fixture.transportAdapter.sentEventsWithSessionTraceState.last {
@@ -694,7 +706,9 @@ class SentryClientTest: XCTestCase {
     func testCaptureExceptionWithSession_WithBeforeSendReturnsNil() {
         let eventId = fixture.getSut(configureOptions: { options in
             options.beforeSend = { _ in return nil }
-        }).capture(exception, with: fixture.session, with: fixture.scope)
+        }).capture(exception, with: fixture.scope) { _ in
+            self.fixture.session
+        }
 
         eventId.assertIsEmpty()
         assertLastSentEnvelopeIsASession()
@@ -731,7 +745,9 @@ class SentryClientTest: XCTestCase {
         let session = SentrySession(releaseName: "")
 
         fixture.getSut().capture(session: session)
-        fixture.getSut().capture(exception, with: session, with: Scope())
+        fixture.getSut().capture(exception, with: Scope()) { _ in
+            session
+        }
             .assertIsNotEmpty()
         fixture.getSut().captureCrash(fixture.event, with: session, with: Scope())
             .assertIsNotEmpty()
@@ -850,7 +866,9 @@ class SentryClientTest: XCTestCase {
         _ = SentryEnvelope(event: Event())
         let eventId = fixture.getSut(configureOptions: { options in
             options.dsn = nil
-        }).capture(self.exception, with: fixture.session, with: fixture.scope)
+        }).capture(self.exception, with: fixture.scope) { _ in
+            self.fixture.session
+        }
 
         eventId.assertIsEmpty()
         assertNothingSent()
@@ -860,7 +878,9 @@ class SentryClientTest: XCTestCase {
         _ = SentryEnvelope(event: Event())
         let eventId = fixture.getSut(configureOptions: { options in
             options.dsn = nil
-        }).captureError(self.error, with: fixture.session, with: fixture.scope)
+        }).captureError(self.error, with: fixture.scope) { _ in
+            self.fixture.session
+        }
 
         eventId.assertIsEmpty()
         assertNothingSent()

Tests/SentryTests/SentryHubTests.swift

@@ -392,6 +392,26 @@ class SentryHubTests: XCTestCase {
         // only session init is sent
         XCTAssertEqual(1, fixture.client.captureSessionInvocations.count)
     }
+
+    func testCaptureWithoutIncreasingErrorCount() {
+        let sut = fixture.getSut()
+        sut.startSession()
+        fixture.client.sessionIncrementsErrorCount = false
+        sut.capture(error: fixture.error, scope: fixture.scope).assertIsNotEmpty()
+
+        XCTAssertEqual(1, fixture.client.captureErrorWithSessionInvocations.count)
+        if let errorArguments = fixture.client.captureErrorWithSessionInvocations.first {
+            XCTAssertEqual(fixture.error, errorArguments.error as NSError)
+
+            XCTAssertEqual(errorArguments.session.errors, 0)
+            XCTAssertEqual(SentrySessionStatus.ok, errorArguments.session.status)
+
+            XCTAssertEqual(fixture.scope, errorArguments.scope)
+        }
+
+        // only session init is sent
+        XCTAssertEqual(1, fixture.client.captureSessionInvocations.count)
+    }
 
     func testCaptureErrorWithoutScope() {
         fixture.getSut(fixture.options, fixture.scope).capture(error: fixture.error).assertIsNotEmpty()
@@ -441,6 +461,26 @@ class SentryHubTests: XCTestCase {
         // only session init is sent
         XCTAssertEqual(1, fixture.client.captureSessionInvocations.count)
     }
+
+    func testCaptureExceptionWithoutIncreasingErrorCount() {
+        let sut = fixture.getSut()
+        sut.startSession()
+        fixture.client.sessionIncrementsErrorCount = false
+        sut.capture(exception: fixture.exception, scope: fixture.scope).assertIsNotEmpty()
+
+        XCTAssertEqual(1, fixture.client.captureExceptionWithSessionInvocations.count)
+        if let exceptionArguments = fixture.client.captureExceptionWithSessionInvocations.first {
+            XCTAssertEqual(fixture.exception, exceptionArguments.exception)
+
+            XCTAssertEqual(exceptionArguments.session.errors, 0)
+            XCTAssertEqual(SentrySessionStatus.ok, exceptionArguments.session.status)
+
+            XCTAssertEqual(fixture.scope, exceptionArguments.scope)
+        }
+
+        // only session init is sent
+        XCTAssertEqual(1, fixture.client.captureSessionInvocations.count)
+    }
 
     @available(tvOS 10.0, *)
     @available(OSX 10.12, *)

Tests/SentryTests/TestClient.swift

@@ -83,16 +83,17 @@ class TestClient: Client {
         captureExceptionWithScopeInvocations.record((exception, scope))
         return SentryId()
     }
-
+
+    var sessionIncrementsErrorCount = true
     var captureErrorWithSessionInvocations = Invocations<(error: Error, session: SentrySession, scope: Scope)>()
-    override func captureError(_ error: Error, with session: SentrySession, with scope: Scope) -> SentryId {
-        captureErrorWithSessionInvocations.record((error, session, scope))
+    override func captureError(_ error: Error, with scope: Scope, withSession sessionBlock: @escaping (Bool) -> SentrySession) -> SentryId {
+        captureErrorWithSessionInvocations.record((error, sessionBlock(sessionIncrementsErrorCount), scope))
         return SentryId()
     }
 
     var captureExceptionWithSessionInvocations = Invocations<(exception: NSException, session: SentrySession, scope: Scope)>()
-    override func capture(_ exception: NSException, with session: SentrySession, with scope: Scope) -> SentryId {
-        captureExceptionWithSessionInvocations.record((exception, session, scope))
+    override func capture(_ exception: NSException, with scope: Scope, withSession sessionBlock: @escaping (Bool) -> SentrySession) -> SentryId {
+        captureExceptionWithSessionInvocations.record((exception, sessionBlock(sessionIncrementsErrorCount), scope))
         return SentryId()
     }