From dd439dcb42093fde4452e320f3a9c4bfa2066def Mon Sep 17 00:00:00 2001 From: Jeff Davidson Date: Thu, 15 Oct 2015 15:26:10 -0700 Subject: 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 | 35 ++++++++++++++++++++++ src/main/java/com/android/volley/Request.java | 21 ++++++++++++- .../com/android/volley/toolbox/BasicNetwork.java | 19 +++++++++--- 3 files changed, 70 insertions(+), 5 deletions(-) create mode 100644 src/main/java/com/android/volley/ClientError.java (limited to 'src/main') diff --git a/src/main/java/com/android/volley/ClientError.java b/src/main/java/com/android/volley/ClientError.java new file mode 100644 index 0000000..a8c8141 --- /dev/null +++ b/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(); + } +} + diff --git a/src/main/java/com/android/volley/Request.java b/src/main/java/com/android/volley/Request.java index 5b42d1f..8200f6e 100644 --- a/src/main/java/com/android/volley/Request.java +++ b/src/main/java/com/android/volley/Request.java @@ -20,7 +20,6 @@ import android.net.TrafficStats; 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 abstract class Request implements Comparable> { /** 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 abstract class Request implements Comparable> { 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. diff --git a/src/main/java/com/android/volley/toolbox/BasicNetwork.java b/src/main/java/com/android/volley/toolbox/BasicNetwork.java index 4b1603b..37c35ec 100644 --- a/src/main/java/com/android/volley/toolbox/BasicNetwork.java +++ b/src/main/java/com/android/volley/toolbox/BasicNetwork.java @@ -21,6 +21,7 @@ import android.os.SystemClock; 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,14 +144,14 @@ public class BasicNetwork implements Network { } 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); @@ -158,12 +159,22 @@ public class BasicNetwork implements Network { 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()); } } } -- cgit v1.2.3