fix: add check to drop events that return a 400 response from Segment (#296)

* fix: add check to drop events that return a 400 response from Segment

* add test

* add fix for linux in test

* fix tests

* remove redundant log

Author: alanjcharles

Sources/Segment/Plugins/SegmentDestination.swift

@@ -146,6 +146,12 @@ public class SegmentDestination: DestinationPlugin, Subscriber, FlushCompletion
                         case .success(_):
                             storage.remove(file: url)
                             self.cleanupUploads()
+                        
+                        // we don't want to retry events in a given batch when a 400
+                        // response for malformed JSON is returned
+                        case .failure(Segment.HTTPClientErrors.statusCode(code: 400)):
+                            storage.remove(file: url)
+                            self.cleanupUploads()
                         default:
                             break
                     }

Tests/Segment-Tests/Analytics_Tests.swift

@@ -865,4 +865,56 @@ final class Analytics_Tests: XCTestCase {
         let d: Double? = trackEvent?.properties?.value(forKeyPath: "TestNaN")
         XCTAssertNil(d)
     }
+    
+    // Linux doesn't know what URLProtocol is and on watchOS it somehow works differently and isn't hit.
+    #if !os(Linux) && !os(watchOS)
+    func testFailedSegmentResponse() throws {
+        //register our network blocker (returns 400 response)
+        guard URLProtocol.registerClass(FailedNetworkCalls.self) else {
+            XCTFail(); return }
+        
+        let analytics = Analytics(configuration: Configuration(writeKey: "networkTest"))
+        
+        waitUntilStarted(analytics: analytics)
+        
+        //set the httpClient to use our blocker session
+        let segment = analytics.find(pluginType: SegmentDestination.self)
+        let configuration = URLSessionConfiguration.ephemeral
+        configuration.allowsCellularAccess = true
+        configuration.timeoutIntervalForRequest = 30
+        configuration.timeoutIntervalForRequest = 60
+        configuration.httpMaximumConnectionsPerHost = 2
+        configuration.protocolClasses = [FailedNetworkCalls.self]
+        configuration.httpAdditionalHeaders = [
+            "Content-Type": "application/json; charset=utf-8",
+            "Authorization": "Basic test",
+            "User-Agent": "analytics-ios/\(Analytics.version())"
+        ]
+        
+        let blockSession = URLSession(configuration: configuration, delegate: nil, delegateQueue: nil)
+        
+        segment?.httpClient?.session = blockSession
+        
+        analytics.track(name: "test track", properties: ["Malformed Paylod": "My Failed Prop"])
+        
+        //get fileUrl from track call
+        let storedEvents: [URL]? = analytics.storage.read(.events)
+        let fileURL = storedEvents![0]
+        
+        
+        let expectation = XCTestExpectation()
+        
+        analytics.flush {
+            expectation.fulfill()
+        }
+        
+        wait(for: [expectation], timeout: 1.0)
+        
+        let newStoredEvents: [URL]? = analytics.storage.read(.events)
+        
+        XCTAssert(!(newStoredEvents?.contains(fileURL))!)
+        
+        XCTAssertFalse(FileManager.default.fileExists(atPath: fileURL.path))
+    }
+    #endif
 }

Tests/Segment-Tests/Support/TestUtilities.swift

@@ -179,4 +179,27 @@ class BlockNetworkCalls: URLProtocol {
     }
 }
 
+class FailedNetworkCalls: URLProtocol {
+    var initialURL: URL? = nil
+    override class func canInit(with request: URLRequest) -> Bool {
+        
+        return true
+    }
+    
+    override class func canonicalRequest(for request: URLRequest) -> URLRequest {
+        return request
+    }
+    
+    override var cachedResponse: CachedURLResponse? { return nil }
+    
+    override func startLoading() {
+        client?.urlProtocol(self, didReceive: HTTPURLResponse(url: URL(string: "http://api.segment.com")!, statusCode: 400, httpVersion: nil, headerFields: ["blocked": "true"])!, cacheStoragePolicy: .notAllowed)
+        client?.urlProtocolDidFinishLoading(self)
+    }
+    
+    override func stopLoading() {
+        
+    }
+}
+
 #endif