Make it so that repeat: can terminate on nil OR an object and return that.

[dmitrym0]

Hi @couchdeveloper,

This is a small pull request that makes it so that repeat: behaves slightly differently (and more consistently with then say). Instead of terminating on nil and returning a constant, it terminates if and only if the value returned from a repeat block is not a promise. Furthermore, when the termination occurs the result is returned up the chain.

I'd like to get some feedback from you. My plan is to use this in my own code, in order to poll a webservice (may take a few repetitions) and then return the result.

What do you think?

Thanks,

Dmitry

[couchdeveloper]

On 17.12.2014, at 21:58, Dmitry [email protected] wrote:

Hi @couchdeveloper https://github.com/couchdeveloper,

This is a small pull request that makes it so that repeat: behaves slightly differently (and more consistently with then say). Instead of terminating on nil and returning a constant, it terminates if and only if the value returned from a repeat block is not a promise. Furthermore, when the termination occurs the result is returned up the chain.

I'd like to get some feedback from you. My plan is to use this in my own code, in order to poll a webservice (may take a few repetitions) and then return the result.

What do you think?

If I understood you correctly, you are trying to implement some kind of “retry” functionality. I think this is a great idea! But wouldn’t it be better to leave repeat: as is and introduce a new method:

typedef id (^ rxp_retry_block)(id input);

+ (instancetype) retry:(rxp_retry_block)tryOnce;

retry: could also have an additional parameter, specifying the “retry policy” like max number of tries, min duration between tries, delay algorithm (e.g.: exponential, linear, etc.).

Is it this what you are searching for?

Thanks,

Dmitry

You can merge this Pull Request by running

git pull https://github.com/dmitrym0/RXPromise master Or view, comment on, or merge it at:

https://github.com/couchdeveloper/RXPromise/pull/30 https://github.com/couchdeveloper/RXPromise/pull/30 Commit Summary

Make it so that repeat: can terminate on nil OR an object and return that File Changes

M RXPromise Libraries/Source/RXPromise+RXExtension.h https://github.com/couchdeveloper/RXPromise/pull/30/files#diff-0 (2) M RXPromise Libraries/Source/RXPromise+RXExtension.mm https://github.com/couchdeveloper/RXPromise/pull/30/files#diff-1 (20) M Samples/Sample7/main.m https://github.com/couchdeveloper/RXPromise/pull/30/files#diff-2 (4) M Test/Tests/RXPromiseTest.mm https://github.com/couchdeveloper/RXPromise/pull/30/files#diff-3 (23) Patch Links:

https://github.com/couchdeveloper/RXPromise/pull/30.patch https://github.com/couchdeveloper/RXPromise/pull/30.patch https://github.com/couchdeveloper/RXPromise/pull/30.diff https://github.com/couchdeveloper/RXPromise/pull/30.diff — Reply to this email directly or view it on GitHub https://github.com/couchdeveloper/RXPromise/pull/30.

[dmitrym0]

Thanks for the feedback. I thought about splitting this behavior up initially, but figured that retry and repeat (as you describe them) were sufficiently similar and could be encapsulated in one call. However I understand why you want to keep them separate.

I will refactor the new behavior out. I'm not sure about the extra parameter to be honest. I think it'll increase the complexity, perhaps unnecessarily so. In my use case, the code running inside the block is responsible for scheduling and cancellation, but I can see how it would be useful to others.. Were you thinking of something like

+ (instancetype) retry:(rxp_retry_block)tryOnce;

and

+ (instancetype) retry:(rxp_retry_block)tryOnce withPolicy:... ?

[couchdeveloper]

I'm not sure about the extra parameter to be honest. I think it'll increase the complexity, perhaps unnecessarily so.

Perhaps yes ;)

So the most simple API would be:

typedef id (^rxp_retry_block)();

+ (instancetype) retry:(rxp_retry_block)tryOnce;

where the return value of rxp_retry_block might be either "something" or a RXPromise. If it's not a RXPromise, method retry: will immediately invoke the block tryOnce again.

If this is sufficient for you it's absolutely OK to extend the RXPromise API in this way.

