Merge 14dcd93 into aa45f36

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Ensure flushing envelopes directly after capturing them (#3915)
+
 ## 8.25.0
 
 ### Features

Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme

@@ -79,15 +79,6 @@
                <Test
                   Identifier = "SentryCrashReportStore_Tests/testStoresLoadsMultipleReports">
                </Test>
-               <Test
-                  Identifier = "SentryHttpTransportTests/testFlush_BlocksCallingThread_FinishesFlushingWhenSent()">
-               </Test>
-               <Test
-                  Identifier = "SentryHttpTransportTests/testFlush_CalledSequentially_BlocksTwice()">
-               </Test>
-               <Test
-                  Identifier = "SentryHttpTransportTests/testFlush_WhenNoInternet_BlocksAndFinishes()">
-               </Test>
                <Test
                   Identifier = "SentryNetworkTrackerIntegrationTests/testGetCaptureFailedRequestsEnabled()">
                </Test>

Sources/Sentry/SentryHttpTransport.m

@@ -199,7 +199,12 @@ - (SentryFlushResult)flush:(NSTimeInterval)timeout
 #endif
     }
 
-    [self sendAllCachedEnvelopes];
+    // We are waiting for the dispatch group below, which we leave in finished sending. As
+    // sendAllCachedEnvelopes does some IO, it could block the calling thread longer than the
+    // desired flush duration. Therefore, we dispatch the sendAllCachedEnvelopes async. Furthermore,
+    // when calling flush directly after captureEnvelope, it could happen that SDK doesn't store the
+    // envelope to disk, which happens async, before starting to flush.
+    [self.dispatchQueue dispatchAsyncWithBlock:^{ [self sendAllCachedEnvelopes]; }];
 
     intptr_t result = dispatch_group_wait(self.dispatchGroup, dispatchTimeout);
 

Tests/SentryTests/Networking/SentryHttpTransportTests.swift

@@ -1,3 +1,4 @@
+import Nimble
 @testable import Sentry
 import SentryTestUtils
 import XCTest
