[Review] SE-0112: Improved NSError Bridging


(Mohsen Ramezanpoor) #1

* I would like to see more work on `RecoverableError`. I know this is a more or less faithful translation of `NSErrorRecoveryAttempting`, but I don't think that's a particularly good API, and I would like to see it revisited.

The design I'd prefer would look more like this:

protocol RecoverableError: Error {
var recoveryOptions: [ErrorRecoveryOption] { get }
}

typealias ErrorRecoveryCompletionHandler = (recovered: Bool) ->Void

protocol ErrorRecoveryOption {
var localizedName: String { get }
func attemptRecovery(completion: ErrorRecoveryCompletionHandler) {…}
}

struct AnyErrorRecoveryOption: ErrorRecoveryOption {
typealias RecoveryAttempter = (ErrorRecoveryCompletionHandler) ->Void

var localizedName: String
var attempter: RecoveryAttempter

init(localizedName: String, attempter: RecoveryAttempter) {…}

func attemptRecovery(completion: CompletionHandler) { attempter(completion) }
}

Though further from the equivalent Foundation API, I think this is a much cleaner design. It requires no `switch` statements, avoids issues with matching indices between the option array and the recovery attempter, and makes it easier for interested subtypes to add or remove recovery options.

In particular, in my view, there should be only one way to attempt recovery, and that way should not be application-modal. If a client wants to recover modally, we should invoke the non-modal API and then spin the runloop until the completion handler runs.

If a better `RecoverableError` design isn't feasible in Swift 3, I think we can defer it. In a pinch, `CustomNSError` provides the ability to specify error-handling behavior the old-fashioned way.

I agree. This feels like a much better API, specially as it’s not tied to Cocoa (e.g. via `CustomNSError `). One tweak I would suggest is for the completion handler to return an error:

typealias ErrorRecoveryCompletionHandler = (recoveryError: Error?) ->Void

If the recovery from an error fails, It’s almost certainly because *another* error occurred.

I realise this is an even larger departure from Cocoa, but it should not be difficult to do technically. A negative result returned by, say, `NSErrorRecoveryAttempting` can will be converted to a new `RecoveryFailed` error struct which wraps the original error; conversely, a “recovered” boolean can be created by simply checking if the recovery error is `nil`.