If you want that retry: will invoke subsequent calls of the retry block with some delay for the next call (possibly a dynamic delay, e.g.: exponential increasing), you'll need this extra parameter "retry policy" - whatever it actually is. And yes, this will complicate the implementation since you also need to implement a means to cancel retry: when the retry block is currently delayed (in addition to when it is actually executing). You can implement this sort of cancelable "dispatch_after" utilizing a dispatch timer. IMHO, this kind of stuff would also be very interesting ;)

[dmitrym0]

I started implementing this as you suggested but I'm hitting a bit of a roadblock, so I'd appreciate a suggestion.

If we're assuming that retry: will rerun the block if it returns anything other than RXPromise doesn't that mean the block is running synchronously?

Consider this example (which is in fact what I'm trying to implement, pseudocodish objective-c):

[downloadManager prepareDownload].retry:^id{
  [NetworkOperation queryWebServicesToSeeIfDownloadIsReadyWithSucess:^() {
  // #1
  } andOnFailure:^() {
  // #2
  }];
  // #3 
}].then(^id(id result){
  NSLog(@"Download ready, proceed");
  return nil;
}, ^id(NSError* error){
  NSLog(@"Error occured: %@", error);
  return nil;
});

Based on your spec above, the block must return immediately at #3 above, so this appears not to work as intended. What am I missing?

Thinking some more, it appears that the retry block always needs to return a promise, and only retry if there's no value returned in this promise. In the example above, I'm always expecting a value ("download ready") or an error (when the server hasn't been able to prepare the download for whatever reason).

What do you think?

[couchdeveloper]

If we're assuming that retry: will rerun the block if it returns anything other than RXPromise doesn't that mean the block is running synchronously?

Well, yes this should be true.

In the promise API, a "task" is usually considered to be asynchronous and it may take a long time to finish. Since a promise can be cancelled, the underlying task could be cancelled through canceling the returned promise (if this is implemented by the task). Cancellation is an important characteristic. Thus a long-lasting task should return a promise. When your task does not return a promise it should be sufficiently short so that cancellation is not required and that undesired effects through blocking a thread is negligible.

If your block does always block for a significant duration it would be better to return always a promise and check instead the kind of error returned through the promise:

If the error justifies a retry - say for a network task the error equals "host not reachable" - you would retry the task. Otherwise, the task either succeeds or it fails with an error where retrying makes no sense.

In order to have a universally applicable API, perhaps it makes sense to define foremost the API which defines which errors should cause a retry and which errors should abort the task - for example through a "Predicate" parameter, possibly a Block which takes a parameter error which is the error of the previous task.

So far, we would get this API:

typedef RXPromise* (^rxp_try_once_t)();
typedef BOOL (^rxp_retry_predicate_t)(NSError* error);
+ (instancetype) retryIf:(rxp_retry_predicate_t)predicate task:(rxp_try_once_t)tryOnce;

(as always, the naming of these methods are debatable ;) )

The predicate may have additional parameters - for example the current count of retries etc. Whether and when a certain task should be retried can be much better handled with a sort of "RetryPolicy" class. For example, when dealing with network requests you may want to incorporate the iOS "Reachability" utility which is implemented in the SystemConfiguration Framework. (See also: Reachability Sample).

It becomes obvious that the actual implementation of a retry policy depends heavily on the actual task that should be tried.

This API may look as follows:

+ (instancetype) retryWithPolicy:(RetryPolicy)policy task:(rxp_try_once_t)tryOnce;

where RetryPolicy still needs to be defined.

I'm in doubt whether it is possible to implement the retry method with a simple method or with some helper functions. A viable approach to implement retryIf:task: and retryWithPolicy:task: will likely require a subclass of NSOperation (or a similar class of your own) which encapsulates the retry mechanics and state variables. This task should (well, it MUST) be cancelable. This operation must also be thread-safe, since cancellation will access shared variables. retryIf:task: and retryWithPolicy:task: would then create this operation, set parameters and let it run until it succeeds, or until it "fails hard", or until it has been cancelled by the user.

[dmitrym0]

Closing, stale.