diff --git a/Sources/KlaviyoSwift/APIRequestErrorHandling.swift b/Sources/KlaviyoSwift/APIRequestErrorHandling.swift index 9b20cd59..8a465c64 100644 --- a/Sources/KlaviyoSwift/APIRequestErrorHandling.swift +++ b/Sources/KlaviyoSwift/APIRequestErrorHandling.swift @@ -33,10 +33,9 @@ enum InvalidField: Equatable { } } -private func getDelaySeconds(for count: Int) -> Int { - let delay = Int(pow(2.0, Double(count))) +private func addJitter(to value: Int) -> Int { let jitter = environment.randomInt() - return min(delay + jitter, ErrorHandlingConstants.maxBackoff) + return value + jitter } private func parseError(_ data: Data) -> [InvalidField]? { @@ -100,7 +99,7 @@ func handleRequestError( environment.emitDeveloperWarning("Invalid data supplied for request. Skipping.") return .deQueueCompletedResults(request) - case .rateLimitError: + case let .rateLimitError(retryAfter): var requestRetryCount = 0 var totalRetryCount = 0 var nextBackoff = 0 @@ -112,7 +111,9 @@ func handleRequestError( case let .retryWithBackoff(requestCount, totalCount, _): requestRetryCount = requestCount + 1 totalRetryCount = totalCount + 1 - nextBackoff = getDelaySeconds(for: totalRetryCount) + let exponentialBackOff = Int(pow(2.0, Double(totalRetryCount))) + + nextBackoff = addJitter(to: retryAfter ?? exponentialBackOff) } return .requestFailed( request, .retryWithBackoff( diff --git a/Sources/KlaviyoSwift/KlaviyoAPI.swift b/Sources/KlaviyoSwift/KlaviyoAPI.swift index 5254a37c..347701e1 100644 --- a/Sources/KlaviyoSwift/KlaviyoAPI.swift +++ b/Sources/KlaviyoSwift/KlaviyoAPI.swift @@ -22,7 +22,7 @@ struct KlaviyoAPI { enum KlaviyoAPIError: Error { case httpError(Int, Data) - case rateLimitError + case rateLimitError(Int?) case missingOrInvalidResponse(URLResponse?) case networkError(Error) case internalError(String) @@ -70,7 +70,8 @@ struct KlaviyoAPI { if httpResponse.statusCode == 429 { requestRateLimited(request) - return .failure(KlaviyoAPIError.rateLimitError) + let retryAfter = Int(httpResponse.value(forHTTPHeaderField: "Retry-After") ?? "0") + return .failure(KlaviyoAPIError.rateLimitError(retryAfter)) } guard 200..<300 ~= httpResponse.statusCode else { diff --git a/Sources/KlaviyoSwift/KlaviyoState.swift b/Sources/KlaviyoSwift/KlaviyoState.swift index bec5a1a2..6f9a72d4 100644 --- a/Sources/KlaviyoSwift/KlaviyoState.swift +++ b/Sources/KlaviyoSwift/KlaviyoState.swift @@ -120,27 +120,24 @@ struct KlaviyoState: Equatable, Codable { } mutating func updateEmail(email: String) { - guard email != self.email else { - return + if email.isNotEmptyOrSame(as: self.email, identifier: "email") { + self.email = email + enqueueProfileOrTokenRequest() } - self.email = email - enqueueProfileOrTokenRequest() } mutating func updateExternalId(externalId: String) { - guard externalId != self.externalId else { - return + if externalId.isNotEmptyOrSame(as: self.externalId, identifier: "external Id") { + self.externalId = externalId + enqueueProfileOrTokenRequest() } - self.externalId = externalId - enqueueProfileOrTokenRequest() } mutating func updatePhoneNumber(phoneNumber: String) { - guard phoneNumber != self.phoneNumber else { - return + if phoneNumber.isNotEmptyOrSame(as: self.phoneNumber, identifier: "phone number") { + self.phoneNumber = phoneNumber + enqueueProfileOrTokenRequest() } - self.phoneNumber = phoneNumber - enqueueProfileOrTokenRequest() } mutating func enqueueProfileOrTokenRequest() { @@ -179,9 +176,20 @@ struct KlaviyoState: Equatable, Codable { } mutating func updateStateWithProfile(profile: Profile) { - email = profile.email ?? email - phoneNumber = profile.phoneNumber ?? phoneNumber - externalId = profile.externalId ?? externalId + if let profileEmail = profile.email, + profileEmail.isNotEmptyOrSame(as: self.email, identifier: "email") { + email = profileEmail + } + + if let profilePhoneNumber = profile.phoneNumber, + profilePhoneNumber.isNotEmptyOrSame(as: self.phoneNumber, identifier: "phone number") { + phoneNumber = profilePhoneNumber + } + + if let profileExternalId = profile.externalId, + profileExternalId.isNotEmptyOrSame(as: self.externalId, identifier: "external id") { + externalId = profileExternalId + } } mutating func updateRequestAndStateWithPendingProfile(profile: CreateProfilePayload) -> CreateProfilePayload { @@ -373,6 +381,14 @@ private func removeStateFile(at file: URL) { } } +private func logDevWarning(for identifier: String) { + environment.emitDeveloperWarning(""" + \(identifier) is either empty or same as what is already set earlier. + The SDK will ignore this change, please use resetProfile for + resetting profile identifiers + """) +} + /// Loads SDK state from disk /// - Parameter apiKey: the API key that uniquely identiifies the company /// - Returns: an instance of the `KlaviyoState` @@ -487,3 +503,14 @@ extension Profile { return profile } } + +extension String { + fileprivate func isNotEmptyOrSame(as state: String?, identifier: String) -> Bool { + let incoming = self + if incoming.isEmpty || incoming == state { + logDevWarning(for: identifier) + } + + return !incoming.isEmpty && incoming != state + } +} diff --git a/Sources/KlaviyoSwift/StateManagement.swift b/Sources/KlaviyoSwift/StateManagement.swift index cfccd47d..d41b8260 100644 --- a/Sources/KlaviyoSwift/StateManagement.swift +++ b/Sources/KlaviyoSwift/StateManagement.swift @@ -422,13 +422,15 @@ struct KlaviyoReducer: ReducerProtocol { pushToken: tokenData.pushToken, enablement: tokenData.pushEnablement.rawValue, background: tokenData.pushBackground.rawValue, - profile: profile, + profile: profile.profile(from: state), anonymousId: anonymousId) )) } else { request = KlaviyoAPI.KlaviyoRequest( apiKey: apiKey, - endpoint: .createProfile(.init(data: .init(profile: profile, anonymousId: anonymousId)))) + endpoint: .createProfile( + .init(data: .init(profile: profile.profile(from: state), anonymousId: anonymousId)) + )) } state.enqueueRequest(request: request) @@ -491,3 +493,16 @@ extension Event { uniqueId: uniqueId) } } + +extension Profile { + fileprivate func profile(from state: KlaviyoState) -> Profile { + Profile( + email: state.email, + phoneNumber: state.phoneNumber, + externalId: state.externalId, + firstName: firstName, + image: image, + location: location, + properties: properties) + } +} diff --git a/Tests/KlaviyoSwiftTests/APIRequestErrorHandlingTests.swift b/Tests/KlaviyoSwiftTests/APIRequestErrorHandlingTests.swift index 49c06ad9..2beb192c 100644 --- a/Tests/KlaviyoSwiftTests/APIRequestErrorHandlingTests.swift +++ b/Tests/KlaviyoSwiftTests/APIRequestErrorHandlingTests.swift @@ -298,6 +298,25 @@ class APIRequestErrorHandlingTests: XCTestCase { } } + func testRetryWithRetryAfter() async throws { + var initialState = INITIALIZED_TEST_STATE() + initialState.retryInfo = .retryWithBackoff(requestCount: 3, totalRetryCount: 3, currentBackoff: 4) + let request = initialState.buildProfileRequest(apiKey: initialState.apiKey!, anonymousId: initialState.anonymousId!) + initialState.requestsInFlight = [request] + let store = TestStore(initialState: initialState, reducer: KlaviyoReducer()) + + environment.analytics.klaviyoAPI.send = { _ in .failure(.rateLimitError(20)) } + + _ = await store.send(.sendRequest) + + await store.receive(.requestFailed(request, .retryWithBackoff(requestCount: 4, totalRetryCount: 4, currentBackoff: 20)), timeout: TIMEOUT_NANOSECONDS) { + $0.flushing = false + $0.queue = [request] + $0.requestsInFlight = [] + $0.retryInfo = .retryWithBackoff(requestCount: 4, totalRetryCount: 4, currentBackoff: 20) + } + } + // MARK: - Missing or invalid response @MainActor diff --git a/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift b/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift index 48c6e083..09449939 100644 --- a/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift +++ b/Tests/KlaviyoSwiftTests/StateManagementEdgeCaseTests.swift @@ -151,6 +151,13 @@ class StateManagementEdgeCaseTests: XCTestCase { } } + func testSetEmptyEmail() async throws { + let initialState = INITIALIZED_TEST_STATE() + let store = TestStore(initialState: initialState, reducer: KlaviyoReducer()) + + _ = await store.send(.setEmail("")) + } + // MARK: - Set External Id @MainActor @@ -182,6 +189,13 @@ class StateManagementEdgeCaseTests: XCTestCase { } } + func testSetEmptyExternalId() async throws { + let initialState = INITIALIZED_TEST_STATE() + let store = TestStore(initialState: initialState, reducer: KlaviyoReducer()) + + _ = await store.send(.setExternalId("")) + } + // MARK: - Set Phone number @MainActor @@ -212,6 +226,13 @@ class StateManagementEdgeCaseTests: XCTestCase { } } + func testSetEmptyPhoneNumber() async throws { + let initialState = INITIALIZED_TEST_STATE() + let store = TestStore(initialState: initialState, reducer: KlaviyoReducer()) + + _ = await store.send(.setPhoneNumber("")) + } + // MARK: - Set Push Token @MainActor @@ -353,4 +374,31 @@ class StateManagementEdgeCaseTests: XCTestCase { _ = await store.send(.enqueueProfile(profile)) await fulfillment(of: [expection]) } + + func testSetProfileWithEmptyStringIdentifiers() async throws { + let initialState = KlaviyoState( + apiKey: TEST_API_KEY, + email: "foo@bar.com", + anonymousId: environment.analytics.uuid().uuidString, + phoneNumber: "99999999", + externalId: "12345", + pushTokenData: .init(pushToken: "blob_token", + pushEnablement: .authorized, + pushBackground: .available, + deviceData: .init(context: environment.analytics.appContextInfo())), + queue: [], + requestsInFlight: [], + initalizationState: .initialized, + flushing: true) + + let store = TestStore(initialState: initialState, reducer: KlaviyoReducer()) + + _ = await store.send(.enqueueProfile(Profile(email: "", phoneNumber: "", externalId: ""))) { + $0.email = nil // since we reset state + $0.phoneNumber = nil // since we reset state + $0.externalId = nil // since we reset state + $0.enqueueProfileOrTokenRequest() + $0.pushTokenData = nil + } + } }