Having Result's Failure type conform to Swift.Error is needlessly restrictive

Evolution Discussion
1

I was really bummed to see that Result was accepted with the Failure type restricted to conforming to Swift.Error. Unfortunately I wasn't part of the original discussion on this or I would have raised my objections at that time.

I understand that the Swift team wants to encourage people to use structured errors instead of just throwing e.g. a String in there, but in practice, this is needlessly restrictive.

For example, the issue I'm running into right now is I want to return something like Result<Model, (Error, sourceText: String)> from a method that decodes JSON, but I can't because (Error, sourceText: String) does not conform to Error. This failure type meets the desired goal of the Swift team (it contains a full error object rather than just a description), but I'm not allowed to do it. The only way to do this is to introduce something like struct ErrorWithSource { var error: Error; var sourceText: String } but that's more awkward to work with, and the need to define bespoke error types for individual functions encourages me to just avoid using Result to begin with.

1 Like
2

Sorry to hear your feedback wasn't heard, there seem to be a number of complaints recently around review periods being too short.

I think the decision made sense from the starting point of ‘We want to express throwable function results in the type system, how do we do it?’. Having a type that aligns with the error-handling model allows better opportunities for language integration, such as allowing auto-conversion of async APIs producing Results to ‘async throws’ functions, once the modifier is added to the language.

From that perspective, the type being generally useful is just a handy extra. If the goal of the proposal was to add a type which fits most use-cases, I expect we would have an Either<A, B> enum in the standard library now, not Result.

4 Likes
3

The issue here isn't that the review period was too short but that the proposal fundamentally changed after the first review returned it for revision, adding the Error constraint on the Failure type. From what I understand, the change was made to encourage and require users to model failure states as Error conforming types, as a principled stand on error handling in Swift. Frankly, the proposal would likely have had that constraint, if not for the fact that Error didn't conform to itself until work was done to make it so while the Result proposal was under review.

2 Likes
4

The alternative to Result<Success, Failure: Error> is not an Either type. I've expressed my opinion in the past about how Result<T,E> is different from Either for semantic reasons, even if the types are structurally the same.

The overall point here is that my "failure" case may encompass more than just an Error and having to create a bespoke second Error-conforming type just to carry the additional data is busywork that needlessly complicates my API (and in fact obscures the real error underneath).

1 Like
5

For what it's worth, this is the point at which I would tend to naturally create a custom SourceTextError Error-conforming struct that contains both underlyingError and sourceText properties.

4 Likes
6

You can certainly do that, but there's 3 issues with this:

  1. It's extra work all to recreate what should be a tuple.
  2. It obscures the actual failure information from the caller. All they see is a thing called SourceTextError, and have to go digging into it to realize that it contains an underlying error and additional data.
  3. And related, since SourceTextError is an Error in its own right, people will absolutely just treat it as an Error and e.g. throw it, which we don't want because it's not the real error, it's just a wrapper.
7

I don't really understand point 3. If you consider the source text to be a significant property of the error, why would you not want them thrown together? If it's not that significant, why worry about being able to return it as part of the Result? Just log it and return just the error.

8 Likes
8

I know my argument is very subjective, but I use tuples very little in my code, because I will actually prefer to have a nominal type I can conform to protocols instead of an anonymous tuple, especially in cases like this where the tuple sounds awfully like an Error.

7 Likes
9

The source text is not a property of the error. It's just additional data I want to have in this particular case along with the error. It's "here's an error, and here's the source text with the error", and I want to return it because my caller doesn't have access to that source text otherwise.

The problem with creating a new type to wrap this and having that new type conform to Error is I don't want my caller to treat that wrapper as an error. I don't want it to be converted to NSError and logged to Crashlytics, because that's not going to provide the actual information I want to see in my logs.

10

You could just as easily have information you want to an error when throwing. The primary reason Result has this constraint is that it is aligned with throws (and the potential direction of typed throws). Swift could have been designed to allow arbitrary values to be thrown but it wasn't. Instead, it uses the Error protocol.

If you think that was a mistake I think you should make that argument directly. If you don't think it was a mistake and think Result should be treated differently for some reason why is that the case? Why shouldn't the nominal type be aligned with the language feature?

2 Likes
11

throws works only in the case where you're returning a value to your caller, which means your caller knows what inputs you were called with and so there's no need give your caller more context for the error.

Result is designed to support callback-based flows, where you can't use throws because you're passing the error into something you call rather into your caller. In a callback-based flow, the code you're calling doesn't necessarily have the context for where the error occurred, because it occurred asynchronously.

In my particular case, I'm processing PubSub events, decoding the message data to a model object via JSONDecoder, and passing the result to an event handler. If the decode fails, I want to provide the source text for the JSON event to the handler along with the decode error so that way my handler can choose to include that JSON in any error logging it may do. I could write a wrapper error type, but it just doesn't make sense to me to have an error type whose sole meaning is "some arbitrary error occurred plus some associated value" which you then have to rip apart to get the actual error. It would be counterproductive to log this wrapper type anywhere because it has no meaning on its own, it's purely a container for the actual error plus the associated JSON source, and logging it would only serve to obfuscate the real error. The fact that I'm forced to declare that this wrapper type conforms to Error means that I'm allowing my caller to do stuff with this type that it shouldn't do, with no benefit.

12

This is exactly what Error is for and generally describes errors in Swift. They're all "some arbitrary error" with potentially "some associated value[s]". The benefit to conforming random types to Error is that they can then be used with Swift's built in error handling, including throws and Result.

This isn't going to change. So it seems you have a few options:

  1. Make (another) thread about letting tuples conform to protocols and see if it goes anywhere this time.
  2. Make a real type that can conform to Error.
  3. Don't use Result. Instead use an Either type or something like that that isn't constrained and manually bridge back into Swift's native error handling when you need.

2 seems the easiest option, given your objection isn't practical but rather philosophical. Besides which I would argue that your type does have meaning on its own, just like any error type.

8 Likes
13

No they emphatically are not. A DecodingError is not an arbitrary error. It's a specific error with a specific meaning. A CocoaError is not an arbitrary error. It's a specific error with a specific meaning. And so on. I should not be taking an Error and wrapping it up in a brand new struct unless my struct is providing additional semantic information to the error. The fact that the Error was provided by a particular API isn't relevant information because I already know it's being given to me by that API. Or to put it another way, if that was relevant information then every single API that throws an error should be expected to wrap the error in a brand new single-valued struct.

That's not a benefit. The fact that I can throw something doesn't automatically mean I should be able to. If the "ability to be used with Swift's built-in error handling" was an implicit benefit then String should conform to Error, Int should conform to Error, Date should conform to Error, etc.

A tuple should not conform to Error. That makes no sense.

That defeats the entire point of having Result in the standard library.

14

There's precedent in Cocoa for errors that wrap other errors; see NSUnderlyingErrorKey.

1 Like
15

It sounds like you're concerned that the extra data, which you claim isn't important, will be ignored. You aren't being consistent.

In any event, your wrapper struct can implement CustomStringConvertible to ensure that logging will include the source text. While this seems like a lot of work to handle a simple case, you have the burden of having unique or specialized requirements, and that often requires extra work on your part. I don't see why Result should be a formless blob just to cater to every conceivable use-case. At that point, it would be better not to have the type in the standard lib.

16

When I write asynchronous callbacks, those callbacks are generally closures and they capture whatever they need from the context. But if you are subscribing to events from multiple sources (such as in an observer of notification pattern), then your closure can't necessarily know the source in advance.

If you are in control of the subscribing APIs, maybe you could add a second context parameter to the callbacks instead of stuffing everything into Result. If you are not, then there's little choice but to include it inside the error type that must conform to Error.

I must say I do agree with the current Error constraint though. Have you ever captured an error and rethrown it (asynchronously) in another thread? You lose the context from the call stack then too. If this needs improvement with Result, the improvement deserves to be in added to errors when they are thrown too. And thus to me this thread should be about finding better ways to add context to Error, not relaxing the constraint in Result.

17

Sure, though this is used when the original error isn't an appropriate description of the problem at the current API level (e.g. FileManager will wrap a POSIXError with a CocoaError because the fact that Cocoa's file management routines are implemented by POSIX APIs is an implementation detail and not something that FileManager's clients should have to deal with). When my API involves decoding JSON into a model object, and and I get a decoding error, that decoding error is the appropriate thing to pass to my clients already, and wrapping it serves no purpose.

