[Revised] SE-0235: Add Result to the Standard Library


(John McCall) #41

Note that it's equally unclear how to translate that into async/await, which is not coincidental. async throws -> Data with an inout URLResponse? parameter?


#42

There are 2 possibles I think.

One is what you said.
(Result<Data, Error>, URLResponse?) -> Void.
This is simple and obviously better than original (Data?, URLResponse?, Error?) -> Void
But it is not best in a point.
I want to represents that URLResponse is never nil when success if I can.

Second is below.

struct URLSessionDataTaskError : Error {
    var response: URLResponse?
    var error: Error
}

(Result<(Data, URLResponse), URLSessionDataTaskError>) -> Void

But user needs to handle inner error property and this is far from standard way.

So I think first one is better.


#43

This change is not welcome at all.

It is very painful to developers who use the currently untyped throwing mechanism, and plan to use Result<..., Swift.Error>.

Just look:

// Given
func loadFoo(completion: @escaping (Result<Foo, Error>) -> ()) { ... }
func transform(_ foo: Foo) throws -> Bar { ... }
func handle(_ result: Result<Bar, Error>) { ... }

// When flatMap accepts a throwing closure
func loadFoo { result: Result<Foo, Error> in
    // Nice :-)
    let barResult = result.flatMap(transform)
    handle(barResult)
}

// When flatMap does not accept a throwing closure
func loadFoo { result: Result<Foo, Error> in 
    // Not nice at all :-(
    let barResult = result.flatMap { value in
        Result { try transform(value) }
    }
    handle(barResult)
}

When there is no flatMap variant which accepts a throwing closure, it becomes painful to write, and read, chains of throwing methods.


(John McCall) #44

I'm not an expert in this API, but yes, I tend to agree that it would be better if the information from URLResponse was somehow reflected in the error when it's meaningful. I think NSErrors are supposed to be self-contained descriptions of the failure, which I'm surprised can happen here without the information from the URLResponse.


(Yuta Koshizawa) #45

Because dataTask(with:completionHandler:) returns a URLSessionDataTask, it is impossible to be converted into async/await directly. I think we need an extension method like below.

extension URLSessionDataTask {
    func completion() async throws -> (Data, URLResponse) { ... }
}

Then we need to provide URLResponse for error cases by throwing an error like @omochimetaru proposed.

enum may be preferred.

enum URLSessionDataTaskError: Error {
    case foo(message: String)
    case bar(message: String, response: URLResponse) // when received a response but fails
    ...
}

