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

Catch Clause Not Invoked #76

Closed
jasperblues opened this issue Feb 7, 2021 · 7 comments
Closed

Catch Clause Not Invoked #76

jasperblues opened this issue Feb 7, 2021 · 7 comments

Comments

@jasperblues
Copy link

jasperblues commented Feb 7, 2021

I've set up a somewhat complex Promise chain where the catch clause is not invoked as expected.

Here's a test-case for Promise (fails) and another for PromiseKit (passes).

Test case is derived from a real world app scenario that didn't work as I expected it would. Perhaps it is a user error or misunderstanding.

Failing Test:

import Foundation
import Promise

class Chain {

    func verify(userId: String, withToken: String) -> Promise<Void> {
        chunk1()
            .then { [self] result in
                chunk2()
            }
            .then {
                print("All done!")
            }
    }

    func chunk1() -> Promise<String> {
        first()
            .then { [self] result in
                second()
            }
            .then { [self] result in
                third()
            }
            .then { [self] result in
                fourth()
            }
    }

    func chunk2() -> Promise<String> {
        fifth()
            .then { [self] result in
                sixth()
            }
            .then { [self] result in
                seventh()
            }
            .then { [self] result in
                eighth()
            }
    }

    func first() -> Promise<String> {
        Promise<String>(work: { fulfill, reject in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 1"
                print(message)
                fulfill(message)
            }
        })
    }

    func second() -> Promise<String> {
        Promise<String>(work: { fulfill, reject in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 2"
                print(message)
                fulfill(message)
            }
        })
    }

    func third() -> Promise<String> {
        Promise<String>(work: { fulfill, reject in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 3"
                print(message)
                fulfill(message)
            }
        })
    }

    func fourth() -> Promise<String> {
        Promise<String>(work: { fulfill, reject in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 4"
                print(message)
                fulfill(message)
            }
        })
    }

    func fifth() -> Promise<String> {
        Promise<String>(work: { fulfill, reject in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 5"
                print(message)
                fulfill(message)
            }
        })
    }

    func sixth() -> Promise<String> {
        Promise<String>(work: { fulfill, reject in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 6"
                print(message)
                reject(RuntimeError(reason: "bang at 6"))
            }
        })
    }

    func seventh() -> Promise<String> {
        Promise<String>(work: { fulfill, reject in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 7"
                print(message)
                fulfill(message)
            }
        })
    }

    func eighth() -> Promise<String> {
        Promise<String>(work: { fulfill, reject in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 8"
                print(message)
                fulfill(message)
            }
        })
    }

    func ninth() -> Promise<String> {
        Promise<String>(work: { fulfill, reject in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 9"
                print(message)
                fulfill(message)
            }
        })
    }

}

Failing test:

class PromiseTest : XCTestCase {


    func test_it_handles_chain() {

        var result: Bool? = nil

        let chain = Chain()

        chain.verify(userId: "foo", withToken: "foo")
            .then {
                result = false
            }
            .catch { error in
                result = true
            }


        expect {
            result != nil
        }

        print("Go the final result: \(result)")
        XCTAssertEqual(result, true)

    }

}

PromiseKit Version

class PKChain {

    func verify(userId: String, withToken: String) -> Promise<Void> {
        chunk1()
            .then { [self] result in
                chunk2()
            }
            .done { result in
                print("All done!")
            }
    }

    func chunk1() -> Promise<String> {
        first()
            .then { [self] result in
                second()
            }
            .then { [self] result in
                third()
            }
            .then { [self] result in
                fourth()
            }
    }

    func chunk2() -> Promise<String> {
        fifth()
            .then { [self] result in
                sixth()
            }
            .then { [self] result in
                seventh()
            }
            .then { [self] result in
                eighth()
            }
    }

    func first() -> Promise<String> {
        Promise { seal in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 1"
                print(message)
                seal.fulfill(message)
            }
        }
    }

    func second() -> Promise<String> {
        Promise { seal in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 2"
                print(message)
                seal.fulfill(message)
            }
        }
    }

    func third() -> Promise<String> {
        Promise { seal in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 3"
                print(message)
                seal.fulfill(message)
            }
        }
    }

    func fourth() -> Promise<String> {
        Promise { seal in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 4"
                print(message)
                seal.fulfill(message)
            }
        }
    }

    func fifth() -> Promise<String> {
        Promise { seal in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 5"
                print(message)
                seal.fulfill(message)
            }
        }
    }

    func sixth() -> Promise<String> {
        Promise { seal in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 6"
                print(message)
                seal.reject(RuntimeError(reason: "bang at 6"))
            }
        }
    }

    func seventh() -> Promise<String> {
        Promise { seal in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 7"
                print(message)
                seal.fulfill(message)
            }
        }
    }

    func eighth() -> Promise<String> {
        Promise { seal in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 8"
                print(message)
                seal.fulfill(message)
            }
        }
    }

    func ninth() -> Promise<String> {
        Promise { seal in
            DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) {
                let message = "message 9"
                print(message)
                seal.fulfill(message)
            }
        }
    }

}

