0

My application must do two things in general:

  • Accept only one network request at the same time
  • Retry if request failed

That's how I implement it:

public class RequestsLocker {

    private volatile boolean isLocked;

    public <T> Observable.Transformer<T, T> applyLocker() {
        if(!isLocked()) {
            return observable -> observable
                    .doOnSubscribe(() -> {
                        lockChannel();
                    })
                    .doOnUnsubscribe(() -> {
                        freeChannel();
                    });
        } else {
            return observable -> Observable.error(new ChannelBusyException("Channel is busy now."));
        }
    }

    private void lockChannel() {
        isLocked = true;
    }

    private void freeChannel() {
        isLocked = false;
    }

    public boolean isLocked() {
        return isLocked;
    }

}

Looks nice.

Now my retryWhen implementation:

public static Observable<?> retryWhenAnyIoExceptionWithDelay(Observable<? extends Throwable> observable) {
    return observable.flatMap(error -> {
        // For IOExceptions, we  retry
        if (error instanceof IOException) {
            return Observable.timer(2, TimeUnit.SECONDS);
        }

        // For anything else, don't retry
        return Observable.error(error);
    });
}

There is how I use it:

public Observable<List<QueueCarItem>> finishService(int id, PaymentType paymentType, String notes) {
    return carsQueueApi.finishService(id, new FinishCarServiceRequest(paymentType.getName(), notes))
            .compose(requestsLocker.applyLocker(RequestsLocker.RequestChannel.CHANGE));
}

...

public void finishCarService(QueueCarItem carItem, PaymentType paymentType,
                             String notes, Subscriber<List<QueueCarItem>> subscriber) {
    queueApiMediator.finishService(carItem.getId(), paymentType, notes)
            .subscribeOn(ioScheduler)
            .observeOn(uiScheduler)
            .doOnError(this::handleError)
            .retryWhen(RxOperatorsHelpers::retryWhenAnyIoExceptionWithDelay)
            .subscribe(subscriber);
}

The main problem that doOnUnsubscribe() called on any error and then locker is open for any new request until the timer expires and resubscribing happens again. That's the problem. While the timer is ticking user can make another request.

How I can fix it?

Alexandr
  • 3,859
  • 5
  • 32
  • 56
  • 1
    Could you post some code showing how you actually use `applyLocker` transformer and the `retryWhenAnyIoExceptionWithDelay`? – JohnWowUs Jul 11 '16 at 07:21

3 Answers3

2

The problem is that you're applying your transformer to the source observable i.e. before your retrywhen. When there is an error you're always going to unsubscribe from and then resubscribe to the source observable leading to your doOnUnsubscribe being called.

I suggest you try

public Observable<List<QueueCarItem>> finishService(int id, PaymentType paymentType, String notes) {
    return carsQueueApi.finishService(id, new FinishCarServiceRequest(paymentType.getName(), notes));            
}


public void finishCarService(QueueCarItem carItem, PaymentType paymentType,
                             String notes, Subscriber<List<QueueCarItem>> subscriber) {
    queueApiMediator.finishService(carItem.getId(), paymentType, notes)
            .subscribeOn(ioScheduler)
            .observeOn(uiScheduler)
            .doOnError(this::handleError)
            .retryWhen(RxOperatorsHelpers::retryWhenAnyIoExceptionWithDelay)
            .compose(requestsLocker.applyLocker(RequestsLocker.RequestChannel.CHANGE));
            .subscribe(subscriber);
}

PS: The apply locker transformer looks a bit different i.e. it doesn't take an argument in the code you linked.

JohnWowUs
  • 3,053
  • 1
  • 13
  • 20
  • Yeah, I understand. That's the problem: I decided to try CleanAcritecture by android10. So, `RequetsLocker` and `finishService` method is on the `data layer`. When `finishCarService` method on the domain. Now I thinking about moving retryWhen operator to the `data layer`. But it breaks philosophy of CA, as `data layer` must decide "WHAT to do", not "HOW". Another way — let `RequestLocker` to `domain layer`. In this case, I must notify `domain layer` about the source of data as `RequestLocker` must be applied only to network request, not to `getFromCache`. That's also a bad practice. Meh.. – Alexandr Jul 11 '16 at 15:26
1

Using retryWhen, to avoid unsubscribe onError you must use onErrorResumeNext which wont unsubscribe you.

Take a look of this example

/**
 * Here we can see how onErrorResumeNext works and emit an item in case that an error occur in the pipeline and an exception is propagated
 */
@Test
public void observableOnErrorResumeNext() {
    Subscription subscription = Observable.just(null)
                                          .map(Object::toString)
                                          .doOnError(failure -> System.out.println("Error:" + failure.getCause()))
                                          .retryWhen(errors -> errors.doOnNext(o -> count++)
                                                                     .flatMap(t -> count > 3 ? Observable.error(t) : Observable.just(null)),
                                                     Schedulers.newThread())
                                          .onErrorResumeNext(t -> {
                                              System.out.println("Error after all retries:" + t.getCause());
                                              return Observable.just("I save the world for extinction!");
                                          })
                                          .subscribe(s -> System.out.println(s));
    new TestSubscriber((Observer) subscription).awaitTerminalEvent(500, TimeUnit.MILLISECONDS);
}

Also about concurrency, if you do an operation in a flatMap operator, you can specify the Max concurrent.

public final <R> Observable<R> flatMap(Func1<? super T, ? extends Observable<? extends R>> func, int maxConcurrent) {
    if (getClass() == ScalarSynchronousObservable.class) {
        return ((ScalarSynchronousObservable<T>)this).scalarFlatMap(func);
    }
    return merge(map(func), maxConcurrent);
}

You can see more examples here https://github.com/politrons/reactive

paul
  • 12,873
  • 23
  • 91
  • 153
0

My current solution is not to unlock RequestLocker on IoException as in this case request will be repeated after delay.

public <T> Observable.Transformer<T, T> applyLocker() {
    if(!isLocked()) {
        return observable -> observable.doOnSubscribe(() -> {
            lockChannel();
        }).doOnNext(obj -> {
            freeChannel();
        }).doOnError(throwable -> {
            if(throwable instanceof IOException) {
                return; // as any request will be repeated in case of IOException
            }
            freeChannel(channel);
        });
    } else {
        return observable -> Observable.error(new ChannelBusyException("Channel is busy now"));
    }
}
Alexandr
  • 3,859
  • 5
  • 32
  • 56