From a78e4e9acc6f40c93e54981fdd250cb12cfc7597 Mon Sep 17 00:00:00 2001 From: Alan Charles Date: Mon, 12 Feb 2024 14:36:42 -0700 Subject: [PATCH 1/5] fix: add check to drop events that return a 400 response from Segment --- Sources/Segment/Plugins/SegmentDestination.swift | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/Sources/Segment/Plugins/SegmentDestination.swift b/Sources/Segment/Plugins/SegmentDestination.swift index 0cf6fea6..01407d04 100644 --- a/Sources/Segment/Plugins/SegmentDestination.swift +++ b/Sources/Segment/Plugins/SegmentDestination.swift @@ -146,6 +146,13 @@ 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)): + analytics.log(message: "Server returned a 400 response: malformed JSON") + storage.remove(file: url) + self.cleanupUploads() default: break } From a04009d33db475de39852ab34a1caa859241537c Mon Sep 17 00:00:00 2001 From: Alan Charles Date: Tue, 13 Feb 2024 13:38:07 -0700 Subject: [PATCH 2/5] add test --- Tests/Segment-Tests/Analytics_Tests.swift | 37 +++++++++++++++++++ .../Segment-Tests/Support/TestUtilities.swift | 23 ++++++++++++ 2 files changed, 60 insertions(+) diff --git a/Tests/Segment-Tests/Analytics_Tests.swift b/Tests/Segment-Tests/Analytics_Tests.swift index 45460984..bfa55da6 100644 --- a/Tests/Segment-Tests/Analytics_Tests.swift +++ b/Tests/Segment-Tests/Analytics_Tests.swift @@ -865,4 +865,41 @@ final class Analytics_Tests: XCTestCase { let d: Double? = trackEvent?.properties?.value(forKeyPath: "TestNaN") XCTAssertNil(d) } + + 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").errorHandler( {error in + XCTFail("Network Error: \(error)") + })) + + 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 + + var storedFiles = analytics.storage.eventFiles(includeUnfinished: true) + + analytics.track(name: "test track", properties: ["MalformedPaylod": "My Failed Prop"]) + analytics.flush() + + XCTAssert(storedFiles.count == 0) + } } diff --git a/Tests/Segment-Tests/Support/TestUtilities.swift b/Tests/Segment-Tests/Support/TestUtilities.swift index 060e0186..710954aa 100644 --- a/Tests/Segment-Tests/Support/TestUtilities.swift +++ b/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 From 50bb5cdd6e51f3b88e480c23360c32235c838b43 Mon Sep 17 00:00:00 2001 From: Alan Charles Date: Tue, 13 Feb 2024 13:42:15 -0700 Subject: [PATCH 3/5] add fix for linux in test --- Tests/Segment-Tests/Analytics_Tests.swift | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Tests/Segment-Tests/Analytics_Tests.swift b/Tests/Segment-Tests/Analytics_Tests.swift index bfa55da6..d57bca53 100644 --- a/Tests/Segment-Tests/Analytics_Tests.swift +++ b/Tests/Segment-Tests/Analytics_Tests.swift @@ -866,6 +866,8 @@ final class Analytics_Tests: XCTestCase { 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 { @@ -902,4 +904,5 @@ final class Analytics_Tests: XCTestCase { XCTAssert(storedFiles.count == 0) } + #endif } From 4522446eecb2eca9e5eac1794f63c8ba138350f1 Mon Sep 17 00:00:00 2001 From: Alan Charles Date: Tue, 13 Feb 2024 15:20:19 -0700 Subject: [PATCH 4/5] fix tests --- Tests/Segment-Tests/Analytics_Tests.swift | 30 ++++++++++++++++------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/Tests/Segment-Tests/Analytics_Tests.swift b/Tests/Segment-Tests/Analytics_Tests.swift index d57bca53..d4101659 100644 --- a/Tests/Segment-Tests/Analytics_Tests.swift +++ b/Tests/Segment-Tests/Analytics_Tests.swift @@ -873,9 +873,7 @@ final class Analytics_Tests: XCTestCase { guard URLProtocol.registerClass(FailedNetworkCalls.self) else { XCTFail(); return } - let analytics = Analytics(configuration: Configuration(writeKey: "networkTest").errorHandler( {error in - XCTFail("Network Error: \(error)") - })) + let analytics = Analytics(configuration: Configuration(writeKey: "networkTest")) waitUntilStarted(analytics: analytics) @@ -897,12 +895,26 @@ final class Analytics_Tests: XCTestCase { segment?.httpClient?.session = blockSession - var storedFiles = analytics.storage.eventFiles(includeUnfinished: true) - - analytics.track(name: "test track", properties: ["MalformedPaylod": "My Failed Prop"]) - analytics.flush() - - XCTAssert(storedFiles.count == 0) + 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 } From 5fb4c4a889705f866eb24a72cea51523ceccac5f Mon Sep 17 00:00:00 2001 From: Alan Charles Date: Tue, 13 Feb 2024 15:23:59 -0700 Subject: [PATCH 5/5] remove redundant log --- Sources/Segment/Plugins/SegmentDestination.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/Segment/Plugins/SegmentDestination.swift b/Sources/Segment/Plugins/SegmentDestination.swift index 01407d04..988ead2f 100644 --- a/Sources/Segment/Plugins/SegmentDestination.swift +++ b/Sources/Segment/Plugins/SegmentDestination.swift @@ -150,7 +150,6 @@ public class SegmentDestination: DestinationPlugin, Subscriber, FlushCompletion // 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)): - analytics.log(message: "Server returned a 400 response: malformed JSON") storage.remove(file: url) self.cleanupUploads() default: