How to switch catches on error types?

[epologee]

Thanks for writing #58, that's a nice feature, thanks!

I was looking for a way to mimick a switch-like decision tree with these error types. But from the looks of the current implementation, consecutive catch blocks will all be called when the promise is rejected:

(this test just serves to illustrate all three catch blocks are executed, sequentially)

There also seems to be no error-type based recover-method that would allow me to catch (and squelch) errors of a specific type, so if I want to be "picky" about specific error types, I still have to write the decision tree within the catch block, instead of leveraging the Promise API.

How would you approach something like this?
Cheers,
Eric-Paul

[khanlou]

Yeah so it seems like there are two issues here:

First, you want to be able to catch only uncaught errors. While we can write code that handles this pretty easily, I think it does represent a conceptual problem. Any time you filter out certain errors from being handled within a catch block, you run the risk of errors being accidentally filtered out and causing silent bugs. For example, twice now I've run into the issue with PromiseKit where it silently filters out cancellation errors from catch blocks causing bugs. You could imagine a situation where you have this function in a library:

func fetchFlight(id: ID) -> Promise<Flight> {
    return network.fetchFlight(by: id).catch(type: AirTrafficError.self, handleError)
}

and then a consumer comes along and calls this code:

fetchFlight(id: 123).catch({ error in })

What errors should be yielded here? They don't know that AirTrafficErrors have already been caught, and they might want to know about them anyway.

So then you end up with a situation where you want to disambiguate with names, such as catchOnlyUncaught or catchAll which end up kind of ugly and more confusing.

You could also address it by passing a second block to .catch(type:_:) for if the error doesn't match the type, but that has its own limitations and doesn't compose nicely.

It's a real problem and it almost makes me wish I hadn't added the matchers to the library.

The second issue is that you'd like a recover method that only recovers errors matching a certain type, and as far as I can tell, this is a slam dunk. If you want to make a PR with support for that function, I'd be happy to review and integrate it!

[epologee]

The second issue is that you'd like a recover method that only recovers errors matching a certain type, and as far as I can tell, this is a slam dunk. If you want to make a PR with support for that function, I'd be happy to review and integrate it!

I'm going to give this a shot!

[epologee]

Closing this issue in favor of #63.

[khanlou]

I was looking for a way to mimick a switch-like decision tree with these error types. But from the looks of the current implementation, consecutive catch blocks will all be called when the promise is rejected.

I've been thinking about this more, and I think we can manage it without too much more complexity by adding a new case to the state enum called .handledError(Error). It would switch into this state when the error is handled. I'm still not sure this is the right call, since you might want to handle some errors in a method, then return a promise and handle more errors outside. We could add a .allowErrorsToBeHandledAgain() method which could reset you back to the .rejected(Error) state? Just throwing out ideas here.

[epologee]

Interesting idea. I like the explicit wording of .allowErrorsToBeHandledAgain().

For my project, I went a somewhat different direction and added a silence(type:silencing:) method, that optionalizes the promise's value type, and just take a Void-returning closure to execute whatever you need to happen.

I do like the resulting declarations you can make. Here's an example from my tests:

AirTrafficClient()
    .suggestions("ea", for: .departure)
    .then { _ in fail("should have gone into the silencing-block") }
    .silence(type: AirTrafficError.self) { expect($0.errorStatusCode) == 400  }
    .catch { fail($0.localizedDescription) }
    .always { done() }

Unfortunately my approach can't handle already-optional input type, and would turn it into double-optional ??. Since my project didn't need it right then, I just gave up trying to make it work with generics 😬😊 and added a runtime error to prevent you from trying. This does make the implementation less clean than I would have liked:

/**
 Allows you to silence/squelch/eat Promise-errors of a particular type, and turn the original Promise.Value
 into an optional one. Due to this last implementation detail, this extension will unfortunately not work
 for Promises that already define an optional return type.
 */
public extension Promise {
    func silence<E: Error>(type errorType: E.Type, _ silencing: ((E) throws -> Void)? = nil) -> Promise<Value?> {
        let anyType: Any.Type = Value.self
        if anyType is OptionalMarker.Type {
            fatalError("this method should not be used with optional Promise-values")
        }

        return Promise<Value?> { fulfill, reject in
            self.then(fulfill)
                .catch { anyError in
                    guard let error = anyError as? E else {
                        reject(anyError)
                        return
                    }

                    do {
                        try silencing?(error)
                        fulfill(nil)
                    } catch let error {
                        reject(error)
                    }
            }
        }
    }
}

private protocol OptionalMarker: ExpressibleByNilLiteral {}
extension Optional: OptionalMarker {}

[khanlou]

For this particular use case, what I would actually do is instead of using optionals, you could map to another type that's shaped like an optional but isn't one, and then just use recover. So you'd do

enum SuggestionResult {
     case suggestion(Suggestion)
     case errorCode400
}

And then you can map to this new result type and just use recover

AirTrafficClient()
    .suggestions("ea", for: .departure)
    .then { value in SuggestionResult.suggestion(value) }
    .then { _ in fail("should have gone into the silencing-block") }
    .recover(type: AirTrafficError.self) { if $0.errorStatusCode == 400 { return Promise(value: .errorCode400 } else { return Promise(error: $0) } }
    .catch { fail($0.localizedDescription) }
    .always { done() }

Would that work for your use case?

[epologee]

It actually would. It would also alleviate the need to explain what a nil-value actually represents. Hmm, now I'm rethinking that silence(...) extension...

[epologee]

Would the compiler accept a more map-like sibling of recover(type:) next to the more flatMap-like one we introduced with this PR? The one thing I'm tripping over in the example you gave is the requirement of returning a full Promise(value: ...)-instance, instead of just the value.

[khanlou]

Yeah, it seems like it should be possible, but you'd probably want it to be a throwing function so you can have an error escape hatch. Worth trying!