Merge branch 'main' into sam/remove-test-rollout-pixel

* main: VPN clean-up (#1041) Deprecate PixelKit daily pixel suffixes (#1060) Update C-S-S to 6.29.0 (#1062) Send pixel on sync secure storage read failure (#1058)
duckduckgo · Nov 6, 2024 · 527617c · 527617c
2 parents 40b7a37 + 1d228b8
commit 527617c
Show file tree

Hide file tree

Showing 12 changed files with 56 additions and 125 deletions.
diff --git a/Package.resolved b/Package.resolved
@@ -14,8 +14,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/duckduckgo/content-scope-scripts",
       "state" : {
-        "revision" : "48fee2508995d4ac02d18b3d55424adedcb4ce4f",
-        "version" : "6.28.0"
+        "revision" : "6cab7bdb584653a5dc007cc1ae827ec41c5a91bc",
+        "version" : "6.29.0"
       }
     },
     {

diff --git a/Package.swift b/Package.swift
@@ -52,7 +52,7 @@ let package = Package(
         .package(url: "https://github.com/duckduckgo/sync_crypto", exact: "0.3.0"),
         .package(url: "https://github.com/gumob/PunycodeSwift.git", exact: "3.0.0"),
         .package(url: "https://github.com/duckduckgo/privacy-dashboard", exact: "7.1.1"),
-        .package(url: "https://github.com/duckduckgo/content-scope-scripts", exact: "6.28.0"),
+        .package(url: "https://github.com/duckduckgo/content-scope-scripts", exact: "6.29.0"),
         .package(url: "https://github.com/httpswift/swifter.git", exact: "1.5.0"),
         .package(url: "https://github.com/duckduckgo/bloom_cpp.git", exact: "3.0.0"),
         .package(url: "https://github.com/1024jp/GzipSwift.git", exact: "6.0.1")

diff --git a/Sources/DDGSync/DDGSync.swift b/Sources/DDGSync/DDGSync.swift
@@ -42,7 +42,14 @@ public class DDGSync: DDGSyncing {
     }
 
     public var account: SyncAccount? {
-        try? dependencies.secureStore.account()
+        do {
+            return try dependencies.secureStore.account()
+        } catch {
+            if let syncError = error as? SyncError {
+                dependencies.errorEvents.fire(syncError, error: syncError)
+            }
+            return nil
+        }
     }
 
     public var scheduler: Scheduling {

diff --git a/Sources/DDGSync/SyncError.swift b/Sources/DDGSync/SyncError.swift
@@ -190,4 +190,14 @@ extension SyncError: CustomNSError {
         }
     }
 
+    public var errorUserInfo: [String: Any] {
+        switch self {
+        case .failedToReadSecureStore(let status), .failedToWriteSecureStore(let status), .failedToRemoveSecureStore(let status):
+            let underlyingError = NSError(domain: "secError", code: Int(status))
+            return [NSUnderlyingErrorKey: underlyingError]
+        default:
+            return [:]
+        }
+    }
+
 }
diff --git a/Sources/NetworkProtection/Diagnostics/NetworkProtectionConnectionTester.swift b/Sources/NetworkProtection/Diagnostics/NetworkProtectionConnectionTester.swift
@@ -119,12 +119,7 @@ final class NetworkProtectionConnectionTester {
         let tunnelInterface = try await networkInterface(forInterfaceNamed: tunnelIfName)
         self.tunnelInterface = tunnelInterface
 
-        do {
-            try await scheduleTimer(testImmediately: testImmediately)
-        } catch {
-            Logger.networkProtectionConnectionTester.log("🔴 Stopping connection tester early")
-            throw error
-        }
+        await scheduleTimer(testImmediately: testImmediately)
     }
 
     func stop() {
@@ -163,16 +158,11 @@ final class NetworkProtectionConnectionTester {
 
     // MARK: - Timer scheduling
 
-    private func scheduleTimer(testImmediately: Bool) async throws {
+    private func scheduleTimer(testImmediately: Bool) async {
         stopScheduledTimer()
 
         if testImmediately {
-            do {
-                try await testConnection()
-            } catch {
-                Logger.networkProtectionConnectionTester.log("Rethrowing exception")
-                throw error
-            }
+            await testConnection()
         }
 
         task = Task.periodic(interval: intervalBetweenTests) { [weak self] in
@@ -181,7 +171,7 @@ final class NetworkProtectionConnectionTester {
             // The error we're ignoring here is only used when this class is initialized
             // with an immediate test, to know whether the connection is up while the user
             // still sees "Connecting..."
-            try? await self?.testConnection()
+            await self?.testConnection()
         }
     }
 
@@ -192,7 +182,7 @@ final class NetworkProtectionConnectionTester {
 
     // MARK: - Testing the connection
 
-    func testConnection() async throws {
+    func testConnection() async {
         guard let tunnelInterface = tunnelInterface else {
             Logger.networkProtectionConnectionTester.error("No interface to test!")
             return

diff --git a/Sources/NetworkProtection/Diagnostics/NetworkProtectionError.swift b/Sources/NetworkProtection/Diagnostics/NetworkProtectionError.swift
@@ -41,11 +41,6 @@ public enum NetworkProtectionError: LocalizedError, CustomNSError {
     case failedToEncodeRegisterKeyRequest
     case failedToFetchRegisteredServers(Error?)
     case failedToParseRegisteredServersResponse(Error)
-    case failedToEncodeRedeemRequest
-    case invalidInviteCode
-    case failedToRedeemInviteCode(Error?)
-    case failedToRetrieveAuthToken(AuthenticationFailureResponse)
-    case failedToParseRedeemResponse(Error)
     case invalidAuthToken
     case serverListInconsistency
 
@@ -91,11 +86,6 @@ public enum NetworkProtectionError: LocalizedError, CustomNSError {
         case .failedToEncodeRegisterKeyRequest: return 104
         case .failedToFetchRegisteredServers: return 105
         case .failedToParseRegisteredServersResponse: return 106
-        case .failedToEncodeRedeemRequest: return 107
-        case .invalidInviteCode: return 108
-        case .failedToRedeemInviteCode: return 109
-        case .failedToRetrieveAuthToken: return 110
-        case .failedToParseRedeemResponse: return 111
         case .invalidAuthToken: return 112
         case .serverListInconsistency: return 113
         case .failedToFetchServerStatus: return 114
@@ -130,9 +120,6 @@ public enum NetworkProtectionError: LocalizedError, CustomNSError {
                 .couldNotGetPeerHostName,
                 .couldNotGetInterfaceAddressRange,
                 .failedToEncodeRegisterKeyRequest,
-                .failedToEncodeRedeemRequest,
-                .invalidInviteCode,
-                .failedToRetrieveAuthToken,
                 .invalidAuthToken,
                 .serverListInconsistency,
                 .failedToCastKeychainValueToData,
@@ -147,8 +134,7 @@ public enum NetworkProtectionError: LocalizedError, CustomNSError {
                 .vpnAccessRevoked:
             return [:]
         case .failedToFetchServerList(let error),
-                .failedToFetchRegisteredServers(let error),
-                .failedToRedeemInviteCode(let error):
+                .failedToFetchRegisteredServers(let error):
             guard let error else {
                 return [:]
             }
@@ -160,7 +146,6 @@ public enum NetworkProtectionError: LocalizedError, CustomNSError {
                 .failedToFetchLocationList(let error),
                 .failedToParseLocationListResponse(let error),
                 .failedToParseRegisteredServersResponse(let error),
-                .failedToParseRedeemResponse(let error),
                 .wireGuardSetNetworkSettings(let error),
                 .startWireGuardBackend(let error),
                 .setWireguardConfig(let error),

diff --git a/Sources/NetworkProtection/Networking/NetworkProtectionClient.swift b/Sources/NetworkProtection/Networking/NetworkProtectionClient.swift
@@ -37,11 +37,6 @@ public enum NetworkProtectionClientError: CustomNSError, NetworkProtectionErrorC
     case failedToEncodeRegisterKeyRequest
     case failedToFetchRegisteredServers(Error)
     case failedToParseRegisteredServersResponse(Error)
-    case failedToEncodeRedeemRequest
-    case invalidInviteCode
-    case failedToRedeemInviteCode(Error)
-    case failedToRetrieveAuthToken(AuthenticationFailureResponse)
-    case failedToParseRedeemResponse(Error)
     case invalidAuthToken
     case accessDenied
 
@@ -56,11 +51,6 @@ public enum NetworkProtectionClientError: CustomNSError, NetworkProtectionErrorC
         case .failedToEncodeRegisterKeyRequest: return .failedToEncodeRegisterKeyRequest
         case .failedToFetchRegisteredServers(let error): return .failedToFetchRegisteredServers(error)
         case .failedToParseRegisteredServersResponse(let error): return .failedToParseRegisteredServersResponse(error)
-        case .failedToEncodeRedeemRequest: return .failedToEncodeRedeemRequest
-        case .invalidInviteCode: return .invalidInviteCode
-        case .failedToRedeemInviteCode(let error): return .failedToRedeemInviteCode(error)
-        case .failedToRetrieveAuthToken(let response): return .failedToRetrieveAuthToken(response)
-        case .failedToParseRedeemResponse(let error): return .failedToParseRedeemResponse(error)
         case .invalidAuthToken: return .invalidAuthToken
         case .accessDenied: return .vpnAccessRevoked
         }
@@ -75,11 +65,6 @@ public enum NetworkProtectionClientError: CustomNSError, NetworkProtectionErrorC
         case .failedToEncodeRegisterKeyRequest: return 4
         case .failedToFetchRegisteredServers: return 5
         case .failedToParseRegisteredServersResponse: return 6
-        case .failedToEncodeRedeemRequest: return 7
-        case .invalidInviteCode: return 8
-        case .failedToRedeemInviteCode: return 9
-        case .failedToRetrieveAuthToken: return 10
-        case .failedToParseRedeemResponse: return 11
         case .invalidAuthToken: return 12
         case .accessDenied: return 13
         case .failedToFetchServerStatus: return 14
@@ -95,15 +80,10 @@ public enum NetworkProtectionClientError: CustomNSError, NetworkProtectionErrorC
                 .failedToParseServerListResponse(let error),
                 .failedToFetchRegisteredServers(let error),
                 .failedToParseRegisteredServersResponse(let error),
-                .failedToRedeemInviteCode(let error),
-                .failedToParseRedeemResponse(let error),
                 .failedToFetchServerStatus(let error),
                 .failedToParseServerStatusResponse(let error):
             return [NSUnderlyingErrorKey: error as NSError]
         case .failedToEncodeRegisterKeyRequest,
-                .failedToEncodeRedeemRequest,
-                .invalidInviteCode,
-                .failedToRetrieveAuthToken,
                 .invalidAuthToken,
                 .accessDenied:
             return [:]
@@ -159,10 +139,6 @@ enum RegisterServerSelection {
     }
 }
 
-struct RedeemInviteCodeRequestBody: Encodable {
-    let code: String
-}
-
 struct ExchangeAccessTokenRequestBody: Encodable {
     let token: String
 }
@@ -171,10 +147,6 @@ struct AuthenticationSuccessResponse: Decodable {
     let token: String
 }
 
-public struct AuthenticationFailureResponse: Decodable {
-    public let message: String
-}
-
 final class NetworkProtectionBackendClient: NetworkProtectionClient {
 
     enum Constants {
@@ -435,57 +407,6 @@ final class NetworkProtectionBackendClient: NetworkProtectionClient {
         }
     }
 
-    private func retrieveAuthToken<RequestBody: Encodable>(
-        requestBody: RequestBody,
-        endpoint: URL
-    ) async -> Result<String, NetworkProtectionClientError> {
-        let requestBodyData: Data
-
-        do {
-            requestBodyData = try JSONEncoder().encode(requestBody)
-        } catch {
-            return .failure(.failedToEncodeRedeemRequest)
-        }
-
-        var request = URLRequest(url: endpoint)
-        request.allHTTPHeaderFields = APIRequest.Headers().httpHeaders
-        request.addValue("application/json", forHTTPHeaderField: "Content-Type")
-        request.httpMethod = "POST"
-        request.httpBody = requestBodyData
-
-        let responseData: Data
-
-        do {
-            let (data, response) = try await URLSession.shared.data(for: request)
-            guard let response = response as? HTTPURLResponse else {
-                throw AuthTokenError.noResponse
-            }
-            switch response.statusCode {
-            case 200:
-                responseData = data
-            case 400:
-                return .failure(.invalidInviteCode)
-            default:
-                do {
-                    // Try to redeem the subscription backend error response first:
-                    let decodedRedemptionResponse = try decoder.decode(AuthenticationFailureResponse.self, from: data)
-                    return .failure(.failedToRetrieveAuthToken(decodedRedemptionResponse))
-                } catch {
-                    throw AuthTokenError.unexpectedStatus(status: response.statusCode)
-                }
-            }
-        } catch {
-            return .failure(NetworkProtectionClientError.failedToRedeemInviteCode(error))
-        }
-
-        do {
-            let decodedRedemptionResponse = try decoder.decode(AuthenticationSuccessResponse.self, from: responseData)
-            return .success(decodedRedemptionResponse.token)
-        } catch {
-            return .failure(NetworkProtectionClientError.failedToParseRedeemResponse(error))
-        }
-    }
-
 }
 
 extension URL {

diff --git a/Sources/NetworkProtection/PacketTunnelProvider.swift b/Sources/NetworkProtection/PacketTunnelProvider.swift
@@ -884,7 +884,7 @@ open class PacketTunnelProvider: NEPacketTunnelProvider {
 
     // MARK: - Tunnel Stop: Support Methods
 
-    /// Do not call this directly.  Call `stopTunnel(with:)` or `cancelTunnel(with:)` instead.
+    /// Do not call this directly, call `cancelTunnel(with:)` instead.
     ///
     @MainActor
     private func stopTunnel() async throws {

diff --git a/Sources/PixelKit/PixelKit.swift b/Sources/PixelKit/PixelKit.swift
@@ -41,9 +41,14 @@ public final class PixelKit {
         /// Sent once per day. The last timestamp for this pixel is stored and compared to the current date. Pixels of this type will have `_d` appended to their name.
         case daily
 
-        /// Sent once per day with a `_d` suffix, in addition to every time it is called with a `_c` suffix.
+        /// [Legacy] Sent once per day with a `_d` suffix, in addition to every time it is called with a `_c` suffix.
         /// This means a pixel will get sent twice the first time it is called per-day, and subsequent calls that day will only send the `_c` variant.
         /// This is useful in situations where pixels receive spikes in volume, as the daily pixel can be used to determine how many users are actually affected.
+        case legacyDailyAndCount
+
+        /// Sent once per day with a `_daily` suffix, in addition to every time it is called with a `_count` suffix.
+        /// This means a pixel will get sent twice the first time it is called per-day, and subsequent calls that day will only send the `_count` variant.
+        /// This is useful in situations where pixels receive spikes in volume, as the daily pixel can be used to determine how many users are actually affected.
         case dailyAndCount
 
         fileprivate var description: String {
@@ -58,6 +63,8 @@ public final class PixelKit {
                 "Legacy Daily"
             case .daily:
                 "Daily"
+            case .legacyDailyAndCount:
+                "Legacy Daily and Count"
             case .dailyAndCount:
                 "Daily and Count"
             }
@@ -233,7 +240,7 @@ public final class PixelKit {
             } else {
                 printDebugInfo(pixelName: pixelName + "_d", frequency: frequency, parameters: newParams, skipped: true)
             }
-        case .dailyAndCount:
+        case .legacyDailyAndCount:
             reportErrorIf(pixel: pixelName, endsWith: "_u")
             reportErrorIf(pixel: pixelName, endsWith: "_d") // Because is added automatically
             reportErrorIf(pixel: pixelName, endsWith: "_c") // Because is added automatically
@@ -245,6 +252,18 @@ public final class PixelKit {
             }
 
             fireRequestWrapper(pixelName + "_c", headers, newParams, allowedQueryReservedCharacters, true, frequency, onComplete)
+        case .dailyAndCount:
+            reportErrorIf(pixel: pixelName, endsWith: "_u")
+            reportErrorIf(pixel: pixelName, endsWith: "_daily") // Because is added automatically
+            reportErrorIf(pixel: pixelName, endsWith: "_count") // Because is added automatically
+            if !pixelHasBeenFiredToday(pixelName) {
+                fireRequestWrapper(pixelName + "_daily", headers, newParams, allowedQueryReservedCharacters, true, frequency, onComplete)
+                updatePixelLastFireDate(pixelName: pixelName)
+            } else {
+                printDebugInfo(pixelName: pixelName + "_daily", frequency: frequency, parameters: newParams, skipped: true)
+            }
+
+            fireRequestWrapper(pixelName + "_count", headers, newParams, allowedQueryReservedCharacters, true, frequency, onComplete)
         }
     }
 

diff --git a/Sources/PixelKitTestingUtilities/XCTestCase+PixelKit.swift b/Sources/PixelKitTestingUtilities/XCTestCase+PixelKit.swift
@@ -97,7 +97,7 @@ public extension XCTestCase {
         let knownExpectedParameters = knownExpectedParameters(for: event)
         let callbackExecutedExpectation = expectation(description: "The PixelKit callback has been executed")
 
-        if frequency == .dailyAndCount {
+        if frequency == .legacyDailyAndCount {
             callbackExecutedExpectation.expectedFulfillmentCount = 2
         }
 
@@ -123,7 +123,7 @@ public extension XCTestCase {
                 firedParameters[key] == value
             }, file: file, line: line)
 
-            if frequency == .dailyAndCount {
+            if frequency == .legacyDailyAndCount {
                 XCTAssertTrue(firedPixelName.hasPrefix(expectations.pixelName))
                 XCTAssertTrue(firedPixelName.hasSuffix("_c") || firedPixelName.hasSuffix("_d"))
                 XCTAssertEqual(firedPixelName.count, expectations.pixelName.count + 2)
@@ -155,9 +155,12 @@ public extension XCTestCase {
             expectedPixelNames.append(originalName)
         case .daily:
             expectedPixelNames.append(originalName.appending("_d"))
-        case .dailyAndCount:
+        case .legacyDailyAndCount:
             expectedPixelNames.append(originalName.appending("_d"))
             expectedPixelNames.append(originalName.appending("_c"))
+        case .dailyAndCount:
+            expectedPixelNames.append(originalName.appending("_daily"))
+            expectedPixelNames.append(originalName.appending("_count"))
         }
         return expectedPixelNames
     }

diff --git a/Tests/NetworkProtectionTests/NetworkProtectionErrorTests.swift b/Tests/NetworkProtectionTests/NetworkProtectionErrorTests.swift
@@ -40,8 +40,6 @@ final class NetworkProtectionErrorTests: XCTestCase {
             .couldNotGetPeerHostName,
             .couldNotGetInterfaceAddressRange,
             .failedToEncodeRegisterKeyRequest,
-            .failedToEncodeRedeemRequest,
-            .invalidInviteCode,
             .invalidAuthToken,
             .serverListInconsistency,
             .wireGuardCannotLocateTunnelFileDescriptor,
@@ -72,8 +70,6 @@ final class NetworkProtectionErrorTests: XCTestCase {
             .failedToParseLocationListResponse(underlyingError),
             .failedToFetchRegisteredServers(underlyingError),
             .failedToParseRegisteredServersResponse(underlyingError),
-            .failedToRedeemInviteCode(underlyingError),
-            .failedToParseRedeemResponse(underlyingError),
             .wireGuardSetNetworkSettings(underlyingError),
             .startWireGuardBackend(underlyingError),
             .unhandledError(function: #function, line: #line, error: underlyingError),