Promisekit: Add retry feature

[kevin-hirsch]

Hello @mxcl,

First, thank you for your great library! :+1:

I was thinking that it would be great to have a retry feature when a promise fails. For example, when calling a web API, it would be great to retry when it returns a 5xx error.

Nice features could be:

• Have a number of tries retry(2)
• Have a delay retry(2, after: 1)
• Have a closure to decide when to retry or not retry(2, after: 1, when: { ((ErrorType) -> Bool) in … }

It would look something like this:

// Simple retry
NSURLConnection.GET(…).then { response in
    // Success code
}
.retry(2)
.error { error in
    UIAlertView(…).show()
}

// Retries 2 times and then enter the `error` function if the promise 
// failed 3 times (1 original + 2 retries)

// retry with delay (could be of type NSTimeInterval (seconds))
NSURLConnection.GET(…).then { response in
    // Success code
}
.retry(2, after: 3)
.error { error in
    UIAlertView(…).show()
}

// Retries 2 times. Waits each time 3 seconds before retrying

// retry with when a certain error occurred
NSURLConnection.GET(…).then { response in
    // Success code
}.retry(2, after: 3) { error -> Bool in
    // Retry only if the statusCode of the error is a server error (5xx)
    return error.statusCode = 5xx
}.error { error in
    // This closure is called only if the error status code was other than 5xx 
    // or there was a 5xx error after 2 retries
    UIAlertView(…).show()
}

// Retries 2 times only if 5xx internal server error. Waits each time 3 seconds before retrying.

What do you think? :)

[adamszeptycki]

I like the idea ;)

[wangii]

when iOS device is unlocked, the first http request always fails with the UnderlyingCocoaError, due to Apple's incorrect implementation of keep alive. before retry mechanism is implemented, how to handle it gracefully? currently I use ugly nested .error handler.

[mdhornet90]

This would be an awesome addition!

[mxcl]

Can you list concrete situations and use-cases where you have needed this? Not saying no, just want to understand the problem properly.

[jonasman]

This is a good idea to implement in PromiseKit
Here is an alternative version while you don't have it natively:

the promiseClosure in the beginning is needed otherwise so that the promise can resolve again and again.

func retry<T>(promise:(() -> Promise<T>), times:Int = 5, delaySeconds:NSTimeInterval = 0.0) -> Promise<T> {
    return promise().recover { (error) -> Promise<T> in
        if times == 0 {
             throw error
        } else {
            if delaySeconds > 0.0 {
                return delay(delaySeconds).thenInBackground { retry(promise, times: times - 1, delaySeconds: delaySeconds) }
            } else {
                return retry(promise, times: times - 1, delaySeconds: delaySeconds)
            }
        }
    }
}

func delay(delay:NSTimeInterval) -> Promise<()> {

    let (promise, fulfill, _) = Promise<()>.pendingPromise()

    let delayTime = dispatch_time(DISPATCH_TIME_NOW, Int64(delay * Double(NSEC_PER_SEC)))
    dispatch_after(delayTime, dispatch_get_main_queue()) {
        fulfill()
    }

    return promise
}

[mxcl]

It would have to be something like:

retry(times: 2, delay: 2) {
    NSURLSesssion.GET(url)
}.then { data in
    //…
}

Because the thing to be retried has to be captured in a closure or we could not retry it.

[mxcl]

I'm still reluctant to add this as I feel it is an anti-pattern to retry, can someone provide a concrete example of a time it was important to have this feature?

[jonasman]

I use retry in our app for some critical network requests.
We use an endpoint to give urls for the other endpoints. If this first call fails it makes all the other calls to fail. The initial call will be recalled again anyway when the user presses retry or goes to another screen. But just because one network call failed we shouldn't display an error to the user before we try for a couple of times.

[mxcl]

The endpoint cannot be fixed? Adding retry to the library feels like fixing things with duct tape when really we should be fixing the underlying cause properly.

[jonasman]

For example, if for any reason your internet is down for 1 sec a retry would help.

Sent from my iPhone

On 27/07/2016, at 19:58, Max Howell [email protected] wrote:

The endpoint cannot be fixed? Adding retry to the library feels like fixing things with duct tape when really we should be fixing the underlying cause properly.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

[mdhornet90]

I've been thinking about this since I first endorsed the inclusion of it,
and I realized it's a feature very specific to networking. While promises
are used extensively for async network calls, that's not all the library
can be used for, and so "retry" would be useless or unnecessary in a lot of
valid use cases. This might make more sense as an extension to the library.

That said, I haven't come up with a decent pattern when there is a network
call that fails and I want to retry. Everything I've written feels
ungainly, even with PromiseKit.

