Add a more intricate retryWhen example, with both exception type check and retry count

[jbripley]

No description provided.

[samuelgruetter]

Thanks for providing this example. That's imho a very good way of testing that the signatures of the methods make sense, and I have a suspicion that this might not be the case here. Currently, the signature looks as follows:

def retryWhen(handler: Observable[Notification[Any]] => Observable[Any]): Observable[T] 

And I wonder why it's not like this:

def retryWhen(handler: Observable[Throwable] => Observable[Any]): Observable[T] 

since the handler only has to react if an error (Throwable) occurs, and all it has to indicate is whether to abort (by calling onComplete or not onError) or to retry (by emitting Any item whose value is irrelevant).

But maybe we can also retry if the source observable completes or emits a certain item. In that case, it would make sense to have the first signature, so that the handler can also react on OnNext/OnComplete. In that case, it would be nice to have an example for this as well.

The goal of the examples in RxScalaDemo is to clarify for readers how the operators work. What happened to me here, however, that after reading it, I have more questions than before ;-) So I think we should strive for improvement here.

[jbripley]

Yeah, it took me a while to figure out how to extract the actual Throwable instance from the Notification.

Though I'm glad the functionality exist, since I removed a lot of intricate boilerplate code in favour of this, it could definitely be simpler to use.

As far as I understand, the current RxJava implementation only trigger retryWhen foronError Notifications, so your simplified signature change with Observable[Throwable] makes sense in that regard.

[samuelgruetter]

Ok, so let's ask @headinthebox and @benjchristensen why the signature of retryWhen does not look like this:

def retryWhen(handler: Observable[Throwable] => Observable[Any]): Observable[T] 

[abersnaze]

It was for consistency with repeatWhen. Because there is no value for onCompleted we used Observable[Notification[Any]]. Both repeatWhen and retryWhen use the same underlying implementation.

I guess retryWhen could be Observable[Throwable] and repeatWhen could be Observable[Void].

[jbripley]

I really like the two proposed signature changes. Maybe change Observable[Void] to Observable[Unit] in RxScala @samuelgruetter?

Is it to late to change this for 1.0 @benjchristensen? Maybe deprecate the current signatures in 0.20.x, remove them from 1.x, and then add the new signatures to both 0.20.x and 1.x?

For reference, this how the submitted code would look like with the proposed signature change:

@Test def retryWhenDifferentExceptionsExample(): Unit = {
  var observableCreateCount = 1 // Just to support switching which Exception is produced
  Observable[String]({ subscriber =>
    println("subscribing")
    if (observableCreateCount <= 2) {
      subscriber.onError(new IOException("IO Fail"))
    } else {
      subscriber.onError(new RuntimeException("Other failure"))
    }
    observableCreateCount += 1
  }).retryWhen({ throwableObservable =>
    throwableObservable.zip(Observable.from(1 to 3)).flatMap({ case (error, retryCount) =>
      error match {
        // Only retry 2 times if we get a IOException and then error out with the third IOException.
        // Let the other Exception's pass through and complete the Observable.
        case _: IOException =>
          if (retryCount <= 3) {
            println("IOException delay retry by " + retryCount + " second(s)")
            Observable.timer(Duration(retryCount, TimeUnit.SECONDS))
          } else {
            Observable.error(error)
          }

        case _ =>
          println("got error " + error + ", will stop retrying")
          Observable.empty
      }
    })
  }).toBlocking.foreach(s => println(s))
}

[samuelgruetter]

Yes, in Scala, I would prefer retryWhen with Observable[Throwable] and repeatWhen with Observable[Unit].
I think we could also just change it in Scala and leave it in Java, if you don't want to make the change in Java.

[jbripley]

I would prefer if the API for RxScala was inline with RxJava as much as possible, barring difference in language idioms. So updating both would be better IMO.

I could take a swing at doing PR's for both RxScala and RxJava during the weekend, if we feel it's not too late to change the RxJava signatures.

[benjchristensen]

The RxJava changes have been made. I'll release a new release candidate soon for you to depend on for RxScala.