Invoke RetryPolicy#retry in the blocking executor. (#393)

When using the synchronous RequestQueue, retry will be invoked in the
NetworkDispatcher thread. Clients may be relying on the ability to
perform blocking operations in retry(); a common example would be
clearing auth tokens in the event of an auth error, which may
require disk I/O.

In order to keep the same API contract when using AsyncRequestQueue,
rework NetworkUtility to permit BasicAsyncNetwork to invoke retry in
the blocking executor.

src/main/java/com/android/volley/toolbox/BasicAsyncNetwork.java

@@ -29,6 +29,7 @@
 import com.android.volley.Request;
 import com.android.volley.RequestTask;
 import com.android.volley.VolleyError;
+import com.android.volley.toolbox.NetworkUtility.RetryInfo;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.HttpURLConnection;
@@ -126,13 +127,39 @@ private void onRequestFailed(
             @Nullable HttpResponse httpResponse,
             @Nullable byte[] responseContents) {
         try {
-            NetworkUtility.handleException(
-                    request, exception, requestStartMs, httpResponse, responseContents);
+            RetryInfo retryInfo =
+                    NetworkUtility.shouldRetryException(
+                            request, exception, requestStartMs, httpResponse, responseContents);
+            // RetryPolicy#retry may need a background thread, so invoke in the blocking executor.
+            getBlockingExecutor()
+                    .execute(new InvokeRetryPolicyTask<>(request, retryInfo, callback));
         } catch (VolleyError volleyError) {
             callback.onError(volleyError);
-            return;
         }
-        performRequest(request, callback);
+    }
+
+    private class InvokeRetryPolicyTask<T> extends RequestTask<T> {
+        final Request<T> request;
+        final RetryInfo retryInfo;
+        final OnRequestComplete callback;
+
+        InvokeRetryPolicyTask(Request<T> request, RetryInfo retryInfo, OnRequestComplete callback) {
+            super(request);
+            this.request = request;
+            this.retryInfo = retryInfo;
+            this.callback = callback;
+        }
+
+        @Override
+        public void run() {
+            try {
+                NetworkUtility.attemptRetryOnException(request, retryInfo);
+                // attemptRetryOnException didn't throw, so proceed with the next attempt.
+                performRequest(request, callback);
+            } catch (VolleyError e) {
+                callback.onError(e);
+            }
+        }
     }
 
     @Override

src/main/java/com/android/volley/toolbox/BasicNetwork.java

@@ -22,6 +22,7 @@
 import com.android.volley.NetworkResponse;
 import com.android.volley.Request;
 import com.android.volley.VolleyError;
+import com.android.volley.toolbox.NetworkUtility.RetryInfo;
 import java.io.IOException;
 import java.io.InputStream;
 import java.net.HttpURLConnection;
@@ -140,8 +141,11 @@ public NetworkResponse performRequest(Request<?> request) throws VolleyError {
             } catch (IOException e) {
                 // This will either throw an exception, breaking us from the loop, or will loop
                 // again and retry the request.
-                NetworkUtility.handleException(
-                        request, e, requestStart, httpResponse, responseContents);
+                RetryInfo retryInfo =
+                        NetworkUtility.shouldRetryException(
+                                request, e, requestStart, httpResponse, responseContents);
+                // We should already be on a background thread, so we can invoke the retry inline.
+                NetworkUtility.attemptRetryOnException(request, retryInfo);
             }
         }
     }

src/main/java/com/android/volley/toolbox/NetworkUtility.java