(Tjeerd in 't Veen) #46

I am very happy with this proposal.

Having used Result extensively, there is one more insight I’d like to share; not something that needs to be solved right away or ever, but is a bit cumbersome:

Once you start using combinators, such as map and flatMap, and mapError, it is a bit awkward that flatMap does not change the error type. As a result—pun intended—this may catch developers off guard, and the mapError always has to move above the flatMap operation if error types are different.

My question is: Should we consider a flatMap-like operation that allows for returning a Result with different error types? If you follow Rust, this question sometimes popup and developers come up with their own functions to deal with this.

mapAny on the SwiftPM comes close when working with AnyError, but perhaps this is something to take into account.


(John McCall) #47

Four comments:

  • The core team isn't strongly opposed to the entire idea of having this operation; we just don't want to call it flatMap and don't have an alternative name readily at hand.
  • Your example is somewhat rigged: your first example looks really clean because you happen to have a function which exactly matches the signature required by flatMap without needing extra parameters or anything.
  • You can easily keep a functional style here without a flatMap variant by writing a generic higher-order function which turns a throwing function into a Result-producing function.
  • You could also just drop the functional style and write let barResult = Result { try transform(result.unwrapped()) } instead. unwrapped() is almost always going to be the cleanest way of combining throwing code with Result.

(Adrian Zubarev) #48

Can you or any other core team member provide the reasoning why it was decided for that asymmetric naming scheme instead of something like this?!

mapValue
mapError

flatMapValue
flatMapError

I think if we start to add more operations to this type in the future and for disambiguation need to add Value as part of the name it will become even more misaligned.


I second this. Even though we would have case success(Value) you can still add a constrained extension for some more convenience when Value is Void.

extension Result where Value == Void {
  public static var success: Result<Value, Error> {
    return .success(())
  }
}

(John McCall) #49

The value-focused operations are clearly the more central ones; I don't think there's anything wrong with acknowledging that in the naming.

If you'll accept an argument based in abstract nonsense, it's also true that Swift consistently uses flatMap for the monadic bind operation of whatever type it's defined on, and as a monad Result is expected to vary in its value type, not its error type.


(Adrian Zubarev) #50

Well I can live with that, but it still feels a little distanced from the already established naming scheme in the stdlib: mapValues, compactMapValues


#51

The core team isn't strongly opposed to the entire idea of having this operation; we just don't want to call it flatMap and don't have an alternative name readily at hand.

That's good to hear. But I have to point that this framing is unlikely to be the correct one.

Since this "operation" is essentially the same because Result<..., Error> is the dual of do/try/catch, this alternative name may never emerge. It is flatMap.

You have to wonder if this focus on naming is worth keeping, if the consequence is the killing of this feature.

Your example is somewhat rigged: your first example looks really clean because you happen to have a function which exactly matches the signature required by flatMap without needing extra parameters or anything.

Come on... Excuse me to use a throwing function!!!

And even with parameters, the difference would still be interesting;

result.flatMap { try transform($0, extraArgument) }
// vs.
result.flatMap { value in Result { try transform(value, extraArgument) } }

You can easily keep a functional style here without a flatMap variant by writing a generic higher-order function which turns a throwing function into a Result -producing function.

This is the same point as the first, above. And now, you are pushing the baby to application code.

You could also just drop the functional style and write let barResult = Result { try transform(result.unwrapped()) } instead. unwrapped() is almost always going to be the cleanest way of combining throwing code with Result .

You are a member of the Core Team. I plead your judgment about the UX problem in the Standard Library that this missing feature introduces. Untyped throws is still a thing, isn't it?


(Adrian Zubarev) #52

Just a few other minor things:

  • Since the final branching date for Swift 5 is over it means Result won't made it into Swift 5 and we would have to wait until Swift 5.1?
  • Similar story with the self-conforming Error, does it require a standalone proposal or are we waiving it through just like that?

(Chéyo Jiménez) #53

I prefer unwrapping()

try someResult.unwrapping()

I think unwrap() should be reserved and probably provided along side the throwing function but as a way to force unwrap similar to rust’s version.

https://doc.rust-lang.org/rust-by-example/error/option_unwrap.html


(Jeffrey Macko) #54

Should we consider adding a Validated type which is Result with an array of error instead of only one error ?


(Pyry Jahkola) #55

You could achieve this by creating your own Error type which contains an array of nested errors as a property.


(Matt Gallagher) #56

I love it; pragmatic and conservative. I echo @nicklockwood et al on the .success/.failure case naming. Giving enum cases the same name as their associated content leads to repetitious statements like if let error = error { result = .error(error) } which gets brain numbing to read.

I choose to assume this means that self-conformance for all protocol existentials, auto-synthesized enum case sugar (including enum pattern matching as a boolean expression) and default arguments for generic parameters are all guaranteed for Swift 5.1


(Erik Little) #57

I am rather lukewarm on the proposal now that it has been decided to constrain the error case to Swift.Error. I don't see the value this adds over an unconstrained version, and I don't like to think that Result should in anyway be related to error handling.

My one bit of input would be that we should continue to include fold. I don't buy into the argument that we should hold out on adding it in a later proposal. As for the name of the method, I would rather just keep with fold, or have some authoritarian decision made on its naming, but holding out because of some future sugaring of switch syntax feels as bad an argument as just saying we shouldn't add it period. The only real argument I see for holding off on adding fold is to generalize that functionality, and define it as a protocol so that it could be used from many types that offer similar functionality. But does that really work well for different types that might have better names for the functionality offered by that method?


(Brandon Williams) #58

I'm still +1 to the proposal, but find the Swift.Error constraint unfortunate. It means we can never use tuples, functions or 3rd party types that don't conform to Error as errors.

For example, we can't do Result<A, [MyError]>, even if MyError: Error. So we need a wrapper ErrorArray<A>: Error { let values: [A] }. But now this wrapper doesn't behave anything like an array, so we are forced to continually unwrap+transform+wrap, or conform it to Collection so that it behaves like an array.

I know there is a lot of guidance for giving a "proper" type to errors, like enums and wrappers, but that can often be too much of a burden in a local domain. A proliferation of wrapper types can also be its own kind of anti-pattern, leading to types that don't carry their own weight because you don't want to implement all of the API's that the wrapped type has on the wrapper.


(Adrian Zubarev) #59

Why? You can do this:

ˋˋˋ
extension Array : Error where Element : Error {}
ˋˋˋ

Or is your point different and you want some error types not conform to Error protocol?


(Brandon Williams) #60

The common guidance is that you shouldn't conform 3rd party types to 3rd party protocols. Otherwise you are in for a world of hurt. I believe SPM or NIO ran into this recently, but I may be mistaken.

And if the standard library decides to add that, where does it end? We may also want it for dictionaries, optionals, array slices, etc... May we want it for all collections who elements are errors, but it's not possible to extend protocols in that way.

My point is that the Error constraint is far more restrictive than some may believe at first. Some of it due to a lack of Swift features that could be fixed in the future (like tuples and functions), and others due to things that will probably never be fixed in Swift.