Retry more errors in Volley's BasicNetwork.

Always retry I/O errors while reading the HTTP response entity.
Furthermore, if a client opts in, retry HTTP 500 errors indicating
something went wrong on the server.

This resolves a longstanding TODO to only throw a ServerError on 5xx
errors by adding a ClientError for 4xx errors. For backwards
compatibility, this extends ServerError.

Note that Volley already may retry a request that reached the server
if the connection times out, which means that lack of idempotency
shouldn't be a concern here if it wasn't already. But if we wanted to
be even safer, we could require clients to opt into the additional
retry cases, at the cost of a somewhat more polluted API.

Add unit tests for most failure scenarios.

Bug: 23152983
Change-Id: I92cf35c66ccf98a1682adf41654afeb8634911db

src/main/java/com/android/volley/ClientError.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright (C) 2015 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.android.volley;
+
+/**
+ * Indicates that the server responded with an error response indicating that the client has erred.
+ *
+ * For backwards compatibility, extends ServerError which used to be thrown for all server errors,
+ * including 4xx error codes indicating a client error.
+ */
+@SuppressWarnings("serial")
+public class ClientError extends ServerError {
+    public ClientError(NetworkResponse networkResponse) {
+        super(networkResponse);
+    }
+
+    public ClientError() {
+        super();
+    }
+}
+

src/main/java/com/android/volley/Request.java

@@ -20,7 +20,6 @@
 import android.net.Uri;
 import android.os.Handler;
 import android.os.Looper;
-import android.os.SystemClock;
 import android.text.TextUtils;
 
 import com.android.volley.VolleyLog.MarkerLog;
@@ -90,6 +89,9 @@ public interface Method {
     /** Whether or not a response has been delivered for this request yet. */
     private boolean mResponseDelivered = false;
 
+    /** Whether the request should be retried in the event of an HTTP 5xx (server) error. */
+    private boolean mShouldRetryServerErrors = false;
+
     /** The retry policy for this request. */
     private RetryPolicy mRetryPolicy;
 
@@ -473,6 +475,23 @@ public final boolean shouldCache() {
         return mShouldCache;
     }
 
+    /**
+     * Sets whether or not the request should be retried in the event of an HTTP 5xx (server) error.
+     *
+     * @return This Request object to allow for chaining.
+     */
+    public final Request<?> setShouldRetryServerErrors(boolean shouldRetryServerErrors) {
+        mShouldRetryServerErrors = shouldRetryServerErrors;
+        return this;
+    }
+
+    /**
+     * Returns true if this request should be retried in the event of an HTTP 5xx (server) error.
+     */
+    public final boolean shouldRetryServerErrors() {
+        return mShouldRetryServerErrors;
+    }
+
     /**
      * Priority values.  Requests will be processed from higher priorities to
      * lower priorities, in FIFO order.

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

@@ -21,6 +21,7 @@
 import com.android.volley.AuthFailureError;
 import com.android.volley.Cache;
 import com.android.volley.Cache.Entry;
+import com.android.volley.ClientError;
 import com.android.volley.Network;
 import com.android.volley.NetworkError;
 import com.android.volley.NetworkResponse;
@@ -143,27 +144,37 @@ public NetworkResponse performRequest(Request<?> request) throws VolleyError {
             } catch (MalformedURLException e) {
                 throw new RuntimeException("Bad URL " + request.getUrl(), e);
             } catch (IOException e) {
-                int statusCode = 0;
-                NetworkResponse networkResponse = null;
+                int statusCode;
                 if (httpResponse != null) {
                     statusCode = httpResponse.getStatusLine().getStatusCode();
                 } else {
                     throw new NoConnectionError(e);
                 }
                 VolleyLog.e("Unexpected response code %d for %s", statusCode, request.getUrl());
+                NetworkResponse networkResponse;
                 if (responseContents != null) {
                     networkResponse = new NetworkResponse(statusCode, responseContents,
                             responseHeaders, false, SystemClock.elapsedRealtime() - requestStart);
                     if (statusCode == HttpStatus.SC_UNAUTHORIZED ||
                             statusCode == HttpStatus.SC_FORBIDDEN) {
                         attemptRetryOnException("auth",
                                 request, new AuthFailureError(networkResponse));
+                    } else if (statusCode >= 400 && statusCode <= 499) {
+                        // Don't retry other client errors.
+                        throw new ClientError(networkResponse);
+                    } else if (statusCode >= 500 && statusCode <= 599) {
+                        if (request.shouldRetryServerErrors()) {
+                            attemptRetryOnException("server",
+                                    request, new ServerError(networkResponse));
+                        } else {
+                            throw new ServerError(networkResponse);
+                        }
                     } else {
-                        // TODO: Only throw ServerError for 5xx status codes.
+                        // 3xx? No reason to retry.
                         throw new ServerError(networkResponse);
                     }
                 } else {
-                    throw new NetworkError(networkResponse);
+                    attemptRetryOnException("network", request, new NetworkError());
                 }
             }
         }

src/test/java/com/android/volley/mock/MockHttpStack.java

@@ -22,13 +22,16 @@
 
 import org.apache.http.HttpResponse;
 
+import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 
 public class MockHttpStack implements HttpStack {
 
     private HttpResponse mResponseToReturn;
 
+    private IOException mExceptionToThrow;
+
     private String mLastUrl;
 
     private Map<String, String> mLastHeaders;
@@ -51,9 +54,16 @@ public void setResponseToReturn(HttpResponse response) {
         mResponseToReturn = response;
     }
 
+    public void setExceptionToThrow(IOException exception) {
+        mExceptionToThrow = exception;
+    }
+
     @Override
     public HttpResponse performRequest(Request<?> request, Map<String, String> additionalHeaders)
-            throws AuthFailureError {
+            throws IOException, AuthFailureError {
+        if (mExceptionToThrow != null) {
+            throw mExceptionToThrow;
+        }
         mLastUrl = request.getUrl();
         mLastHeaders = new HashMap<String, String>();
         if (request.getHeaders() != null) {