@@ -33,7 +34,7 @@ class SentryHttpTransportTests: XCTestCase {
         let reachability = TestSentryReachability()
 #endif // !os(watchOS)
 
-        let flushTimeout: TimeInterval = 0.5
+        let flushTimeout: TimeInterval = 2.0
 
         let userFeedback: UserFeedback
         let userFeedbackRequest: SentryNSURLRequest
@@ -106,29 +107,19 @@ class SentryHttpTransportTests: XCTestCase {
             clientReportRequest = buildRequest(clientReportEnvelope)
         }
 
-        var sut: SentryHttpTransport {
-#if !os(watchOS)
-            return SentryHttpTransport(
-                options: options,
-                fileManager: fileManager,
-                requestManager: requestManager,
-                requestBuilder: requestBuilder,
-                rateLimits: rateLimits,
-                envelopeRateLimit: EnvelopeRateLimit(rateLimits: rateLimits),
-                dispatchQueueWrapper: dispatchQueueWrapper
-            )
-
-#else // os(watchOS)
+        func  getSut(dispatchQueueWrapper: SentryDispatchQueueWrapper? = nil) -> SentryHttpTransport {
+
+            let dispatchQueue = dispatchQueueWrapper ?? self.dispatchQueueWrapper
+
             return SentryHttpTransport(
                 options: options,
                 fileManager: fileManager,
                 requestManager: requestManager,
                 requestBuilder: requestBuilder,
                 rateLimits: rateLimits,
                 envelopeRateLimit: EnvelopeRateLimit(rateLimits: rateLimits),
-                dispatchQueueWrapper: dispatchQueueWrapper
+                dispatchQueueWrapper: dispatchQueue
             )
-#endif // !os(watchOS)
         }
     }
 
@@ -150,7 +141,7 @@ class SentryHttpTransportTests: XCTestCase {
         fixture.fileManager.deleteAllEnvelopes()
         fixture.requestManager.returnResponse(response: HTTPURLResponse())
 
-        sut = fixture.sut
+        sut = fixture.getSut()
     }
 
     override func tearDown() {
@@ -167,7 +158,7 @@ class SentryHttpTransportTests: XCTestCase {
 
         waitForAllRequests()
         givenOkResponse()
-        let sut = fixture.sut
+        let sut = fixture.getSut()
         XCTAssertNotNil(sut)
         waitForAllRequests()
 
@@ -614,7 +605,7 @@ class SentryHttpTransportTests: XCTestCase {
 
         // Interact with sut in extra function so ARC deallocates it
         func getSut() {
-            let sut = fixture.sut
+            let sut = fixture.getSut()
             sut.send(envelope: fixture.eventEnvelope)
             waitForAllRequests()
         }
@@ -738,14 +729,58 @@ class SentryHttpTransportTests: XCTestCase {
     }
 
     func testFlush_WhenNoEnvelopes_BlocksAndFinishes() {
-        assertFlushBlocksAndFinishesSuccessfully()
+        let sut = fixture.getSut(dispatchQueueWrapper: SentryDispatchQueueWrapper())
+
+        var blockingDurationSum: TimeInterval = 0.0
+        let flushInvocations = 100
+
+        for _ in  0..<flushInvocations {
+            let beforeFlush = getAbsoluteTime()
+            expect(sut.flush(self.fixture.flushTimeout)).to(equal(.success), description: "Flush should not time out.")
+            let blockingDuration = getDurationNs(beforeFlush, getAbsoluteTime()).toTimeInterval()
+
+            blockingDurationSum += blockingDuration
+        }
+
+        let blockingDurationAverage = blockingDurationSum / Double(flushInvocations)
+
+        expect(blockingDurationAverage) < 0.1
     }
 
     func testFlush_WhenNoInternet_BlocksAndFinishes() {
-        givenCachedEvents()
         givenNoInternetConnection()
 
-        assertFlushBlocksAndFinishesSuccessfully()
+        let sut = fixture.getSut(dispatchQueueWrapper: SentryDispatchQueueWrapper())
+
+        sut.send(envelope: fixture.eventEnvelope)
+        sut.send(envelope: fixture.eventEnvelope)
+
+        var blockingDurationSum: TimeInterval = 0.0
+        let flushInvocations = 100
+
+        for _ in  0..<flushInvocations {
+            let beforeFlush = getAbsoluteTime()
+            expect(sut.flush(self.fixture.flushTimeout)).to(equal(.success), description: "Flush should not time out.")
+            let blockingDuration = getDurationNs(beforeFlush, getAbsoluteTime()).toTimeInterval()
+
+            blockingDurationSum += blockingDuration
+        }
+
+        let blockingDurationAverage = blockingDurationSum / Double(flushInvocations)
+
+        expect(blockingDurationAverage) < 0.1
+    }
+
+    func testFlush_CallingFlushDirectlyAfterCapture_Flushes() {
+        let sut = fixture.getSut(dispatchQueueWrapper: SentryDispatchQueueWrapper())
+
+        for _ in 0..<10 {
+            sut.send(envelope: fixture.eventEnvelope)
+
+            expect(sut.flush(self.fixture.flushTimeout)).to(equal(.success), description: "Flush should not time out.")
+
+            expect(self.fixture.fileManager.getAllEnvelopes().count) == 0
+        }
     }
 
     func testFlush_CalledMultipleTimes_ImmediatelyReturnsFalse() {
@@ -815,15 +850,15 @@ class SentryHttpTransportTests: XCTestCase {
 
     func testDealloc_StopsReachabilityMonitoring() {
         func deallocSut() {
-            _ = fixture.sut
+            _ = fixture.getSut()
         }
         deallocSut()
 
         XCTAssertEqual(1, fixture.reachability.stopMonitoringInvocations.count)
     }
 
     func testDealloc_TriggerNetworkReachable_NoCrash() {
-        _ = fixture.sut
+        _ = fixture.getSut()
 
         fixture.reachability.triggerNetworkReachable()
     }
@@ -945,11 +980,4 @@ class SentryHttpTransportTests: XCTestCase {
         XCTAssertNotNil(dict)
         XCTAssertEqual(0, dict?.count)
     }
-
-    private func assertFlushBlocksAndFinishesSuccessfully() {
-        let beforeFlush = getAbsoluteTime()
-        XCTAssertEqual(.success, sut.flush(fixture.flushTimeout), "Flush should not time out.")
-        let blockingDuration = getDurationNs(beforeFlush, getAbsoluteTime()).toTimeInterval()
-        XCTAssertLessThan(blockingDuration, 0.1)
-    }
 }