diff options
| author | Andrej Mihajlov <and@mullvad.net> | 2023-02-28 14:37:44 +0100 |
|---|---|---|
| committer | Andrej Mihajlov <and@mullvad.net> | 2023-02-28 14:37:44 +0100 |
| commit | c3825cade57944600cebe16d6567674cc050082f (patch) | |
| tree | 7f490b0d602aafa85157e9e795119919be8289ad | |
| parent | f2e71de32cde3c8c29dc12349b1d1e147fbc6296 (diff) | |
| parent | 0e2300104d4578984529ab76e86ea7e4955b642d (diff) | |
| download | mullvadvpn-c3825cade57944600cebe16d6567674cc050082f.tar.xz mullvadvpn-c3825cade57944600cebe16d6567674cc050082f.zip | |
Merge branch 'drop-op-completion'
49 files changed, 460 insertions, 637 deletions
diff --git a/ios/MullvadREST/RESTAccessTokenManager.swift b/ios/MullvadREST/RESTAccessTokenManager.swift index 5a9dba1a52..d8d7ef5521 100644 --- a/ios/MullvadREST/RESTAccessTokenManager.swift +++ b/ios/MullvadREST/RESTAccessTokenManager.swift @@ -25,37 +25,36 @@ extension REST { func getAccessToken( accountNumber: String, - completionHandler: @escaping (OperationCompletion<REST.AccessTokenData, REST.Error>) - -> Void + completionHandler: @escaping (Result<REST.AccessTokenData, Swift.Error>) -> Void ) -> Cancellable { - let operation = ResultBlockOperation<REST.AccessTokenData, REST.Error>( - dispatchQueue: dispatchQueue - ) { operation in + let operation = ResultBlockOperation<REST.AccessTokenData>(dispatchQueue: dispatchQueue) + + operation.setExecutionBlock { operation in if let tokenData = self.tokens[accountNumber], tokenData.expiry > Date() { - operation.finish(completion: .success(tokenData)) + operation.finish(result: .success(tokenData)) return } let task = self.proxy.getAccessToken( accountNumber: accountNumber, retryStrategy: .noRetry - ) { completion in + ) { result in self.dispatchQueue.async { - switch completion { + switch result { case let .success(tokenData): self.tokens[accountNumber] = tokenData - case let .failure(error): + case let .failure(error) where !error.isOperationCancellationError: self.logger.error( error: error, message: "Failed to fetch access token." ) - case .cancelled: + default: break } - operation.finish(completion: completion) + operation.finish(result: result) } } diff --git a/ios/MullvadREST/RESTAuthorization.swift b/ios/MullvadREST/RESTAuthorization.swift index c75a20f8cd..cce6e23f1f 100644 --- a/ios/MullvadREST/RESTAuthorization.swift +++ b/ios/MullvadREST/RESTAuthorization.swift @@ -11,9 +11,8 @@ import MullvadTypes import Operations protocol RESTAuthorizationProvider { - typealias Completion = OperationCompletion<REST.Authorization, REST.Error> - - func getAuthorization(completion: @escaping (Completion) -> Void) -> Cancellable + func getAuthorization(completion: @escaping (Result<REST.Authorization, Swift.Error>) -> Void) + -> Cancellable } extension REST { @@ -28,13 +27,15 @@ extension REST { self.accountNumber = accountNumber } - func getAuthorization(completion: @escaping (Completion) -> Void) -> Cancellable { - return accessTokenManager - .getAccessToken(accountNumber: accountNumber) { operationCompletion in - completion(operationCompletion.map { tokenData in - return tokenData.accessToken - }) - } + func getAuthorization( + completion: @escaping (Result<REST.Authorization, Swift.Error>) + -> Void + ) -> Cancellable { + return accessTokenManager.getAccessToken(accountNumber: accountNumber) { result in + completion(result.map { tokenData in + return tokenData.accessToken + }) + } } } } diff --git a/ios/MullvadREST/RESTNetworkOperation.swift b/ios/MullvadREST/RESTNetworkOperation.swift index 9c8e0cbf2a..64f07a3e93 100644 --- a/ios/MullvadREST/RESTNetworkOperation.swift +++ b/ios/MullvadREST/RESTNetworkOperation.swift @@ -12,7 +12,7 @@ import MullvadTypes import Operations extension REST { - class NetworkOperation<Success>: ResultOperation<Success, REST.Error> { + class NetworkOperation<Success>: ResultOperation<Success> { private let requestHandler: RESTRequestHandler private let responseHandler: AnyResponseHandler<Success> @@ -76,7 +76,7 @@ extension REST { dispatchPrecondition(condition: .onQueue(dispatchQueue)) guard !isCancelled else { - finish(completion: .cancelled) + finish(result: .failure(OperationError.cancelled)) return } @@ -87,17 +87,18 @@ extension REST { } requiresAuthorization = true - authorizationTask = authorizationProvider.getAuthorization { completion in + authorizationTask = authorizationProvider.getAuthorization { result in self.dispatchQueue.async { - switch completion { + switch result { case let .success(authorization): self.didReceiveAuthorization(authorization) case let .failure(error): - self.didFailToRequestAuthorization(error) - - case .cancelled: - self.finish(completion: .cancelled) + if error.isOperationCancellationError { + self.finish(result: .failure(error)) + } else { + self.didFailToRequestAuthorization(error) + } } } } @@ -107,7 +108,7 @@ extension REST { dispatchPrecondition(condition: .onQueue(dispatchQueue)) guard !isCancelled else { - finish(completion: .cancelled) + finish(result: .failure(OperationError.cancelled)) return } @@ -125,7 +126,7 @@ extension REST { } } - private func didFailToRequestAuthorization(_ error: REST.Error) { + private func didFailToRequestAuthorization(_ error: Swift.Error) { dispatchPrecondition(condition: .onQueue(dispatchQueue)) logger.error( @@ -133,7 +134,7 @@ extension REST { message: "Failed to request authorization." ) - finish(completion: .failure(error)) + finish(result: .failure(error)) } private func didReceiveURLRequest(_ restRequest: REST.Request, endpoint: AnyIPEndpoint) { @@ -141,7 +142,7 @@ extension REST { guard let transport = transportProvider() else { logger.error("Failed to obtain transport.") - finish(completion: .failure(.transport(NoTransportError()))) + finish(result: .failure(REST.Error.transport(NoTransportError()))) return } @@ -190,7 +191,7 @@ extension REST { message: "Failed to create URLRequest." ) - finish(completion: .failure(error)) + finish(result: .failure(error)) } private func didReceiveError( @@ -203,7 +204,7 @@ extension REST { if let urlError = error as? URLError { switch urlError.code { case .cancelled: - finish(completion: .cancelled) + finish(result: .failure(OperationError.cancelled)) return case .notConnectedToInternet, .internationalRoamingOff, .callIsActive: @@ -237,7 +238,7 @@ extension REST { switch handlerResult { case let .success(output): // Response handler produced value. - finish(completion: .success(output)) + finish(result: .success(output)) case let .decoding(decoderBlock): // Response handler returned a block decoding value. @@ -245,7 +246,7 @@ extension REST { .mapError { error -> REST.Error in return .decodeResponse(error) } - finish(completion: OperationCompletion(result: decodeResult)) + finish(result: decodeResult.mapError { $0 }) case let .unhandledResponse(serverErrorResponse): // Response handler couldn't handle the response. @@ -257,11 +258,9 @@ extension REST { retryInvalidAccessTokenError = false startRequest() } else { - finish( - completion: .failure( - .unhandledResponse(response.statusCode, serverErrorResponse) - ) - ) + finish(result: .failure( + REST.Error.unhandledResponse(response.statusCode, serverErrorResponse) + )) } } } @@ -272,7 +271,7 @@ extension REST { if retryStrategy.maxRetryCount > 0 { logger.debug("Ran out of retry attempts (\(retryStrategy.maxRetryCount))") } - finish(completion: .failure(wrapRequestError(error))) + finish(result: .failure(wrapRequestError(error))) return } @@ -288,7 +287,7 @@ extension REST { guard let waitDelay = retryDelayIterator.next() else { logger.debug("Retry delay iterator failed to produce next value.") - finish(completion: .failure(wrapRequestError(error))) + finish(result: .failure(wrapRequestError(error))) return } @@ -302,7 +301,7 @@ extension REST { } timer.setCancelHandler { [weak self] in - self?.finish(completion: .cancelled) + self?.finish(result: .failure(OperationError.cancelled)) } timer.schedule(wallDeadline: .now() + waitDelay.timeInterval) diff --git a/ios/MullvadREST/RESTProxy.swift b/ios/MullvadREST/RESTProxy.swift index fe9bf7386a..5c8a14df81 100644 --- a/ios/MullvadREST/RESTProxy.swift +++ b/ios/MullvadREST/RESTProxy.swift @@ -12,8 +12,7 @@ import Operations extension REST { public class Proxy<ConfigurationType: ProxyConfiguration> { - public typealias CompletionHandler<Success> = (OperationCompletion<Success, REST.Error>) - -> Void + public typealias CompletionHandler<Success> = (Result<Success, Swift.Error>) -> Void /// Synchronization queue used by network operations. let dispatchQueue: DispatchQueue diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj index 6875501ade..a5bc1234df 100644 --- a/ios/MullvadVPN.xcodeproj/project.pbxproj +++ b/ios/MullvadVPN.xcodeproj/project.pbxproj @@ -248,10 +248,9 @@ 58D223AF294C8A630029F5F8 /* NoFailedDependenciesCondition.swift in Sources */ = {isa = PBXBuildFile; fileRef = 581813A228E09DCD002817DE /* NoFailedDependenciesCondition.swift */; }; 58D223B0294C8A630029F5F8 /* TransformOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58059DDB28465E8F002B1049 /* TransformOperation.swift */; }; 58D223B1294C8A630029F5F8 /* OperationCondition.swift in Sources */ = {isa = PBXBuildFile; fileRef = 589D28772846250500F9A7B3 /* OperationCondition.swift */; }; - 58D223B2294C8A630029F5F8 /* OperationCompletion.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5840BE34279EDB16002836BA /* OperationCompletion.swift */; }; + 58D223B2294C8A630029F5F8 /* OperationError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5840BE34279EDB16002836BA /* OperationError.swift */; }; 58D223B3294C8A630029F5F8 /* OperationObserver.swift in Sources */ = {isa = PBXBuildFile; fileRef = 589D28792846250500F9A7B3 /* OperationObserver.swift */; }; 58D223B4294C8A630029F5F8 /* InputInjectionBuilder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58DF5B752852108E00E92647 /* InputInjectionBuilder.swift */; }; - 58D223B5294C8A630029F5F8 /* ResultOperation+Output.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58059DDF2846823E002B1049 /* ResultOperation+Output.swift */; }; 58D223B6294C8A630029F5F8 /* BlockCondition.swift in Sources */ = {isa = PBXBuildFile; fileRef = 581813A428E09DE2002817DE /* BlockCondition.swift */; }; 58D223B7294C8A630029F5F8 /* MutuallyExclusive.swift in Sources */ = {isa = PBXBuildFile; fileRef = 581813A628E09DF2002817DE /* MutuallyExclusive.swift */; }; 58D223B8294C8A630029F5F8 /* ResultOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58F7D26427EB50A300E4D821 /* ResultOperation.swift */; }; @@ -643,7 +642,6 @@ 5803B4B12940A48700C23744 /* TunnelStore.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TunnelStore.swift; sourceTree = "<group>"; }; 58059DDB28465E8F002B1049 /* TransformOperation.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = TransformOperation.swift; sourceTree = "<group>"; }; 58059DDD28468158002B1049 /* OutputOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OutputOperation.swift; sourceTree = "<group>"; }; - 58059DDF2846823E002B1049 /* ResultOperation+Output.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ResultOperation+Output.swift"; sourceTree = "<group>"; }; 5807E2BF2432038B00F5FF30 /* String+Split.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "String+Split.swift"; sourceTree = "<group>"; }; 5807E2C1243203D000F5FF30 /* StringTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = StringTests.swift; sourceTree = "<group>"; }; 5808273928487E3E006B77A4 /* Base.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Base.xcconfig; sourceTree = "<group>"; }; @@ -694,7 +692,7 @@ 583DA21325FA4B5C00318683 /* LocationDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocationDataSource.swift; sourceTree = "<group>"; }; 583E1E292848DF67004838B3 /* OperationObserverTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OperationObserverTests.swift; sourceTree = "<group>"; }; 5840250322B11AB700E4CFEC /* MullvadEndpoint.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MullvadEndpoint.swift; sourceTree = "<group>"; }; - 5840BE34279EDB16002836BA /* OperationCompletion.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OperationCompletion.swift; sourceTree = "<group>"; }; + 5840BE34279EDB16002836BA /* OperationError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OperationError.swift; sourceTree = "<group>"; }; 5842102D282D3FC200F24E46 /* ResultBlockOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ResultBlockOperation.swift; sourceTree = "<group>"; }; 5842102F282D8A3C00F24E46 /* UpdateAccountDataOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UpdateAccountDataOperation.swift; sourceTree = "<group>"; }; 58421031282E42B000F24E46 /* UpdateDeviceDataOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UpdateDeviceDataOperation.swift; sourceTree = "<group>"; }; @@ -1545,13 +1543,12 @@ 581813A628E09DF2002817DE /* MutuallyExclusive.swift */, 581813A028E09DBB002817DE /* NoCancelledDependenciesCondition.swift */, 581813A228E09DCD002817DE /* NoFailedDependenciesCondition.swift */, - 5840BE34279EDB16002836BA /* OperationCompletion.swift */, + 5840BE34279EDB16002836BA /* OperationError.swift */, 589D28772846250500F9A7B3 /* OperationCondition.swift */, 589D28792846250500F9A7B3 /* OperationObserver.swift */, 58059DDD28468158002B1049 /* OutputOperation.swift */, 5842102D282D3FC200F24E46 /* ResultBlockOperation.swift */, 58F7D26427EB50A300E4D821 /* ResultOperation.swift */, - 58059DDF2846823E002B1049 /* ResultOperation+Output.swift */, 58059DDB28465E8F002B1049 /* TransformOperation.swift */, ); path = Operations; @@ -2503,12 +2500,11 @@ 58D223B6294C8A630029F5F8 /* BlockCondition.swift in Sources */, 58D223BA294C8A630029F5F8 /* BackgroundObserver.swift in Sources */, 58D223AD294C8A630029F5F8 /* AsyncBlockOperation.swift in Sources */, - 58D223B5294C8A630029F5F8 /* ResultOperation+Output.swift in Sources */, 58D223AC294C8A630029F5F8 /* GroupOperation.swift in Sources */, 58D223BB294C8A630029F5F8 /* NoCancelledDependenciesCondition.swift in Sources */, 58D223B3294C8A630029F5F8 /* OperationObserver.swift in Sources */, 58D223B7294C8A630029F5F8 /* MutuallyExclusive.swift in Sources */, - 58D223B2294C8A630029F5F8 /* OperationCompletion.swift in Sources */, + 58D223B2294C8A630029F5F8 /* OperationError.swift in Sources */, 58D223AF294C8A630029F5F8 /* NoFailedDependenciesCondition.swift in Sources */, 58D223B9294C8A630029F5F8 /* AsyncOperationQueue.swift in Sources */, 58D223B8294C8A630029F5F8 /* ResultOperation.swift in Sources */, diff --git a/ios/MullvadVPN/AccountInteractor.swift b/ios/MullvadVPN/AccountInteractor.swift index 8444a426db..c454181d95 100644 --- a/ios/MullvadVPN/AccountInteractor.swift +++ b/ios/MullvadVPN/AccountInteractor.swift @@ -56,10 +56,7 @@ final class AccountInteractor { func restorePurchases( for accountNumber: String, - completionHandler: @escaping (OperationCompletion< - REST.CreateApplePaymentResponse, - StorePaymentManagerError - >) -> Void + completionHandler: @escaping (Result<REST.CreateApplePaymentResponse, Error>) -> Void ) -> Cancellable { return storePaymentManager.restorePurchases( for: accountNumber, @@ -69,7 +66,7 @@ final class AccountInteractor { func requestProducts( with productIdentifiers: Set<StoreSubscription>, - completionHandler: @escaping (OperationCompletion<SKProductsResponse, Error>) -> Void + completionHandler: @escaping (Result<SKProductsResponse, Error>) -> Void ) -> Cancellable { return storePaymentManager.requestProducts( with: productIdentifiers, diff --git a/ios/MullvadVPN/AccountViewController.swift b/ios/MullvadVPN/AccountViewController.swift index 1d4b82b56e..34f16b3233 100644 --- a/ios/MullvadVPN/AccountViewController.swift +++ b/ios/MullvadVPN/AccountViewController.swift @@ -329,10 +329,10 @@ class AccountViewController: UIViewController { case let .success(response): self.showTimeAddedConfirmationAlert(with: response, context: .restoration) - case let .failure(error): + case let .failure(error as StorePaymentManagerError): self.showRestorePurchasesErrorAlert(error: error) - case .cancelled: + default: break } diff --git a/ios/MullvadVPN/AddressCacheTracker.swift b/ios/MullvadVPN/AddressCacheTracker.swift index 7fb0e79826..8c3100f92b 100644 --- a/ios/MullvadVPN/AddressCacheTracker.swift +++ b/ios/MullvadVPN/AddressCacheTracker.swift @@ -84,22 +84,17 @@ final class AddressCacheTracker { timer = nil } - func updateEndpoints( - completionHandler: ((OperationCompletion<Bool, Error>) -> Void)? = nil - ) -> Cancellable { - let operation = ResultBlockOperation<Bool, Error> { operation in + func updateEndpoints(completionHandler: ((Result<Bool, Error>) -> Void)? = nil) -> Cancellable { + let operation = ResultBlockOperation<Bool> { operation in guard self.nextScheduleDate() <= Date() else { - operation.finish(completion: .success(false)) + operation.finish(result: .success(false)) return } - let task = self.apiProxy.getAddressList(retryStrategy: .default) { completion in - self.setEndpoints(from: completion) + let task = self.apiProxy.getAddressList(retryStrategy: .default) { result in + self.setEndpoints(from: result) - let mappedCompletion = completion.map { _ in true } - .eraseFailureType() - - operation.finish(completion: mappedCompletion) + operation.finish(result: result.map { _ in true }) } operation.addCancellationBlock { @@ -130,25 +125,25 @@ final class AddressCacheTracker { return _nextScheduleDate() } - private func setEndpoints(from completion: OperationCompletion<[AnyIPEndpoint], REST.Error>) { + private func setEndpoints(from result: Result<[AnyIPEndpoint], Error>) { nslock.lock() defer { nslock.unlock() } - switch completion { + switch result { case let .success(endpoints): store.setEndpoints(endpoints) + lastFailureAttemptDate = nil - case let .failure(error): + case let .failure(error as REST.Error): logger.error( error: error, message: "Failed to update address cache." ) + fallthrough - case .cancelled: - break + default: + lastFailureAttemptDate = Date() } - - lastFailureAttemptDate = completion.isSuccess ? nil : Date() } private func scheduleEndpointsUpdate(startTime: DispatchWallTime) { diff --git a/ios/MullvadVPN/AppDelegate.swift b/ios/MullvadVPN/AppDelegate.swift index 89b9b505f2..3ed73a43fa 100644 --- a/ios/MullvadVPN/AppDelegate.swift +++ b/ios/MullvadVPN/AppDelegate.swift @@ -209,10 +209,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD forTaskWithIdentifier: ApplicationConfiguration.addressCacheUpdateTaskIdentifier, using: nil ) { task in - let handle = self.addressCacheTracker.updateEndpoints { completion in + let handle = self.addressCacheTracker.updateEndpoints { result in self.scheduleAddressCacheUpdateTask() - task.setTaskCompleted(success: completion.isSuccess) + task.setTaskCompleted(success: result.isSuccess) } task.expirationHandler = { @@ -373,12 +373,12 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD } } - let migrateSettingsOperation = ResultBlockOperation<SettingsMigrationResult, Error>( + let migrateSettingsOperation = ResultBlockOperation<SettingsMigrationResult>( dispatchQueue: .main ) { operation in SettingsManager.migrateStore(with: self.proxyFactory) { migrationResult in let finishHandler = { - operation.finish(completion: .success(migrationResult)) + operation.finish(result: .success(migrationResult)) } guard case let .failure(error) = migrationResult, @@ -412,7 +412,7 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD } initTunnelManagerOperation.addDependency(migrateSettingsOperation) - let reconnectTunnelOperation = TransformOperation<SettingsMigrationResult, Void, Error>( + let reconnectTunnelOperation = TransformOperation<SettingsMigrationResult, Void>( dispatchQueue: .main ) { migrationResult in if case .success = migrationResult { diff --git a/ios/MullvadVPN/DeviceManagementInteractor.swift b/ios/MullvadVPN/DeviceManagementInteractor.swift index 87ee7f204a..7aa202a2cd 100644 --- a/ios/MullvadVPN/DeviceManagementInteractor.swift +++ b/ios/MullvadVPN/DeviceManagementInteractor.swift @@ -22,28 +22,26 @@ class DeviceManagementInteractor { @discardableResult func getDevices( - _ completionHandler: @escaping (OperationCompletion<[REST.Device], Error>) + _ completionHandler: @escaping (Result<[REST.Device], Error>) -> Void ) -> Cancellable { return devicesProxy.getDevices( accountNumber: accountNumber, - retryStrategy: .default - ) { completion in - completionHandler(completion.eraseFailureType()) - } + retryStrategy: .default, + completion: completionHandler + ) } @discardableResult func deleteDevice( _ identifier: String, - completionHandler: @escaping (OperationCompletion<Bool, Error>) -> Void + completionHandler: @escaping (Result<Bool, Error>) -> Void ) -> Cancellable { return devicesProxy.deleteDevice( accountNumber: accountNumber, identifier: identifier, - retryStrategy: .default - ) { completion in - completionHandler(completion.eraseFailureType()) - } + retryStrategy: .default, + completion: completionHandler + ) } } diff --git a/ios/MullvadVPN/DeviceManagementViewController.swift b/ios/MullvadVPN/DeviceManagementViewController.swift index d96d9172fd..d3b0d006da 100644 --- a/ios/MullvadVPN/DeviceManagementViewController.swift +++ b/ios/MullvadVPN/DeviceManagementViewController.swift @@ -85,16 +85,16 @@ class DeviceManagementViewController: UIViewController, RootContainment { func fetchDevices( animateUpdates: Bool, - completionHandler: ((OperationCompletion<Void, Error>) -> Void)? = nil + completionHandler: ((Result<Void, Error>) -> Void)? = nil ) { - interactor.getDevices { [weak self] completion in + interactor.getDevices { [weak self] result in guard let self = self else { return } - if let devices = completion.value { + if let devices = result.value { self.setDevices(devices, animated: animateUpdates) } - completionHandler?(completion.ignoreOutput()) + completionHandler?(result.map { _ in () }) } } @@ -236,14 +236,15 @@ class DeviceManagementViewController: UIViewController, RootContainment { } case let .failure(error): - self.logger.error( - error: error, - message: "Failed to delete device." - ) - completionHandler(error) - - case .cancelled: - completionHandler(nil) + if error.isOperationCancellationError { + completionHandler(nil) + } else { + self.logger.error( + error: error, + message: "Failed to delete device." + ) + completionHandler(error) + } } } } diff --git a/ios/MullvadVPN/LoginViewController.swift b/ios/MullvadVPN/LoginViewController.swift index a0c13358a3..ed113160e9 100644 --- a/ios/MullvadVPN/LoginViewController.swift +++ b/ios/MullvadVPN/LoginViewController.swift @@ -36,7 +36,7 @@ protocol LoginViewControllerDelegate: AnyObject { func loginViewController( _ controller: LoginViewController, shouldHandleLoginAction action: LoginAction, - completion: @escaping (OperationCompletion<StoredAccountData?, Error>) -> Void + completion: @escaping (Result<StoredAccountData?, Error>) -> Void ) func loginViewControllerDidFinishLogin(_ controller: LoginViewController) @@ -184,9 +184,7 @@ class LoginViewController: UIViewController, RootContainment { self?.endLogin(.success(action)) case let .failure(error): - self?.endLogin(.failure(error)) - case .cancelled: - self?.endLogin(.default) + self?.endLogin(error.isOperationCancellationError ? .default : .failure(error)) } } } diff --git a/ios/MullvadVPN/Operations/ProductsRequestOperation.swift b/ios/MullvadVPN/Operations/ProductsRequestOperation.swift index f9ddb09f72..380dc5267e 100644 --- a/ios/MullvadVPN/Operations/ProductsRequestOperation.swift +++ b/ios/MullvadVPN/Operations/ProductsRequestOperation.swift @@ -9,7 +9,7 @@ import Operations import StoreKit -public final class ProductsRequestOperation: ResultOperation<SKProductsResponse, Error>, +public final class ProductsRequestOperation: ResultOperation<SKProductsResponse>, SKProductsRequestDelegate { private let productIdentifiers: Set<String> @@ -52,7 +52,7 @@ public final class ProductsRequestOperation: ResultOperation<SKProductsResponse, self.retryCount += 1 self.retry(error: error) } else { - self.finish(completion: .failure(error)) + self.finish(result: .failure(error)) } } } @@ -61,7 +61,7 @@ public final class ProductsRequestOperation: ResultOperation<SKProductsResponse, _ request: SKProductsRequest, didReceive response: SKProductsResponse ) { - finish(completion: .success(response)) + finish(result: .success(response)) } // MARK: - Private @@ -80,7 +80,7 @@ public final class ProductsRequestOperation: ResultOperation<SKProductsResponse, } retryTimer?.setCancelHandler { [weak self] in - self?.finish(completion: .failure(error)) + self?.finish(result: .failure(error)) } retryTimer?.schedule(wallDeadline: .now() + retryDelay) diff --git a/ios/MullvadVPN/OutOfTimeInteractor.swift b/ios/MullvadVPN/OutOfTimeInteractor.swift index cd4989461a..4166cb1e36 100644 --- a/ios/MullvadVPN/OutOfTimeInteractor.swift +++ b/ios/MullvadVPN/OutOfTimeInteractor.swift @@ -61,9 +61,9 @@ final class OutOfTimeInteractor { func restorePurchases( for accountNumber: String, - completionHandler: @escaping (OperationCompletion< + completionHandler: @escaping (Result< REST.CreateApplePaymentResponse, - StorePaymentManagerError + Error >) -> Void ) -> Cancellable { return storePaymentManager.restorePurchases( @@ -74,7 +74,7 @@ final class OutOfTimeInteractor { func requestProducts( with productIdentifiers: Set<StoreSubscription>, - completionHandler: @escaping (OperationCompletion<SKProductsResponse, Error>) -> Void + completionHandler: @escaping (Result<SKProductsResponse, Error>) -> Void ) -> Cancellable { return storePaymentManager.requestProducts( with: productIdentifiers, diff --git a/ios/MullvadVPN/OutOfTimeViewController.swift b/ios/MullvadVPN/OutOfTimeViewController.swift index ac8c021bd3..70f7ea57bc 100644 --- a/ios/MullvadVPN/OutOfTimeViewController.swift +++ b/ios/MullvadVPN/OutOfTimeViewController.swift @@ -301,17 +301,17 @@ class OutOfTimeViewController: UIViewController, RootContainment { paymentState = .restoringPurchases - _ = interactor.restorePurchases(for: accountData.number) { [weak self] completion in + _ = interactor.restorePurchases(for: accountData.number) { [weak self] result in guard let self = self else { return } - switch completion { + switch result { case let .success(response): self.showAlertIfNoTimeAdded(with: response, context: .restoration) - case let .failure(error): + case let .failure(error as StorePaymentManagerError): self.showRestorePurchasesErrorAlert(error: error) - case .cancelled: + default: break } diff --git a/ios/MullvadVPN/ProblemReportInteractor.swift b/ios/MullvadVPN/ProblemReportInteractor.swift index 744eedcbac..bac2c375a4 100644 --- a/ios/MullvadVPN/ProblemReportInteractor.swift +++ b/ios/MullvadVPN/ProblemReportInteractor.swift @@ -40,7 +40,7 @@ final class ProblemReportInteractor { func sendReport( email: String, message: String, - completion: @escaping (OperationCompletion<Void, REST.Error>) -> Void + completion: @escaping (Result<Void, Error>) -> Void ) -> Cancellable { let request = REST.ProblemReportRequest( address: email, diff --git a/ios/MullvadVPN/ProblemReportSubmissionOverlayView.swift b/ios/MullvadVPN/ProblemReportSubmissionOverlayView.swift index 160128dce9..0b5a8b5bf5 100644 --- a/ios/MullvadVPN/ProblemReportSubmissionOverlayView.swift +++ b/ios/MullvadVPN/ProblemReportSubmissionOverlayView.swift @@ -17,7 +17,7 @@ class ProblemReportSubmissionOverlayView: UIView { enum State { case sending case sent(_ email: String) - case failure(REST.Error) + case failure(Error) var title: String? { switch self { @@ -96,7 +96,11 @@ class ProblemReportSubmissionOverlayView: UIView { return combinedAttributedString case let .failure(error): - return error.displayErrorDescription.flatMap { NSAttributedString(string: $0) } + if let error = error as? REST.Error { + return error.displayErrorDescription.flatMap { NSAttributedString(string: $0) } + } else { + return NSAttributedString(string: error.localizedDescription) + } } } } diff --git a/ios/MullvadVPN/ProblemReportViewController.swift b/ios/MullvadVPN/ProblemReportViewController.swift index c894664679..4a1bd4596b 100644 --- a/ios/MullvadVPN/ProblemReportViewController.swift +++ b/ios/MullvadVPN/ProblemReportViewController.swift @@ -654,7 +654,7 @@ final class ProblemReportViewController: UIViewController, UITextFieldDelegate { private func didSendProblemReport( viewModel: ViewModel, - completion: OperationCompletion<Void, REST.Error> + completion: Result<Void, Error> ) { switch completion { case .success: @@ -665,9 +665,6 @@ final class ProblemReportViewController: UIViewController, UITextFieldDelegate { case let .failure(error): submissionOverlayView.state = .failure(error) - - case .cancelled: - submissionOverlayView.state = .failure(.network(URLError(.cancelled))) } navigationItem.setHidesBackButton(false, animated: true) diff --git a/ios/MullvadVPN/RelayCacheTracker/RelayCacheTracker.swift b/ios/MullvadVPN/RelayCacheTracker/RelayCacheTracker.swift index dedfa1825d..75b73b2119 100644 --- a/ios/MullvadVPN/RelayCacheTracker/RelayCacheTracker.swift +++ b/ios/MullvadVPN/RelayCacheTracker/RelayCacheTracker.swift @@ -94,28 +94,22 @@ final class RelayCacheTracker { timerSource = nil } - func updateRelays( - completionHandler: ( - (OperationCompletion<RelaysFetchResult, Error>) -> Void - )? = nil - ) -> Cancellable { - let operation = ResultBlockOperation<RelaysFetchResult, Error>( - dispatchQueue: nil - ) { operation in + func updateRelays(completionHandler: ((Result<RelaysFetchResult, Error>) -> Void)? = nil) + -> Cancellable + { + let operation = ResultBlockOperation<RelaysFetchResult>(dispatchQueue: nil) { operation in let cachedRelays = try? self.getCachedRelays() if self.getNextUpdateDate() > Date() { - operation.finish(completion: .success(.throttled)) + operation.finish(result: .success(.throttled)) return } let task = self.apiProxy.getRelays( etag: cachedRelays?.etag, retryStrategy: .noRetry - ) { completion in - operation.finish( - completion: self.handleResponse(completion: completion) - ) + ) { result in + operation.finish(result: self.handleResponse(result: result)) } operation.addCancellationBlock { @@ -181,10 +175,10 @@ final class RelayCacheTracker { return max(nextUpdate, Date()) } - private func handleResponse( - completion: OperationCompletion<REST.ServerRelaysCacheResponse, REST.Error> - ) -> OperationCompletion<RelaysFetchResult, Error> { - let mappedCompletion = completion.tryMap { response -> RelaysFetchResult in + private func handleResponse(result: Result<REST.ServerRelaysCacheResponse, Error>) + -> Result<RelaysFetchResult, Error> + { + return result.tryMap { response -> RelaysFetchResult in switch response { case let .newContent(etag, relays): try self.storeResponse(etag: etag, relays: relays) @@ -194,16 +188,14 @@ final class RelayCacheTracker { case .notModified: return .sameContent } - } + }.inspectError { error in + guard !error.isOperationCancellationError else { return } - if let error = mappedCompletion.error { logger.error( error: error, message: "Failed to update relays." ) } - - return mappedCompletion } private func storeResponse(etag: String?, relays: REST.ServerRelaysResponse) throws { diff --git a/ios/MullvadVPN/Result+Extensions.swift b/ios/MullvadVPN/Result+Extensions.swift index 0ea447752e..204df0714e 100644 --- a/ios/MullvadVPN/Result+Extensions.swift +++ b/ios/MullvadVPN/Result+Extensions.swift @@ -26,6 +26,30 @@ extension Result { return error } } + + var isSuccess: Bool { + switch self { + case .success: + return true + case .failure: + return false + } + } + + func tryMap<NewSuccess>(_ body: (Success) throws -> NewSuccess) -> Result<NewSuccess, Error> { + return Result<NewSuccess, Error> { + let value = try self.get() + + return try body(value) + } + } + + @discardableResult func inspectError(_ body: (Failure) -> Void) -> Self { + if case let .failure(error) = self { + body(error) + } + return self + } } extension Result { diff --git a/ios/MullvadVPN/SceneDelegate.swift b/ios/MullvadVPN/SceneDelegate.swift index ac680b9906..92e3867dcd 100644 --- a/ios/MullvadVPN/SceneDelegate.swift +++ b/ios/MullvadVPN/SceneDelegate.swift @@ -629,16 +629,16 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate, UISplitViewControllerDe func loginViewController( _ controller: LoginViewController, shouldHandleLoginAction action: LoginAction, - completion: @escaping (OperationCompletion<StoredAccountData?, Error>) -> Void + completion: @escaping (Result<StoredAccountData?, Error>) -> Void ) { setEnableSettingsButton(isEnabled: false, from: controller) - tunnelManager.setAccount(action: action.setAccountAction) { operationCompletion in - switch operationCompletion { + tunnelManager.setAccount(action: action.setAccountAction) { result in + switch result { case .success: // RootContainer's settings button will be re-enabled in // `loginViewControllerDidFinishLogin` - completion(operationCompletion) + completion(result) case let .failure(error): // Show device management controller when too many devices detected during log in. @@ -664,7 +664,7 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate, UISplitViewControllerDe ) // Return .cancelled to login controller upon success. - completion(operationCompletion.flatMap { .cancelled }) + completion(result.flatMap { _ in .failure(OperationError.cancelled) }) self?.setEnableSettingsButton(isEnabled: true, from: controller) } @@ -672,9 +672,9 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate, UISplitViewControllerDe fallthrough } - case .cancelled: + case .failure(OperationError.cancelled): self.setEnableSettingsButton(isEnabled: true, from: controller) - completion(operationCompletion) + completion(result) } } } diff --git a/ios/MullvadVPN/SettingsManager/Migrations/MigrationFromV1ToV2.swift b/ios/MullvadVPN/SettingsManager/Migrations/MigrationFromV1ToV2.swift index cea71d5924..a7f8883c0f 100644 --- a/ios/MullvadVPN/SettingsManager/Migrations/MigrationFromV1ToV2.swift +++ b/ios/MullvadVPN/SettingsManager/Migrations/MigrationFromV1ToV2.swift @@ -19,8 +19,10 @@ final class MigrationFromV1ToV2: Migration { private var accountTask: Cancellable? private var deviceTask: Cancellable? - private var accountCompletion: OperationCompletion<REST.AccountData, REST.Error> = .cancelled - private var devicesCompletion: OperationCompletion<[REST.Device], REST.Error> = .cancelled + private var accountCompletion: Result<REST.AccountData, Error> = .failure( + OperationError.cancelled + ) + private var devicesCompletion: Result<[REST.Device], Error> = .failure(OperationError.cancelled) private let legacySettings: LegacyTunnelSettings diff --git a/ios/MullvadVPN/StorePaymentManager/SendStoreReceiptOperation.swift b/ios/MullvadVPN/StorePaymentManager/SendStoreReceiptOperation.swift index 73e9788c2b..4afb212585 100644 --- a/ios/MullvadVPN/StorePaymentManager/SendStoreReceiptOperation.swift +++ b/ios/MullvadVPN/StorePaymentManager/SendStoreReceiptOperation.swift @@ -13,10 +13,8 @@ import MullvadTypes import Operations import StoreKit -class SendStoreReceiptOperation: ResultOperation< - REST.CreateApplePaymentResponse, - StorePaymentManagerError ->, SKRequestDelegate { +class SendStoreReceiptOperation: ResultOperation<REST.CreateApplePaymentResponse>, SKRequestDelegate +{ private let apiProxy: REST.APIProxy private let accountNumber: String @@ -75,7 +73,7 @@ class SendStoreReceiptOperation: ResultOperation< error: error, message: "Failed to read the AppStore receipt." ) - finish(completion: .failure(.readReceipt(error))) + finish(result: .failure(StorePaymentManagerError.readReceipt(error))) } } @@ -92,7 +90,7 @@ class SendStoreReceiptOperation: ResultOperation< error: error, message: "Failed to read the AppStore receipt after refresh." ) - self.finish(completion: .failure(.readReceipt(error))) + self.finish(result: .failure(StorePaymentManagerError.readReceipt(error))) } } } @@ -103,7 +101,7 @@ class SendStoreReceiptOperation: ResultOperation< error: error, message: "Failed to refresh the AppStore receipt." ) - self.finish(completion: .failure(.readReceipt(error))) + self.finish(result: .failure(StorePaymentManagerError.readReceipt(error))) } } @@ -138,8 +136,8 @@ class SendStoreReceiptOperation: ResultOperation< accountNumber: accountNumber, receiptString: receiptData, retryStrategy: .noRetry - ) { completion in - switch completion { + ) { result in + switch result { case let .success(response): self.logger.info( """ @@ -148,18 +146,19 @@ class SendStoreReceiptOperation: ResultOperation< New expiry: \(response.newExpiry.logFormatDate()) """ ) - self.finish(completion: .success(response)) + self.finish(result: .success(response)) case let .failure(error): - self.logger.error( - error: error, - message: "Failed to send the AppStore receipt." - ) - self.finish(completion: .failure(.sendReceipt(error))) - - case .cancelled: - self.logger.debug("Receipt submission cancelled.") - self.finish(completion: .cancelled) + if error.isOperationCancellationError { + self.logger.debug("Receipt submission cancelled.") + self.finish(result: .failure(error)) + } else { + self.logger.error( + error: error, + message: "Failed to send the AppStore receipt." + ) + self.finish(result: .failure(StorePaymentManagerError.sendReceipt(error))) + } } } } diff --git a/ios/MullvadVPN/StorePaymentManager/StorePaymentManager.swift b/ios/MullvadVPN/StorePaymentManager/StorePaymentManager.swift index 5aa5db8316..40e1b6b7e3 100644 --- a/ios/MullvadVPN/StorePaymentManager/StorePaymentManager.swift +++ b/ios/MullvadVPN/StorePaymentManager/StorePaymentManager.swift @@ -110,7 +110,7 @@ final class StorePaymentManager: NSObject, SKPaymentTransactionObserver { func requestProducts( with productIdentifiers: Set<StoreSubscription>, - completionHandler: @escaping (OperationCompletion<SKProductsResponse, Error>) -> Void + completionHandler: @escaping (Result<SKProductsResponse, Error>) -> Void ) -> Cancellable { let productIdentifiers = productIdentifiers.productIdentifiersSet let operation = ProductsRequestOperation( @@ -149,10 +149,7 @@ final class StorePaymentManager: NSObject, SKPaymentTransactionObserver { func restorePurchases( for accountNumber: String, - completionHandler: @escaping (OperationCompletion< - REST.CreateApplePaymentResponse, - StorePaymentManagerError - >) -> Void + completionHandler: @escaping (Result<REST.CreateApplePaymentResponse, Error>) -> Void ) -> Cancellable { return sendStoreReceipt( accountNumber: accountNumber, @@ -184,15 +181,12 @@ final class StorePaymentManager: NSObject, SKPaymentTransactionObserver { accountNumber: String, completionHandler: @escaping (StorePaymentManagerError?) -> Void ) { - let accountOperation = ResultBlockOperation< - REST.AccountData, - REST.Error - >(dispatchQueue: .main) { op in + let accountOperation = ResultBlockOperation<REST.AccountData>(dispatchQueue: .main) { op in let task = self.accountsProxy.getAccountData( accountNumber: accountNumber, retryStrategy: .default - ) { completion in - op.finish(completion: completion) + ) { result in + op.finish(result: result) } op.addCancellationBlock { @@ -207,16 +201,8 @@ final class StorePaymentManager: NSObject, SKPaymentTransactionObserver { )) accountOperation.completionQueue = .main - accountOperation.completionHandler = { completion in - var error: REST.Error? - - if case .cancelled = completion { - error = .network(URLError(.cancelled)) - } else { - error = completion.error - } - - completionHandler(error.map { .validateAccount($0) }) + accountOperation.completionHandler = { result in + completionHandler(result.error.map { StorePaymentManagerError.validateAccount($0) }) } operationQueue.addOperation(accountOperation) @@ -225,10 +211,7 @@ final class StorePaymentManager: NSObject, SKPaymentTransactionObserver { private func sendStoreReceipt( accountNumber: String, forceRefresh: Bool, - completionHandler: @escaping (OperationCompletion< - REST.CreateApplePaymentResponse, - StorePaymentManagerError - >) -> Void + completionHandler: @escaping (Result<REST.CreateApplePaymentResponse, Error>) -> Void ) -> Cancellable { let operation = SendStoreReceiptOperation( apiProxy: apiProxy, @@ -333,10 +316,10 @@ final class StorePaymentManager: NSObject, SKPaymentTransactionObserver { return } - _ = sendStoreReceipt(accountNumber: accountNumber, forceRefresh: false) { completion in + _ = sendStoreReceipt(accountNumber: accountNumber, forceRefresh: false) { result in var event: StorePaymentEvent? - switch completion { + switch result { case let .success(response): self.paymentQueue.finishTransaction(transaction) @@ -346,7 +329,7 @@ final class StorePaymentManager: NSObject, SKPaymentTransactionObserver { serverResponse: response )) - case let .failure(error): + case let .failure(error as StorePaymentManagerError): event = StorePaymentEvent.failure(StorePaymentFailure( transaction: transaction, payment: transaction.payment, @@ -354,7 +337,7 @@ final class StorePaymentManager: NSObject, SKPaymentTransactionObserver { error: error )) - case .cancelled: + default: break } diff --git a/ios/MullvadVPN/StorePaymentManager/StorePaymentManagerError.swift b/ios/MullvadVPN/StorePaymentManager/StorePaymentManagerError.swift index 660d181df4..f7df5e4489 100644 --- a/ios/MullvadVPN/StorePaymentManager/StorePaymentManagerError.swift +++ b/ios/MullvadVPN/StorePaymentManager/StorePaymentManagerError.swift @@ -16,7 +16,7 @@ enum StorePaymentManagerError: LocalizedError, WrappingError { case noAccountSet /// Failure to validate the account token. - case validateAccount(REST.Error) + case validateAccount(Error) /// Failure to handle payment transaction. Contains error returned by StoreKit. case storePayment(Error) @@ -25,7 +25,7 @@ enum StorePaymentManagerError: LocalizedError, WrappingError { case readReceipt(Error) /// Failure to send the AppStore receipt to backend. - case sendReceipt(REST.Error) + case sendReceipt(Error) var errorDescription: String? { switch self { diff --git a/ios/MullvadVPN/StorePaymentManagerError+Display.swift b/ios/MullvadVPN/StorePaymentManagerError+Display.swift index 3015c69081..a2adb551ad 100644 --- a/ios/MullvadVPN/StorePaymentManagerError+Display.swift +++ b/ios/MullvadVPN/StorePaymentManagerError+Display.swift @@ -21,8 +21,8 @@ extension StorePaymentManagerError: DisplayError { comment: "" ) - case let .validateAccount(restError): - let reason = restError.displayErrorDescription ?? "" + case let .validateAccount(error): + let reason = (error as? DisplayError)?.displayErrorDescription ?? "" return String( format: NSLocalizedString( @@ -60,8 +60,8 @@ extension StorePaymentManagerError: DisplayError { ) } - case let .sendReceipt(restError): - let reason = restError.displayErrorDescription ?? "" + case let .sendReceipt(error): + let reason = (error as? DisplayError)?.displayErrorDescription ?? "" let errorFormat = NSLocalizedString( "SEND_RECEIPT_ERROR", tableName: "StorePaymentManager", diff --git a/ios/MullvadVPN/TransportMonitor/PacketTunnelTransport.swift b/ios/MullvadVPN/TransportMonitor/PacketTunnelTransport.swift index aa85eda6b2..4ee628751d 100644 --- a/ios/MullvadVPN/TransportMonitor/PacketTunnelTransport.swift +++ b/ios/MullvadVPN/TransportMonitor/PacketTunnelTransport.swift @@ -9,6 +9,7 @@ import Foundation import protocol MullvadREST.RESTTransport import MullvadTypes +import Operations import TunnelProviderMessaging struct PacketTunnelTransport: RESTTransport { @@ -30,9 +31,6 @@ struct PacketTunnelTransport: RESTTransport { return tunnel.sendRequest(proxyRequest) { result in switch result { - case .cancelled: - completion(nil, nil, URLError(.cancelled)) - case let .success(reply): completion( reply.data, @@ -41,7 +39,9 @@ struct PacketTunnelTransport: RESTTransport { ) case let .failure(error): - completion(nil, nil, error) + let returnError = error.isOperationCancellationError ? URLError(.cancelled) : error + + completion(nil, nil, returnError) } } } diff --git a/ios/MullvadVPN/TunnelManager/LoadTunnelConfigurationOperation.swift b/ios/MullvadVPN/TunnelManager/LoadTunnelConfigurationOperation.swift index 4f5bfa60b8..20b5564e6f 100644 --- a/ios/MullvadVPN/TunnelManager/LoadTunnelConfigurationOperation.swift +++ b/ios/MullvadVPN/TunnelManager/LoadTunnelConfigurationOperation.swift @@ -11,7 +11,7 @@ import MullvadLogging import MullvadTypes import Operations -class LoadTunnelConfigurationOperation: ResultOperation<Void, Error> { +class LoadTunnelConfigurationOperation: ResultOperation<Void> { private let logger = Logger(label: "LoadTunnelConfigurationOperation") private let interactor: TunnelInteractor @@ -54,7 +54,7 @@ class LoadTunnelConfigurationOperation: ResultOperation<Void, Error> { interactor.setTunnel(tunnel, shouldRefreshTunnelState: true) interactor.setConfigurationLoaded() - finish(completion: .success(())) + finish(result: .success(())) } private func readSettings() -> Result<TunnelSettingsV2?, Error> { diff --git a/ios/MullvadVPN/TunnelManager/ReconnectTunnelOperation.swift b/ios/MullvadVPN/TunnelManager/ReconnectTunnelOperation.swift index 42ed31c941..bbaa83850e 100644 --- a/ios/MullvadVPN/TunnelManager/ReconnectTunnelOperation.swift +++ b/ios/MullvadVPN/TunnelManager/ReconnectTunnelOperation.swift @@ -13,7 +13,7 @@ import Operations import RelayCache import RelaySelector -class ReconnectTunnelOperation: ResultOperation<Void, Error> { +class ReconnectTunnelOperation: ResultOperation<Void> { private let interactor: TunnelInteractor private let selectNewRelay: Bool private var task: Cancellable? @@ -31,20 +31,19 @@ class ReconnectTunnelOperation: ResultOperation<Void, Error> { override func main() { guard let tunnel = interactor.tunnel else { - finish(completion: .failure(UnsetTunnelError())) + finish(result: .failure(UnsetTunnelError())) return } do { let selectorResult = selectNewRelay ? try interactor.selectRelay() : nil - task = tunnel.reconnectTunnel( - relaySelectorResult: selectorResult - ) { [weak self] completion in - self?.finish(completion: completion) - } + task = tunnel + .reconnectTunnel(relaySelectorResult: selectorResult) { [weak self] result in + self?.finish(result: result) + } } catch { - finish(completion: .failure(error)) + finish(result: .failure(error)) } } diff --git a/ios/MullvadVPN/TunnelManager/RotateKeyOperation.swift b/ios/MullvadVPN/TunnelManager/RotateKeyOperation.swift index 88a4f99b5e..5f74e5ee2a 100644 --- a/ios/MullvadVPN/TunnelManager/RotateKeyOperation.swift +++ b/ios/MullvadVPN/TunnelManager/RotateKeyOperation.swift @@ -13,7 +13,7 @@ import MullvadTypes import Operations import class WireGuardKitTypes.PrivateKey -class RotateKeyOperation: ResultOperation<Bool, Error> { +class RotateKeyOperation: ResultOperation<Bool> { private let interactor: TunnelInteractor private let devicesProxy: REST.DevicesProxy @@ -41,7 +41,7 @@ class RotateKeyOperation: ResultOperation<Bool, Error> { override func main() { guard case let .loggedIn(accountData, deviceData) = interactor.deviceState else { - finish(completion: .failure(InvalidDeviceStateError())) + finish(result: .failure(InvalidDeviceStateError())) return } @@ -52,7 +52,7 @@ class RotateKeyOperation: ResultOperation<Bool, Error> { if nextRotationDate > Date() { logger.debug("Throttle private key rotation.") - finish(completion: .success(false)) + finish(result: .success(false)) return } else { logger.debug("Private key is old enough, rotate right away.") @@ -70,11 +70,11 @@ class RotateKeyOperation: ResultOperation<Bool, Error> { identifier: deviceData.identifier, publicKey: newPrivateKey.publicKey, retryStrategy: .default - ) { completion in + ) { result in self.dispatchQueue.async { self.didRotateKey( newPrivateKey: newPrivateKey, - completion: completion + result: result ) } } @@ -85,11 +85,8 @@ class RotateKeyOperation: ResultOperation<Bool, Error> { task = nil } - private func didRotateKey( - newPrivateKey: PrivateKey, - completion: OperationCompletion<REST.Device, REST.Error> - ) { - switch completion { + private func didRotateKey(newPrivateKey: PrivateKey, result: Result<REST.Device, Error>) { + switch result { case let .success(device): logger.debug("Successfully rotated device key. Persisting settings...") @@ -103,20 +100,19 @@ class RotateKeyOperation: ResultOperation<Bool, Error> { interactor.setDeviceState(.loggedIn(accountData, deviceData), persist: true) - finish(completion: .success(true)) + finish(result: .success(true)) default: - finish(completion: .failure(InvalidDeviceStateError())) + finish(result: .failure(InvalidDeviceStateError())) } case let .failure(error): - logger.error( - error: error, - message: "Failed to rotate device key." - ) - finish(completion: .failure(error)) - - case .cancelled: - finish(completion: .cancelled) + if !error.isOperationCancellationError { + logger.error( + error: error, + message: "Failed to rotate device key." + ) + } + finish(result: .failure(error)) } } } diff --git a/ios/MullvadVPN/TunnelManager/SendTunnelProviderMessageOperation.swift b/ios/MullvadVPN/TunnelManager/SendTunnelProviderMessageOperation.swift index 9fc30565f5..b7fafa326d 100644 --- a/ios/MullvadVPN/TunnelManager/SendTunnelProviderMessageOperation.swift +++ b/ios/MullvadVPN/TunnelManager/SendTunnelProviderMessageOperation.swift @@ -20,7 +20,7 @@ private let connectingStateWaitDelay: TimeInterval = 5 /// Default timeout in seconds. private let defaultTimeout: TimeInterval = 5 -final class SendTunnelProviderMessageOperation<Output>: ResultOperation<Output, Error> { +final class SendTunnelProviderMessageOperation<Output>: ResultOperation<Output> { typealias DecoderHandler = (Data?) throws -> Output private let tunnel: Tunnel @@ -68,11 +68,11 @@ final class SendTunnelProviderMessageOperation<Output>: ResultOperation<Output, override func operationDidCancel() { if isExecuting { - finish(completion: .cancelled) + finish(result: .failure(OperationError.cancelled)) } } - override func finish(completion: Completion) { + override func finish(result: Result<Output, Error>) { // Release status observer. removeVPNStatusObserver() @@ -81,7 +81,7 @@ final class SendTunnelProviderMessageOperation<Output>: ResultOperation<Output, waitForConnectingStateWork?.cancel() // Finish operation. - super.finish(completion: completion) + super.finish(result: result) } private func removeVPNStatusObserver() { @@ -91,7 +91,7 @@ final class SendTunnelProviderMessageOperation<Output>: ResultOperation<Output, private func setTimeoutTimer(connectingStateWaitDelay delay: TimeInterval) { let workItem = DispatchWorkItem { [weak self] in - self?.finish(completion: .failure(SendTunnelProviderMessageError.timeout)) + self?.finish(result: .failure(SendTunnelProviderMessageError.timeout)) } // Cancel pending timeout work. @@ -124,7 +124,7 @@ final class SendTunnelProviderMessageOperation<Output>: ResultOperation<Output, sendMessage() case .invalid, .disconnecting, .disconnected: - finish(completion: .failure(SendTunnelProviderMessageError.tunnelDown(status))) + finish(result: .failure(SendTunnelProviderMessageError.tunnelDown(status))) @unknown default: break @@ -180,7 +180,7 @@ final class SendTunnelProviderMessageOperation<Output>: ResultOperation<Output, do { messageData = try message.encode() } catch { - finish(completion: .failure(error)) + finish(result: .failure(error)) return } @@ -192,11 +192,11 @@ final class SendTunnelProviderMessageOperation<Output>: ResultOperation<Output, self.dispatchQueue.async { let decodingResult = Result { try self.decoderHandler(responseData) } - self.finish(completion: OperationCompletion(result: decodingResult)) + self.finish(result: decodingResult) } } } catch { - finish(completion: .failure(SendTunnelProviderMessageError.system(error))) + finish(result: .failure(SendTunnelProviderMessageError.system(error))) } } } diff --git a/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift b/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift index e2c1de0656..d07170bdc2 100644 --- a/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift +++ b/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift @@ -62,7 +62,7 @@ private struct SetAccountContext: OperationInputContext { } } -class SetAccountOperation: ResultOperation<StoredAccountData?, Error> { +class SetAccountOperation: ResultOperation<StoredAccountData?> { private let interactor: TunnelInteractor private let accountsProxy: REST.AccountsProxy private let devicesProxy: REST.DevicesProxy @@ -156,7 +156,7 @@ class SetAccountOperation: ResultOperation<StoredAccountData?, Error> { private func completeOperation(accountData: StoredAccountData?) { guard !isCancelled else { - finish(completion: .cancelled) + finish(result: .failure(OperationError.cancelled)) return } @@ -165,13 +165,13 @@ class SetAccountOperation: ResultOperation<StoredAccountData?, Error> { } if let error = errors.first { - finish(completion: .failure(error)) + finish(result: .failure(error)) } else { - finish(completion: .success(accountData)) + finish(result: .success(accountData)) } } - private func getAccountDataOperation() -> ResultOperation<StoredAccountData, Error>? { + private func getAccountDataOperation() -> ResultOperation<StoredAccountData>? { switch action { case .new: return getCreateAccountOperation() @@ -184,19 +184,20 @@ class SetAccountOperation: ResultOperation<StoredAccountData?, Error> { } } - private func getCreateAccountOperation() -> ResultBlockOperation<StoredAccountData, Error> { - let operation = ResultBlockOperation<StoredAccountData, Error>(dispatchQueue: dispatchQueue) + private func getCreateAccountOperation() -> ResultBlockOperation<StoredAccountData> { + let operation = ResultBlockOperation<StoredAccountData>(dispatchQueue: dispatchQueue) operation.setExecutionBlock { operation in self.logger.debug("Create new account...") - let task = self.accountsProxy.createAccount(retryStrategy: .default) { completion in - let mappedCompletion = completion.mapError { error -> Error in + let task = self.accountsProxy.createAccount(retryStrategy: .default) { result in + let result = result.inspectError { error in + guard !error.isOperationCancellationError else { return } + self.logger.error( error: error, message: "Failed to create new account." ) - return error }.map { newAccountData -> StoredAccountData in self.logger.debug("Created new account.") @@ -207,7 +208,7 @@ class SetAccountOperation: ResultOperation<StoredAccountData?, Error> { ) } - operation.finish(completion: mappedCompletion) + operation.finish(result: result) } operation.addCancellationBlock { @@ -219,9 +220,9 @@ class SetAccountOperation: ResultOperation<StoredAccountData?, Error> { } private func getExistingAccountOperation(accountNumber: String) - -> ResultOperation<StoredAccountData, Error> + -> ResultOperation<StoredAccountData> { - let operation = ResultBlockOperation<StoredAccountData, Error>(dispatchQueue: dispatchQueue) + let operation = ResultBlockOperation<StoredAccountData>(dispatchQueue: dispatchQueue) operation.setExecutionBlock { operation in self.logger.debug("Request account data...") @@ -229,13 +230,14 @@ class SetAccountOperation: ResultOperation<StoredAccountData?, Error> { let task = self.accountsProxy.getAccountData( accountNumber: accountNumber, retryStrategy: .default - ) { completion in - let mappedCompletion = completion.mapError { error -> Error in + ) { result in + let result = result.inspectError { error in + guard !error.isOperationCancellationError else { return } + self.logger.error( error: error, message: "Failed to receive account data." ) - return error }.map { accountData -> StoredAccountData in self.logger.debug("Received account data.") @@ -246,7 +248,7 @@ class SetAccountOperation: ResultOperation<StoredAccountData?, Error> { ) } - operation.finish(completion: mappedCompletion) + operation.finish(result: result) } operation.addCancellationBlock { @@ -257,12 +259,12 @@ class SetAccountOperation: ResultOperation<StoredAccountData?, Error> { return operation } - private func getDeleteDeviceOperation() -> ResultBlockOperation<Void, Error>? { + private func getDeleteDeviceOperation() -> AsyncBlockOperation? { guard case let .loggedIn(accountData, deviceData) = interactor.deviceState else { return nil } - let operation = ResultBlockOperation<Void, Error>(dispatchQueue: dispatchQueue) + let operation = AsyncBlockOperation(dispatchQueue: dispatchQueue) operation.setExecutionBlock { operation in self.logger.debug("Delete current device...") @@ -271,23 +273,22 @@ class SetAccountOperation: ResultOperation<StoredAccountData?, Error> { accountNumber: accountData.number, identifier: deviceData.identifier, retryStrategy: .default - ) { completion in - let mappedCompletion = completion - .mapError { error -> Error in - self.logger.error( - error: error, - message: "Failed to delete device." - ) - return error - }.map { isDeleted in - if isDeleted { - self.logger.debug("Deleted device.") - } else { - self.logger.debug("Device is already deleted.") - } - } + ) { result in + switch result { + case let .success(isDeleted): + self.logger.debug(isDeleted ? "Deleted device." : "Device is already deleted.") + + case let .failure(error) where !error.isOperationCancellationError: + self.logger.error( + error: error, + message: "Failed to delete device." + ) + + default: + break + } - operation.finish(completion: mappedCompletion) + operation.finish(error: result.error) } operation.addCancellationBlock { @@ -336,12 +337,11 @@ class SetAccountOperation: ResultOperation<StoredAccountData?, Error> { } private func getCreateDeviceOperation() - -> TransformOperation<StoredAccountData, (PrivateKey, REST.Device), Error> + -> TransformOperation<StoredAccountData, (PrivateKey, REST.Device)> { let createDeviceOperation = TransformOperation< StoredAccountData, - (PrivateKey, REST.Device), - Error + (PrivateKey, REST.Device) >(dispatchQueue: dispatchQueue) createDeviceOperation.setExecutionBlock { storedAccountData, operation in @@ -369,17 +369,16 @@ class SetAccountOperation: ResultOperation<StoredAccountData?, Error> { accountNumber: storedAccountData.number, request: request, retryStrategy: .default - ) { completion in - let mappedCompletion = completion + ) { result in + let result = result .map { device in return (privateKey, device) } - .mapError { error -> Error in + .inspectError { error in self.logger.error(error: error, message: "Failed to create device.") - return error } - operation.finish(completion: mappedCompletion) + operation.finish(result: result) } operation.addCancellationBlock { @@ -391,10 +390,11 @@ class SetAccountOperation: ResultOperation<StoredAccountData?, Error> { } private func getSaveSettingsOperation() - -> TransformOperation<SetAccountResult, StoredAccountData, Error> + -> TransformOperation<SetAccountResult, StoredAccountData> { let saveSettingsOperation = TransformOperation< - SetAccountResult, StoredAccountData, Error + SetAccountResult, + StoredAccountData >(dispatchQueue: dispatchQueue) saveSettingsOperation.setExecutionBlock { input in diff --git a/ios/MullvadVPN/TunnelManager/StartTunnelOperation.swift b/ios/MullvadVPN/TunnelManager/StartTunnelOperation.swift index 12934a4790..6954fd3c24 100644 --- a/ios/MullvadVPN/TunnelManager/StartTunnelOperation.swift +++ b/ios/MullvadVPN/TunnelManager/StartTunnelOperation.swift @@ -14,7 +14,7 @@ import RelayCache import RelaySelector import TunnelProviderMessaging -class StartTunnelOperation: ResultOperation<Void, Error> { +class StartTunnelOperation: ResultOperation<Void> { typealias EncodeErrorHandler = (Error) -> Void private let interactor: TunnelInteractor @@ -36,7 +36,7 @@ class StartTunnelOperation: ResultOperation<Void, Error> { override func main() { guard case .loggedIn = interactor.deviceState else { - finish(completion: .failure(InvalidDeviceStateError())) + finish(result: .failure(InvalidDeviceStateError())) return } @@ -47,21 +47,21 @@ class StartTunnelOperation: ResultOperation<Void, Error> { tunnelStatus.state = .disconnecting(.reconnect) } - finish(completion: .success(())) + finish(result: .success(())) case .disconnected, .pendingReconnect: do { let selectorResult = try interactor.selectRelay() makeTunnelProviderAndStartTunnel(selectorResult: selectorResult) { error in - self.finish(completion: OperationCompletion(error: error)) + self.finish(result: error.map { .failure($0) } ?? .success(())) } } catch { - finish(completion: .failure(error)) + finish(result: .failure(error)) } default: - finish(completion: .success(())) + finish(result: .success(())) } } diff --git a/ios/MullvadVPN/TunnelManager/StopTunnelOperation.swift b/ios/MullvadVPN/TunnelManager/StopTunnelOperation.swift index 2c63b1ae2e..38e60c7cf4 100644 --- a/ios/MullvadVPN/TunnelManager/StopTunnelOperation.swift +++ b/ios/MullvadVPN/TunnelManager/StopTunnelOperation.swift @@ -9,7 +9,7 @@ import Foundation import Operations -class StopTunnelOperation: ResultOperation<Void, Error> { +class StopTunnelOperation: ResultOperation<Void> { private let interactor: TunnelInteractor init( @@ -33,11 +33,11 @@ class StopTunnelOperation: ResultOperation<Void, Error> { tunnelStatus.state = .disconnecting(.nothing) } - finish(completion: .success(())) + finish(result: .success(())) case .connected, .connecting, .reconnecting, .waitingForConnectivity: guard let tunnel = interactor.tunnel else { - finish(completion: .failure(UnsetTunnelError())) + finish(result: .failure(UnsetTunnelError())) return } @@ -47,16 +47,16 @@ class StopTunnelOperation: ResultOperation<Void, Error> { tunnel.saveToPreferences { error in self.dispatchQueue.async { if let error = error { - self.finish(completion: .failure(error)) + self.finish(result: .failure(error)) } else { tunnel.stop() - self.finish(completion: .success(())) + self.finish(result: .success(())) } } } case .disconnected, .disconnecting, .pendingReconnect: - finish(completion: .success(())) + finish(result: .success(())) } } } diff --git a/ios/MullvadVPN/TunnelManager/Tunnel+Messaging.swift b/ios/MullvadVPN/TunnelManager/Tunnel+Messaging.swift index 339e9b0da6..7dea6451f6 100644 --- a/ios/MullvadVPN/TunnelManager/Tunnel+Messaging.swift +++ b/ios/MullvadVPN/TunnelManager/Tunnel+Messaging.swift @@ -27,7 +27,7 @@ extension Tunnel { /// Packet tunnel will reconnect to the current relay if relay selector result is not provided. func reconnectTunnel( relaySelectorResult: RelaySelectorResult?, - completionHandler: @escaping (OperationCompletion<Void, Error>) -> Void + completionHandler: @escaping (Result<Void, Error>) -> Void ) -> Cancellable { let operation = SendTunnelProviderMessageOperation( dispatchQueue: dispatchQueue, @@ -43,7 +43,7 @@ extension Tunnel { /// Request status from packet tunnel process. func getTunnelStatus( - completionHandler: @escaping (OperationCompletion<PacketTunnelStatus, Error>) -> Void + completionHandler: @escaping (Result<PacketTunnelStatus, Error>) -> Void ) -> Cancellable { let operation = SendTunnelProviderMessageOperation( dispatchQueue: dispatchQueue, @@ -60,7 +60,7 @@ extension Tunnel { /// Send HTTP request via packet tunnel process bypassing VPN. func sendRequest( _ proxyRequest: ProxyURLRequest, - completionHandler: @escaping (OperationCompletion<ProxyURLResponse, Error>) -> Void + completionHandler: @escaping (Result<ProxyURLResponse, Error>) -> Void ) -> Cancellable { let operation = SendTunnelProviderMessageOperation( dispatchQueue: dispatchQueue, diff --git a/ios/MullvadVPN/TunnelManager/TunnelManager.swift b/ios/MullvadVPN/TunnelManager/TunnelManager.swift index 3854f3ca54..6ac13e7ac3 100644 --- a/ios/MullvadVPN/TunnelManager/TunnelManager.swift +++ b/ios/MullvadVPN/TunnelManager/TunnelManager.swift @@ -67,7 +67,7 @@ final class TunnelManager: StorePaymentObserver { private var privateKeyRotationTimer: DispatchSourceTimer? private var lastKeyRotationData: ( attempt: Date, - completion: OperationCompletion<Bool, Error> + completion: Result<Bool, Error> )? private var isRunningPeriodicPrivateKeyRotation = false @@ -198,11 +198,11 @@ final class TunnelManager: StorePaymentObserver { logger.debug("Schedule next private key rotation at \(scheduleDate.logFormatDate()).") } - private func setFinishedKeyRotation(_ completion: OperationCompletion<Bool, Error>) { + private func setFinishedKeyRotation(_ result: Result<Bool, Error>) { nslock.lock() defer { nslock.unlock() } - lastKeyRotationData = (Date(), completion) + lastKeyRotationData = (Date(), result) updatePrivateKeyRotationTimer() } @@ -252,15 +252,15 @@ final class TunnelManager: StorePaymentObserver { operationQueue.addOperation(loadTunnelOperation) } - func startTunnel(completionHandler: ((OperationCompletion<Void, Error>) -> Void)? = nil) { + func startTunnel(completionHandler: ((Error?) -> Void)? = nil) { let operation = StartTunnelOperation( dispatchQueue: internalQueue, interactor: TunnelInteractorProxy(self), - completionHandler: { [weak self] completion in + completionHandler: { [weak self] result in guard let self = self else { return } DispatchQueue.main.async { - if let error = completion.error { + if let error = result.error { self.logger.error( error: error, message: "Failed to start the tunnel." @@ -273,7 +273,7 @@ final class TunnelManager: StorePaymentObserver { } } - completionHandler?(completion) + completionHandler?(result.error) } } ) @@ -288,15 +288,15 @@ final class TunnelManager: StorePaymentObserver { operationQueue.addOperation(operation) } - func stopTunnel(completionHandler: ((OperationCompletion<Void, Error>) -> Void)? = nil) { + func stopTunnel(completionHandler: ((Error?) -> Void)? = nil) { let operation = StopTunnelOperation( dispatchQueue: internalQueue, interactor: TunnelInteractorProxy(self) - ) { [weak self] completion in + ) { [weak self] result in guard let self = self else { return } DispatchQueue.main.async { - if let error = completion.error { + if let error = result.error { self.logger.error( error: error, message: "Failed to stop the tunnel." @@ -309,7 +309,7 @@ final class TunnelManager: StorePaymentObserver { } } - completionHandler?(completion) + completionHandler?(result.error) } } @@ -325,7 +325,7 @@ final class TunnelManager: StorePaymentObserver { func reconnectTunnel( selectNewRelay: Bool, - completionHandler: ((OperationCompletion<Void, Error>) -> Void)? = nil + completionHandler: ((Error?) -> Void)? = nil ) { let operation = ReconnectTunnelOperation( dispatchQueue: internalQueue, @@ -334,10 +334,10 @@ final class TunnelManager: StorePaymentObserver { ) operation.completionQueue = .main - operation.completionHandler = { [weak self] completion in - self?.didReconnectTunnel(completion: completion) + operation.completionHandler = { [weak self] result in + self?.didReconnectTunnel(error: result.error) - completionHandler?(completion) + completionHandler?(result.error) } operation.addObserver( @@ -356,7 +356,7 @@ final class TunnelManager: StorePaymentObserver { func setAccount( action: SetAccountAction, - completionHandler: @escaping (OperationCompletion<StoredAccountData?, Error>) -> Void + completionHandler: @escaping (Result<StoredAccountData?, Error>) -> Void ) { let operation = SetAccountOperation( dispatchQueue: internalQueue, @@ -367,10 +367,10 @@ final class TunnelManager: StorePaymentObserver { ) operation.completionQueue = .main - operation.completionHandler = { [weak self] completion in + operation.completionHandler = { [weak self] result in self?.resetKeyRotationData() - completionHandler(completion) + completionHandler(result) } operation.addObserver(BackgroundObserver( @@ -461,7 +461,7 @@ final class TunnelManager: StorePaymentObserver { func rotatePrivateKey( forceRotate: Bool, - completionHandler: @escaping (OperationCompletion<Bool, Error>) -> Void + completionHandler: @escaping (Result<Bool, Error>) -> Void ) -> Cancellable { var rotationInterval: TimeInterval? if !forceRotate { @@ -476,24 +476,23 @@ final class TunnelManager: StorePaymentObserver { ) operation.completionQueue = .main - operation.completionHandler = { [weak self] completion in + operation.completionHandler = { [weak self] result in guard let self = self else { return } - self.setFinishedKeyRotation(completion) + self.setFinishedKeyRotation(result) - switch completion { + switch result { case .success: self.reconnectTunnel(selectNewRelay: true) { _ in - completionHandler(completion) + completionHandler(result) } case let .failure(error): - self.checkIfDeviceRevoked(error) - - completionHandler(.failure(error)) + if !error.isOperationCancellationError { + self.checkIfDeviceRevoked(error) + } - case .cancelled: - completionHandler(completion) + completionHandler(result) } } @@ -814,15 +813,12 @@ final class TunnelManager: StorePaymentObserver { ) } - private func didReconnectTunnel(completion: OperationCompletion<Void, Error>) { + private func didReconnectTunnel(error: Error?) { nslock.lock() defer { nslock.unlock() } - if let error = completion.error { - logger.error( - error: error, - message: "Failed to reconnect the tunnel." - ) + if let error = error, !error.isOperationCancellationError { + logger.error(error: error, message: "Failed to reconnect the tunnel.") } // Refresh tunnel status only when connecting or reasserting to pick up the next relay, diff --git a/ios/MullvadVPN/TunnelManager/UpdateAccountDataOperation.swift b/ios/MullvadVPN/TunnelManager/UpdateAccountDataOperation.swift index e0e4f5229d..00531bc2e0 100644 --- a/ios/MullvadVPN/TunnelManager/UpdateAccountDataOperation.swift +++ b/ios/MullvadVPN/TunnelManager/UpdateAccountDataOperation.swift @@ -12,7 +12,7 @@ import MullvadREST import MullvadTypes import Operations -class UpdateAccountDataOperation: ResultOperation<Void, Error> { +class UpdateAccountDataOperation: ResultOperation<Void> { private let logger = Logger(label: "UpdateAccountDataOperation") private let interactor: TunnelInteractor private let accountsProxy: REST.AccountsProxy @@ -31,16 +31,16 @@ class UpdateAccountDataOperation: ResultOperation<Void, Error> { override func main() { guard case let .loggedIn(accountData, _) = interactor.deviceState else { - finish(completion: .failure(InvalidDeviceStateError())) + finish(result: .failure(InvalidDeviceStateError())) return } task = accountsProxy.getAccountData( accountNumber: accountData.number, retryStrategy: .default - ) { completion in + ) { result in self.dispatchQueue.async { - self.didReceiveAccountData(completion: completion) + self.didReceiveAccountData(result: result) } } } @@ -50,15 +50,14 @@ class UpdateAccountDataOperation: ResultOperation<Void, Error> { task = nil } - private func didReceiveAccountData( - completion: OperationCompletion<REST.AccountData, REST.Error> - ) { - let mappedCompletion = completion.mapError { error -> Error in + private func didReceiveAccountData(result: Result<REST.AccountData, Error>) { + let result = result.inspectError { error in + guard !error.isOperationCancellationError else { return } + self.logger.error( error: error, message: "Failed to fetch account expiry." ) - return error }.tryMap { accountData in switch interactor.deviceState { case .loggedIn(var storedAccountData, let storedDeviceData): @@ -73,6 +72,6 @@ class UpdateAccountDataOperation: ResultOperation<Void, Error> { } } - finish(completion: mappedCompletion) + finish(result: result) } } diff --git a/ios/MullvadVPN/TunnelManager/UpdateDeviceDataOperation.swift b/ios/MullvadVPN/TunnelManager/UpdateDeviceDataOperation.swift index 877b654b55..2f0f276153 100644 --- a/ios/MullvadVPN/TunnelManager/UpdateDeviceDataOperation.swift +++ b/ios/MullvadVPN/TunnelManager/UpdateDeviceDataOperation.swift @@ -13,7 +13,7 @@ import MullvadTypes import Operations import class WireGuardKitTypes.PublicKey -class UpdateDeviceDataOperation: ResultOperation<StoredDeviceData, Error> { +class UpdateDeviceDataOperation: ResultOperation<StoredDeviceData> { private let interactor: TunnelInteractor private let devicesProxy: REST.DevicesProxy @@ -32,7 +32,7 @@ class UpdateDeviceDataOperation: ResultOperation<StoredDeviceData, Error> { override func main() { guard case let .loggedIn(accountData, deviceData) = interactor.deviceState else { - finish(completion: .failure(InvalidDeviceStateError())) + finish(result: .failure(InvalidDeviceStateError())) return } @@ -40,11 +40,9 @@ class UpdateDeviceDataOperation: ResultOperation<StoredDeviceData, Error> { accountNumber: accountData.number, identifier: deviceData.identifier, retryStrategy: .default, - completion: { [weak self] completion in + completion: { [weak self] result in self?.dispatchQueue.async { - self?.didReceiveDeviceResponse( - completion: completion - ) + self?.didReceiveDeviceResponse(result: result) } } ) @@ -55,23 +53,21 @@ class UpdateDeviceDataOperation: ResultOperation<StoredDeviceData, Error> { task = nil } - private func didReceiveDeviceResponse(completion: OperationCompletion<REST.Device, REST.Error>) - { - let mappedCompletion = completion - .tryMap { device -> StoredDeviceData in - switch interactor.deviceState { - case .loggedIn(let storedAccount, var storedDevice): - storedDevice.update(from: device) - let newDeviceState = DeviceState.loggedIn(storedAccount, storedDevice) - interactor.setDeviceState(newDeviceState, persist: true) + private func didReceiveDeviceResponse(result: Result<REST.Device, Error>) { + let result = result.tryMap { device -> StoredDeviceData in + switch interactor.deviceState { + case .loggedIn(let storedAccount, var storedDevice): + storedDevice.update(from: device) + let newDeviceState = DeviceState.loggedIn(storedAccount, storedDevice) + interactor.setDeviceState(newDeviceState, persist: true) - return storedDevice + return storedDevice - default: - throw InvalidDeviceStateError() - } + default: + throw InvalidDeviceStateError() } + } - finish(completion: mappedCompletion) + finish(result: result) } } diff --git a/ios/Operations/AsyncOperation.swift b/ios/Operations/AsyncOperation.swift index 61630573d3..2939f0708e 100644 --- a/ios/Operations/AsyncOperation.swift +++ b/ios/Operations/AsyncOperation.swift @@ -188,9 +188,10 @@ open class AsyncOperation: Operation { public func addCondition(_ condition: OperationCondition) { operationLock.lock() + defer { operationLock.unlock() } + assert(state < .evaluatingConditions) _conditions.append(condition) - operationLock.unlock() } private func evaluateConditions() { diff --git a/ios/Operations/InputInjectionBuilder.swift b/ios/Operations/InputInjectionBuilder.swift index a4c63a6a00..0c87ab767a 100644 --- a/ios/Operations/InputInjectionBuilder.swift +++ b/ios/Operations/InputInjectionBuilder.swift @@ -56,15 +56,15 @@ public final class InputInjectionBuilder<OperationType, Context> return self } - public func injectCompletion<T, Success, Failure>( + public func injectResult<T, Success>( from dependency: T, - via block: @escaping (inout Context, T.Completion) -> Void + via block: @escaping (inout Context, Result<T.Output, Error>) -> Void ) -> Self - where T: ResultOperation<Success, Failure> + where T: ResultOperation<Success> { inputBlocks.append { context in - if let completion = dependency.completion { - block(&context, completion) + if let result = dependency.result { + block(&context, result) } } diff --git a/ios/Operations/OperationCompletion.swift b/ios/Operations/OperationCompletion.swift deleted file mode 100644 index 49a34e0884..0000000000 --- a/ios/Operations/OperationCompletion.swift +++ /dev/null @@ -1,159 +0,0 @@ -// -// OperationCompletion.swift -// Operations -// -// Created by pronebird on 24/01/2022. -// Copyright © 2022 Mullvad VPN AB. All rights reserved. -// - -import Foundation - -public enum OperationCompletion<Success, Failure: Error> { - case cancelled - case success(Success) - case failure(Failure) - - public var isSuccess: Bool { - if case .success = self { - return true - } else { - return false - } - } - - public var value: Success? { - if case let .success(value) = self { - return value - } else { - return nil - } - } - - public var error: Failure? { - if case let .failure(error) = self { - return error - } else { - return nil - } - } - - public var result: Result<Success, Failure>? { - switch self { - case let .success(value): - return .success(value) - case let .failure(error): - return .failure(error) - case .cancelled: - return nil - } - } - - public init(result: Result<Success, Failure>) { - switch result { - case let .success(value): - self = .success(value) - case let .failure(error): - self = .failure(error) - } - } - - public init(error: Failure?) where Success == Void { - if let error = error { - self = .failure(error) - } else { - self = .success(()) - } - } - - public func get() throws -> Success { - if let result = result { - return try result.get() - } else { - throw OperationCancellationError() - } - } - - public func map<NewSuccess>(_ block: (Success) -> NewSuccess) - -> OperationCompletion<NewSuccess, Failure> - { - switch self { - case let .success(value): - return .success(block(value)) - case let .failure(error): - return .failure(error) - case .cancelled: - return .cancelled - } - } - - public func mapError<NewFailure: Error>(_ block: (Failure) -> NewFailure) - -> OperationCompletion<Success, NewFailure> - { - switch self { - case let .success(value): - return .success(value) - case let .failure(error): - return .failure(block(error)) - case .cancelled: - return .cancelled - } - } - - public func flatMap<NewSuccess>(_ block: (Success) -> OperationCompletion<NewSuccess, Failure>) - -> OperationCompletion<NewSuccess, Failure> - { - switch self { - case let .success(value): - return block(value) - case let .failure(error): - return .failure(error) - case .cancelled: - return .cancelled - } - } - - public func flatMapError<NewFailure: Error>( - _ block: (Failure) - -> OperationCompletion<Success, NewFailure> - ) -> OperationCompletion<Success, NewFailure> { - switch self { - case let .success(value): - return .success(value) - case let .failure(error): - return block(error) - case .cancelled: - return .cancelled - } - } - - public func tryMap<NewSuccess>(_ block: (Success) throws -> NewSuccess) - -> OperationCompletion<NewSuccess, Error> - { - switch self { - case let .success(value): - do { - return .success(try block(value)) - } catch { - return .failure(error) - } - case let .failure(error): - return .failure(error) - case .cancelled: - return .cancelled - } - } - - public func ignoreOutput() -> OperationCompletion<Void, Failure> { - return map { _ in () } - } - - public func eraseFailureType() -> OperationCompletion<Success, Error> { - return mapError { $0 } - } -} - -public struct OperationCancellationError: LocalizedError { - public var errorDescription: String? { - return "Operation was cancelled." - } -} diff --git a/ios/Operations/OperationError.swift b/ios/Operations/OperationError.swift new file mode 100644 index 0000000000..3b96cf6c82 --- /dev/null +++ b/ios/Operations/OperationError.swift @@ -0,0 +1,32 @@ +// +// OperationError.swift +// Operations +// +// Created by pronebird on 24/01/2022. +// Copyright © 2022 Mullvad VPN AB. All rights reserved. +// + +import Foundation + +public enum OperationError: LocalizedError, Equatable { + /// Unsatisfied operation requirement. + case unsatisfiedRequirement + + /// Operation cancelled. + case cancelled + + public var errorDescription: String? { + switch self { + case .unsatisfiedRequirement: + return "Unsatisfied operation requirement." + case .cancelled: + return "Operation was cancelled." + } + } +} + +extension Error { + public var isOperationCancellationError: Bool { + return (self as? OperationError) == .cancelled + } +} diff --git a/ios/Operations/ResultBlockOperation.swift b/ios/Operations/ResultBlockOperation.swift index 9329a566e1..5f21cd202e 100644 --- a/ios/Operations/ResultBlockOperation.swift +++ b/ios/Operations/ResultBlockOperation.swift @@ -8,11 +8,8 @@ import Foundation -public final class ResultBlockOperation<Success, Failure: Error>: ResultOperation< - Success, - Failure -> { - public typealias ExecutionBlock = (ResultBlockOperation<Success, Failure>) -> Void +public final class ResultBlockOperation<Success>: ResultOperation<Success> { + public typealias ExecutionBlock = (ResultBlockOperation<Success>) -> Void public typealias ThrowingExecutionBlock = () throws -> Success private var executionBlock: ExecutionBlock? @@ -78,10 +75,7 @@ public final class ResultBlockOperation<Success, Failure: Error>: ResultOperatio executionBlock = nil } - public func setExecutionBlock( - _ block: @escaping (ResultBlockOperation<Success, Failure>) - -> Void - ) { + public func setExecutionBlock(_ block: @escaping ExecutionBlock) { dispatchQueue.async { assert(!self.isExecuting && !self.isFinished) self.executionBlock = block @@ -106,15 +100,9 @@ public final class ResultBlockOperation<Success, Failure: Error>: ResultOperatio -> ExecutionBlock { return { operation in - do { - let value = try executionBlock() - - operation.finish(completion: .success(value)) - } catch { - let castedError = error as! Failure + let result = Result { try executionBlock() } - operation.finish(completion: .failure(castedError)) - } + operation.finish(result: result) } } } diff --git a/ios/Operations/ResultOperation+Output.swift b/ios/Operations/ResultOperation+Output.swift deleted file mode 100644 index ba31220a46..0000000000 --- a/ios/Operations/ResultOperation+Output.swift +++ /dev/null @@ -1,15 +0,0 @@ -// -// ResultOperation+Output.swift -// Operations -// -// Created by pronebird on 31/05/2022. -// Copyright © 2022 Mullvad VPN AB. All rights reserved. -// - -import Foundation - -extension ResultOperation: OutputOperation { - public var output: Success? { - return completion?.value - } -} diff --git a/ios/Operations/ResultOperation.swift b/ios/Operations/ResultOperation.swift index 9be0f1c0f4..c884ceb9b2 100644 --- a/ios/Operations/ResultOperation.swift +++ b/ios/Operations/ResultOperation.swift @@ -9,20 +9,27 @@ import Foundation /// Base class for operations producing result. -open class ResultOperation<Success, Failure: Error>: AsyncOperation { - public typealias Completion = OperationCompletion<Success, Failure> - public typealias CompletionHandler = (Completion) -> Void +open class ResultOperation<Success>: AsyncOperation, OutputOperation { + public typealias CompletionHandler = (Result<Success, Error>) -> Void private let nslock = NSLock() - private var completionValue: Completion? + private var _output: Success? private var _completionQueue: DispatchQueue? private var _completionHandler: CompletionHandler? private var pendingFinish = false - public var completion: Completion? { + public var result: Result<Success, Error>? { nslock.lock() defer { nslock.unlock() } - return completionValue + + return _output.map { .success($0) } ?? error.map { .failure($0) } + } + + public var output: Success? { + nslock.lock() + defer { nslock.unlock() } + + return _output } public var completionQueue: DispatchQueue? { @@ -34,8 +41,9 @@ open class ResultOperation<Success, Failure: Error>: AsyncOperation { } set { nslock.lock() + defer { nslock.unlock() } + _completionQueue = newValue - nslock.unlock() } } @@ -72,25 +80,19 @@ open class ResultOperation<Success, Failure: Error>: AsyncOperation { @available(*, unavailable) override public func finish() { - _finish(error: nil) + _finish(result: .failure(OperationError.cancelled)) } @available(*, unavailable) override public func finish(error: Error?) { - _finish(error: error) + _finish(result: .failure(error ?? OperationError.cancelled)) } - open func finish(completion: Completion) { - nslock.lock() - if completionValue == nil { - completionValue = completion - } - nslock.unlock() - - _finish(error: completion.error) + open func finish(result: Result<Success, Error>) { + _finish(result: result) } - private func _finish(error: Error?) { + private func _finish(result: Result<Success, Error>) { nslock.lock() // Bail if operation is already finishing. guard !pendingFinish else { @@ -108,7 +110,9 @@ open class ResultOperation<Success, Failure: Error>: AsyncOperation { _completionHandler = nil // Copy completion value. - let completion = completionValue ?? .cancelled + if case let .success(output) = result { + _output = output + } // Copy completion queue. let completionQueue = _completionQueue @@ -116,7 +120,12 @@ open class ResultOperation<Success, Failure: Error>: AsyncOperation { let block = { // Call completion handler. - completionHandler?(completion) + completionHandler?(result) + + var error: Error? + if case let .failure(failure) = result { + error = failure + } // Finish operation. super.finish(error: error) diff --git a/ios/Operations/TransformOperation.swift b/ios/Operations/TransformOperation.swift index 58048d1033..0b144707be 100644 --- a/ios/Operations/TransformOperation.swift +++ b/ios/Operations/TransformOperation.swift @@ -8,11 +8,8 @@ import Foundation -public final class TransformOperation<Input, Output, Failure: Error>: - ResultOperation<Output, Failure>, - InputOperation -{ - public typealias ExecutionBlock = (Input, TransformOperation<Input, Output, Failure>) -> Void +public final class TransformOperation<Input, Output>: ResultOperation<Output>, InputOperation { + public typealias ExecutionBlock = (Input, TransformOperation<Input, Output>) -> Void public typealias ThrowingExecutionBlock = (Input) throws -> Output public typealias InputBlock = () -> Input? @@ -69,7 +66,7 @@ public final class TransformOperation<Input, Output, Failure: Error>: _input = inputValue guard let inputValue = inputValue, let executionBlock = executionBlock else { - finish(completion: .cancelled) + finish(result: .failure(OperationError.unsatisfiedRequirement)) return } @@ -125,15 +122,9 @@ public final class TransformOperation<Input, Output, Failure: Error>: -> ExecutionBlock { return { input, operation in - do { - let value = try executionBlock(input) + let result = Result { try executionBlock(input) } - operation.finish(completion: .success(value)) - } catch { - let castedError = error as! Failure - - operation.finish(completion: .failure(castedError)) - } + operation.finish(result: result) } } } diff --git a/ios/OperationsTests/OperationConditionTests.swift b/ios/OperationsTests/OperationConditionTests.swift index ca4a15f20a..b35e687319 100644 --- a/ios/OperationsTests/OperationConditionTests.swift +++ b/ios/OperationsTests/OperationConditionTests.swift @@ -78,7 +78,7 @@ class OperationConditionTests: XCTestCase { let expectToNeverExecute = expectation(description: "Expect child to never execute.") expectToNeverExecute.isInverted = true - let parent = ResultBlockOperation<Void, URLError> { + let parent = ResultBlockOperation<Void> { throw URLError(.badURL) } diff --git a/ios/OperationsTests/OperationInputInjectionTests.swift b/ios/OperationsTests/OperationInputInjectionTests.swift index 828afd2643..559228e348 100644 --- a/ios/OperationsTests/OperationInputInjectionTests.swift +++ b/ios/OperationsTests/OperationInputInjectionTests.swift @@ -11,11 +11,11 @@ import XCTest class OperationInputInjectionTests: XCTestCase { func testInject() throws { - let provider = ResultBlockOperation<Int, Error> { + let provider = ResultBlockOperation<Int> { return 1 } - let consumer = TransformOperation<Int, Int, Error> { input in + let consumer = TransformOperation<Int, Int> { input in return input + 1 } @@ -29,11 +29,11 @@ class OperationInputInjectionTests: XCTestCase { } func testInjectVia() throws { - let provider = ResultBlockOperation<Int, Error> { + let provider = ResultBlockOperation<Int> { return 1 } - let consumer = TransformOperation<String, Int, Error> { input in + let consumer = TransformOperation<String, Int> { input in return Int(input)! } @@ -62,15 +62,15 @@ class OperationInputInjectionTests: XCTestCase { let operationQueue = AsyncOperationQueue() - let providerA = ResultBlockOperation<Int, Error> { + let providerA = ResultBlockOperation<Int> { return 1 } - let providerB = ResultBlockOperation<Int, Error> { + let providerB = ResultBlockOperation<Int> { return 2 } - let consumer = TransformOperation<Int, String, Error> { input in + let consumer = TransformOperation<Int, String> { input in return "\(input)" } diff --git a/ios/PacketTunnel/PacketTunnelProvider.swift b/ios/PacketTunnel/PacketTunnelProvider.swift index c8bdfd2ea2..8b59185b90 100644 --- a/ios/PacketTunnel/PacketTunnelProvider.swift +++ b/ios/PacketTunnel/PacketTunnelProvider.swift @@ -597,31 +597,37 @@ class PacketTunnelProvider: NEPacketTunnelProvider, TunnelMonitorDelegate { var newAccountExpiry: Date? var newDeviceRevoked: Bool? - switch accountOperation.completion { + switch accountOperation.result { case let .failure(error): - self.providerLogger.error( - error: error, - message: "Failed to fetch account data." - ) + if !error.isOperationCancellationError { + self.providerLogger.error( + error: error, + message: "Failed to fetch account data." + ) + } case let .success(accountData): newAccountExpiry = accountData.expiry - case .none, .cancelled: break + case .none: + break } - switch deviceOperation.completion { + switch deviceOperation.result { case let .failure(error): - if error.compareErrorCode(.deviceNotFound) { + if let restError = error as? REST.Error, + restError.compareErrorCode(.deviceNotFound) + { newDeviceRevoked = true - } else { + } else if !error.isOperationCancellationError { self.providerLogger.error( error: error, message: "Failed to fetch device data." ) } - case .none, .cancelled, .success: break + case .none, .success: + break } if var deviceCheck = self.deviceCheck { @@ -657,9 +663,9 @@ class PacketTunnelProvider: NEPacketTunnelProvider, TunnelMonitorDelegate { } private func createGetAccountDataOperation(accountNumber: String) - -> ResultOperation<REST.AccountData, REST.Error> + -> ResultOperation<REST.AccountData> { - let operation = ResultBlockOperation<REST.AccountData, REST.Error>( + let operation = ResultBlockOperation<REST.AccountData>( dispatchQueue: dispatchQueue ) @@ -667,8 +673,8 @@ class PacketTunnelProvider: NEPacketTunnelProvider, TunnelMonitorDelegate { let task = self.accountsProxy.getAccountData( accountNumber: accountNumber, retryStrategy: .noRetry - ) { completion in - operation.finish(completion: completion) + ) { result in + operation.finish(result: result) } operation.addCancellationBlock { @@ -680,17 +686,17 @@ class PacketTunnelProvider: NEPacketTunnelProvider, TunnelMonitorDelegate { } private func createGetDeviceDataOperation(accountNumber: String, identifier: String) - -> ResultOperation<REST.Device, REST.Error> + -> ResultOperation<REST.Device> { - let operation = ResultBlockOperation<REST.Device, REST.Error>(dispatchQueue: dispatchQueue) + let operation = ResultBlockOperation<REST.Device>(dispatchQueue: dispatchQueue) operation.setExecutionBlock { operation in let task = self.devicesProxy.getDevice( accountNumber: accountNumber, identifier: identifier, retryStrategy: .noRetry - ) { completion in - operation.finish(completion: completion) + ) { result in + operation.finish(result: result) } operation.addCancellationBlock { |