No I'm concerned that the actual error will be ignored. The error with semantic meaning got wrapped by one without, so unless you go digging into the underlying error, you actually lose meaning with this wrapping. I don't want to design an API that forces all of my users to dig into the underlying error in order to actually get the information they need, and I don't want the "obvious action" (taking the error and logging it to analytics) to produce a bad result.

Logging the original JSON in the same string as the actual error text is very explicitly a bad thing. That makes it really hard to coalesce error logs together or extract any meaningful information in aggregate. I want to log the actual error, which can then be handled appropriately in aggregate, and I want to attach the source text as secondary information that I can use if I dig into the individual logged errors as part of diagnosing the issue.

More generally, literally the only reason to have the Failure type conform to Error is because the Swift core team doesn't want to encourage people to use String as their error type. That's it. There's no technical benefit, as we can trivially make get() into a conditional extension. And I would argue that forcing the Failure type to conform to Error actually serves no purpose and is a holdover from thinking of this as a replacement for throws.

throws is specifically an error mechanism, and everything it throws should be an error. We already have a separate mechanism for returning one of two different value types (namely, a 2-variant enum with associated values), and the purpose of throws is very explicitly to implement an error strategy, including propagating errors up the stack, along with explicit catch blocks to catch them, etc.

Result is not specifically an error mechansim. It's the representation of the outcome of a failable computation. The failure state for a computation may not necessarily be an error. It will probably include an error, though even that isn't a requirement. More generally, Result does not directly tie into the rest of the error-handling mechanism. There is no automatic propagation of Results up the stack, no do/catch block construct that deals with it, and no automatic coalescing of multiple error types into a single existential (e.g. there's no built-in way to combine a Result<Int,FooError> with a Result<Int,BarError>). The fact that the Failure type is constrained on Error has no benefit and only makes Result more awkward to use outside of the limited scenario of "I'm taking a throws API and representing it with Result".

Also, there's precedent in Cocoa for wanting to have Failure not specifically conform to Error. What I'm thinking about is URLSession.dataTask(with:completion:). This API cannot be converted to use Result without retaining the ugly optional URLResponse? type in the handler, even though we know for a fact that the success case includes a URLResponse and we'd really like to make it non-optional. The ideal completion handler type here is (Result<(URLResponse, Data), (URLResponse?, Error)>) -> Void, but this is not a type that can be written as long as Failure is constrained to Error.

1 Like
18

Can you elaborate on what you think the logging site would look like? To my mind, you would be relying on the callee to unwrap the tuple to log what you think is important information.

{ result in
    // result.failure is a DecodeError
    if case let Result.failure(error) = result {
        log(error.error, extra: error.sourceText)
    }
}

{ result in
    // result.failure is a tuple (Error, String)
    if case let Result.failure(error) = result {
        log(error.0, extra: error.1)
    }
}

I'm not seeing how the latter is intrinsically better than the former.

19

Swift is verbose when you need to wrap types, and this is worth discussing. I think here the decision was good for the language, we need Error and Result should contain this type. But it would be nice–for sure—to have anonymous structs or even have tuples conform to protocols (can of worms perhaps) or some other some-such to make wrapping your tuple in some error type easier.

I've done a lot of work with Vapor lately and have experienced these kinds of pains frequently.

One thing I love about Swift is I know my pains are acknowledged by the core Swift developers and that they talk and think about how they can be alleviated, if it's possible, pragmatically.

I think you can acknowledge that error handling is important and if Result is to have more syntactic sugar available in the future it is important it have its failure type be Error and this is why we have this situation.

But that's not to say there aren't other possible way to alleviate your pains, and they are worthy of discussing and pitching.

1 Like
20

The problem with the former is the "obvious" way to write the handler actually just looks like

{ result in
    if case let Result.failure(error) = result {
        log(error)
   }
}

Unless you go looking at the definition of the error type you wouldn't realize that you're logging the wrong thing. Of course, if Failure wasn't constrained on Error then my failure type would not be loggable at all (as this log API only takes errors), thus requiring the caller to actually look at it and realize that it contains an actual error.

I agree it would be nice to have tuples be able to conform to protocols. But I've already said that I very explicitly do not want this particular failure type to conform to Error because having it be logged as-is to my error log is incorrect as that obscures the actual underlying error. I do not want someone to take a (Error, sourceText: String) tuple and be able to treat that whole tuple as if it were an Error in its own right.