Rx friendly presenters - Githubissues

We have been using rx with GWTP and this is how we are using right now. In general, subscribing to an observable is pretty dangerous, and spread subscriptions over the code is not a good idea. Observables have a pretty big difference with traditional events (gwt ui events for example), if an error happens, the subscriptions die! so to avoid what you need to do is to create some kind of scope and register the observable so the "scope" manages the subscription.

This matches perfectly with the component model, in our case gwt widgets and your custom presenter are 2 kinds of components. And binding the lifecycle of this component to the subscription of the observables works perfectly. If you need to use it in a widget you can use RxGWT RxWidget and I have created here a base Presenter which binds the start/stop lifecycle with the subscription of the registered observables. Also, there is an observable called request() that just notifies when a request is pushed to the presenter.

This has weird results, usually, you end up doing everything in the constructor (but it is lazy, don't worry, the constructor is still lightweight, means run quickly, do almost nothing! even if it has a lot of code) and you usually don't need any field. You end up with a list of register call which describes the behavior of the presenter like a narrative: when something happens then do this thing.

This PR is not ready to be merged, and I need to update the test, but I just upload so you might see if you like it or not and if you see that this way it should be safer to develop.

Thanks for this PR, looks really interesting. I will take closer look on Monday.

I think that I understand the problem now. But my original pull request already went through several refactorings, also in the aspect of error handling and resubscriptions to Observables. Please hold on with your changes. I would like to use your proposal on top of a new PR.

@ibaca I tried to address all the problems with dying subscriptions. For this reason I unified the way uncaught errors are handled all over the code and now I retry forever for standard UI triggered subscriptions. I discovered that subscription can die not only when exception is thrown while observing, but also if it is thrown while executing subscription Consumer. Both cases are addressed now in my base Presenter:

https://github.com/xemantic/github-users/blob/master/src/main/java/com/xemantic/ankh/shared/presenter/Presenter.java

And here is the test:

https://github.com/xemantic/github-users/blob/master/src/test/java/com/xemantic/ankh/shared/presenter/PresenterTest.java

With this base Presenter support and small DSL there I can have quite concise and more human readable presenter code now, e.g.:

https://github.com/xemantic/github-users/blob/master/src/main/java/com/xemantic/githubusers/logic/user/UserQueryPresenter.java

I decided not to start subscriptions in constructor now, maybe I will come to this idea later.

Retrying requests with exponential backoff I would like to handle with in the service access layer later on, which means in this case UserService implementation. But in order to check if all the error handling works with resubscriptions I added several test cases, and supporting utilities like junit rule for verifying uncaught exceptions:

https://github.com/xemantic/github-users/blob/master/src/test/java/com/xemantic/ankh/test/ExpectedUncaughtExceptionTest.java

Here are test cases for unexpected exceptions:

https://github.com/xemantic/github-users/blob/master/src/test/java/com/xemantic/githubusers/logic/drawer/DrawerPresenterTest.java (start_unexpectedExceptionWhenOpeningProjectOnGitHubForThe1StTime_shouldResubscribe)

https://github.com/xemantic/github-users/blob/master/src/test/java/com/xemantic/githubusers/logic/user/UserListPresenterTest.java (all onError test cases)

And finally I also added own emitting Observable to stream pages on demand, but always try to deliver the first page immediately on subscription, feels a bit like a hack, but works and has a good test coverage.

https://github.com/xemantic/github-users/blob/master/src/main/java/com/xemantic/ankh/shared/request/Page.java

Thanks for your contribution. It gave me a lot of input for refactoring. I will close this PR now.

hehe, to be honest, I don't like too much 😜! RxJava already handles errors correctly, I don't understand why you need so much code to do something that already works perfectly. If your app works correctly no error handling should happen, if some unexpected error is thrown, then just retry, nothing more, most of the time this is just useful during development, although if you upload some bug to production those retries are pretty useful too. And this is the outer level, obviously, if you have some transport layer, you should probably handle error differently, but this is not what the presenter subscription is for. You should think more like the typical defensive try-catch in a thread, just in case some pretty bad thing happen, please do not kill the thread. But you do not try to solve your ddbb error in this defensive try-catch.

Observables are lazy and just describes a behavior. Actually, creating an Observable is almost like adding methods to a class! nothing happens, just describes behavior. This is why it ends up in the constructor you create them just one time as you only define method one time. It is not bad to create it later, as you might some time create some anonymous classes or lambdas inside some nested calls, but, if you can you should try to define it once.

Also, the on method is... not ok, I use register bc it actually means register. Again, it is like creating a method, and you register this method in the class. Also, the CallDefiner just hides the pretty good RxJava API in an obscure unique use case. IMO adds nothing and it just limits or enforces you to do something, not sure what.

Hehe, and please, subscribe inside a subscription (talking about Page) some reactivex programmer has just die! 👎 Why don't you like the scanWith strategy?

I guess I should provide some rationale behind decisions I made. Some of them must be coming out of my ignorance as I am still not as experienced with reactive paradigm as I wish to be. :( I would like to ask you some questions, and then maybe ask your opinions as well.

In the pull request you do something like:

snackbarMessage$.doOnNext(e -> view.show(e.getMessage())

Instead of:

snackbarMessage$.subscribe(e -> view.show(e.getMessage())

I understand that this way you are able to delay the actual subscription. But are they any side effects? For example it seems that the Consumer passed to doOnNext can be called on a different Scheduler than the one calling the Consumer passed to subscribe. This is exactly what I wanted to avoid as in the future I would like to differentiate rendering/worker threads (platform agnostic). The second question - should I strive for delaying the actual subscription, or is it just for convenience or reusability of presenters?

The idea behind "never ever subscribe" is that you should develop code that doesn't care about the subscription and you focus on describing how the system reacts. Ideally, you should end up with a unique pure function, hehe so with cyclejs, but for sure if you came from classic imperative UI this means rewrite and rethink everything (bad idea 😉). So, my current solution is the compoenet+reactions approach. The components, in our case, are widgets or presenters. And reactions are Observable.

This limitation has a pretty practical advantage, if you never ever subscribe, it is impossible you forget to unsubscribe anything. So the situation where someone forgets to call a removeLister which end up producing memory leaks or some weird widget which is not visible but surprising is still been updating cannot happen.

As I commented, this also makes trivial to be sure the subscription never dies. But you should think just a simple case, do not think of all possible cases, do not try to fix all error handling here (transport, validation, offline, etc!), just the programming bug case. You define your reaction "on button click then gets the input int add +1 and set to label", if for example you have some bug and input resturns null and you try to add +1 you probably don't want neither the subscription to die nor to show a big error to the user, actually, if you are lucky you can ever not show anything to the user, send the error to your server and whenever the user sets a valid value in the input it will not even notice that the reaction was failing during a time (althoug a small indication something is wrong is pretty useful in general). So, what I mean is that this final tryAgain operator is just for fatal error, 99% of the time bugs.

I like to imagine this component+reactions strategy as pipes you can add to your component. Each pipe is a reaction, ie. a behavior and the component is your widget. hyper-tx3-evo-04 So your component control when to turn on and off the flow throw your pipes. And hehe, the doOnNext, which make our life (imperative coders using mutable systems like widgets) much easier, are like pipes leaks. Those leaks mutate the external world when some events happen.

So, in general, subscribing is completely forbidden. This also enforces you to use Observable better, for example in the Pager you subscribe inside a subscription, this is an extremely bad idea and you ignore (poorly reinvent) the whole composition power of reactivex, you lose the error chain and the subscription chain, so you never ever should do that. You can always go to any reactivex forum and ask how to do whatever you are trying to do, combining observables is pretty different from imperative programming and at first is super confusing, it feels like start programming again and you don't even know how for-loop works 😛.

So, you ask about "delaying the subscription" and ideally you should not care about the "when", you only describe "what" should happen. But, yep, you are right, it can happen in any scheduler, in GWT doesn't care, but for sure in android/ios might be a problem. As I said, you probably care about the thread on those "leaks" hehe, the ones that are mutating the world and are not quite functionalish... so there is your solution. Whenever you want to mutate your view just wrap it in some function, thanks to the nice RxJava API you better use compose.

private ObservableTransformer<Board, Board> mutateUi(Consumer<? super Board> fn) {
    return o -> o.observeOn(eventLoop).doOnNext(fn);
}

And use it like .compose(mutateUi(n -> {/*safe to mutate UI bc you are in the UI event loop*/})).

You can get inspired by this https://github.com/ReactiveX/RxAndroid/blob/2.x/rxandroid/src/main/java/io/reactivex/android/MainThreadDisposable.java but hehe, I really think you should NOT do what this example does, you should try to detect if you are already in the event loop and handle doOnNext without any scheduler, and use the eventLoop scheduler only when you really need it. Uhm, I'm not sure what is the best solution but for now, I think the best is that you handle it manually, so you (as developer) should be more or less aware that you are not in the main loop, in this case, just add the .observeOn(mainLoop) as needed. So you can still use the mutateUi wrapper but use it only to throw an eager exception if it detects that the action is not being executed in the UI event loop.

xemantic / github-users

Rx friendly presenters #7