@@ -113,38 +113,53 @@ static byte[] inputStreamToBytes(InputStream in, int contentLength, ByteArrayPoo
 
     /**
      * Attempts to prepare the request for a retry. If there are no more attempts remaining in the
-     * request's retry policy, a timeout exception is thrown.
+     * request's retry policy, the provided exception is thrown.
+     *
+     * <p>Must be invoked from a background thread, as client implementations of RetryPolicy#retry
+     * may make blocking calls.
      *
      * @param request The request to use.
      */
-    private static void attemptRetryOnException(
-            final String logPrefix, final Request<?> request, final VolleyError exception)
+    static void attemptRetryOnException(final Request<?> request, final RetryInfo retryInfo)
             throws VolleyError {
         final RetryPolicy retryPolicy = request.getRetryPolicy();
         final int oldTimeout = request.getTimeoutMs();
         try {
-            retryPolicy.retry(exception);
+            retryPolicy.retry(retryInfo.errorToRetry);
         } catch (VolleyError e) {
             request.addMarker(
-                    String.format("%s-timeout-giveup [timeout=%s]", logPrefix, oldTimeout));
+                    String.format(
+                            "%s-timeout-giveup [timeout=%s]", retryInfo.logPrefix, oldTimeout));
             throw e;
         }
-        request.addMarker(String.format("%s-retry [timeout=%s]", logPrefix, oldTimeout));
+        request.addMarker(String.format("%s-retry [timeout=%s]", retryInfo.logPrefix, oldTimeout));
+    }
+
+    static class RetryInfo {
+        private final String logPrefix;
+        private final VolleyError errorToRetry;
+
+        private RetryInfo(String logPrefix, VolleyError errorToRetry) {
+            this.logPrefix = logPrefix;
+            this.errorToRetry = errorToRetry;
+        }
     }
 
     /**
      * Based on the exception thrown, decides whether to attempt to retry, or to throw the error.
-     * Also handles logging.
+     *
+     * <p>If this method returns without throwing, {@link #attemptRetryOnException} should be called
+     * with the provided {@link RetryInfo} to consult the client's retry policy.
      */
-    static void handleException(
+    static RetryInfo shouldRetryException(
             Request<?> request,
             IOException exception,
             long requestStartMs,
             @Nullable HttpResponse httpResponse,
             @Nullable byte[] responseContents)
             throws VolleyError {
         if (exception instanceof SocketTimeoutException) {
-            attemptRetryOnException("socket", request, new TimeoutError());
+            return new RetryInfo("socket", new TimeoutError());
         } else if (exception instanceof MalformedURLException) {
             throw new RuntimeException("Bad URL " + request.getUrl(), exception);
         } else {
@@ -153,11 +168,9 @@ static void handleException(
                 statusCode = httpResponse.getStatusCode();
             } else {
                 if (request.shouldRetryConnectionErrors()) {
-                    attemptRetryOnException("connection", request, new NoConnectionError());
-                    return;
-                } else {
-                    throw new NoConnectionError(exception);
+                    return new RetryInfo("connection", new NoConnectionError());
                 }
+                throw new NoConnectionError(exception);
             }
             VolleyLog.e("Unexpected response code %d for %s", statusCode, request.getUrl());
             NetworkResponse networkResponse;
@@ -173,24 +186,21 @@ static void handleException(
                                 responseHeaders);
                 if (statusCode == HttpURLConnection.HTTP_UNAUTHORIZED
                         || statusCode == HttpURLConnection.HTTP_FORBIDDEN) {
-                    attemptRetryOnException("auth", request, new AuthFailureError(networkResponse));
-                } else if (statusCode >= 400 && statusCode <= 499) {
+                    return new RetryInfo("auth", new AuthFailureError(networkResponse));
+                }
+                if (statusCode >= 400 && statusCode <= 499) {
                     // Don't retry other client errors.
                     throw new ClientError(networkResponse);
-                } else if (statusCode >= 500 && statusCode <= 599) {
+                }
+                if (statusCode >= 500 && statusCode <= 599) {
                     if (request.shouldRetryServerErrors()) {
-                        attemptRetryOnException(
-                                "server", request, new ServerError(networkResponse));
-                    } else {
-                        throw new ServerError(networkResponse);
+                        return new RetryInfo("server", new ServerError(networkResponse));
                     }
-                } else {
-                    // 3xx? No reason to retry.
-                    throw new ServerError(networkResponse);
                 }
-            } else {
-                attemptRetryOnException("network", request, new NetworkError());
+                // Server error and client has opted out of retries, or 3xx. No reason to retry.
+                throw new ServerError(networkResponse);
             }
+            return new RetryInfo("network", new NetworkError());
         }
     }
 }