On Wed, Jul 27, 2016 at 9:58 AM, Max Howell [email protected]
wrote:

The endpoint cannot be fixed? Adding retry to the library feels like
fixing things with duct tape when really we should be fixing the underlying
cause properly.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/mxcl/PromiseKit/issues/350#issuecomment-235650265,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAZ4nIsHIdw-KrtaizwmCcgFkgFKijxAks5qZ45SgaJpZM4HDJEE
.

[mxcl]

Here’s code you can use:

func retry<T>(var times: Int, body: () -> Promise<T>) -> Promise<T>
    func attempt() -> Promise<T> {
        return body().recover { error -> Promise in
            guard times-- > 0 else { throw error }
            return after(2).then(attempt)
        }
    }
    return attempt()
}

Usage:

retry(2: times) { flakeyTask() }.then {
    //…
}

[zhenjiangliu24]

@discardableResult func retry<T>(times: Int, wait: TimeInterval = 2, body: @escaping () -> Promise<T>) -> Promise<T> {
    var attempts = times
    func attempt() -> Promise<T> {
        return body().recover { error -> Promise<T> in
            attempts -= 1
            guard attempts > 0 else { throw error }
            return after(interval: wait).then(execute: attempt)
        }
    }
    return attempt()
}

@mxcl This is what we tried for the retry for Promisekit, but did not work. Can you double check the code please?

[zlangley]

@zhenjiangliu24 What doesn't work about it? It seems to work fine for me.

[zhenjiangliu24]

@zlangley for me, what inside the retry function is been called only once.

[zhenjiangliu24]

 func samplePromise(result: Bool) -> Promise<Int> {

        return Promise<Int> { fulfill, reject in
            guard result else {
                count += 1
                reject(PromiseError.noResult); return
            }
            fulfill(count)

        }
    }

    func testRetry() {
        retry(times: 5) {
            self.samplePromise(result: false)
            }.then{_ in
                print("goes here.")
        }
        XCTAssert(count == 5, "retry failed")
    }

This is our unit test.

[zlangley]

XCTAssert will be called immediately after retry is kicked off; it will not wait for the async work to execute and will therefore always fail.

Consider using XCTestExpectation.

func testRetry() {
    enum Error: Swift.Error {
        case test
    }

    let ex = expectation(description: "")

    var count = 0
    retry(times: 5, wait: 0) { _ -> Promise<Void> in
        count += 1
        if count == 5 {
            ex.fulfill()
        }
        return Promise(error: Error.test)
    }.then {
        XCTFail()
    }
    waitForExpectations(timeout: 1)
}

[zhenjiangliu24]

@zlangley It works! Thanks man, you are awesome.

[basememara]

Does this also work for when? I can't seem to get it to work with the following:

func testWhenRetry() {
        let ex = expectation(description: "test -when- with retry")

        var promises: [Promise<Void>] = (1...5).map { _ in
            promises.append(Promise { fulfill, reject in
                self.count += 1
            })
        }
        
        retry(times: 5) {  _ -> Promise<Void> in
            when(resolved: promises)
                .always { _ in print("when.always \(self.count)") }
            
            if self.count == 10 {
                ex.fulfill()
            }

            return Promise(error: PromiseError.noResult)
        }

        waitForExpectations(timeout: 5)
        XCTAssert(count == 10, "test -when- failed")
    }

I think it's because the recover in the retry function is being called on when, but shouldn't it be called on the individual promises? It would be slick if the retry skips the promises that were fulfilled and keep going through the rejected. Or is there a better approach to what I'm doing?

[zlangley]

There are a number of issues in the test you provided. For one, the count will never exceed 5 because each of the promises in promises is only instantiated once; it's only the block provided to retry that will be executed multiple times. You're also not ever resolving any of the promises in promises. And remember that your XCTAssert will always fail because it will be called immediately.

If you have further questions, please open a new issue or ask them via gitter. :)

[nalexn]

For those who is looking for more robust implementation of the retry function that takes arbitrary number of retries and delay:

func retry<T>(times: Int, cooldown: TimeInterval, body: @escaping () -> Promise<T>) -> Promise<T> {
  var retryCounter = 0
  func attempt() -> Promise<T> {
    return body().recover(policy: CatchPolicy.allErrorsExceptCancellation) { error -> Promise<T> in
      retryCounter += 1
      guard retryCounter <= times else {
        throw error
      }
      return after(interval: cooldown).then(execute: attempt)
    }
  }
  return attempt()
}

[namanhams]

Is there anyway to do this in Objective C ? I can't seem to be able to use the recover function in Objective-C

[mxcl]

catch behaves the same as recover in ObjC

[thomers]

@namanhams have you managed to achieve this in Objective C?