google / promises

Promises is a modern framework that provides a synchronization construct for Swift and Objective-C.
Apache License 2.0
3.8k stars 294 forks source link

Promise<String> fulfil not working as expected #88

Closed lucabartoletti closed 5 years ago

lucabartoletti commented 5 years ago

Hi, while writing some code I start noticing that a catch block was called even if the promise was fulfil. The catch was called in a complex chain so in the attempt to find the problem I reduced my code to:

    public func test() -> Promise<String> {
        return Promise<String> { fulfil, _ in
            fulfil("String")
        }
    }

called as follow:

        controller.test().then { (string) in
            print("result: \(string)")
        }.catch { (error) in
            print("Error: \(error.localizedDescription)")
        }

Running the above I get Error: The operation couldn’t be completed. (Swift.String error 1.)

I tracked down where promises decides that the above is an error to: Promise+Asynch.swift:28 where for some reason I truly don't understand if let error = value as? NSError generate a cast from String to NSError

While changing the code above to

    public func test() -> Promise<NSString> {
        return Promise<NSString> { fulfil, _ in
            let string = String("12")
            fulfil(string as NSString)
        }
    }

then the fulfil goes well and the .then is called

I tried to reproduce the behaviour I see in my project in the Promises test suite without success.

I don't understand what is going on here, any help is appreciated.

shoumikhin commented 5 years ago

Hi Luca,

Could you please share a complete example of your controller that has that test function, and also specify the Xcode & Swift versions you're using? You may also open Promises.xcworkspace in Xcode, navigate to Promises.playground and experiment there.

For example, the code:

func test() -> Promise<String> {
  return Promise<String> { fulfill, _ in
    fulfill("String")
  }
}

test().then { string in
  print("result: \(string)")
}.catch { error in
  print("Error: \(error.localizedDescription)")
}

works as expected for me, so I clearly need more surrounding context to understand what's going on your side.

Thanks.

ghost commented 5 years ago

Hi Luca, feel free to reopen if you can still reproduce this issue.

nandrienk commented 4 years ago

I have the same issue in my project. And also can't reproduce it in Playground. So maybe you have an idea in which cases it can appear. The same code works in different ways.

shoumikhin commented 4 years ago

Hi Nikita, can you elaborate more? A code snippet would help. Is it related to Promise getting rejected when you expect it to be fulfilled? Please provide more context.

nandrienk commented 4 years ago

Yes, here is simplified example of my app code.

public func logIn(login: String, password: String) -> Promise<String?> {
    let promise = Promise<String?> {
        resolve, reject in
        // here happens https request in result we have Result<String?,Error>
        SCSdk.API.SignIn.auth(login: login, password: password).execute {
            [weak self] result in
            switch result {
                case .success(let accessToken):
                resolve(accessToken!)
                case .failure(let error):
                reject(error)
            }
        }
    }
    return promise
}

After I call this method like this.

func auth(login: String, password: String) {
    SCSdk.session.logIn(login: login, password: password).then {
        accessToken in
        print(accessToken)
    }.catch {
        error in
        print("Error:\(error)")
    }
}

And here is a problem usually I get "Error: accessToken". While checking breakpoints I see that resolve(fullFill) called, but after it goes to .catch. In this block my string with token interprets as error. If I change String to Int it works as expected.

shoumikhin commented 4 years ago

Hi Nikita,

I believe part of the problem is the promise type the login func returns, which is the optional string, whereas you fulfill the promise with a non-optional string, if I’m not mistaken?

nandrienk commented 4 years ago