Test case:

class PKPromisesTest : XCTestCase {

    func test_it_handles_chain() {

        var result: Bool? = nil

        let chain = PKChain()

        chain.verify(userId: "foo", withToken: "foo")
            .done { string in
                result = false
            }
            .catch { error in
                print("Ahahahahah error: \(error)")
                result = true
            }


        expect {
            result != nil
        }

        print("Go the final result: \(result)")
        XCTAssertEqual(result, true)

    }

}

Source for Expect/Async Function:

(Sometimes I find XCTest style expectations fussy, so I roll this one:)

func expect(condition: @escaping () -> Bool) {
    expectWithin(seconds: 7, condition: condition)
}

func expectWithin(seconds: TimeInterval, condition: @escaping () -> Bool) {
    for _ in 0..<Int(seconds) {
        if condition() {
            return
        } else {
            RunLoop.current.run(until: Date(timeIntervalSinceNow: 1))
        }
    }
    fatalError("Condition didn't happen before timeout:\(seconds)")
}
@jasperblues
Copy link
Author

jasperblues commented Feb 7, 2021

When I change to the following it works as expected:

    func verify(userId: String, withToken: String) -> Promise<Void> {
        chunk1()
            .then { [self] result in
               chunk2()
            }
            .then { result in <--- 'result in' is missing from original version. 
                print("All done!")
            }
            .catch { error in
                print("Got an error")
            }
    }

result in is missing from the original version. It will also work with an explicit rather than implied return chunk2(). Looks like the Swift compiler tried to get clever?

Is it possible to put a compile-time check so that the return value of previous promise must be consumed in the next?

@jasperblues
Copy link
Author

Real World Code

    func verify(userId: String, withToken: String) -> Promise<Void> {
        openClient()
            .then { networking in
                networking.putAsync("/api/users/\(userId)/verify/", parameters: ["token": withToken])
            }
    }

^-- Here's the real-world culprit

    func verify(userId: String, withToken: String) -> Promise<Void> {
        openClient()
            .then { networking in
                networking.putAsync("/api/users/\(userId)/verify/", parameters: ["token": withToken])
            }.then { result in
                print("Got result: \(result)")
            }
    }

^-- But like this it works as expected. Need to be careful with Promise<Void>

@khanlou
Copy link
Owner

khanlou commented Feb 7, 2021

Thanks for following up, this would have been tough to figure out without that. So, if you use the void-returning tap version of then, it won't wait until a promise fired in there is returned before continuing on to the next promise. It's definitely a subtle behavior, but I'm not sure if there's a way around it other than perhaps renaming the methods?

@khanlou
Copy link
Owner

khanlou commented Feb 7, 2021

@jasperblues
Copy link
Author

All clear what was happening now. Perhaps some documentation is the right solution for this.

My IDE (I like AppCode) always suggests to me that return is redundant for single line closures, however if I had written:

openClient()
            .then { networking in
                return networking.putAsync("/api/users/\(userId)/verify/", parameters: ["token": withToken])
            }

. . . with an explicit return, then it would not have compiled (method signature is Promise<Void> and it would've been more obvious that another step in the chain was required .

  • Let the previous step fulfill
  • Map the JSONResponse return type to void.

But then it could also be the case that the previous step returned Void too and I still would have tripped.

Helper Method To Convert Typed Promise to Void or Await For Void to Fulfill

extension Promise {

    func await() -> Promise<Void> {
        self.then { result in }
    }

}

Usage

Rather than writing this:

    func verify(userId: String, withToken: String) -> Promise<Void> {
        openClient()
            .then { networking in
                networking.putAsync("/api/users/\(userId)/verify/", parameters: ["token": withToken])
            }.then { result in } <-- Which looks a bit tedious 
    }

I created a helper that awaits the previous step in the chain and returns a Void promise like this:

    func verify(userId: String, withToken: String) -> Promise<Void> {
        openClient()
            .then { networking in
                networking.putAsync("/api/users/\(userId)/verify/", parameters: ["token": withToken])
            }.await() <-- Slightly better? 
    }

Perhaps it could be a pattern when it comes to Promise<Void>? Maybe await is a bad name though, as it might be confused with JavaScript/TypeScript/etc do-notation of promises.

Using Promise Had a Learning Curve

I found that this library is super super simple to learn, except for some potential pitfalls with Promise. Assuming others might trip in the same areas that I did, then documentation could include:

  • I posted a question about how to fulfill a Promise with void, which you kindly answered. fulfill(()) vs fulfill()
  • The pitfall above.

@jasperblues
Copy link
Author

All done from my side, so close at will. Thanks again!

@khanlou
Copy link
Owner

khanlou commented Feb 10, 2021

Closing this as it seems to have been resolved.

Just a quick question for you @jasperblues: if you add back the explicit return, does it also work with that?

@khanlou khanlou closed this as completed Feb 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants