From 16be79e9f3dd183a73c8e3ace762a234d7e78f2c Mon Sep 17 00:00:00 2001 From: Graeme Arthur Date: Thu, 11 Apr 2024 18:43:10 +0200 Subject: [PATCH 1/6] Specific tunnelcontroller StartError reporting --- .../NetworkProtectionTunnelController.swift | 40 ++++++++++++++----- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/DuckDuckGo/NetworkProtectionTunnelController.swift b/DuckDuckGo/NetworkProtectionTunnelController.swift index aaba16d63b..2b3c88fe5e 100644 --- a/DuckDuckGo/NetworkProtectionTunnelController.swift +++ b/DuckDuckGo/NetworkProtectionTunnelController.swift @@ -37,9 +37,24 @@ final class NetworkProtectionTunnelController: TunnelController { // MARK: - Starting & Stopping the VPN - enum StartError: LocalizedError { - case connectionStatusInvalid + enum StartError: LocalizedError, CustomNSError { case simulateControllerFailureError + case loadFromPreferencesFailed(Error) + case saveToPreferencesFailed(Error) + case startVPNFailed(Error) + case fetchAuthTokenFailed(Error) + + public static let errorDomain = "com.duckduckgo.NetworkProtectionTunnelController.StartError.domain" + + public var errorCode: Int { + switch self { + case .simulateControllerFailureError: 0 + case .loadFromPreferencesFailed: 1 + case .saveToPreferencesFailed: 2 + case .startVPNFailed: 3 + case .fetchAuthTokenFailed: 4 + } + } } init() { @@ -147,7 +162,11 @@ final class NetworkProtectionTunnelController: TunnelController { } options["activationAttemptId"] = UUID().uuidString as NSString - options["authToken"] = try tokenStore.fetchToken() as NSString? + do { + options["authToken"] = try tokenStore.fetchToken() as NSString? + } catch { + throw StartError.fetchAuthTokenFailed(error) + } options[NetworkProtectionOptionKey.selectedEnvironment] = VPNSettings(defaults: .networkProtectionGroupDefaults) .selectedEnvironment.rawValue as NSString @@ -160,8 +179,9 @@ final class NetworkProtectionTunnelController: TunnelController { ) } } catch { + // Not this one Pixel.fire(pixel: .networkProtectionActivationRequestFailed, error: error) - throw error + throw StartError.startVPNFailed(error) } } @@ -203,17 +223,16 @@ final class NetworkProtectionTunnelController: TunnelController { private func setupAndSave(_ tunnelManager: NETunnelProviderManager) async throws { setup(tunnelManager) - try await tunnelManager.saveToPreferences() - try await tunnelManager.loadFromPreferences() - try await tunnelManager.saveToPreferences() + try await saveToPreferences(tunnelManager) + try await loadFromPreferences(tunnelManager) + try await saveToPreferences(tunnelManager) } private func saveToPreferences(_ tunnelManager: NETunnelProviderManager) async throws { do { try await tunnelManager.saveToPreferences() } catch { - Pixel.fire(pixel: .networkProtectionFailedToSaveToPreferences, error: error) - throw error + throw StartError.saveToPreferencesFailed(error) } } @@ -221,8 +240,7 @@ final class NetworkProtectionTunnelController: TunnelController { do { try await tunnelManager.loadFromPreferences() } catch { - Pixel.fire(pixel: .networkProtectionFailedToLoadFromPreferences, error: error) - throw error + throw StartError.loadFromPreferencesFailed(error) } } From 6588a90203d407473ba921d341df18fab2a79eba Mon Sep 17 00:00:00 2001 From: Graeme Arthur Date: Thu, 11 Apr 2024 18:45:17 +0200 Subject: [PATCH 2/6] Remove silly comment --- DuckDuckGo/NetworkProtectionTunnelController.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/DuckDuckGo/NetworkProtectionTunnelController.swift b/DuckDuckGo/NetworkProtectionTunnelController.swift index 2b3c88fe5e..51589d4804 100644 --- a/DuckDuckGo/NetworkProtectionTunnelController.swift +++ b/DuckDuckGo/NetworkProtectionTunnelController.swift @@ -179,7 +179,6 @@ final class NetworkProtectionTunnelController: TunnelController { ) } } catch { - // Not this one Pixel.fire(pixel: .networkProtectionActivationRequestFailed, error: error) throw StartError.startVPNFailed(error) } From 06238f0a580ee865072130b7e28812f8ba366567 Mon Sep 17 00:00:00 2001 From: Graeme Arthur Date: Fri, 12 Apr 2024 11:02:12 +0200 Subject: [PATCH 3/6] Use auto domain for tunnelcontroller start error --- DuckDuckGo/NetworkProtectionTunnelController.swift | 2 -- 1 file changed, 2 deletions(-) diff --git a/DuckDuckGo/NetworkProtectionTunnelController.swift b/DuckDuckGo/NetworkProtectionTunnelController.swift index 51589d4804..e0309ddbc3 100644 --- a/DuckDuckGo/NetworkProtectionTunnelController.swift +++ b/DuckDuckGo/NetworkProtectionTunnelController.swift @@ -44,8 +44,6 @@ final class NetworkProtectionTunnelController: TunnelController { case startVPNFailed(Error) case fetchAuthTokenFailed(Error) - public static let errorDomain = "com.duckduckgo.NetworkProtectionTunnelController.StartError.domain" - public var errorCode: Int { switch self { case .simulateControllerFailureError: 0 From f265a340b0b714a2213214c192d77bc8a92d682f Mon Sep 17 00:00:00 2001 From: Graeme Arthur Date: Fri, 12 Apr 2024 11:39:39 +0200 Subject: [PATCH 4/6] Send underlying error with pixel --- DuckDuckGo/NetworkProtectionTunnelController.swift | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/DuckDuckGo/NetworkProtectionTunnelController.swift b/DuckDuckGo/NetworkProtectionTunnelController.swift index e0309ddbc3..4b75439e45 100644 --- a/DuckDuckGo/NetworkProtectionTunnelController.swift +++ b/DuckDuckGo/NetworkProtectionTunnelController.swift @@ -53,6 +53,20 @@ final class NetworkProtectionTunnelController: TunnelController { case .fetchAuthTokenFailed: 4 } } + + public var errorUserInfo: [String: Any] { + switch self { + case + .simulateControllerFailureError: + return [:] + case + .loadFromPreferencesFailed(let error), + .saveToPreferencesFailed(let error), + .startVPNFailed(let error), + .fetchAuthTokenFailed(let error): + return ["NSUnderlyingError": error] + } + } } init() { From 32cfb1fbb5c6ea6fd086696f8a02885a6dcf2d7d Mon Sep 17 00:00:00 2001 From: Graeme Arthur Date: Fri, 12 Apr 2024 11:39:58 +0200 Subject: [PATCH 5/6] Filter out permission denied errors --- DuckDuckGo/NetworkProtectionTunnelController.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/DuckDuckGo/NetworkProtectionTunnelController.swift b/DuckDuckGo/NetworkProtectionTunnelController.swift index 4b75439e45..b2f4caefdd 100644 --- a/DuckDuckGo/NetworkProtectionTunnelController.swift +++ b/DuckDuckGo/NetworkProtectionTunnelController.swift @@ -243,6 +243,14 @@ final class NetworkProtectionTunnelController: TunnelController { do { try await tunnelManager.saveToPreferences() } catch { + let nsError = error as NSError + if nsError.code == NEVPNError.Code.configurationReadWriteFailed.rawValue, + nsError.localizedDescription == "permission denied" { + // This is a user denying the system permissions prompt to add the config + // Maybe we should fire another pixel here, but not a start failure as this is an imaginable scenario + // The code could be caused by a number of problems so I'm using the localizedDescription to catch that case + return + } throw StartError.saveToPreferencesFailed(error) } } From 0e2dd0de01fee2cab8b5c2a104e8ab8487771dc1 Mon Sep 17 00:00:00 2001 From: Graeme Arthur Date: Fri, 12 Apr 2024 12:30:00 +0200 Subject: [PATCH 6/6] User underlying error key constant --- DuckDuckGo/NetworkProtectionTunnelController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGo/NetworkProtectionTunnelController.swift b/DuckDuckGo/NetworkProtectionTunnelController.swift index b2f4caefdd..ea50513c9a 100644 --- a/DuckDuckGo/NetworkProtectionTunnelController.swift +++ b/DuckDuckGo/NetworkProtectionTunnelController.swift @@ -64,7 +64,7 @@ final class NetworkProtectionTunnelController: TunnelController { .saveToPreferencesFailed(let error), .startVPNFailed(let error), .fetchAuthTokenFailed(let error): - return ["NSUnderlyingError": error] + return [NSUnderlyingErrorKey: error] } } }