Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make all error enums extensible #969

Open
weissi opened this issue Apr 12, 2019 · 4 comments
Open

make all error enums extensible #969

weissi opened this issue Apr 12, 2019 · 4 comments
Labels
⚠️ needs-major-version-bump For PRs that when merged cause a bump of the major version, ie. x.0.0 -> (x+1).0.0
Milestone

Comments

@weissi
Copy link
Member

weissi commented Apr 12, 2019

There are basically two options:

  1. Swift has open enums for everyone by the time we start working on NIO 3 in which case we just need to open all error enums.
  2. Swift still won't have open enums for everyone and then we might want to do the following pattern. We probably should start with this ASAP for new error types:
public struct SandwichError: Error, Equatable, CustomStringConvertible {
    private enum SandwichErrorCode: Int {
        case tooLittleSalami = 1
        case tooLittleMustard
        case noTomatoes
        case noBread
    }

    private var code: SandwichErrorCode

    private init(code: SandwichErrorCode) {
        self.code = code
    }

    public var description: String {
        return "SandwichError.\(String(describing: self.code))"
    }

    public static let tooLittleSalami: SandwichError = .init(code: .tooLittleSalami)
    public static let tooLittleMustard: SandwichError = .init(code: .tooLittleMustard)
    public static let noTomatoes: SandwichError = .init(code: .noTomatoes)
    public static let noBread: SandwichError = .init(code: .noBread)
}

func inspectErrorWithSwitch(_ error: Error) {
    if let error = error as? SandwichError {
        switch error {
        case .tooLittleSalami:
            print("NEED MORE SALAMI!")
        case .tooLittleMustard:
            print("NEED MORE MUSTARD!")
        case .noTomatoes:
            print("Can I please get a tomato?")
        default:
            print("ooh, some error I didn't know existed: \(error)")
        }
    } else {
        print("totally unexpected error")
    }
}

func inspectErrorWithCatch(_ e: Error) {
    do {
        throw e
    } catch let error as SandwichError where error == .tooLittleSalami {
        print("NEED MORE SALAMI!")
    } catch let error as SandwichError where error == .tooLittleMustard {
        print("NEED MORE MUSTARD!")
    } catch let error as SandwichError where error == .noTomatoes {
        print("Can I please get a tomato?")
    } catch let error as SandwichError {
        print("ooh, some error I didn't know existed: \(error)")
    } catch {
        print("totally unexpected error")
    }
}

struct OtherRandomError: Error {}

inspectErrorWithSwitch(SandwichError.tooLittleSalami)
inspectErrorWithSwitch(SandwichError.tooLittleMustard)
inspectErrorWithSwitch(SandwichError.noTomatoes)
inspectErrorWithSwitch(SandwichError.noBread)
inspectErrorWithSwitch(OtherRandomError())

inspectErrorWithCatch(SandwichError.tooLittleSalami)
inspectErrorWithCatch(SandwichError.tooLittleMustard)
inspectErrorWithCatch(SandwichError.noTomatoes)
inspectErrorWithCatch(SandwichError.noBread)
inspectErrorWithCatch(OtherRandomError())
@weissi weissi added the ⚠️ needs-major-version-bump For PRs that when merged cause a bump of the major version, ie. x.0.0 -> (x+1).0.0 label Apr 12, 2019
@weissi weissi added this to the 3.0.0 milestone Apr 12, 2019
@weissi weissi changed the title make all errors extensible make all error enums extensible Apr 12, 2019
@MrMage
Copy link
Contributor

MrMage commented Apr 15, 2019

Sorry for the silly question, but how would SandwichError be more extensible in this case than if SandwichError itself were the enum?

@Lukasa
Copy link
Contributor

Lukasa commented Apr 15, 2019

Because enums are frozen, which means that the Swift compiler verifies that switches over those enums are exhaustive. If we add an enum case, that will break some kinds of previously-exhaustive switch statements, which is a semver major.

However, switch statements over equatable structs cannot be verified to be exhaustive, and so they enforce the use of a default case. That's useful for us, because it means we can safely add extra cases without breaking those switch statements.

In the future the Swift team may bring open enums to third-party libraries, at which point we will transition back to using open enumerations.

@MrMage
Copy link
Contributor

MrMage commented Apr 15, 2019

@Lukasa thank you for the explanation! I was somehow thinking of user-extensible, not framework-extensible. Would have never thought that extending an enum would be a breaking change, but it makes total sense in this light.

@weissi
Copy link
Member Author

weissi commented Apr 15, 2019

Big unsolved problem with SandwichError is obviously pattern matching for cases that need values. For example:

public struct PizzaError: Error, Equatable, CustomStringConvertible {
    private enum PizzaErrorCode: Equatable {
        case unacceptableIngredient(String)
        case tooCold
    }

    private var code: PizzaErrorCode

    private init(code: PizzaErrorCode) {
        self.code = code
    }

    public var description: String {
        return "PizzaError.\(String(describing: self.code))"
    }

    public static func unacceptableIngredient(_ which: String) -> PizzaError {
        return .init(code: .unacceptableIngredient(which))
    }

    public static let tooCold: PizzaError = .init(code: .tooCold)
}

let e1 = PizzaError.unacceptableIngredient("chicken")
let e2 = PizzaError.unacceptableIngredient("barbeque sauce")
let e3 = PizzaError.tooCold

let someError: PizzaError = [e1, e2, e3].randomElement()!

switch someError {
case .tooCold:
    print("too cold!")
// // THIS DOES NOT WORK
// case .unacceptableIngredient(let foo):
//    print("bad ingredient: \(foo)")

// THIS DOES WORK but is bad because we'd need to match the exact value
case .unacceptableIngredient("chicken"):
    print("chicken doesn't belong on pizza")
default:
    print("so picky: \(someError)")
}

// we can build `match*` or something properties that yield the associated
// values, but that's a lot of work and not very ergonomic to work with.
extension PizzaError {
    var matchUnacceptableIngredient: String? {
        switch self.code {
        case .unacceptableIngredient(let which):
            return which
        default:
            return nil
        }
    }
}

if let badIngredient = someError.matchUnacceptableIngredient {
    print("\(badIngredient) on pizza is unacceptable")
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚠️ needs-major-version-bump For PRs that when merged cause a bump of the major version, ie. x.0.0 -> (x+1).0.0
Projects
None yet
Development

No branches or pull requests

3 participants