boncey / Flickr4Java

Java API For Flickr. Fork of FlickrJ
BSD 2-Clause "Simplified" License
176 stars 155 forks source link

timeout logic #65

Open dermoritz opened 10 years ago

dermoritz commented 10 years ago

i observed that there seem no timeouts in flickr4java: it could happen that flickr api operation run forever. In my case this always happens on isp-24h reconnect. would it be possible to implement configurable timeout? The timeout itself could also be easily implemented in my program but the problem is i can not simply retry the operation because i don't know the status of the connection/authorization. On a reconnect authorization process must run. If an operation timed out due to slow connection a retry would suffice. So an combination of timeout/check status/retry logic would be perfect.

boncey commented 10 years ago

Hiya, this all sounds pretty sensible.

I don't really have any free time to work on this right now however. Patches are welcome of course if you feel like having a crack at it.

dermoritz commented 10 years ago

In meantime i implemented it myself. I made a class Retrier. The method it provides could easily be made static (the only non static thing is the logger). Here it is:

package de.ml.flickrUploader.config;

import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;

import org.slf4j.Logger;

import com.google.common.util.concurrent.SimpleTimeLimiter;
import com.google.common.util.concurrent.TimeLimiter;
import com.google.common.util.concurrent.UncheckedTimeoutException;

import de.ml.flickrUploader.config.Retrier.Retry;

/**
 * Implements retry logic by publishing an interface.
 *
 */
public class Retrier {

    /**
     * Logger.
     */
    @Log
    private Logger log;

    /**
     * Used for retry logic.
     */
    private static final TimeLimiter TIME_LIMITER = new SimpleTimeLimiter();

    /**
     * Delegate interface. Implementing classes can call
     * {@link Retrier#retryOnException(Class, int, Retry)}
     *
     * @author Moritz Löser (moritz.loeser@prodyna.com)
     *
     */
    public interface Retry {
        /**
         * This method will be called.
         *
         * @throws Exception
         *             all kinds of exceptions are allowed.
         */
        void tryThis() throws Exception;
    }

    public final int retryOnExceptionWithTimeOut(final int secondsTimeout, final int retryCountOnException,
            final Retry r, final List<Class<? extends Exception>> exceptions) throws Exception {
        int tryCount = 0;
        // you can only escape by successfully call the method, throw an
        // unexpected exception or exceed retries
        while (true) {
            try {
                TIME_LIMITER.callWithTimeout(new Callable<Void>() {

                    @Override
                    public Void call() throws Exception {
                        r.tryThis();
                        return null;
                    }
                }, secondsTimeout, TimeUnit.SECONDS, false);
                break;
            } catch (Exception e) {
                boolean handled = false;
                //try to match current exception with list elements
                for (Class<? extends Exception> exceptionClass : exceptions) {
                    log.debug("checking exception " + exceptionClass.getName());
                    if (exceptionClass.isInstance(e)) {
                        log.debug("Expected exception detected - retry. Exception: " + e.toString());
                        if (++tryCount > retryCountOnException) {
                            throw new Exception("Giving up after " + retryCountOnException
                                    + " retries on this exception: ", e);
                        }
                        handled = true;
                        break;
                        //now handle time out
                    } else if (UncheckedTimeoutException.class.isInstance(e)) {
                        log.debug("Timeout - retry.");
                        if (++tryCount > retryCountOnException) {
                            throw new TimeoutException("Giving up after " + retryCountOnException
                                    + " retries operation timed out.");
                        }
                        handled = true;
                        break;
                    }
                }
                //exception not handled yet - not in list or no time out -> rethrow
                if (!handled) {
                    log.debug("Exception not eligible for retry,  class: " + e.getClass().getName()+ " message: " + e.getMessage());
                    throw e;
                }
            }
        }
        return tryCount;
    }
}

It uses Google Guava's SimpleTimeLimiter. You give the exceptions eligible for retry as list of exception classes. In my app i retry on "IllegalArgumentException"(see the other issue i opened), on "OAuthConnectionException" and on "FlickrException". This class should be easily reused in all kinds of code. The only thing left is to find the right place in flickr4java to use it. On one hand it is good to centralize the use ("TimeOutAndExceptionHandler") but on the other hand it is important to react on different kind of exceptions differently. I guess a retry on some flickr exceptions makes sense but on other it is waste of time. This is not a patch but probably you can use it on some time.