Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: allow RetryWhen to count only consecutive errors #16

Open
RobLewis opened this issue Feb 14, 2019 · 2 comments
Open

Feature request: allow RetryWhen to count only consecutive errors #16

RobLewis opened this issue Feb 14, 2019 · 2 comments

Comments

@RobLewis
Copy link

The semantics would be "Retry when the count of consecutive errors has not exceeded a certain value." The error count would be reset upon a successful onNext call.

It could be done with a builder function like .consecutiveErrorCountLessThan( int n ). (If 2+ billion consecutive errors isn't enough, make the parameter a long :-)

I've implemented something like this as a composable ObservableTransformer. It also accepts an optional list of the types of Exception to be counted. Caveat: it's not well tested yet.

public class RetryLimiter<T> implements ObservableTransformer<T,T> {
    
    private final static String TAG = RetryLimiter.class.getSimpleName();
    
    private int errorCount = 0;
    private int maxErrorCount;
    
    private List<Class> countedExceptions;  // note errors not in this list (if defined) are not counted and will continue retries
    
    // constructor passes max tolerated consecutive errors (count is rezeroed upon a successful emission)
    public RetryLimiter( int maxErrors ) {
        if( maxErrors < 0 ) throw new IllegalArgumentException( "maxErrors cannot be < 0" );
        maxErrorCount = maxErrors;
    }
    
    // constructor that accepts a list of the types of exceptions to count (others are ignored)
    public RetryLimiter( int maxErrors, List<Class> exceptions ) {
        this( maxErrors );
        if( exceptions != null ) {
            for( Class c : exceptions ) {  // check that the supplied list contains only Throwable (sub)classes
                if( !Throwable.class.isAssignableFrom( c ) ) {  // is c a Throwable or subclass of Throwable?
                    throw new IllegalArgumentException( "List can only contain class Throwable or its subclasses" );
                }
            }
            countedExceptions = exceptions;
        } else {
            throw new NullPointerException( "If supplied, Exception Class list cannot be null" );
        }
    }
    
    @Override
    public ObservableSource<T> apply( Observable<T> upstream ) {
        return upstream
                .doOnError( err -> {
                    if( countedExceptions != null ) {
                        for( Class t : countedExceptions ) {
                            if( err.getClass().equals( t ) ) {
                                errorCount++;
                                break;        // don't count more than once
                            }
                        }
                    } else {  // null list of counted Exceptions
                        errorCount++;
                    }
                    if( DEBUG ) Log.d( TAG, "Consecutive error #" + errorCount + ": " + err.toString() );
                } )  
                .doOnNext( next -> errorCount = 0 )
                .retryUntil( () -> errorCount > maxErrorCount );
    }
}

Also, what about including an option for a general Predicate function to decide whether or not to retry? I realize this is essentially the same as the standard .retryUntil( Predicate ) but it could be a useful addition: .isTrue( Predicate p ). Not to gild the lily, but it could perhaps be two functions: .and( Predicate p ) and .or( Predicate p ). Arguments to the Predicate are TBD but would presumably include the Throwable.

@davidmoten
Copy link
Owner

Ok, can you give me a use case (just wondering if there's another way to do it)?

I use retryWhen for IO calls especially and usually do so with a capped exponential backoff:

.retryWhen(
  RetryWhen.delays(
    Flowable.just(1, 2, 4, 8, 16, 30).compose(Transformers.repeatLast()),
  TimeUnit.SECONDS)

Another trick is to add .timeout(30, TimeUnit.SECONDS) to the source stream to detect no emissions.

@davidmoten
Copy link
Owner

In your email you mentioned that your use case involves network IO. I suggest that a time-based retry is more appropriate. If you want to fail after 7 attempts with exponential backoff:

.retryWhen(
  RetryWhen.delays(
    Flowable.just(1, 2, 4, 8, 16, 30),
  TimeUnit.SECONDS)

Is there any reason why time-based retries are not the go?

Note also that I would expect the retryWhen operator to be attached to each of the IO calls (the Singles you mentioned) rather than further downstream after the flatmap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants