Problematic timeout setting for volley request ?

[stream101]

Hi there,

I found that a network timeout setting is kind of problematic. The following is a code snippet from HttpUtils.java:

public static final int TIMEOUT_MILLIS = 20000; //20 seconds. Long but not too short for GPRS connections and not endless.
try {
            response = futureListener.get(TIMEOUT_MILLIS, TimeUnit.MILLISECONDS);
        } catch (final InterruptedException | ExecutionException e) {
            Log.error("cacheUriToFile(): Exception from request.", e);
            throw new FileCacheException("Exception from request.", e);
        } catch (final TimeoutException e) {
            Log.error("cacheUriToFile(): Timeout Exception from request.", e);
            throw new FileCacheException("Timeout Exception from request.", e);

        }

From the comments, I guess the purpose of TIMEOUT_MILLIS is to set a 20 sec timeout of this network request. But volley library uses setRetryPolicy(TIMEOUT_MS, MAX_RETRIES, BACKOFF_MULT) to set timeout and retry policy. RequestFuture() only blocks and waits 20 sec for the result to be ready, and therefore cannot control the timeout of network request. So I think the expected behavior here is to add request.setRetryPolicy() to set the timeout value ?

Best,

[murraycu]

So you think that when the RequestFuture times out then we could be orphaning the actual request that could actually continue even though we don't care about its result? That would indeed seem less than ideal.

But I wonder if it's wise to still provide a RequestFuture.get() timeout. Maybe there are other things (other than the network communication) that can cause it to take longer.

[stream101]

The default timeout of volley library is 2.5 second. And if it fails, it will automatically retry once at back-off multiplier 1. That means, the maximum timeout of a default volly network requests is 2.5s (first timeout) + 2.5s(back-off) + 2.5s (retry timeout) = 7.5s, which is far more less than 20 seconds.

I didn't mean to delete Request.future(), but in addition to that, we also need to set network request's timeout separately. For example:

request.setRetryPolicy(20000);
try {
        response = futureListener.get(30000, TimeUnit.MILLISECONDS);
}

So what do you think ? If you agree with this policy, I would be happy to submit a pull request of the change.

[murraycu]

I'm happy to use the default volley timeout time (and backoff time and retry counts). They are probably well chosen. If our RequestFuture waits longer when that fails, it's no great problem as long as we don't wait too long. It seems more important to give volley enough time to do what it usually does, while not waiting forever (or for an excessively long time). It would be nice if we didn't have to mention any magic number, of course.

So, thanks, but it doesn't look like changing this would be useful.

[stream101]

Well, I am afraid it is hard to say that the default value is well chosen. Actually 2.5 second is too short, especially when you load some file under weak connection. Originally I was not aware of this short timeout too, but later I got so many timeout exceptions in practice, so I increased it in my project. There is no one-size-for-all answer, but I suggest to be conservative on choosing the value to fit into yours :)

Thanks,

[murraycu]

A patch to increase the timeout slightly would be welcome even if I end up commenting it out until I see a real need for it to fix a real problem. There's a chance that this will improve things on bad connections, though bad connections just might be too bad to use at all. Thanks.

[murraycu]

Closing this for now until there's some sign that there is a real problem here for real users.

[AhmadullahSaikat]

I use default timeout. But 5 minutes left onErrorResponse not called. I used alertdialog & cancellable(false); So this is a very bad problem for me..

How can I set timeout 10 second? I don't want to start that process again.

[murraycu]

This is the android-galaxyzoo project, not the volley project.