Merge branch 'add-operation-error'

10 files changed, 76 insertions, 75 deletions

diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj
index fb9f67f90b..a3b3440984 100644
--- a/ios/MullvadVPN.xcodeproj/project.pbxproj
+++ b/ios/MullvadVPN.xcodeproj/project.pbxproj
@@ -11,7 +11,6 @@
 		58059DDC28465E8F002B1049 /* TransformOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58059DDB28465E8F002B1049 /* TransformOperation.swift */; };
 		58059DDE28468158002B1049 /* OutputOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58059DDD28468158002B1049 /* OutputOperation.swift */; };
 		58059DE02846823E002B1049 /* ResultOperation+Output.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58059DDF2846823E002B1049 /* ResultOperation+Output.swift */; };
-		58059DE228468255002B1049 /* ResultOperation+Fallible.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58059DE128468255002B1049 /* ResultOperation+Fallible.swift */; };
 		5806767C27048E9B00C858CB /* PacketTunnelProvider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58CE5E7B224146470008646E /* PacketTunnelProvider.swift */; };
 		5807483B27DB8A980020ECBF /* WireGuardKitTypes in Frameworks */ = {isa = PBXBuildFile; productRef = 5807483A27DB8A980020ECBF /* WireGuardKitTypes */; };
 		5807E2C02432038B00F5FF30 /* String+Split.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5807E2BF2432038B00F5FF30 /* String+Split.swift */; };
@@ -74,7 +73,6 @@
 		5835B7CC233B76CB0096D79F /* TunnelManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5835B7CB233B76CB0096D79F /* TunnelManager.swift */; };
 		5838318B27C40A3900000571 /* Pinger.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5838318A27C40A3900000571 /* Pinger.swift */; };
 		583DA21425FA4B5C00318683 /* LocationDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = 583DA21325FA4B5C00318683 /* LocationDataSource.swift */; };
-		583E1E1B2848DE1C004838B3 /* ResultOperation+Fallible.swift in Sources */ = {isa = PBXBuildFile; fileRef = 58059DE128468255002B1049 /* ResultOperation+Fallible.swift */; };
 		583E1E1C2848DE1C004838B3 /* AsyncOperationQueue.swift in Sources */ = {isa = PBXBuildFile; fileRef = 589D28782846250500F9A7B3 /* AsyncOperationQueue.swift */; };
 		583E1E1E2848DE1C004838B3 /* OperationCondition.swift in Sources */ = {isa = PBXBuildFile; fileRef = 589D28772846250500F9A7B3 /* OperationCondition.swift */; };
 		583E1E202848DE1C004838B3 /* OperationObserver.swift in Sources */ = {isa = PBXBuildFile; fileRef = 589D28792846250500F9A7B3 /* OperationObserver.swift */; };
@@ -361,7 +359,6 @@
 		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>"; };
-		58059DE128468255002B1049 /* ResultOperation+Fallible.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ResultOperation+Fallible.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>"; };
@@ -668,7 +665,6 @@
 				5846226426E0D9630035F7C2 /* ProductsRequestOperation.swift */,
 				5842102D282D3FC200F24E46 /* ResultBlockOperation.swift */,
 				58F7D26427EB50A300E4D821 /* ResultOperation.swift */,
-				58059DE128468255002B1049 /* ResultOperation+Fallible.swift */,
 				58059DDF2846823E002B1049 /* ResultOperation+Output.swift */,
 				58059DDB28465E8F002B1049 /* TransformOperation.swift */,
 			);
@@ -1279,7 +1275,6 @@
 				583E1E1C2848DE1C004838B3 /* AsyncOperationQueue.swift in Sources */,
 				5820676226E75D8500655B05 /* REST.swift in Sources */,
 				58A8055E2716EA6700681642 /* AnyIPAddress.swift in Sources */,
-				583E1E1B2848DE1C004838B3 /* ResultOperation+Fallible.swift in Sources */,
 				58DF5B7D28521AAC00E92647 /* OutputOperation.swift in Sources */,
 				58DF5B79285217F300E92647 /* TransformOperation.swift in Sources */,
 				5857F23024C843ED00CF6F47 /* ChainedError.swift in Sources */,
@@ -1323,7 +1318,6 @@
 				58B993B12608A34500BA7811 /* LoginContentView.swift in Sources */,
 				58E6771F24ADFE7800AA26E7 /* SettingsNavigationController.swift in Sources */,
 				58A1AA8C23F5584C009F7EA6 /* ConnectionPanelView.swift in Sources */,
-				58059DE228468255002B1049 /* ResultOperation+Fallible.swift in Sources */,
 				582BB1B3229574F40055B6EF /* SettingsAccountCell.swift in Sources */,
 				588527B2276B3F0700BAA373 /* LoadTunnelConfigurationOperation.swift in Sources */,
 				58289082286B590900478596 /* UIFont+Monospaced.swift in Sources */,
diff --git a/ios/MullvadVPN/Operations/AsyncOperation.swift b/ios/MullvadVPN/Operations/AsyncOperation.swift
index 1ac471cf6a..1655098af6 100644
--- a/ios/MullvadVPN/Operations/AsyncOperation.swift
+++ b/ios/MullvadVPN/Operations/AsyncOperation.swift
@@ -62,6 +62,10 @@ class AsyncOperation: Operation {
     /// Access must be guarded with `stateLock`.
     private var __isCancelled: Bool = false
 
+    /// Backing variable for `error`.
+    /// Access must be guarded with `stateLock`.
+    private var _error: Error?
+
     /// Operation state.
     @objc private var state: State {
         get {
@@ -96,6 +100,19 @@ class AsyncOperation: Operation {
         }
     }
 
+    private(set) var error: Error? {
+        get {
+            stateLock.lock()
+            defer { stateLock.unlock() }
+            return _error
+        }
+        set {
+            stateLock.lock()
+            defer { stateLock.unlock() }
+            _error = newValue
+        }
+    }
+
     final override var isReady: Bool {
         stateLock.lock()
         defer { stateLock.unlock() }
@@ -323,22 +340,18 @@ class AsyncOperation: Operation {
     }
 
     func finish() {
-        var notifyDidFinish = false
+        finish(error: nil)
+    }
 
-        operationLock.lock()
-        if state < .finished {
-            state = .finished
-            notifyDidFinish = true
-        }
-        operationLock.unlock()
+    func finish(error: Error?) {
+        guard tryFinish(error: error) else { return }
 
-        if notifyDidFinish {
-            dispatchQueue.async {
-                self.operationDidFinish()
+        dispatchQueue.async {
+            self.operationDidFinish()
 
-                for observer in self.observers {
-                    observer.operationDidFinish(self)
-                }
+            let anError = self.error
+            for observer in self.observers {
+                observer.operationDidFinish(self, error: anError)
             }
         }
     }
@@ -358,14 +371,24 @@ class AsyncOperation: Operation {
 
     private func checkReadiness() {
         operationLock.lock()
+        defer { operationLock.unlock() }
 
         if state == .pending, !_isCancelled, super.isReady {
             evaluateConditions()
         }
-
-        operationLock.unlock()
     }
 
+    private func tryFinish(error: Error?) -> Bool {
+        operationLock.lock()
+        defer { operationLock.unlock() }
+
+        guard state < .finished else { return false }
+
+        self.error = error
+        state = .finished
+
+        return true
+    }
 
     // MARK: - Subclass overrides
 
diff --git a/ios/MullvadVPN/Operations/AsyncOperationQueue.swift b/ios/MullvadVPN/Operations/AsyncOperationQueue.swift
index 8f8b1b8060..2476f2abd6 100644
--- a/ios/MullvadVPN/Operations/AsyncOperationQueue.swift
+++ b/ios/MullvadVPN/Operations/AsyncOperationQueue.swift
@@ -67,7 +67,7 @@ private final class ExclusivityManager {
 
             operationsByCategory[category] = operations
 
-            let blockObserver = OperationBlockObserver(didFinish: { [weak self] op in
+            let blockObserver = OperationBlockObserver(didFinish: { [weak self] op, error in
                 self?.removeOperation(op, categories: categories)
             })
 
diff --git a/ios/MullvadVPN/Operations/BackgroundObserver.swift b/ios/MullvadVPN/Operations/BackgroundObserver.swift
index e5d83c6235..6e8e0562c7 100644
--- a/ios/MullvadVPN/Operations/BackgroundObserver.swift
+++ b/ios/MullvadVPN/Operations/BackgroundObserver.swift
@@ -44,7 +44,7 @@ class BackgroundObserver: OperationObserver {
         // no-op
     }
 
-    func operationDidFinish(_ operation: Operation) {
+    func operationDidFinish(_ operation: Operation, error: Error?) {
         if let taskIdentifier = taskIdentifier {
             application.endBackgroundTask(taskIdentifier)
         }
diff --git a/ios/MullvadVPN/Operations/OperationCondition.swift b/ios/MullvadVPN/Operations/OperationCondition.swift
index da3e5f56a1..64d4f385fa 100644
--- a/ios/MullvadVPN/Operations/OperationCondition.swift
+++ b/ios/MullvadVPN/Operations/OperationCondition.swift
@@ -8,10 +8,6 @@
 
 import Foundation
 
-protocol FallibleOperation {
-    var error: Error? { get }
-}
-
 protocol OperationCondition {
     var name: String { get }
     var isMutuallyExclusive: Bool { get }
@@ -53,8 +49,7 @@ final class NoFailedDependenciesCondition: OperationCondition {
 
     func evaluate(for operation: Operation, completion: @escaping (Bool) -> Void) {
         let satisfy = operation.dependencies.allSatisfy { operation in
-            if let fallibleOperation = operation as? FallibleOperation,
-                fallibleOperation.error != nil {
+            if let operation = operation as? AsyncOperation, operation.error != nil {
                 return false
             }
 
diff --git a/ios/MullvadVPN/Operations/OperationObserver.swift b/ios/MullvadVPN/Operations/OperationObserver.swift
index 28ce7ee348..0cb42e54f1 100644
--- a/ios/MullvadVPN/Operations/OperationObserver.swift
+++ b/ios/MullvadVPN/Operations/OperationObserver.swift
@@ -12,23 +12,24 @@ protocol OperationObserver {
     func didAttach(to operation: Operation)
     func operationDidStart(_ operation: Operation)
     func operationDidCancel(_ operation: Operation)
-    func operationDidFinish(_ operation: Operation)
+    func operationDidFinish(_ operation: Operation, error: Error?)
 }
 
 /// Block based operation observer.
 class OperationBlockObserver<OperationType: Operation>: OperationObserver {
     typealias VoidBlock = (OperationType) -> Void
+    typealias FinishBlock = (OperationType, Error?) -> Void
 
     private let _didAttach: VoidBlock?
     private let _didStart: VoidBlock?
     private let _didCancel: VoidBlock?
-    private let _didFinish: VoidBlock?
+    private let _didFinish: FinishBlock?
 
     init(
         didAttach: VoidBlock? = nil,
         didStart: VoidBlock? = nil,
         didCancel: VoidBlock? = nil,
-        didFinish: VoidBlock? = nil
+        didFinish: FinishBlock? = nil
     )
     {
         _didAttach = didAttach
@@ -55,9 +56,9 @@ class OperationBlockObserver<OperationType: Operation>: OperationObserver {
         }
     }
 
-    func operationDidFinish(_ operation: Operation) {
+    func operationDidFinish(_ operation: Operation, error: Error?) {
         if let operation = operation as? OperationType {
-            _didFinish?(operation)
+            _didFinish?(operation, error)
         }
     }
 }
diff --git a/ios/MullvadVPN/Operations/ResultOperation+Fallible.swift b/ios/MullvadVPN/Operations/ResultOperation+Fallible.swift
deleted file mode 100644
index fac45cbf93..0000000000
--- a/ios/MullvadVPN/Operations/ResultOperation+Fallible.swift
+++ /dev/null
@@ -1,15 +0,0 @@
-//
-//  ResultOperation+Fallible.swift
-//  MullvadVPN
-//
-//  Created by pronebird on 31/05/2022.
-//  Copyright © 2022 Mullvad VPN AB. All rights reserved.
-//
-
-import Foundation
-
-extension ResultOperation: FallibleOperation {
-    var error: Error? {
-        return completion?.error
-    }
-}
diff --git a/ios/MullvadVPN/Operations/ResultOperation.swift b/ios/MullvadVPN/Operations/ResultOperation.swift
index 1d14b0cb4f..dbbb050d4c 100644
--- a/ios/MullvadVPN/Operations/ResultOperation.swift
+++ b/ios/MullvadVPN/Operations/ResultOperation.swift
@@ -13,42 +13,42 @@ class ResultOperation<Success, Failure: Error>: AsyncOperation {
     typealias Completion = OperationCompletion<Success, Failure>
     typealias CompletionHandler = (Completion) -> Void
 
-    fileprivate let stateLock = NSLock()
+    private let nslock = NSLock()
     private var completionValue: Completion?
     private var _completionQueue: DispatchQueue?
     private var _completionHandler: CompletionHandler?
     private var pendingFinish = false
 
     var completion: Completion? {
-        stateLock.lock()
-        defer { stateLock.unlock() }
+        nslock.lock()
+        defer { nslock.unlock() }
         return completionValue
     }
 
     var completionQueue: DispatchQueue? {
         get {
-            stateLock.lock()
-            defer { stateLock.unlock() }
+            nslock.lock()
+            defer { nslock.unlock() }
 
             return _completionQueue
         }
         set {
-            stateLock.lock()
+            nslock.lock()
             _completionQueue = newValue
-            stateLock.unlock()
+            nslock.unlock()
         }
     }
 
     var completionHandler: CompletionHandler? {
         get {
-            stateLock.lock()
-            defer { stateLock.unlock() }
+            nslock.lock()
+            defer { nslock.unlock() }
 
             return _completionHandler
         }
         set {
-            stateLock.lock()
-            defer { stateLock.unlock() }
+            nslock.lock()
+            defer { nslock.unlock() }
             if !pendingFinish {
                 _completionHandler = newValue
             }
@@ -73,24 +73,29 @@ class ResultOperation<Success, Failure: Error>: AsyncOperation {
 
     @available(*, unavailable)
     override func finish() {
-        _finish()
+        _finish(error: nil)
+    }
+
+    @available(*, unavailable)
+    override func finish(error: Error?) {
+        _finish(error: error)
     }
 
     func finish(completion: Completion) {
-        stateLock.lock()
+        nslock.lock()
         if completionValue == nil {
             completionValue = completion
         }
-        stateLock.unlock()
+        nslock.unlock()
 
-        _finish()
+        _finish(error: completion.error)
     }
 
-    fileprivate func _finish() {
-        stateLock.lock()
+    private func _finish(error: Error?) {
+        nslock.lock()
         // Bail if operation is already finishing.
         guard !pendingFinish else {
-            stateLock.unlock()
+            nslock.unlock()
             return
         }
 
@@ -108,14 +113,14 @@ class ResultOperation<Success, Failure: Error>: AsyncOperation {
 
         // Copy completion queue.
         let completionQueue = _completionQueue
-        stateLock.unlock()
+        nslock.unlock()
 
         let block = {
             // Call completion handler.
             completionHandler?(completion)
 
             // Finish operation.
-            super.finish()
+            super.finish(error: error)
         }
 
         if let completionQueue = completionQueue {
diff --git a/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift b/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift
index 249ea8887d..c92c6a1c0b 100644
--- a/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift
+++ b/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift
@@ -136,7 +136,7 @@ class SetAccountOperation: ResultOperation<StoredAccountData?, TunnelManager.Err
                     .reduce()
 
                 saveSettingsOperation.addBlockObserver(
-                    OperationBlockObserver(didFinish: { operation in
+                    OperationBlockObserver(didFinish: { operation, error in
                         self.completeOperation(accountData: operation.output)
                     })
                 )
@@ -174,9 +174,7 @@ class SetAccountOperation: ResultOperation<StoredAccountData?, TunnelManager.Err
         }
 
         let errors = children.compactMap { operation -> TunnelManager.Error? in
-            let fallibleOperation = operation as? FallibleOperation
-
-            return fallibleOperation?.error as? TunnelManager.Error
+            return (operation as? AsyncOperation)?.error as? TunnelManager.Error
         }
 
         if let error = errors.first {
diff --git a/ios/MullvadVPNTests/OperationObserverTests.swift b/ios/MullvadVPNTests/OperationObserverTests.swift
index ab4c29f6e3..bc1c92b442 100644
--- a/ios/MullvadVPNTests/OperationObserverTests.swift
+++ b/ios/MullvadVPNTests/OperationObserverTests.swift
@@ -25,7 +25,7 @@ class OperationObserverTests: XCTestCase {
                 expectDidStart.fulfill()
             }, didCancel: { op in
                 expectDidCancel.fulfill()
-            }, didFinish: { op in
+            }, didFinish: { op, error in
                 expectDidFinish.fulfill()
             }
         ))
@@ -52,7 +52,7 @@ class OperationObserverTests: XCTestCase {
                 expectDidStart.fulfill()
             }, didCancel: { op in
                 expectDidCancel.fulfill()
-            }, didFinish: { op in
+            }, didFinish: { op, error in
                 expectDidFinish.fulfill()
             }
         ))