Hi, It was first that I tried, but removing optionality of string does not help. Result the same(

bryanwagner commented 4 years ago

I ran into the same problem when trying to run the chained then example from the documentation. There does appear to be an issue specifically with Promise<String>, or there is some confusion we are running into that probably should be clarified. Could you try running the following unit tests?

import XCTest
@testable import Promises

class PromiseTests: XCTestCase {

  func testPromiseInt() {
    func log(_ message: String) {
      print("testPromiseInt: \(message)")
    }
    func work1(_ value: Int) -> Promise<Int> {
      log("work1: value=\(value)")
      return Promise { value }
    }
    func work2(_ value: Int) -> Promise<Int> {
      log("work2: value=\(value)")
      return Promise { value }
    }

    let exp = expectation(description: "testPromiseInt")
    let p1 = work1(42)
    let p2 = p1.then(work2).then { _ in exp.fulfill() }
    log("isFulfilled: p1=\(p1.isFulfilled), p2=\(p2.isFulfilled)")

    XCTAssert(waitForPromises(timeout: 3))
    waitForExpectations(timeout: 3)
    log("isFulfilled: p1=\(p1.isFulfilled), p2=\(p2.isFulfilled)")
  }

  func testPromiseString() {
    func log(_ message: String) {
      print("testPromiseString: \(message)")
    }
    func work1(_ value: String) -> Promise<String> {
      log("work1: value=\(value)")
      return Promise { value }
    }
    func work2(_ value: String) -> Promise<String> {
      log("work2: value=\(value)")
      return Promise { value }
    }

    let exp = expectation(description: "testPromiseString")
    let p1 = work1("42")
    let p2 = p1.then(work2).then { _ in exp.fulfill() }
    log("isFulfilled: p1=\(p1.isFulfilled), p2=\(p2.isFulfilled)")

    XCTAssert(waitForPromises(timeout: 3))
    waitForExpectations(timeout: 3)
    log("isFulfilled: p1=\(p1.isFulfilled), p2=\(p2.isFulfilled)")
  }

  func testPromiseVec2() {
    struct Vec2 {
      let x: Int, y: Int
    }
    func log(_ message: String) {
      print("testPromiseVec2: \(message)")
    }
    func work1(_ value: Vec2) -> Promise<Vec2> {
      log("work1: value=\(value)")
      return Promise { value }
    }
    func work2(_ value: Vec2) -> Promise<Vec2> {
      log("work2: value=\(value)")
      return Promise { value }
    }

    let exp = expectation(description: "testPromiseVec2")
    let p1 = work1(Vec2(x: 42, y: 43))
    let p2 = p1.then(work2).then { _ in exp.fulfill() }
    log("isFulfilled: p1=\(p1.isFulfilled), p2=\(p2.isFulfilled)")

    XCTAssert(waitForPromises(timeout: 3))
    waitForExpectations(timeout: 3)
    log("isFulfilled: p1=\(p1.isFulfilled), p2=\(p2.isFulfilled)")
  }

  func testPromiseVec3() {
    class Vec3: CustomStringConvertible {
      let x: Int, y: Int, z: Int
      init (x: Int, y: Int, z: Int) {
        self.x = x
        self.y = y
        self.z = z
      }
      public var description: String { return "(\(x), \(y), \(z))" }
    }
    func log(_ message: String) {
      print("testPromiseVec3: \(message)")
    }
    func work1(_ value: Vec3) -> Promise<Vec3> {
      log("work1: value=\(String(describing: value))")
      return Promise { value }
    }
    func work2(_ value: Vec3) -> Promise<Vec3> {
      log("work2: value=\(String(describing: value))")
      return Promise { value }
    }

    let exp = expectation(description: "testPromiseVec3")
    let p1 = work1(Vec3(x: 42, y: 43, z: 44))
    let p2 = p1.then(work2).then { _ in exp.fulfill() }
    log("isFulfilled: p1=\(p1.isFulfilled), p2=\(p2.isFulfilled)")

    XCTAssert(waitForPromises(timeout: 3))
    waitForExpectations(timeout: 3)
    log("isFulfilled: p1=\(p1.isFulfilled), p2=\(p2.isFulfilled)")
  }
}

I get the following output:

Test Case '-[Tests.PromiseTests testPromiseInt]' started.
testPromiseInt: work1: value=42
testPromiseInt: isFulfilled: p1=false, p2=false
testPromiseInt: work2: value=42
testPromiseInt: isFulfilled: p1=true, p2=true
Test Case '-[Tests.PromiseTests testPromiseInt]' passed (0.023 seconds).

Test Case '-[Tests.PromiseTests testPromiseString]' started.
testPromiseString: work1: value=42
testPromiseString: isFulfilled: p1=false, p2=false
<unknown>:0: error: -[Tests.PromiseTests testPromiseString] : Asynchronous wait failed: Exceeded timeout of 3 seconds, with unfulfilled expectations: "testPromiseString".
testPromiseString: isFulfilled: p1=false, p2=false
Test Case '-[Tests.PromiseTests testPromiseString]' failed (3.194 seconds).

Test Case '-[Tests.PromiseTests testPromiseVec2]' started.
testPromiseVec2: work1: value=Vec2(x: 42, y: 43)
testPromiseVec2: isFulfilled: p1=false, p2=false
testPromiseVec2: work2: value=Vec2(x: 42, y: 43)
testPromiseVec2: isFulfilled: p1=true, p2=true
Test Case '-[Tests.PromiseTests testPromiseVec2]' passed (0.024 seconds).

Test Case '-[Tests.PromiseTests testPromiseVec3]' started.
testPromiseVec3: work1: value=(42, 43, 44)
testPromiseVec3: isFulfilled: p1=false, p2=false
testPromiseVec3: work2: value=(42, 43, 44)
testPromiseVec3: isFulfilled: p1=true, p2=true
Test Case '-[Tests.PromiseTests testPromiseVec3]' passed (0.022 seconds).

These tests are derived from the example in the documentation. 3 tests pass but testPromiseString fails. Am I missing something? Otherwise, the solution for now is to avoid using Promise<String> as it appears to be breaking the promise chain. But if you use a user-defined struct as in testPromiseVec2 or a user-defined class as in testPromiseVec3, the promise chain works as expected. I'm using Swift 4.2 and PromisesSwift (1.2.8).

bryanwagner commented 4 years ago

Aha, I've found the source of my issue, which I'd suspect might be a gotcha for others as well. I found this String extension in my dependencies:

extension String: Error {}

After removing it, the above test now passes! I've seen the above extension in examples around the internet and it should probably be considered an anti-pattern. It now makes sense why the above extension would conflict with Promises. Hopefully this helps anyone else running into the same issue.

nandrienk commented 4 years ago

Thanks, your solution helps me a lot, I also have same String extension.