diff --git a/gcloud-java-core/src/main/java/com/google/cloud/BaseServiceException.java b/gcloud-java-core/src/main/java/com/google/cloud/BaseServiceException.java index 6dc87f4abb3e..ad2d6bf07144 100644 --- a/gcloud-java-core/src/main/java/com/google/cloud/BaseServiceException.java +++ b/gcloud-java-core/src/main/java/com/google/cloud/BaseServiceException.java @@ -48,10 +48,16 @@ protected static final class Error implements Serializable { private final Integer code; private final String reason; + private final boolean rejected; public Error(Integer code, String reason) { + this(code, reason, false); + } + + public Error(Integer code, String reason, boolean rejected) { this.code = code; this.reason = reason; + this.rejected = rejected; } /** @@ -61,6 +67,15 @@ public Integer code() { return code; } + /** + * Returns true if the error indicates that the API call was certainly not accepted by the + * server. For instance, if the server returns a rate limit exceeded error, it certainly did not + * process the request and this method will return {@code true}. + */ + public boolean rejected() { + return rejected; + } + /** * Returns the reason that caused the exception. */ @@ -68,11 +83,11 @@ public String reason() { return reason; } - boolean isRetryable(Set retryableErrors) { + boolean isRetryable(boolean idempotent, Set retryableErrors) { for (Error retryableError : retryableErrors) { if ((retryableError.code() == null || retryableError.code().equals(this.code())) && (retryableError.reason() == null || retryableError.reason().equals(this.reason()))) { - return true; + return idempotent || retryableError.rejected(); } } return false; @@ -95,12 +110,14 @@ public BaseServiceException(IOException exception, boolean idempotent) { String reason = null; String location = null; String debugInfo = null; + Boolean retryable = null; if (exception instanceof GoogleJsonResponseException) { GoogleJsonError jsonError = ((GoogleJsonResponseException) exception).getDetails(); if (jsonError != null) { - Error error = error(jsonError); + Error error = new Error(jsonError.getCode(), reason(jsonError)); code = error.code; reason = error.reason; + retryable = isRetryable(idempotent, error); if (reason != null) { GoogleJsonError.ErrorInfo errorInfo = jsonError.getErrors().get(0); location = errorInfo.getLocation(); @@ -110,8 +127,8 @@ public BaseServiceException(IOException exception, boolean idempotent) { code = ((GoogleJsonResponseException) exception).getStatusCode(); } } + this.retryable = MoreObjects.firstNonNull(retryable, isRetryable(idempotent, exception)); this.code = code; - this.retryable = idempotent && isRetryable(exception); this.reason = reason; this.idempotent = idempotent; this.location = location; @@ -119,13 +136,7 @@ public BaseServiceException(IOException exception, boolean idempotent) { } public BaseServiceException(GoogleJsonError error, boolean idempotent) { - super(error.getMessage()); - this.code = error.getCode(); - this.reason = reason(error); - this.idempotent = idempotent; - this.retryable = idempotent && isRetryable(error); - this.location = null; - this.debugInfo = null; + this(error.getCode(), error.getMessage(), reason(error), idempotent); } public BaseServiceException(int code, String message, String reason, boolean idempotent) { @@ -138,7 +149,7 @@ public BaseServiceException(int code, String message, String reason, boolean ide this.code = code; this.reason = reason; this.idempotent = idempotent; - this.retryable = idempotent && new Error(code, reason).isRetryable(retryableErrors()); + this.retryable = isRetryable(idempotent, new Error(code, reason)); this.location = null; this.debugInfo = null; } @@ -147,15 +158,12 @@ protected Set retryableErrors() { return Collections.emptySet(); } - protected boolean isRetryable(GoogleJsonError error) { - return error != null && error(error).isRetryable(retryableErrors()); + protected boolean isRetryable(boolean idempotent, Error error) { + return error.isRetryable(idempotent, retryableErrors()); } - protected boolean isRetryable(IOException exception) { - if (exception instanceof GoogleJsonResponseException) { - return isRetryable(((GoogleJsonResponseException) exception).getDetails()); - } - return exception instanceof SocketTimeoutException; + protected boolean isRetryable(boolean idempotent, IOException exception) { + return idempotent && exception instanceof SocketTimeoutException; } /** @@ -187,8 +195,8 @@ public boolean idempotent() { } /** - * Returns the service location where the error causing the exception occurred. Returns - * {@code null} if not set. + * Returns the service location where the error causing the exception occurred. Returns {@code + * null} if not available. */ public String location() { return location; @@ -223,18 +231,14 @@ public int hashCode() { debugInfo); } - protected static String reason(GoogleJsonError error) { - if (error.getErrors() != null && !error.getErrors().isEmpty()) { + private static String reason(GoogleJsonError error) { + if (error.getErrors() != null && !error.getErrors().isEmpty()) { return error.getErrors().get(0).getReason(); } return null; } - protected static Error error(GoogleJsonError error) { - return new Error(error.getCode(), reason(error)); - } - - protected static String message(IOException exception) { + private static String message(IOException exception) { if (exception instanceof GoogleJsonResponseException) { GoogleJsonError details = ((GoogleJsonResponseException) exception).getDetails(); if (details != null) { diff --git a/gcloud-java-dns/src/main/java/com/google/cloud/dns/DnsException.java b/gcloud-java-dns/src/main/java/com/google/cloud/dns/DnsException.java index 1e1e34b833a5..f725984b6661 100644 --- a/gcloud-java-dns/src/main/java/com/google/cloud/dns/DnsException.java +++ b/gcloud-java-dns/src/main/java/com/google/cloud/dns/DnsException.java @@ -31,16 +31,16 @@ public class DnsException extends BaseServiceException { // see: https://cloud.google.com/dns/troubleshooting private static final Set RETRYABLE_ERRORS = ImmutableSet.of( - new Error(429, null), - new Error(500, null), - new Error(502, null), - new Error(503, null), - new Error(null, "userRateLimitExceeded"), - new Error(null, "rateLimitExceeded")); + new Error(429, null, true), + new Error(500, null, false), + new Error(502, null, false), + new Error(503, null, false), + new Error(null, "userRateLimitExceeded", true), + new Error(null, "rateLimitExceeded", true)); private static final long serialVersionUID = 490302380416260252L; - public DnsException(IOException exception) { - super(exception, true); + public DnsException(IOException exception, boolean idempotent) { + super(exception, idempotent); } private DnsException(int code, String message) { diff --git a/gcloud-java-dns/src/main/java/com/google/cloud/dns/spi/DefaultDnsRpc.java b/gcloud-java-dns/src/main/java/com/google/cloud/dns/spi/DefaultDnsRpc.java index 05b803513acb..08f14a0ba254 100644 --- a/gcloud-java-dns/src/main/java/com/google/cloud/dns/spi/DefaultDnsRpc.java +++ b/gcloud-java-dns/src/main/java/com/google/cloud/dns/spi/DefaultDnsRpc.java @@ -36,8 +36,8 @@ public class DefaultDnsRpc implements DnsRpc { private final Dns dns; private final DnsOptions options; - private static DnsException translate(IOException exception) { - return new DnsException(exception); + private static DnsException translate(IOException exception, boolean idempotent) { + return new DnsException(exception, idempotent); } /** @@ -61,7 +61,8 @@ public ManagedZone create(ManagedZone zone, Map options) throws DnsEx .setFields(FIELDS.getString(options)) .execute(); } catch (IOException ex) { - throw translate(ex); + // todo this can cause misleading report of a failure, intended to be fixed within #924 + throw translate(ex, true); } } @@ -73,7 +74,7 @@ public ManagedZone getZone(String zoneName, Map options) throws DnsEx .setFields(FIELDS.getString(options)) .execute(); } catch (IOException ex) { - DnsException serviceException = translate(ex); + DnsException serviceException = translate(ex, true); if (serviceException.code() == HTTP_NOT_FOUND) { return null; } @@ -93,7 +94,7 @@ public ListResult listZones(Map options) throws DnsExcep .execute(); return of(zoneList.getNextPageToken(), zoneList.getManagedZones()); } catch (IOException ex) { - throw translate(ex); + throw translate(ex, true); } } @@ -103,7 +104,7 @@ public boolean deleteZone(String zoneName) throws DnsException { dns.managedZones().delete(this.options.projectId(), zoneName).execute(); return true; } catch (IOException ex) { - DnsException serviceException = translate(ex); + DnsException serviceException = translate(ex, false); if (serviceException.code() == HTTP_NOT_FOUND) { return false; } @@ -126,7 +127,7 @@ public ListResult listRecordSets(String zoneName, Map options) throws DnsException { return dns.projects().get(this.options.projectId()) .setFields(FIELDS.getString(options)).execute(); } catch (IOException ex) { - throw translate(ex); + throw translate(ex, true); } } @@ -148,7 +149,7 @@ public Change applyChangeRequest(String zoneName, Change changeRequest, Map listChangeRequests(String zoneName, Map opt ChangesListResponse response = request.execute(); return of(response.getNextPageToken(), response.getChanges()); } catch (IOException ex) { - throw translate(ex); + throw translate(ex, true); } } }