Promisekit: Act upon error without doubling up promises

Created on 3 May 2018  Â·  7Comments  Â·  Source: mxcl/PromiseKit

PromiseKit 6.2.5

I've read about doubling up promises and I see why that should be avoided.

But here's my issue: I'd like to act upon an error before it gets send up to the calling object.
Here's how I've achieved this now:

func register(facebookAccessToken: String) -> Promise<Void> {
    return Promise { seal in
        firstly {
            service.registerUser(facebookAccessToken: facebookAccessToken)
        }.map { accessToken in
            self.storeAccessToken(accessToken) // returns Void
        }.then {
            self.updateAuthenticatedUser() // returns Promise
        }.done {
            NotificationCenter.default.post(Notification(name: .userAuthenticated, object: self))
            seal.fulfill(())
        }.catch { error in
            self.discardAccessToken()
            seal.reject(error)
        }
    }
}

Is there a way I can achieve the same without "doubling up promises"?

Also, I'd like to know if I used map (5th line) in the way that was intended, since I'm not returning anything from that method.

Most helpful comment

Apologies, I haven't written this pattern recently and it appears that it has in fact changed due to catch returning PMKFinalizer which cannot be chained with another catch.

For the moment at least, you could do this instead which accomplishes the same though a little less succinctly:

func register(facebookAccessToken: String) -> Promise<Void> {
    let promise = firstly {
        service.registerUser(facebookAccessToken: facebookAccessToken)
    }.map { accessToken in
        self.storeAccessToken(accessToken) // returns Void
    }.then {
        self.updateAuthenticatedUser() // returns Promise
    }.done {
        NotificationCenter.default.post(Notification(name: .userAuthenticated, object: self))
    }

    promise.catch { _ in
        self.discardAccessToken()
    }

    return promise
}

//elsewhere
register(facebookAccessToken: token).catch { error in
    //this catch will get called as well
}

All 7 comments

Does service.registerUser return a promise? It looks like it probably does since there’s no completion on it. In which case the wrapping Promise { seal in … } is unnecessary and would constitute doubling up. You could simply return firstly { ….

Otherwise this looks good to me!

(Edit: removed recommendation for tap in place of map when the latter is in fact the correct choice in this case.)

@nathanhosselton service.registerUser returns Promise<String>.
When I omit return Promise { seal in and return firstly I get the following error:

Cannot convert return expression of type 'PMKFinalizer' to return type 'Promise<Void>'

To me this indicates that I cannot use catchwhen I return firstly.

Apologies, I haven't written this pattern recently and it appears that it has in fact changed due to catch returning PMKFinalizer which cannot be chained with another catch.

For the moment at least, you could do this instead which accomplishes the same though a little less succinctly:

func register(facebookAccessToken: String) -> Promise<Void> {
    let promise = firstly {
        service.registerUser(facebookAccessToken: facebookAccessToken)
    }.map { accessToken in
        self.storeAccessToken(accessToken) // returns Void
    }.then {
        self.updateAuthenticatedUser() // returns Promise
    }.done {
        NotificationCenter.default.post(Notification(name: .userAuthenticated, object: self))
    }

    promise.catch { _ in
        self.discardAccessToken()
    }

    return promise
}

//elsewhere
register(facebookAccessToken: token).catch { error in
    //this catch will get called as well
}

Ah, in fact this is in our migration guide even (scroll to the bottom). And the above is the given solution for catching before and after the return.

Alternatively, you could check the status of the chain with tap and perform your failure cleanup there instead of in a catch:

return firstly {
    service.registerUser(facebookAccessToken: facebookAccessToken)
}.map { accessToken in
    self.storeAccessToken(accessToken) // returns Void
}.then {
    self.updateAuthenticatedUser() // returns Promise
}.tap { result in
    if case .rejected = result { 
        self.discardAccessToken()
    }
}.done {
    NotificationCenter.default.post(Notification(name: .userAuthenticated, object: self))
}

Thank you @nathanhosselton. I personally prefer the first solution. I love that I no longer need the double promises.

We should add this to doubling promised entry. It’s stumped a few for sure.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

dboydor picture dboydor  Â·  4Comments

zvonicek picture zvonicek  Â·  3Comments

daanporon picture daanporon  Â·  6Comments

guidedways picture guidedways  Â·  4Comments

nolanw picture nolanw  Â·  5Comments