Deliver errors in onError for Rx about reprint HOT 7 CLOSED

Thanks for the input. I originally tried using onError, but I found that I like the current design better. The reason is that authentication failures aren't exceptional, and most don't end the event stream. The information passed to onError ends up being the same as to onNext, just held inside an exception, and you need to display that information to the user either way.

I'm not opposed to switching back to using onError for fatal authentication failures, but can you give me an example of what you're trying to do that's not possible but the current design, but would be with onError?

from reprint.

When you're writing your operation, it would look like

getUI().getClicks()
         .flatmap(view -> Observable.zip(Observable.just(someUserInfo), RxReprint.authenticate(), zipToPair()))
         .flatmap(zipped -> getApi().callForAuth(zipped.first, zipped.second))
         .map(doDataMassaging())
         .toList()

Except that for error types it'll also go to the api because the data looks the same, your chain breaks unless you write code to separate from the error cases.

getUI().getClicks()
         .flatmap(view -> Observable.zip(Observable.just(someUserInfo), RxReprint.authenticate(), zipToPair()))
         .flatmap(zipped -> if(itsNotError()) { getApi().callForAuth(zipped.first, zipped.second) } else {  return Observable.empty()})
         .map(doDataMassaging())
         .toList()

    getUI().getClicks()
         .flatmap(view -> Observable.zip(Observable.just(someUserInfo), RxReprint.authenticate(), zipToPair()))
         .filter(noErrorCases())
         .flatmap(zipped -> getApi().callForAuth(zipped.first, zipped.second))
         .map(doDataMassaging())
         .toList()

The reason of having a data stream is to have a single correct track, and a single error track, and use the error operators deal with those cases.

from reprint.

Right. Correct me if I'm wrong, but it looks like you want onNext to be called only on authentication success, and onError to be called in all other cases. I think the issue with that is that authentication failures aren't errors. A failure might be because a fingerprint wasn't read completely, or the sensor is locked out temporarily. In every case, you need to update your ui. But in the first case, the sensor is still running, and more events can get emitted, so onError isn't appropriate since it would terminate the stream.

from reprint.

You have to let the user decide what to do when an error is received, including unsubscribing oneself by letting the chain die. You have to stop delivering onNext() after unsubscription, and cancel any ongoing operation if possible.

If the sensor can be kept open and could potentially deliver a correct value, retry() or retry with a policy will reconnect the observable after that first error. doOnError() to update UI exists precisely for the cases mentioned.

from reprint.

I do stop delivering onNext and cancel the operation after an unsubscribe. I can see that value in having onNext only fire on successful authentication. The problem with implementing that, though, is that there is no way to have the fingerprint sensor on without receiving events from it. I suppose we could cancel the authentication request every time we get a non-fatal error, but I like that currently the user doesn't ever have to restart the call themselves.

from reprint.

Fair enough, if that's the design choice I guess it helps declutter the user code in some way.

from reprint.

With commit 85d66e, I decided to leave the callback based api as it was, but I changed to reactivex contract to only call onNext for a successful authentication, and onError in all other cases.

@pakoito let me know if this still doesn't work for your use case.

from reprint.