From 03d5d30313281209550730375df7a0bbe8f4d284 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Fri, 15 Jan 2016 15:10:21 +0100 Subject: [PATCH 1/5] Refactor BaseServiceException --- .../java/com/google/gcloud/bigquery/Acl.java | 2 +- .../gcloud/bigquery/BigQueryException.java | 42 +++-- .../google/gcloud/spi/DefaultBigQueryRpc.java | 34 +--- .../bigquery/BigQueryExceptionTest.java | 115 +++++++++++++ .../gcloud/bigquery/BigQueryImplTest.java | 4 +- .../google/gcloud/BaseServiceException.java | 157 +++++++++++++++++- .../gcloud/BaseServiceExceptionTest.java | 134 +++++++++++++-- .../gcloud/datastore/DatastoreException.java | 145 +++++----------- .../gcloud/datastore/DatastoreImpl.java | 22 ++- .../gcloud/datastore/DatastoreOptions.java | 23 +-- .../com/google/gcloud/spi/DatastoreRpc.java | 84 +--------- .../gcloud/spi/DefaultDatastoreRpc.java | 71 +++----- .../datastore/DatastoreExceptionTest.java | 84 +++++++--- .../gcloud/datastore/DatastoreTest.java | 21 ++- .../ResourceManagerException.java | 45 +++-- .../gcloud/spi/DefaultResourceManagerRpc.java | 26 +-- .../ResourceManagerExceptionTest.java | 94 +++++++++++ .../ResourceManagerImplTest.java | 4 +- .../google/gcloud/spi/DefaultStorageRpc.java | 24 +-- .../gcloud/storage/BlobReadChannel.java | 2 +- .../gcloud/storage/StorageException.java | 47 ++++-- .../gcloud/storage/RemoteGcsHelperTest.java | 4 +- .../gcloud/storage/StorageExceptionTest.java | 125 ++++++++++++++ .../gcloud/storage/StorageImplTest.java | 4 +- 24 files changed, 873 insertions(+), 440 deletions(-) create mode 100644 gcloud-java-bigquery/src/test/java/com/google/gcloud/bigquery/BigQueryExceptionTest.java create mode 100644 gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerExceptionTest.java create mode 100644 gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageExceptionTest.java diff --git a/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/Acl.java b/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/Acl.java index 2a042c108e00..c1fca9e3b350 100644 --- a/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/Acl.java +++ b/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/Acl.java @@ -104,7 +104,7 @@ static Entity fromPb(Access access) { } // Unreachable throw new BigQueryException(BigQueryException.UNKNOWN_CODE, - "Unrecognized access configuration", false); + "Unrecognized access configuration"); } } diff --git a/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/BigQueryException.java b/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/BigQueryException.java index e92ffacd8f09..930d06d523ab 100644 --- a/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/BigQueryException.java +++ b/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/BigQueryException.java @@ -16,10 +16,14 @@ package com.google.gcloud.bigquery; +import com.google.common.collect.ImmutableSet; import com.google.gcloud.BaseServiceException; import com.google.gcloud.RetryHelper.RetryHelperException; import com.google.gcloud.RetryHelper.RetryInterruptedException; +import java.io.IOException; +import java.util.Set; + /** * BigQuery service exception. * @@ -28,20 +32,30 @@ */ public class BigQueryException extends BaseServiceException { - private static final long serialVersionUID = -5504832700512784654L; - public static final int UNKNOWN_CODE = -1; + // see: https://cloud.google.com/bigquery/troubleshooting-errors + private static final Set RETRYABLE_ERRORS = ImmutableSet.of( + new Error(500, null), + new Error(502, null), + new Error(503, null), + new Error(504, null)); + private static final long serialVersionUID = -5006625989225438209L; private final BigQueryError error; - public BigQueryException(int code, String message, boolean retryable) { - this(code, message, retryable, null); + public BigQueryException(int code, String message) { + this(code, message, null); } - public BigQueryException(int code, String message, boolean retryable, BigQueryError error) { - super(code, message, retryable); + public BigQueryException(int code, String message, BigQueryError error) { + super(code, message, error != null ? error.reason() : null, true); this.error = error; } + public BigQueryException(IOException exception) { + super(exception, true); + this.error = null; + } + /** * Returns the {@link BigQueryError} that caused this exception. Returns {@code null} if none * exists. @@ -50,6 +64,11 @@ public BigQueryError error() { return error; } + @Override + protected Set retryableErrors() { + return RETRYABLE_ERRORS; + } + /** * Translate RetryHelperException to the BigQueryException that caused the error. This method will * always throw an exception. @@ -57,13 +76,8 @@ public BigQueryError error() { * @throws BigQueryException when {@code ex} was caused by a {@code BigQueryException} * @throws RetryInterruptedException when {@code ex} is a {@code RetryInterruptedException} */ - static BigQueryException translateAndThrow(RetryHelperException ex) { - if (ex.getCause() instanceof BigQueryException) { - throw (BigQueryException) ex.getCause(); - } - if (ex instanceof RetryInterruptedException) { - RetryInterruptedException.propagate(); - } - throw new BigQueryException(UNKNOWN_CODE, ex.getMessage(), false); + public static BigQueryException translateAndThrow(RetryHelperException ex) { + BaseServiceException.translateAndThrow(ex); + throw new BigQueryException(UNKNOWN_CODE, ex.getMessage()); } } diff --git a/gcloud-java-bigquery/src/main/java/com/google/gcloud/spi/DefaultBigQueryRpc.java b/gcloud-java-bigquery/src/main/java/com/google/gcloud/spi/DefaultBigQueryRpc.java index 74fdeb74bd64..0a1dc046bf74 100644 --- a/gcloud-java-bigquery/src/main/java/com/google/gcloud/spi/DefaultBigQueryRpc.java +++ b/gcloud-java-bigquery/src/main/java/com/google/gcloud/spi/DefaultBigQueryRpc.java @@ -25,7 +25,6 @@ import static java.net.HttpURLConnection.HTTP_OK; import com.google.api.client.googleapis.json.GoogleJsonError; -import com.google.api.client.googleapis.json.GoogleJsonResponseException; import com.google.api.client.http.ByteArrayContent; import com.google.api.client.http.GenericUrl; import com.google.api.client.http.HttpRequest; @@ -58,10 +57,8 @@ import com.google.api.services.bigquery.model.TableRow; import com.google.common.base.Function; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import com.google.gcloud.bigquery.BigQueryError; import com.google.gcloud.bigquery.BigQueryException; import com.google.gcloud.bigquery.BigQueryOptions; @@ -69,13 +66,10 @@ import java.math.BigInteger; import java.util.List; import java.util.Map; -import java.util.Set; public class DefaultBigQueryRpc implements BigQueryRpc { public static final String DEFAULT_PROJECTION = "full"; - // see: https://cloud.google.com/bigquery/troubleshooting-errors - private static final Set RETRYABLE_CODES = ImmutableSet.of(500, 502, 503, 504); private static final String BASE_RESUMABLE_URI = "https://www.googleapis.com/upload/bigquery/v2/projects/"; // see: https://cloud.google.com/bigquery/loading-data-post-request#resume-upload @@ -94,28 +88,7 @@ public DefaultBigQueryRpc(BigQueryOptions options) { } private static BigQueryException translate(IOException exception) { - BigQueryException translated; - if (exception instanceof GoogleJsonResponseException - && ((GoogleJsonResponseException) exception).getDetails() != null) { - translated = translate(((GoogleJsonResponseException) exception).getDetails()); - } else { - translated = - new BigQueryException(BigQueryException.UNKNOWN_CODE, exception.getMessage(), false); - } - translated.initCause(exception); - return translated; - } - - private static BigQueryException translate(GoogleJsonError exception) { - boolean retryable = RETRYABLE_CODES.contains(exception.getCode()); - BigQueryError bigqueryError = null; - if (exception.getErrors() != null && !exception.getErrors().isEmpty()) { - GoogleJsonError.ErrorInfo error = exception.getErrors().get(0); - bigqueryError = new BigQueryError(error.getReason(), error.getLocation(), error.getMessage(), - (String) error.get("debugInfo")); - } - return new BigQueryException(exception.getCode(), exception.getMessage(), retryable, - bigqueryError); + return new BigQueryException(exception); } @Override @@ -489,10 +462,7 @@ public void write(String uploadId, byte[] toWrite, int toWriteOffset, long destO if (exception != null) { throw exception; } - GoogleJsonError error = new GoogleJsonError(); - error.setCode(code); - error.setMessage(message); - throw translate(error); + throw new BigQueryException(code, message); } } catch (IOException ex) { throw translate(ex); diff --git a/gcloud-java-bigquery/src/test/java/com/google/gcloud/bigquery/BigQueryExceptionTest.java b/gcloud-java-bigquery/src/test/java/com/google/gcloud/bigquery/BigQueryExceptionTest.java new file mode 100644 index 000000000000..66e5289424e2 --- /dev/null +++ b/gcloud-java-bigquery/src/test/java/com/google/gcloud/bigquery/BigQueryExceptionTest.java @@ -0,0 +1,115 @@ +/* + * Copyright 2015 Google Inc. All Rights Reserved. + * + * 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.google.gcloud.bigquery; + +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import com.google.gcloud.BaseServiceException; +import com.google.gcloud.RetryHelper.RetryHelperException; + +import org.junit.Test; + +import java.io.IOException; +import java.net.SocketTimeoutException; + +public class BigQueryExceptionTest { + + @Test + public void testBigqueryException() { + BigQueryException exception = new BigQueryException(500, "message"); + assertEquals(500, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertNull(exception.error()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new BigQueryException(502, "message"); + assertEquals(502, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertNull(exception.error()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new BigQueryException(503, "message"); + assertEquals(503, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertNull(exception.error()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new BigQueryException(504, "message"); + assertEquals(504, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertNull(exception.error()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new BigQueryException(400, "message"); + assertEquals(400, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertNull(exception.error()); + assertFalse(exception.retryable()); + assertTrue(exception.idempotent()); + + BigQueryError error = new BigQueryError("reason", null, null); + exception = new BigQueryException(504, "message", error); + assertEquals(504, exception.code()); + assertEquals("message", exception.getMessage()); + assertEquals("reason", exception.reason()); + assertEquals(error, exception.error()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + IOException cause = new SocketTimeoutException(); + exception = new BigQueryException(cause); + assertNull(exception.reason()); + assertNull(exception.getMessage()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + assertEquals(cause, exception.getCause()); + } + + @Test + public void testTranslateAndThrow() throws Exception { + BigQueryException cause = new BigQueryException(503, "message"); + RetryHelperException exceptionMock = createMock(RetryHelperException.class); + expect(exceptionMock.getCause()).andReturn(cause).times(2); + replay(exceptionMock); + try { + BigQueryException.translateAndThrow(exceptionMock); + } catch (BaseServiceException ex) { + assertEquals(503, ex.code()); + assertEquals("message", ex.getMessage()); + assertTrue(ex.retryable()); + assertTrue(ex.idempotent()); + } finally { + verify(exceptionMock); + } + } +} diff --git a/gcloud-java-bigquery/src/test/java/com/google/gcloud/bigquery/BigQueryImplTest.java b/gcloud-java-bigquery/src/test/java/com/google/gcloud/bigquery/BigQueryImplTest.java index 402edfc4a42f..b54a989fb5e5 100644 --- a/gcloud-java-bigquery/src/test/java/com/google/gcloud/bigquery/BigQueryImplTest.java +++ b/gcloud-java-bigquery/src/test/java/com/google/gcloud/bigquery/BigQueryImplTest.java @@ -1022,7 +1022,7 @@ public void testWriter() { @Test public void testRetryableException() { EasyMock.expect(bigqueryRpcMock.getDataset(DATASET, EMPTY_RPC_OPTIONS)) - .andThrow(new BigQueryException(500, "InternalError", true)) + .andThrow(new BigQueryException(500, "InternalError")) .andReturn(DATASET_INFO_WITH_PROJECT.toPb()); EasyMock.replay(bigqueryRpcMock); bigquery = options.toBuilder().retryParams(RetryParams.defaultInstance()).build().service(); @@ -1034,7 +1034,7 @@ public void testRetryableException() { public void testNonRetryableException() { String exceptionMessage = "Not Implemented"; EasyMock.expect(bigqueryRpcMock.getDataset(DATASET, EMPTY_RPC_OPTIONS)) - .andThrow(new BigQueryException(501, exceptionMessage, false)); + .andThrow(new BigQueryException(501, exceptionMessage)); EasyMock.replay(bigqueryRpcMock); bigquery = options.toBuilder().retryParams(RetryParams.defaultInstance()).build().service(); thrown.expect(BigQueryException.class); diff --git a/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java b/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java index cd0933426756..9f4bfdab994d 100644 --- a/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java +++ b/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java @@ -16,26 +16,126 @@ package com.google.gcloud; +import com.google.api.client.googleapis.json.GoogleJsonError; +import com.google.api.client.googleapis.json.GoogleJsonResponseException; +import com.google.common.base.MoreObjects; + +import java.io.IOException; +import java.io.Serializable; +import java.net.SocketTimeoutException; +import java.util.Collections; +import java.util.Objects; +import java.util.Set; + /** * Base class for all service exceptions. */ public class BaseServiceException extends RuntimeException { - private static final long serialVersionUID = 5028833760039966178L; + protected static final class Error implements Serializable { + + private static final long serialVersionUID = -4019600198652965721L; + + private final Integer code; + private final String reason; + + public Error(Integer code, String reason) { + this.code = code; + this.reason = reason; + } + + /** + * Returns the code associated with this exception. + */ + public Integer code() { + return code; + } + + /** + * Returns the reason that caused the exception. + */ + public String reason() { + return reason; + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this).add("code", code).add("reason", reason).toString(); + } + + @Override + public int hashCode() { + return Objects.hash(code, reason); + } + } + + private static final long serialVersionUID = 759921776378760835L; + public static final int UNKNOWN_CODE = 0; private final int code; private final boolean retryable; + private final String reason; + private final boolean idempotent; - public BaseServiceException(int code, String message, boolean retryable) { - super(message); - this.code = code; - this.retryable = retryable; + public BaseServiceException(IOException exception, boolean idempotent) { + super(message(exception), exception); + if (exception instanceof GoogleJsonResponseException) { + Error error = error(((GoogleJsonResponseException) exception).getDetails()); + this.code = error.code; + this.reason = error.reason; + this.retryable = isRetryable(error); + } else { + this.code = UNKNOWN_CODE; + this.reason = null; + this.retryable = idempotent && isRetryable(exception); + } + this.idempotent = 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); + } + + public BaseServiceException(int code, String message, String reason, boolean idempotent) { + this(code, message, reason, idempotent, null); } - public BaseServiceException(int code, String message, boolean retryable, Exception cause) { + public BaseServiceException(int code, String message, String reason, boolean idempotent, + Exception cause) { super(message, cause); this.code = code; - this.retryable = retryable; + this.reason = reason; + this.idempotent = idempotent; + this.retryable = idempotent && isRetryable(new Error(code, reason)); + } + + protected Set retryableErrors() { + return Collections.emptySet(); + } + + protected boolean isRetryable(GoogleJsonError error) { + return error != null && isRetryable(error(error)); + } + + protected boolean isRetryable(IOException exception) { + if (exception instanceof GoogleJsonResponseException) { + return isRetryable(((GoogleJsonResponseException) exception).getDetails()); + } + return exception instanceof SocketTimeoutException; + } + + protected boolean isRetryable(Error error) { + for (Error retryableError : retryableErrors()) { + if ((retryableError.code() == null || retryableError.code().equals(error.code())) + && (retryableError.reason() == null || retryableError.reason().equals(error.reason()))) { + return true; + } + } + return false; } /** @@ -45,10 +145,53 @@ public int code() { return code; } + /** + * Returns the reason that caused the exception. + */ + public String reason() { + return reason; + } + /** * Returns {@code true} when it is safe to retry the operation that caused this exception. */ public boolean retryable() { return retryable; } + + /** + * Returns {@code true} when the operation that caused this exception had no side effects. + */ + public boolean idempotent() { + return idempotent; + } + + protected 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) { + if (exception instanceof GoogleJsonResponseException) { + return ((GoogleJsonResponseException) exception).getDetails().getMessage(); + } + return exception.getMessage(); + } + + protected static BaseServiceException translateAndThrow( + RetryHelper.RetryHelperException ex) { + if (ex.getCause() instanceof BaseServiceException) { + throw (BaseServiceException) ex.getCause(); + } + if (ex instanceof RetryHelper.RetryInterruptedException) { + RetryHelper.RetryInterruptedException.propagate(); + } + return null; + } } diff --git a/gcloud-java-core/src/test/java/com/google/gcloud/BaseServiceExceptionTest.java b/gcloud-java-core/src/test/java/com/google/gcloud/BaseServiceExceptionTest.java index f30fd3abfb79..a6e22866ed9f 100644 --- a/gcloud-java-core/src/test/java/com/google/gcloud/BaseServiceExceptionTest.java +++ b/gcloud-java-core/src/test/java/com/google/gcloud/BaseServiceExceptionTest.java @@ -16,30 +16,142 @@ package com.google.gcloud; +import static com.google.gcloud.BaseServiceException.UNKNOWN_CODE; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import com.google.api.client.googleapis.json.GoogleJsonError; +import com.google.common.collect.ImmutableSet; import org.junit.Test; +import java.io.IOException; +import java.net.SocketTimeoutException; +import java.util.Set; + /** * Tests for {@link BaseServiceException}. */ public class BaseServiceExceptionTest { - private final int code = 1; - private final String message = "some message"; - private final boolean retryable = true; + private static final int CODE = 1; + private static final int CODE_NO_REASON = 2; + private static final String MESSAGE = "some message"; + private static final String REASON = "some reason"; + private static final boolean RETRYABLE = true; + private static final boolean IDEMPOTENT = true; + private static class CustomServiceException extends BaseServiceException { + + private static final long serialVersionUID = -195251309124875103L; + + public CustomServiceException(int code, String message, String reason, boolean idempotent) { + super(code, message, reason, idempotent); + } + + @Override + protected Set retryableErrors() { + return ImmutableSet.of(new Error(CODE, REASON), new Error(null, REASON), + new Error(CODE_NO_REASON, null)); + } + } @Test public void testBaseServiceException() { - BaseServiceException serviceException = new BaseServiceException(code, message, retryable); - assertEquals(serviceException.code(), code); - assertEquals(serviceException.getMessage(), message); - assertEquals(serviceException.getCause(), null); + BaseServiceException serviceException = new BaseServiceException(CODE, MESSAGE, REASON, + IDEMPOTENT); + assertEquals(CODE, serviceException.code()); + assertEquals(MESSAGE, serviceException.getMessage()); + assertEquals(REASON, serviceException.reason()); + assertFalse(serviceException.retryable()); + assertEquals(IDEMPOTENT, serviceException.idempotent()); + assertNull(serviceException.getCause()); + + serviceException = new BaseServiceException(CODE, MESSAGE, REASON, IDEMPOTENT); + assertEquals(CODE, serviceException.code()); + assertEquals(MESSAGE, serviceException.getMessage()); + assertEquals(REASON, serviceException.reason()); + assertFalse(serviceException.retryable()); + assertEquals(IDEMPOTENT, serviceException.idempotent()); + assertNull(serviceException.getCause()); Exception cause = new RuntimeException(); - serviceException = new BaseServiceException(code, message, retryable, cause); - assertEquals(serviceException.code(), code); - assertEquals(serviceException.getMessage(), message); - assertEquals(serviceException.getCause(), cause); + serviceException = new BaseServiceException(CODE, MESSAGE, REASON, IDEMPOTENT, cause); + assertEquals(CODE, serviceException.code()); + assertEquals(MESSAGE, serviceException.getMessage()); + assertEquals(REASON, serviceException.reason()); + assertFalse(serviceException.retryable()); + assertEquals(IDEMPOTENT, serviceException.idempotent()); + assertEquals(cause, serviceException.getCause()); + + serviceException = new BaseServiceException(CODE, MESSAGE, REASON, false, cause); + assertEquals(CODE, serviceException.code()); + assertEquals(MESSAGE, serviceException.getMessage()); + assertEquals(REASON, serviceException.reason()); + assertFalse(serviceException.retryable()); + assertFalse(serviceException.idempotent()); + assertEquals(cause, serviceException.getCause()); + + IOException exception = new SocketTimeoutException(); + serviceException = new BaseServiceException(exception, true); + assertTrue(serviceException.retryable()); + assertTrue(serviceException.idempotent()); + assertEquals(exception, serviceException.getCause()); + + GoogleJsonError error = new GoogleJsonError(); + error.setCode(CODE); + error.setMessage(MESSAGE); + serviceException = new BaseServiceException(error, true); + assertEquals(CODE, serviceException.code()); + assertEquals(MESSAGE, serviceException.getMessage()); + assertFalse(serviceException.retryable()); + assertTrue(serviceException.idempotent()); + + serviceException = new CustomServiceException(CODE, MESSAGE, REASON, IDEMPOTENT); + assertEquals(CODE, serviceException.code()); + assertEquals(MESSAGE, serviceException.getMessage()); + assertEquals(REASON, serviceException.reason()); + assertEquals(RETRYABLE, serviceException.retryable()); + assertEquals(IDEMPOTENT, serviceException.idempotent()); + + serviceException = new CustomServiceException(CODE_NO_REASON, MESSAGE, null, IDEMPOTENT); + assertEquals(CODE_NO_REASON, serviceException.code()); + assertEquals(MESSAGE, serviceException.getMessage()); + assertNull(serviceException.reason()); + assertEquals(RETRYABLE, serviceException.retryable()); + assertEquals(IDEMPOTENT, serviceException.idempotent()); + + serviceException = new CustomServiceException(UNKNOWN_CODE, MESSAGE, REASON, IDEMPOTENT); + assertEquals(UNKNOWN_CODE, serviceException.code()); + assertEquals(MESSAGE, serviceException.getMessage()); + assertEquals(REASON, serviceException.reason()); + assertEquals(RETRYABLE, serviceException.retryable()); + assertEquals(IDEMPOTENT, serviceException.idempotent()); + } + + @Test + public void testTranslateAndThrow() throws Exception { + BaseServiceException cause = new BaseServiceException(CODE, MESSAGE, REASON, IDEMPOTENT); + RetryHelper.RetryHelperException exceptionMock = createMock(RetryHelper.RetryHelperException.class); + expect(exceptionMock.getCause()).andReturn(cause).times(2); + replay(exceptionMock); + try { + BaseServiceException ex = BaseServiceException.translateAndThrow(exceptionMock); + if (ex != null) { + throw ex; + } + } catch (BaseServiceException ex) { + assertEquals(CODE, ex.code()); + assertEquals(MESSAGE, ex.getMessage()); + assertFalse(ex.retryable()); + assertEquals(IDEMPOTENT, ex.idempotent()); + } finally { + verify(exceptionMock); + } } } diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java index ebef6b44f6b6..946fc9190fc3 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java @@ -16,141 +16,70 @@ package com.google.gcloud.datastore; -import com.google.common.base.MoreObjects; -import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.gcloud.BaseServiceException; -import com.google.gcloud.RetryHelper; import com.google.gcloud.RetryHelper.RetryHelperException; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException.Reason; +import com.google.gcloud.RetryHelper.RetryInterruptedException; -import java.util.HashMap; -import java.util.Map; +import java.io.IOException; +import java.util.Set; +/** + * Datastore service exception. + * + * @see Google Cloud + * Datastore error codes + */ public class DatastoreException extends BaseServiceException { - private static final long serialVersionUID = -2336749234060754893L; - private static final ImmutableMap REASON_TO_ERROR; - private static final ImmutableMap HTTP_TO_ERROR; - - private final DatastoreError error; - - /** - * Represents Datastore errors. - * - * @see Google Cloud - * Datastore error codes - */ - public enum DatastoreError { - - ABORTED(Reason.ABORTED), - DEADLINE_EXCEEDED(Reason.DEADLINE_EXCEEDED), - UNAVAILABLE(Reason.UNAVAILABLE), - FAILED_PRECONDITION(Reason.FAILED_PRECONDITION), - INVALID_ARGUMENT(Reason.INVALID_ARGUMENT), - PERMISSION_DENIED(Reason.PERMISSION_DENIED), - UNAUTHORIZED(false, "Unauthorized", 401), - INTERNAL(Reason.INTERNAL), - RESOURCE_EXHAUSTED(Reason.RESOURCE_EXHAUSTED), - UNKNOWN(false, "Unknown failure", -1); - - private final boolean retryable; - private final String description; - private final int httpStatus; - - DatastoreError(Reason reason) { - this(reason.retryable(), reason.description(), reason.httpStatus()); - } - - DatastoreError(boolean retryable, String description, int httpStatus) { - this.retryable = retryable; - this.description = description; - this.httpStatus = httpStatus; - } + // see https://cloud.google.com/datastore/docs/concepts/errors#Error_Codes" + private static final Set RETRYABLE_ERRORS = ImmutableSet.of( + new Error(409, "ABORTED"), + new Error(403, "DEADLINE_EXCEEDED"), + new Error(503, "UNAVAILABLE")); + private static final long serialVersionUID = 2663750991205874435L; - String description() { - return description; - } - - int httpStatus() { - return httpStatus; - } - - boolean retryable() { - return retryable; - } - - DatastoreException translate(DatastoreRpcException exception, String message) { - return new DatastoreException(this, message, exception); - } - } - - static { - ImmutableMap.Builder builder = ImmutableMap.builder(); - Map httpCodes = new HashMap<>(); - for (DatastoreError error : DatastoreError.values()) { - builder.put(error.name(), error); - httpCodes.put(error.httpStatus(), error); - } - REASON_TO_ERROR = builder.build(); - HTTP_TO_ERROR = ImmutableMap.copyOf(httpCodes); + public DatastoreException(int code, String message, String reason, Exception cause) { + super(code, message, reason, true, cause); } - public DatastoreException(DatastoreError error, String message, Exception cause) { - super(error.httpStatus(), MoreObjects.firstNonNull(message, error.description), - error.retryable(), cause); - this.error = error; + public DatastoreException(int code, String message, String reason) { + super(code, message, reason, true); } - public DatastoreException(DatastoreError error, String message) { - this(error, message, null); + public DatastoreException(IOException exception) { + super(exception, true); } - /** - * Returns the DatastoreError associated with this exception. - */ - public DatastoreError datastoreError() { - return error; - } - - static DatastoreException translateAndThrow(RetryHelperException ex) { - if (ex.getCause() instanceof DatastoreRpcException) { - return translateAndThrow((DatastoreRpcException) ex.getCause()); - } - if (ex instanceof RetryHelper.RetryInterruptedException) { - RetryHelper.RetryInterruptedException.propagate(); - } - throw new DatastoreException(DatastoreError.UNKNOWN, ex.getMessage(), ex); + @Override + protected Set retryableErrors() { + return RETRYABLE_ERRORS; } /** - * Translate DatastoreRpcExceptions to DatastoreExceptions based on their - * HTTP error codes. This method will always throw a new DatastoreException. + * Translate RetryHelperException to the DatastoreException that caused the error. This method + * will always throw an exception. * - * @throws DatastoreException every time + * @throws DatastoreException when {@code ex} was caused by a {@code DatastoreException} + * @throws RetryInterruptedException when {@code ex} is a {@code RetryInterruptedException} */ - static DatastoreException translateAndThrow(DatastoreRpcException exception) { - String message = exception.getMessage(); - DatastoreError error = REASON_TO_ERROR.get(exception.reason()); - if (error == null) { - error = MoreObjects.firstNonNull( - HTTP_TO_ERROR.get(exception.httpStatus()), DatastoreError.UNKNOWN); - } - throw error.translate(exception, message); + public static DatastoreException translateAndThrow(RetryHelperException ex) { + BaseServiceException.translateAndThrow(ex); + throw new DatastoreException(UNKNOWN_CODE, ex.getMessage(), null); } /** - * Throw a DatastoreException with {@code FAILED_PRECONDITION} error and the {@code message} - * in a nested exception. + * Throw a DatastoreException with {@code FAILED_PRECONDITION} reason and the {@code message} in a + * nested exception. * * @throws DatastoreException every time */ static DatastoreException throwInvalidRequest(String massage, Object... params) { - throw new DatastoreException( - DatastoreError.FAILED_PRECONDITION, String.format(massage, params)); + throw new DatastoreException(UNKNOWN_CODE, String.format(massage, params), + "FAILED_PRECONDITION"); } static DatastoreException propagateUserException(Exception ex) { - throw new DatastoreException(DatastoreError.UNKNOWN, ex.getMessage(), ex); + throw new DatastoreException(BaseServiceException.UNKNOWN_CODE, ex.getMessage(), null, ex); } } diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreImpl.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreImpl.java index 43fd75396538..bfcba58f3f2f 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreImpl.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreImpl.java @@ -29,7 +29,6 @@ import com.google.gcloud.RetryHelper.RetryHelperException; import com.google.gcloud.RetryParams; import com.google.gcloud.spi.DatastoreRpc; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException; import com.google.protobuf.ByteString; import java.util.Arrays; @@ -42,7 +41,6 @@ import java.util.Set; import java.util.concurrent.Callable; - final class DatastoreImpl extends BaseService implements Datastore { @@ -58,15 +56,15 @@ public RetryResult afterEval(Exception exception, RetryResult retryResult) { @Override public RetryResult beforeEval(Exception exception) { - if (exception instanceof DatastoreRpcException) { - boolean retryable = ((DatastoreRpcException) exception).retryable(); + if (exception instanceof DatastoreException) { + boolean retryable = ((DatastoreException) exception).retryable(); return retryable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.NO_RETRY; } return Interceptor.RetryResult.CONTINUE_EVALUATION; } }; private static final ExceptionHandler EXCEPTION_HANDLER = ExceptionHandler.builder() - .abortOn(RuntimeException.class, DatastoreRpcException.class) + .abortOn(RuntimeException.class, DatastoreException.class) .interceptor(EXCEPTION_HANDLER_INTERCEPTOR).build(); private final DatastoreRpc datastoreRpc; @@ -105,7 +103,7 @@ QueryResults run(DatastoreV1.ReadOptions readOptionsPb, Query query) { DatastoreV1.RunQueryResponse runQuery(final DatastoreV1.RunQueryRequest requestPb) { try { return RetryHelper.runWithRetries(new Callable() { - @Override public DatastoreV1.RunQueryResponse call() throws DatastoreRpcException { + @Override public DatastoreV1.RunQueryResponse call() throws DatastoreException { return datastoreRpc.runQuery(requestPb); } }, retryParams, EXCEPTION_HANDLER); @@ -139,7 +137,7 @@ public List allocateId(IncompleteKey... keys) { DatastoreV1.AllocateIdsResponse allocateIds(final DatastoreV1.AllocateIdsRequest requestPb) { try { return RetryHelper.runWithRetries(new Callable() { - @Override public DatastoreV1.AllocateIdsResponse call() throws DatastoreRpcException { + @Override public DatastoreV1.AllocateIdsResponse call() throws DatastoreException { return datastoreRpc.allocateIds(requestPb); } }, retryParams, EXCEPTION_HANDLER); @@ -176,7 +174,7 @@ public List add(FullEntity... entities) { if (completeEntity != null) { if (completeEntities.put(completeEntity.key(), completeEntity) != null) { throw DatastoreException.throwInvalidRequest( - "Duplicate entity with the key %s", entity.key()); + "Duplicate entity with the key %s", entity.key()); } mutationPb.addInsert(completeEntity.toPb()); } else { @@ -263,7 +261,7 @@ protected Entity computeNext() { DatastoreV1.LookupResponse lookup(final DatastoreV1.LookupRequest requestPb) { try { return RetryHelper.runWithRetries(new Callable() { - @Override public DatastoreV1.LookupResponse call() throws DatastoreRpcException { + @Override public DatastoreV1.LookupResponse call() throws DatastoreException { return datastoreRpc.lookup(requestPb); } }, retryParams, EXCEPTION_HANDLER); @@ -334,7 +332,7 @@ private DatastoreV1.CommitResponse commitMutation(DatastoreV1.Mutation.Builder m DatastoreV1.CommitResponse commit(final DatastoreV1.CommitRequest requestPb) { try { return RetryHelper.runWithRetries(new Callable() { - @Override public DatastoreV1.CommitResponse call() throws DatastoreRpcException { + @Override public DatastoreV1.CommitResponse call() throws DatastoreException { return datastoreRpc.commit(requestPb); } }, retryParams, EXCEPTION_HANDLER); @@ -352,7 +350,7 @@ DatastoreV1.BeginTransactionResponse beginTransaction( try { return RetryHelper.runWithRetries(new Callable() { @Override - public DatastoreV1.BeginTransactionResponse call() throws DatastoreRpcException { + public DatastoreV1.BeginTransactionResponse call() throws DatastoreException { return datastoreRpc.beginTransaction(requestPb); } }, retryParams, EXCEPTION_HANDLER); @@ -370,7 +368,7 @@ void rollbackTransaction(ByteString transaction) { void rollback(final DatastoreV1.RollbackRequest requestPb) { try { RetryHelper.runWithRetries(new Callable() { - @Override public Void call() throws DatastoreRpcException { + @Override public Void call() throws DatastoreException { datastoreRpc.rollback(requestPb); return null; } diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreOptions.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreOptions.java index 227419d8acc8..2ec0f2be8f2b 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreOptions.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreOptions.java @@ -25,7 +25,6 @@ import com.google.common.collect.Iterables; import com.google.gcloud.ServiceOptions; import com.google.gcloud.spi.DatastoreRpc; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException; import com.google.gcloud.spi.DatastoreRpcFactory; import com.google.gcloud.spi.DefaultDatastoreRpc; @@ -126,20 +125,16 @@ private DatastoreOptions normalize() { .addPathElement(DatastoreV1.Key.PathElement.newBuilder().setKind("__foo__").setName("bar")) .build(); requestPb.addKey(key); - try { - LookupResponse responsePb = rpc().lookup(requestPb.build()); - if (responsePb.getDeferredCount() > 0) { - key = responsePb.getDeferred(0); - } else { - Iterator combinedIter = - Iterables.concat(responsePb.getMissingList(), responsePb.getFoundList()).iterator(); - key = combinedIter.next().getEntity().getKey(); - } - builder.projectId(key.getPartitionId().getDatasetId()); - return new DatastoreOptions(builder); - } catch (DatastoreRpcException e) { - throw DatastoreException.translateAndThrow(e); + LookupResponse responsePb = rpc().lookup(requestPb.build()); + if (responsePb.getDeferredCount() > 0) { + key = responsePb.getDeferred(0); + } else { + Iterator combinedIter = + Iterables.concat(responsePb.getMissingList(), responsePb.getFoundList()).iterator(); + key = combinedIter.next().getEntity().getKey(); } + builder.projectId(key.getPartitionId().getDatasetId()); + return new DatastoreOptions(builder); } @Override diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DatastoreRpc.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DatastoreRpc.java index 3e875ff2b8ba..fd916e0a1c87 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DatastoreRpc.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DatastoreRpc.java @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + package com.google.gcloud.spi; import com.google.api.services.datastore.DatastoreV1.AllocateIdsRequest; @@ -27,92 +28,23 @@ import com.google.api.services.datastore.DatastoreV1.RollbackResponse; import com.google.api.services.datastore.DatastoreV1.RunQueryRequest; import com.google.api.services.datastore.DatastoreV1.RunQueryResponse; +import com.google.gcloud.datastore.DatastoreException; /** * Provides access to the remote Datastore service. */ public interface DatastoreRpc { - public class DatastoreRpcException extends Exception { - - /** - * The reason for the exception. - * - * @see Google - * Cloud Datastore error codes - */ - public enum Reason { - - ABORTED(true, "Request aborted", 409), - DEADLINE_EXCEEDED(true, "Deadline exceeded", 403), - FAILED_PRECONDITION(false, "Invalid request", 412), - INTERNAL(false, "Server returned an error", 500), - INVALID_ARGUMENT(false, "Request parameter has an invalid value", 400), - PERMISSION_DENIED(false, "Unauthorized request", 403), - RESOURCE_EXHAUSTED(false, "Quota exceeded", 402), - UNAVAILABLE(true, "Could not reach service", 503); - - private final boolean retryable; - private final String description; - private final int httpStatus; - - private Reason(boolean retryable, String description, int httpStatus) { - this.retryable = retryable; - this.description = description; - this.httpStatus = httpStatus; - } - - public boolean retryable() { - return retryable; - } - - public String description() { - return description; - } - - public int httpStatus() { - return httpStatus; - } - } - - private final String reason; - private final int httpStatus; - private final boolean retryable; - - public DatastoreRpcException(Reason reason, Throwable cause) { - this(reason.name(), reason.httpStatus, reason.retryable, reason.description, cause); - } - - public DatastoreRpcException(String reason, int httpStatus, boolean retryable, String message, Throwable cause) { - super(message, cause); - this.reason = reason; - this.httpStatus = httpStatus; - this.retryable = retryable; - } - - public String reason() { - return reason; - } - - public int httpStatus() { - return httpStatus; - } - - public boolean retryable() { - return retryable; - } - } - - AllocateIdsResponse allocateIds(AllocateIdsRequest request) throws DatastoreRpcException; + AllocateIdsResponse allocateIds(AllocateIdsRequest request) throws DatastoreException; BeginTransactionResponse beginTransaction(BeginTransactionRequest request) - throws DatastoreRpcException; + throws DatastoreException; - CommitResponse commit(CommitRequest request) throws DatastoreRpcException; + CommitResponse commit(CommitRequest request) throws DatastoreException; - LookupResponse lookup(LookupRequest request) throws DatastoreRpcException; + LookupResponse lookup(LookupRequest request) throws DatastoreException; - RollbackResponse rollback(RollbackRequest request) throws DatastoreRpcException; + RollbackResponse rollback(RollbackRequest request) throws DatastoreException; - RunQueryResponse runQuery(RunQueryRequest request) throws DatastoreRpcException; + RunQueryResponse runQuery(RunQueryRequest request) throws DatastoreException; } diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java index fa993c508a0b..ac00d94692de 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java @@ -29,47 +29,28 @@ import com.google.api.services.datastore.DatastoreV1.RunQueryRequest; import com.google.api.services.datastore.DatastoreV1.RunQueryResponse; import com.google.api.services.datastore.client.Datastore; -import com.google.api.services.datastore.client.DatastoreException; import com.google.api.services.datastore.client.DatastoreFactory; import com.google.api.services.datastore.client.DatastoreOptions.Builder; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableMap; +import com.google.gcloud.datastore.DatastoreException; import com.google.gcloud.datastore.DatastoreOptions; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException.Reason; import org.json.JSONException; import org.json.JSONObject; import org.json.JSONTokener; +import java.io.IOException; import java.net.InetAddress; -import java.net.SocketTimeoutException; import java.net.URL; -import java.util.HashMap; -import java.util.Map; public class DefaultDatastoreRpc implements DatastoreRpc { private final Datastore client; - private static final ImmutableMap STR_TO_REASON; - private static final ImmutableMap HTTP_STATUS_TO_REASON; - - static { - ImmutableMap.Builder builder = ImmutableMap.builder(); - Map httpCodes = new HashMap<>(); - for (Reason reason : Reason.values()) { - builder.put(reason.name(), reason); - httpCodes.put(reason.httpStatus(), reason); - } - STR_TO_REASON = builder.build(); - HTTP_STATUS_TO_REASON = ImmutableMap.copyOf(httpCodes); - } - public DefaultDatastoreRpc(DatastoreOptions options) { String normalizedHost = normalizeHost(options.host()); client = DatastoreFactory.get().create( new Builder() - .transport(options.httpTransportFactory().create()) .dataset(options.projectId()) .host(normalizedHost) .initializer(options.httpRequestInitializer()) @@ -106,90 +87,82 @@ private static boolean includesScheme(String url) { return url.startsWith("http://") || url.startsWith("https://"); } - private static DatastoreRpcException translate(DatastoreException exception) { + private static DatastoreException translate( + com.google.api.services.datastore.client.DatastoreException exception) { String message = exception.getMessage(); - String reasonStr = ""; + int code = exception.getCode(); + String reason = ""; if (message != null) { try { JSONObject json = new JSONObject(new JSONTokener(message)); JSONObject error = json.getJSONObject("error").getJSONArray("errors").getJSONObject(0); - reasonStr = error.getString("reason"); + reason = error.getString("reason"); message = error.getString("message"); } catch (JSONException ignore) { // ignore - will be converted to unknown } } - Reason reason = STR_TO_REASON.get(reasonStr); if (reason == null) { - reason = HTTP_STATUS_TO_REASON.get(exception.getCode()); - } - if (reason != null) { - return new DatastoreRpcException(reason, exception); - } else { - boolean retryable = false; - reasonStr = "Unknown"; - if (exception.getCause() instanceof SocketTimeoutException) { - retryable = true; - reasonStr = "Request timeout"; + if (exception.getCause() instanceof IOException) { + return new DatastoreException((IOException) exception.getCause()); } - return new DatastoreRpcException(reasonStr, exception.getCode(), retryable, message, exception); } + return new DatastoreException(code, message, reason); } @Override public AllocateIdsResponse allocateIds(AllocateIdsRequest request) - throws DatastoreRpcException { + throws DatastoreException { try { return client.allocateIds(request); - } catch (DatastoreException ex) { + } catch (com.google.api.services.datastore.client.DatastoreException ex) { throw translate(ex); } } @Override public BeginTransactionResponse beginTransaction(BeginTransactionRequest request) - throws DatastoreRpcException { + throws DatastoreException { try { return client.beginTransaction(request); - } catch (DatastoreException ex) { + } catch (com.google.api.services.datastore.client.DatastoreException ex) { throw translate(ex); } } @Override - public CommitResponse commit(CommitRequest request) throws DatastoreRpcException { + public CommitResponse commit(CommitRequest request) throws DatastoreException { try { return client.commit(request); - } catch (DatastoreException ex) { + } catch (com.google.api.services.datastore.client.DatastoreException ex) { throw translate(ex); } } @Override - public LookupResponse lookup(LookupRequest request) throws DatastoreRpcException { + public LookupResponse lookup(LookupRequest request) throws DatastoreException { try { return client.lookup(request); - } catch (DatastoreException ex) { + } catch (com.google.api.services.datastore.client.DatastoreException ex) { throw translate(ex); } } @Override - public RollbackResponse rollback(RollbackRequest request) throws DatastoreRpcException { + public RollbackResponse rollback(RollbackRequest request) throws DatastoreException { try { return client.rollback(request); - } catch (DatastoreException ex) { + } catch (com.google.api.services.datastore.client.DatastoreException ex) { throw translate(ex); } } @Override - public RunQueryResponse runQuery(RunQueryRequest request) throws DatastoreRpcException { + public RunQueryResponse runQuery(RunQueryRequest request) throws DatastoreException { try { return client.runQuery(request); - } catch (DatastoreException ex) { + } catch (com.google.api.services.datastore.client.DatastoreException ex) { throw translate(ex); } } } - diff --git a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreExceptionTest.java b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreExceptionTest.java index 019d69cb737b..4d62224172f9 100644 --- a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreExceptionTest.java +++ b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreExceptionTest.java @@ -16,42 +16,80 @@ package com.google.gcloud.datastore; +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; -import com.google.gcloud.datastore.DatastoreException.DatastoreError; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException.Reason; +import com.google.gcloud.BaseServiceException; +import com.google.gcloud.RetryHelper; import org.junit.Test; +import java.io.IOException; +import java.net.SocketTimeoutException; + public class DatastoreExceptionTest { @Test - public void testDatastoreError() throws Exception { - for (Reason reason : Reason.values()) { - DatastoreError error = DatastoreError.valueOf(reason.name()); - assertEquals(reason.retryable(), error.retryable()); - assertEquals(reason.description(), error.description()); - assertEquals(reason.httpStatus(), error.httpStatus()); - } + public void testDatastoreException() throws Exception { + DatastoreException exception = new DatastoreException(409, "message", "ABORTED"); + assertEquals(409, exception.code()); + assertEquals("ABORTED", exception.reason()); + assertEquals("message", exception.getMessage()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new DatastoreException(403, "message", "DEADLINE_EXCEEDED"); + assertEquals(403, exception.code()); + assertEquals("DEADLINE_EXCEEDED", exception.reason()); + assertEquals("message", exception.getMessage()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new DatastoreException(503, "message", "UNAVAILABLE"); + assertEquals(503, exception.code()); + assertEquals("UNAVAILABLE", exception.reason()); + assertEquals("message", exception.getMessage()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new DatastoreException(500, "message", "INTERNAL"); + assertEquals(500, exception.code()); + assertEquals("INTERNAL", exception.reason()); + assertEquals("message", exception.getMessage()); + assertFalse(exception.retryable()); + assertTrue(exception.idempotent()); + + IOException cause = new SocketTimeoutException(); + exception = new DatastoreException(cause); + assertNull(exception.reason()); + assertNull(exception.getMessage()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); - DatastoreException exception = new DatastoreException(DatastoreError.ABORTED, "bla"); - assertEquals(DatastoreError.ABORTED, exception.datastoreError()); } @Test public void testTranslateAndThrow() throws Exception { - DatastoreRpcException toTranslate = null; // should be preserved as a cause - for (Reason reason : Reason.values()) { - try { - toTranslate = new DatastoreRpcException(reason, null); - DatastoreException.translateAndThrow(toTranslate); - fail("Exception expected"); - } catch (DatastoreException ex) { - assertEquals(reason.name(), ex.datastoreError().name()); - assertEquals(toTranslate, ex.getCause()); - } + DatastoreException cause = new DatastoreException(503, "message", "UNAVAILABLE"); + RetryHelper.RetryHelperException exceptionMock = createMock(RetryHelper.RetryHelperException.class); + expect(exceptionMock.getCause()).andReturn(cause).times(2); + replay(exceptionMock); + try { + DatastoreException.translateAndThrow(exceptionMock); + } catch (BaseServiceException ex) { + assertEquals(503, ex.code()); + assertEquals("message", ex.getMessage()); + assertTrue(ex.retryable()); + assertTrue(ex.idempotent()); + } finally { + verify(exceptionMock); } } @@ -61,7 +99,7 @@ public void testThrowInvalidRequest() throws Exception { DatastoreException.throwInvalidRequest("message %s %d", "a", 1); fail("Exception expected"); } catch (DatastoreException ex) { - assertEquals(DatastoreError.FAILED_PRECONDITION, ex.datastoreError()); + assertEquals("FAILED_PRECONDITION", ex.reason()); assertEquals("message a 1", ex.getMessage()); } } diff --git a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java index e6f84c76ad40..8cb88f9e7795 100644 --- a/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java +++ b/gcloud-java-datastore/src/test/java/com/google/gcloud/datastore/DatastoreTest.java @@ -38,8 +38,6 @@ import com.google.gcloud.datastore.StructuredQuery.PropertyFilter; import com.google.gcloud.datastore.testing.LocalGcdHelper; import com.google.gcloud.spi.DatastoreRpc; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException; -import com.google.gcloud.spi.DatastoreRpc.DatastoreRpcException.Reason; import com.google.gcloud.spi.DatastoreRpcFactory; import org.easymock.EasyMock; @@ -201,7 +199,7 @@ public void testTransactionWithRead() { transaction.commit(); fail("Expecting a failure"); } catch (DatastoreException expected) { - assertEquals(DatastoreException.DatastoreError.ABORTED, expected.datastoreError()); + assertEquals("ABORTED", expected.reason()); } } @@ -229,7 +227,7 @@ public void testTransactionWithQuery() { transaction.commit(); fail("Expecting a failure"); } catch (DatastoreException expected) { - assertEquals(DatastoreException.DatastoreError.ABORTED, expected.datastoreError()); + assertEquals("ABORTED", expected.reason()); } } @@ -466,7 +464,7 @@ public void testRunStructuredQuery() { } @Test - public void testQueryPaginationWithLimit() throws DatastoreRpcException { + public void testQueryPaginationWithLimit() throws DatastoreException { DatastoreRpcFactory rpcFactoryMock = EasyMock.createStrictMock(DatastoreRpcFactory.class); DatastoreRpc rpcMock = EasyMock.createStrictMock(DatastoreRpc.class); EasyMock.expect(rpcFactoryMock.create(EasyMock.anyObject(DatastoreOptions.class))) @@ -639,7 +637,7 @@ public void testGetArrayNoDeferredResults() { assertFalse(result.hasNext()); } - public void testGetArrayDeferredResults() throws DatastoreRpcException { + public void testGetArrayDeferredResults() throws DatastoreException { Set requestedKeys = new HashSet<>(); requestedKeys.add(KEY1); requestedKeys.add(KEY2); @@ -654,7 +652,7 @@ public void testGetArrayDeferredResults() throws DatastoreRpcException { assertEquals(requestedKeys, keysOfFoundEntities); } - public void testFetchArrayDeferredResults() throws DatastoreRpcException { + public void testFetchArrayDeferredResults() throws DatastoreException { List foundEntities = createDatastoreForDeferredLookup().fetch(KEY1, KEY2, KEY3, KEY4, KEY5); assertEquals(foundEntities.get(0).key(), KEY1); @@ -665,7 +663,7 @@ public void testFetchArrayDeferredResults() throws DatastoreRpcException { assertEquals(foundEntities.size(), 5); } - private Datastore createDatastoreForDeferredLookup() throws DatastoreRpcException { + private Datastore createDatastoreForDeferredLookup() throws DatastoreException { List keysPb = new ArrayList<>(); keysPb.add(KEY1.toPb()); keysPb.add(KEY2.toPb()); @@ -821,7 +819,7 @@ public void testRetryableException() throws Exception { EasyMock.expect(rpcFactoryMock.create(EasyMock.anyObject(DatastoreOptions.class))) .andReturn(rpcMock); EasyMock.expect(rpcMock.lookup(requestPb)) - .andThrow(new DatastoreRpc.DatastoreRpcException(Reason.UNAVAILABLE, null)) + .andThrow(new DatastoreException(503, "UNAVAILABLE", "UNAVAILABLE", null)) .andReturn(responsePb); EasyMock.replay(rpcFactoryMock, rpcMock); DatastoreOptions options = this.options.toBuilder() @@ -843,7 +841,8 @@ public void testNonRetryableException() throws Exception { EasyMock.expect(rpcFactoryMock.create(EasyMock.anyObject(DatastoreOptions.class))) .andReturn(rpcMock); EasyMock.expect(rpcMock.lookup(requestPb)) - .andThrow(new DatastoreRpc.DatastoreRpcException(Reason.PERMISSION_DENIED, null)) + .andThrow( + new DatastoreException(DatastoreException.UNKNOWN_CODE, "denied", "PERMISSION_DENIED")) .times(1); EasyMock.replay(rpcFactoryMock, rpcMock); RetryParams retryParams = RetryParams.builder().retryMinAttempts(2).build(); @@ -853,7 +852,7 @@ public void testNonRetryableException() throws Exception { .build(); Datastore datastore = options.service(); thrown.expect(DatastoreException.class); - thrown.expectMessage(Reason.PERMISSION_DENIED.description()); + thrown.expectMessage("denied"); datastore.get(KEY1); EasyMock.verify(rpcFactoryMock, rpcMock); } diff --git a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerException.java b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerException.java index 22b5e8bfed7c..3510f7728a8f 100644 --- a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerException.java +++ b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerException.java @@ -16,11 +16,14 @@ package com.google.gcloud.resourcemanager; +import com.google.common.collect.ImmutableSet; import com.google.gcloud.BaseServiceException; -import com.google.gcloud.RetryHelper; import com.google.gcloud.RetryHelper.RetryHelperException; import com.google.gcloud.RetryHelper.RetryInterruptedException; +import java.io.IOException; +import java.util.Set; + /** * Resource Manager service exception. * @@ -29,11 +32,32 @@ */ public class ResourceManagerException extends BaseServiceException { - private static final long serialVersionUID = 6841689911565501705L; - private static final int UNKNOWN_CODE = -1; + // see https://cloud.google.com/resource-manager/v1/errors/core_errors + private static final Set RETRYABLE_ERRORS = ImmutableSet.of( + new Error(503, null), + new Error(500, null), + new Error(429, null), + new Error(403, "concurrentLimitExceeded"), + new Error(403, "limitExceeded"), + new Error(403, "rateLimitExceeded"), + new Error(403, "rateLimitExceededUnreg"), + new Error(403, "servingLimitExceeded"), + new Error(403, "userRateLimitExceeded"), + new Error(403, "userRateLimitExceededUnreg"), + new Error(403, "variableTermLimitExceeded")); + private static final long serialVersionUID = -9207194488966554136L; + + public ResourceManagerException(int code, String message) { + super(code, message, null, true); + } + + public ResourceManagerException(IOException exception) { + super(exception, true); + } - public ResourceManagerException(int code, String message, boolean retryable) { - super(code, message, retryable); + @Override + protected Set retryableErrors() { + return RETRYABLE_ERRORS; } /** @@ -44,13 +68,8 @@ public ResourceManagerException(int code, String message, boolean retryable) { * ResourceManagerException} * @throws RetryInterruptedException when {@code ex} is a {@code RetryInterruptedException} */ - static ResourceManagerException translateAndThrow(RetryHelperException ex) { - if (ex.getCause() instanceof ResourceManagerException) { - throw (ResourceManagerException) ex.getCause(); - } - if (ex instanceof RetryHelper.RetryInterruptedException) { - RetryHelper.RetryInterruptedException.propagate(); - } - throw new ResourceManagerException(UNKNOWN_CODE, ex.getMessage(), false); + public static ResourceManagerException translateAndThrow(RetryHelperException ex) { + BaseServiceException.translateAndThrow(ex); + throw new ResourceManagerException(UNKNOWN_CODE, ex.getMessage()); } } diff --git a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/spi/DefaultResourceManagerRpc.java b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/spi/DefaultResourceManagerRpc.java index ec95207c2e7b..61c622fa0c33 100644 --- a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/spi/DefaultResourceManagerRpc.java +++ b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/spi/DefaultResourceManagerRpc.java @@ -7,30 +7,20 @@ import static java.net.HttpURLConnection.HTTP_FORBIDDEN; import static java.net.HttpURLConnection.HTTP_NOT_FOUND; -import com.google.api.client.googleapis.json.GoogleJsonError; -import com.google.api.client.googleapis.json.GoogleJsonResponseException; import com.google.api.client.http.HttpRequestInitializer; import com.google.api.client.http.HttpTransport; import com.google.api.client.json.jackson.JacksonFactory; import com.google.api.services.cloudresourcemanager.Cloudresourcemanager; import com.google.api.services.cloudresourcemanager.model.ListProjectsResponse; import com.google.api.services.cloudresourcemanager.model.Project; -import com.google.common.collect.ImmutableSet; import com.google.gcloud.resourcemanager.ResourceManagerException; import com.google.gcloud.resourcemanager.ResourceManagerOptions; import java.io.IOException; import java.util.Map; -import java.util.Set; public class DefaultResourceManagerRpc implements ResourceManagerRpc { - // see https://cloud.google.com/resource-manager/v1/errors/core_errors - private static final Set RETRYABLE_CODES = ImmutableSet.of(503, 500, 429); - private static final Set RETRYABLE_REASONS = ImmutableSet.of("concurrentLimitExceeded", - "limitExceeded", "rateLimitExceeded", "rateLimitExceededUnreg", "servingLimitExceeded", - "userRateLimitExceeded", "userRateLimitExceededUnreg", "variableTermLimitExceeded"); - private final Cloudresourcemanager resourceManager; public DefaultResourceManagerRpc(ResourceManagerOptions options) { @@ -44,21 +34,7 @@ public DefaultResourceManagerRpc(ResourceManagerOptions options) { } private static ResourceManagerException translate(IOException exception) { - ResourceManagerException translated; - if (exception instanceof GoogleJsonResponseException) { - translated = translate(((GoogleJsonResponseException) exception).getDetails()); - } else { - translated = new ResourceManagerException(0, exception.getMessage(), false); - } - translated.initCause(exception); - return translated; - } - - private static ResourceManagerException translate(GoogleJsonError exception) { - boolean retryable = - RETRYABLE_CODES.contains(exception.getCode()) || (!exception.getErrors().isEmpty() - && RETRYABLE_REASONS.contains(exception.getErrors().get(0).getReason())); - return new ResourceManagerException(exception.getCode(), exception.getMessage(), retryable); + return new ResourceManagerException(exception); } @Override diff --git a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerExceptionTest.java b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerExceptionTest.java new file mode 100644 index 000000000000..388f38f31c35 --- /dev/null +++ b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerExceptionTest.java @@ -0,0 +1,94 @@ +/* + * Copyright 2015 Google Inc. All Rights Reserved. + * + * 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.google.gcloud.resourcemanager; + +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import com.google.gcloud.BaseServiceException; +import com.google.gcloud.RetryHelper.RetryHelperException; + +import org.junit.Test; + +import java.io.IOException; +import java.net.SocketTimeoutException; + +public class ResourceManagerExceptionTest { + + @Test + public void testResourceManagerException() { + ResourceManagerException exception = new ResourceManagerException(500, "message"); + assertEquals(500, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new ResourceManagerException(503, "message"); + assertEquals(503, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new ResourceManagerException(429, "message"); + assertEquals(429, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new ResourceManagerException(403, "message"); + assertEquals(403, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertFalse(exception.retryable()); + assertTrue(exception.idempotent()); + + IOException cause = new SocketTimeoutException(); + exception = new ResourceManagerException(cause); + assertNull(exception.reason()); + assertNull(exception.getMessage()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + assertEquals(cause, exception.getCause()); + } + + @Test + public void testTranslateAndThrow() throws Exception { + ResourceManagerException cause = new ResourceManagerException(503, "message"); + RetryHelperException exceptionMock = createMock(RetryHelperException.class); + expect(exceptionMock.getCause()).andReturn(cause).times(2); + replay(exceptionMock); + try { + ResourceManagerException.translateAndThrow(exceptionMock); + } catch (BaseServiceException ex) { + assertEquals(503, ex.code()); + assertEquals("message", ex.getMessage()); + assertTrue(ex.retryable()); + assertTrue(ex.idempotent()); + } finally { + verify(exceptionMock); + } + } +} diff --git a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java index fedd10eacdc6..7d1e00496463 100644 --- a/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java +++ b/gcloud-java-resourcemanager/src/test/java/com/google/gcloud/resourcemanager/ResourceManagerImplTest.java @@ -273,7 +273,7 @@ public void testRetryableException() { .build() .service(); EasyMock.expect(resourceManagerRpcMock.get(PARTIAL_PROJECT.projectId(), EMPTY_RPC_OPTIONS)) - .andThrow(new ResourceManagerException(500, "Internal Error", true)) + .andThrow(new ResourceManagerException(500, "Internal Error")) .andReturn(PARTIAL_PROJECT.toPb()); EasyMock.replay(resourceManagerRpcMock); ProjectInfo returnedProject = resourceManagerMock.get(PARTIAL_PROJECT.projectId()); @@ -293,7 +293,7 @@ public void testNonRetryableException() { .service(); EasyMock.expect(resourceManagerRpcMock.get(PARTIAL_PROJECT.projectId(), EMPTY_RPC_OPTIONS)) .andThrow(new ResourceManagerException( - 403, "Project " + PARTIAL_PROJECT.projectId() + " not found.", false)) + 403, "Project " + PARTIAL_PROJECT.projectId() + " not found.")) .once(); EasyMock.replay(resourceManagerRpcMock); thrown.expect(ResourceManagerException.class); diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java b/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java index 29fdc651af9f..dc84a1de5559 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/spi/DefaultStorageRpc.java @@ -34,7 +34,6 @@ import com.google.api.client.googleapis.batch.json.JsonBatchCallback; import com.google.api.client.googleapis.json.GoogleJsonError; -import com.google.api.client.googleapis.json.GoogleJsonResponseException; import com.google.api.client.http.ByteArrayContent; import com.google.api.client.http.GenericUrl; import com.google.api.client.http.HttpHeaders; @@ -59,7 +58,6 @@ import com.google.api.services.storage.model.StorageObject; import com.google.common.base.MoreObjects; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.gcloud.storage.StorageException; @@ -68,12 +66,10 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; -import java.net.SocketTimeoutException; import java.util.ArrayList; import java.util.Iterator; import java.util.List; import java.util.Map; -import java.util.Set; public class DefaultStorageRpc implements StorageRpc { @@ -81,8 +77,6 @@ public class DefaultStorageRpc implements StorageRpc { private final StorageOptions options; private final Storage storage; - // see: https://cloud.google.com/storage/docs/concepts-techniques#practices - private static final Set RETRYABLE_CODES = ImmutableSet.of(504, 503, 502, 500, 429, 408); private static final long MEGABYTE = 1024L * 1024L; private static final int MAX_BATCH_DELETES = 100; @@ -97,25 +91,11 @@ public DefaultStorageRpc(StorageOptions options) { } private static StorageException translate(IOException exception) { - StorageException translated; - if (exception instanceof GoogleJsonResponseException - && ((GoogleJsonResponseException) exception).getDetails() != null) { - translated = translate(((GoogleJsonResponseException) exception).getDetails()); - } else { - boolean retryable = false; - if (exception instanceof SocketTimeoutException) { - retryable = true; - } - translated = new StorageException(0, exception.getMessage(), retryable); - } - translated.initCause(exception); - return translated; + return new StorageException(exception); } private static StorageException translate(GoogleJsonError exception) { - boolean retryable = RETRYABLE_CODES.contains(exception.getCode()) - || "InternalError".equals(exception.getMessage()); - return new StorageException(exception.getCode(), exception.getMessage(), retryable); + return new StorageException(exception); } @Override diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/BlobReadChannel.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/BlobReadChannel.java index 984f5d1f72e9..121f2eb63589 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/BlobReadChannel.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/BlobReadChannel.java @@ -129,7 +129,7 @@ public Tuple call() { if (lastEtag != null && !Objects.equals(result.x(), lastEtag)) { StringBuilder messageBuilder = new StringBuilder(); messageBuilder.append("Blob ").append(blob).append(" was updated while reading"); - throw new StorageException(0, messageBuilder.toString(), false); + throw new StorageException(0, messageBuilder.toString()); } lastEtag = result.x(); buffer = result.y(); diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageException.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageException.java index c1075ae28c8b..e724d8ac6850 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageException.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageException.java @@ -16,11 +16,15 @@ package com.google.gcloud.storage; +import com.google.api.client.googleapis.json.GoogleJsonError; +import com.google.common.collect.ImmutableSet; import com.google.gcloud.BaseServiceException; -import com.google.gcloud.RetryHelper; import com.google.gcloud.RetryHelper.RetryHelperException; import com.google.gcloud.RetryHelper.RetryInterruptedException; +import java.io.IOException; +import java.util.Set; + /** * Storage service exception. * @@ -29,11 +33,33 @@ */ public class StorageException extends BaseServiceException { - private static final long serialVersionUID = 8088235105953640145L; - private static final int UNKNOWN_CODE = -1; + // see: https://cloud.google.com/storage/docs/concepts-techniques#practices + private static final Set RETRYABLE_ERRORS = ImmutableSet.of( + new Error(504, null), + new Error(503, null), + new Error(502, null), + new Error(500, null), + new Error(429, null), + new Error(408, null), + new Error(null, "internalError")); + + private static final long serialVersionUID = -4168430271327813063L; + + public StorageException(int code, String message) { + super(code, message, null, true); + } + + public StorageException(IOException exception) { + super(exception, true); + } + + public StorageException(GoogleJsonError error) { + super(error, true); + } - public StorageException(int code, String message, boolean retryable) { - super(code, message, retryable); + @Override + protected Set retryableErrors() { + return RETRYABLE_ERRORS; } /** @@ -43,13 +69,8 @@ public StorageException(int code, String message, boolean retryable) { * @throws StorageException when {@code ex} was caused by a {@code StorageException} * @throws RetryInterruptedException when {@code ex} is a {@code RetryInterruptedException} */ - static StorageException translateAndThrow(RetryHelperException ex) { - if (ex.getCause() instanceof StorageException) { - throw (StorageException) ex.getCause(); - } - if (ex instanceof RetryHelper.RetryInterruptedException) { - RetryHelper.RetryInterruptedException.propagate(); - } - throw new StorageException(UNKNOWN_CODE, ex.getMessage(), false); + public static StorageException translateAndThrow(RetryHelperException ex) { + BaseServiceException.translateAndThrow(ex); + throw new StorageException(UNKNOWN_CODE, ex.getMessage()); } } diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelperTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelperTest.java index 05b7f5f6fd8c..d06f004fe84c 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelperTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/RemoteGcsHelperTest.java @@ -69,8 +69,8 @@ public class RemoteGcsHelperTest { private static final List BLOB_LIST = ImmutableList.of( BlobInfo.builder(BUCKET_NAME, "n1").build(), BlobInfo.builder(BUCKET_NAME, "n2").build()); - private static final StorageException RETRYABLE_EXCEPTION = new StorageException(409, "", true); - private static final StorageException FATAL_EXCEPTION = new StorageException(500, "", false); + private static final StorageException RETRYABLE_EXCEPTION = new StorageException(409, ""); + private static final StorageException FATAL_EXCEPTION = new StorageException(500, ""); private static final Page BLOB_PAGE = new Page() { @Override diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageExceptionTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageExceptionTest.java new file mode 100644 index 000000000000..cf1d4b394e57 --- /dev/null +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageExceptionTest.java @@ -0,0 +1,125 @@ +/* + * Copyright 2015 Google Inc. All Rights Reserved. + * + * 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.google.gcloud.storage; + +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.verify; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import com.google.api.client.googleapis.json.GoogleJsonError; +import com.google.gcloud.BaseServiceException; +import com.google.gcloud.RetryHelper.RetryHelperException; + +import org.junit.Test; + +import java.io.IOException; +import java.net.SocketTimeoutException; + +public class StorageExceptionTest { + + @Test + public void testStorageException() { + StorageException exception = new StorageException(500, "message"); + assertEquals(500, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new StorageException(502, "message"); + assertEquals(502, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new StorageException(503, "message"); + assertEquals(503, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new StorageException(504, "message"); + assertEquals(504, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new StorageException(429, "message"); + assertEquals(429, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new StorageException(408, "message"); + assertEquals(408, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + + exception = new StorageException(400, "message"); + assertEquals(400, exception.code()); + assertEquals("message", exception.getMessage()); + assertNull(exception.reason()); + assertFalse(exception.retryable()); + assertTrue(exception.idempotent()); + + IOException cause = new SocketTimeoutException(); + exception = new StorageException(cause); + assertNull(exception.reason()); + assertNull(exception.getMessage()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + assertEquals(cause, exception.getCause()); + + GoogleJsonError error = new GoogleJsonError(); + error.setCode(503); + error.setMessage("message"); + exception = new StorageException(error); + assertEquals(503, exception.code()); + assertEquals("message", exception.getMessage()); + assertTrue(exception.retryable()); + assertTrue(exception.idempotent()); + } + + @Test + public void testTranslateAndThrow() throws Exception { + StorageException cause = new StorageException(503, "message"); + RetryHelperException exceptionMock = createMock(RetryHelperException.class); + expect(exceptionMock.getCause()).andReturn(cause).times(2); + replay(exceptionMock); + try { + StorageException.translateAndThrow(exceptionMock); + } catch (BaseServiceException ex) { + assertEquals(503, ex.code()); + assertEquals("message", ex.getMessage()); + assertTrue(ex.retryable()); + assertTrue(ex.idempotent()); + } finally { + verify(exceptionMock); + } + } +} diff --git a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java index 0e1f1a0b2f52..b8da0580cd4a 100644 --- a/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java +++ b/gcloud-java-storage/src/test/java/com/google/gcloud/storage/StorageImplTest.java @@ -1266,7 +1266,7 @@ public Tuple apply(StorageObject f) { public void testRetryableException() { BlobId blob = BlobId.of(BUCKET_NAME1, BLOB_NAME1); EasyMock.expect(storageRpcMock.get(blob.toPb(), EMPTY_RPC_OPTIONS)) - .andThrow(new StorageException(500, "InternalError", true)) + .andThrow(new StorageException(500, "internalError")) .andReturn(BLOB_INFO1.toPb()); EasyMock.replay(storageRpcMock); storage = options.toBuilder().retryParams(RetryParams.defaultInstance()).build().service(); @@ -1279,7 +1279,7 @@ public void testNonRetryableException() { BlobId blob = BlobId.of(BUCKET_NAME1, BLOB_NAME1); String exceptionMessage = "Not Implemented"; EasyMock.expect(storageRpcMock.get(blob.toPb(), EMPTY_RPC_OPTIONS)) - .andThrow(new StorageException(501, exceptionMessage, false)); + .andThrow(new StorageException(501, exceptionMessage)); EasyMock.replay(storageRpcMock); storage = options.toBuilder().retryParams(RetryParams.defaultInstance()).build().service(); thrown.expect(StorageException.class); From acf260ce07f7fdd0729682d74c15c5c88abf7c84 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Fri, 15 Jan 2016 18:20:26 +0100 Subject: [PATCH 2/5] Move exception handler and interceptor to BaseService class --- .../google/gcloud/bigquery/BigQueryImpl.java | 23 --------------- .../java/com/google/gcloud/BaseService.java | 25 +++++++++++++++++ .../gcloud/datastore/DatastoreImpl.java | 28 +------------------ .../resourcemanager/ResourceManagerImpl.java | 25 ----------------- .../google/gcloud/storage/StorageImpl.java | 22 --------------- 5 files changed, 26 insertions(+), 97 deletions(-) diff --git a/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/BigQueryImpl.java b/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/BigQueryImpl.java index 3a1cc658bef3..ad55056474fb 100644 --- a/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/BigQueryImpl.java +++ b/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/BigQueryImpl.java @@ -34,8 +34,6 @@ import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.gcloud.BaseService; -import com.google.gcloud.ExceptionHandler; -import com.google.gcloud.ExceptionHandler.Interceptor; import com.google.gcloud.Page; import com.google.gcloud.PageImpl; import com.google.gcloud.PageImpl.NextPageFetcher; @@ -49,27 +47,6 @@ final class BigQueryImpl extends BaseService implements BigQuery { - private static final Interceptor EXCEPTION_HANDLER_INTERCEPTOR = new Interceptor() { - - private static final long serialVersionUID = -7478333733015750774L; - - @Override - public RetryResult afterEval(Exception exception, RetryResult retryResult) { - return Interceptor.RetryResult.CONTINUE_EVALUATION; - } - - @Override - public RetryResult beforeEval(Exception exception) { - if (exception instanceof BigQueryException) { - boolean retriable = ((BigQueryException) exception).retryable(); - return retriable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.NO_RETRY; - } - return Interceptor.RetryResult.CONTINUE_EVALUATION; - } - }; - static final ExceptionHandler EXCEPTION_HANDLER = ExceptionHandler.builder() - .abortOn(RuntimeException.class).interceptor(EXCEPTION_HANDLER_INTERCEPTOR).build(); - private static class DatasetPageFetcher implements NextPageFetcher { private static final long serialVersionUID = -3057564042439021278L; diff --git a/gcloud-java-core/src/main/java/com/google/gcloud/BaseService.java b/gcloud-java-core/src/main/java/com/google/gcloud/BaseService.java index c028eaede331..d9e6f2db7c95 100644 --- a/gcloud-java-core/src/main/java/com/google/gcloud/BaseService.java +++ b/gcloud-java-core/src/main/java/com/google/gcloud/BaseService.java @@ -16,6 +16,8 @@ package com.google.gcloud; +import com.google.gcloud.ExceptionHandler.Interceptor; + /** * Base class for service objects. * @@ -24,6 +26,29 @@ public abstract class BaseService> implements Service { + public static final Interceptor EXCEPTION_HANDLER_INTERCEPTOR = new Interceptor() { + + private static final long serialVersionUID = -8429573486870467828L; + + @Override + public RetryResult afterEval(Exception exception, RetryResult retryResult) { + return Interceptor.RetryResult.CONTINUE_EVALUATION; + } + + @Override + public RetryResult beforeEval(Exception exception) { + if (exception instanceof BaseServiceException) { + boolean retriable = ((BaseServiceException) exception).retryable(); + return retriable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.NO_RETRY; + } + return Interceptor.RetryResult.CONTINUE_EVALUATION; + } + }; + public static final ExceptionHandler EXCEPTION_HANDLER = ExceptionHandler.builder() + .abortOn(RuntimeException.class) + .interceptor(EXCEPTION_HANDLER_INTERCEPTOR) + .build(); + private final OptionsT options; protected BaseService(OptionsT options) { diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreImpl.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreImpl.java index bfcba58f3f2f..92d18ed4787c 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreImpl.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreImpl.java @@ -23,8 +23,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Sets; import com.google.gcloud.BaseService; -import com.google.gcloud.ExceptionHandler; -import com.google.gcloud.ExceptionHandler.Interceptor; import com.google.gcloud.RetryHelper; import com.google.gcloud.RetryHelper.RetryHelperException; import com.google.gcloud.RetryParams; @@ -41,31 +39,7 @@ import java.util.Set; import java.util.concurrent.Callable; -final class DatastoreImpl extends BaseService - implements Datastore { - - private static final Interceptor EXCEPTION_HANDLER_INTERCEPTOR = - new Interceptor() { - - private static final long serialVersionUID = 6911242958397733203L; - - @Override - public RetryResult afterEval(Exception exception, RetryResult retryResult) { - return Interceptor.RetryResult.CONTINUE_EVALUATION; - } - - @Override - public RetryResult beforeEval(Exception exception) { - if (exception instanceof DatastoreException) { - boolean retryable = ((DatastoreException) exception).retryable(); - return retryable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.NO_RETRY; - } - return Interceptor.RetryResult.CONTINUE_EVALUATION; - } - }; - private static final ExceptionHandler EXCEPTION_HANDLER = ExceptionHandler.builder() - .abortOn(RuntimeException.class, DatastoreException.class) - .interceptor(EXCEPTION_HANDLER_INTERCEPTOR).build(); +final class DatastoreImpl extends BaseService implements Datastore { private final DatastoreRpc datastoreRpc; private final RetryParams retryParams; diff --git a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerImpl.java b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerImpl.java index 2a0e09d9fb31..22f2b350d2f3 100644 --- a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerImpl.java +++ b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerImpl.java @@ -25,8 +25,6 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import com.google.gcloud.BaseService; -import com.google.gcloud.ExceptionHandler; -import com.google.gcloud.ExceptionHandler.Interceptor; import com.google.gcloud.Page; import com.google.gcloud.PageImpl; import com.google.gcloud.PageImpl.NextPageFetcher; @@ -40,29 +38,6 @@ final class ResourceManagerImpl extends BaseService implements ResourceManager { - private static final Interceptor EXCEPTION_HANDLER_INTERCEPTOR = new Interceptor() { - - private static final long serialVersionUID = 2091576149969931704L; - - @Override - public RetryResult afterEval(Exception exception, RetryResult retryResult) { - return Interceptor.RetryResult.CONTINUE_EVALUATION; - } - - @Override - public RetryResult beforeEval(Exception exception) { - if (exception instanceof ResourceManagerException) { - boolean retriable = ((ResourceManagerException) exception).retryable(); - return retriable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.NO_RETRY; - } - return Interceptor.RetryResult.CONTINUE_EVALUATION; - } - }; - static final ExceptionHandler EXCEPTION_HANDLER = ExceptionHandler.builder() - .abortOn(RuntimeException.class) - .interceptor(EXCEPTION_HANDLER_INTERCEPTOR) - .build(); - private final ResourceManagerRpc resourceManagerRpc; ResourceManagerImpl(ResourceManagerOptions options) { diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java index a6c851d0f638..a4b6c56e5ede 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageImpl.java @@ -44,8 +44,6 @@ import com.google.common.primitives.Ints; import com.google.gcloud.AuthCredentials.ServiceAccountAuthCredentials; import com.google.gcloud.BaseService; -import com.google.gcloud.ExceptionHandler; -import com.google.gcloud.ExceptionHandler.Interceptor; import com.google.gcloud.Page; import com.google.gcloud.PageImpl; import com.google.gcloud.PageImpl.NextPageFetcher; @@ -76,26 +74,6 @@ final class StorageImpl extends BaseService implements Storage { - private static final Interceptor EXCEPTION_HANDLER_INTERCEPTOR = new Interceptor() { - - private static final long serialVersionUID = -7758580330857881124L; - - @Override - public RetryResult afterEval(Exception exception, RetryResult retryResult) { - return Interceptor.RetryResult.CONTINUE_EVALUATION; - } - - @Override - public RetryResult beforeEval(Exception exception) { - if (exception instanceof StorageException) { - boolean retriable = ((StorageException) exception).retryable(); - return retriable ? Interceptor.RetryResult.RETRY : Interceptor.RetryResult.NO_RETRY; - } - return Interceptor.RetryResult.CONTINUE_EVALUATION; - } - }; - static final ExceptionHandler EXCEPTION_HANDLER = ExceptionHandler.builder() - .abortOn(RuntimeException.class).interceptor(EXCEPTION_HANDLER_INTERCEPTOR).build(); private static final byte[] EMPTY_BYTE_ARRAY = {}; private static final String EMPTY_BYTE_ARRAY_MD5 = "1B2M2Y8AsgTpgAmY7PhCfg=="; private static final String EMPTY_BYTE_ARRAY_CRC32C = "AAAAAA=="; From 67e5dfce1f7d6903fc75a67d472c85ebb900258e Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Wed, 20 Jan 2016 07:40:53 +0100 Subject: [PATCH 3/5] Rename translateAndThrow in BaseServiceException and make it package scope in other exceptions --- .../com/google/gcloud/bigquery/BigQueryException.java | 4 ++-- .../main/java/com/google/gcloud/BaseServiceException.java | 4 +--- .../java/com/google/gcloud/BaseServiceExceptionTest.java | 8 +++----- .../com/google/gcloud/datastore/DatastoreException.java | 4 ++-- .../gcloud/resourcemanager/ResourceManagerException.java | 4 ++-- .../java/com/google/gcloud/storage/StorageException.java | 4 ++-- 6 files changed, 12 insertions(+), 16 deletions(-) diff --git a/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/BigQueryException.java b/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/BigQueryException.java index 930d06d523ab..b0cca68e3e0a 100644 --- a/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/BigQueryException.java +++ b/gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/BigQueryException.java @@ -76,8 +76,8 @@ protected Set retryableErrors() { * @throws BigQueryException when {@code ex} was caused by a {@code BigQueryException} * @throws RetryInterruptedException when {@code ex} is a {@code RetryInterruptedException} */ - public static BigQueryException translateAndThrow(RetryHelperException ex) { - BaseServiceException.translateAndThrow(ex); + static BaseServiceException translateAndThrow(RetryHelperException ex) { + BaseServiceException.translateAndPropagateIfPossible(ex); throw new BigQueryException(UNKNOWN_CODE, ex.getMessage()); } } diff --git a/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java b/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java index 9f4bfdab994d..351ad6cd188a 100644 --- a/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java +++ b/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java @@ -184,14 +184,12 @@ protected static String message(IOException exception) { return exception.getMessage(); } - protected static BaseServiceException translateAndThrow( - RetryHelper.RetryHelperException ex) { + protected static void translateAndPropagateIfPossible(RetryHelper.RetryHelperException ex) { if (ex.getCause() instanceof BaseServiceException) { throw (BaseServiceException) ex.getCause(); } if (ex instanceof RetryHelper.RetryInterruptedException) { RetryHelper.RetryInterruptedException.propagate(); } - return null; } } diff --git a/gcloud-java-core/src/test/java/com/google/gcloud/BaseServiceExceptionTest.java b/gcloud-java-core/src/test/java/com/google/gcloud/BaseServiceExceptionTest.java index a6e22866ed9f..e3c6abb7d1ee 100644 --- a/gcloud-java-core/src/test/java/com/google/gcloud/BaseServiceExceptionTest.java +++ b/gcloud-java-core/src/test/java/com/google/gcloud/BaseServiceExceptionTest.java @@ -137,14 +137,12 @@ public void testBaseServiceException() { @Test public void testTranslateAndThrow() throws Exception { BaseServiceException cause = new BaseServiceException(CODE, MESSAGE, REASON, IDEMPOTENT); - RetryHelper.RetryHelperException exceptionMock = createMock(RetryHelper.RetryHelperException.class); + RetryHelper.RetryHelperException exceptionMock = + createMock(RetryHelper.RetryHelperException.class); expect(exceptionMock.getCause()).andReturn(cause).times(2); replay(exceptionMock); try { - BaseServiceException ex = BaseServiceException.translateAndThrow(exceptionMock); - if (ex != null) { - throw ex; - } + BaseServiceException.translateAndPropagateIfPossible(exceptionMock); } catch (BaseServiceException ex) { assertEquals(CODE, ex.code()); assertEquals(MESSAGE, ex.getMessage()); diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java index 946fc9190fc3..a7e6785b8a8c 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java @@ -63,8 +63,8 @@ protected Set retryableErrors() { * @throws DatastoreException when {@code ex} was caused by a {@code DatastoreException} * @throws RetryInterruptedException when {@code ex} is a {@code RetryInterruptedException} */ - public static DatastoreException translateAndThrow(RetryHelperException ex) { - BaseServiceException.translateAndThrow(ex); + static DatastoreException translateAndThrow(RetryHelperException ex) { + BaseServiceException.translateAndPropagateIfPossible(ex); throw new DatastoreException(UNKNOWN_CODE, ex.getMessage(), null); } diff --git a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerException.java b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerException.java index 3510f7728a8f..32a2998791c9 100644 --- a/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerException.java +++ b/gcloud-java-resourcemanager/src/main/java/com/google/gcloud/resourcemanager/ResourceManagerException.java @@ -68,8 +68,8 @@ protected Set retryableErrors() { * ResourceManagerException} * @throws RetryInterruptedException when {@code ex} is a {@code RetryInterruptedException} */ - public static ResourceManagerException translateAndThrow(RetryHelperException ex) { - BaseServiceException.translateAndThrow(ex); + static ResourceManagerException translateAndThrow(RetryHelperException ex) { + BaseServiceException.translateAndPropagateIfPossible(ex); throw new ResourceManagerException(UNKNOWN_CODE, ex.getMessage()); } } diff --git a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageException.java b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageException.java index e724d8ac6850..0c952c9a65d6 100644 --- a/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageException.java +++ b/gcloud-java-storage/src/main/java/com/google/gcloud/storage/StorageException.java @@ -69,8 +69,8 @@ protected Set retryableErrors() { * @throws StorageException when {@code ex} was caused by a {@code StorageException} * @throws RetryInterruptedException when {@code ex} is a {@code RetryInterruptedException} */ - public static StorageException translateAndThrow(RetryHelperException ex) { - BaseServiceException.translateAndThrow(ex); + static StorageException translateAndThrow(RetryHelperException ex) { + BaseServiceException.translateAndPropagateIfPossible(ex); throw new StorageException(UNKNOWN_CODE, ex.getMessage()); } } From 34e6806becf14f1b9bf0aeac1f7cc11d2aad95b1 Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Wed, 20 Jan 2016 07:55:39 +0100 Subject: [PATCH 4/5] Add throwable cause to DatastoreException --- .../src/main/java/com/google/gcloud/BaseServiceException.java | 2 +- .../java/com/google/gcloud/datastore/DatastoreException.java | 2 +- .../main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java b/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java index 351ad6cd188a..b91090f94e82 100644 --- a/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java +++ b/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java @@ -105,7 +105,7 @@ public BaseServiceException(int code, String message, String reason, boolean ide } public BaseServiceException(int code, String message, String reason, boolean idempotent, - Exception cause) { + Throwable cause) { super(message, cause); this.code = code; this.reason = reason; diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java index a7e6785b8a8c..ecad69ac635b 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/datastore/DatastoreException.java @@ -39,7 +39,7 @@ public class DatastoreException extends BaseServiceException { new Error(503, "UNAVAILABLE")); private static final long serialVersionUID = 2663750991205874435L; - public DatastoreException(int code, String message, String reason, Exception cause) { + public DatastoreException(int code, String message, String reason, Throwable cause) { super(code, message, reason, true, cause); } diff --git a/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java b/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java index ac00d94692de..c82ff9689f68 100644 --- a/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java +++ b/gcloud-java-datastore/src/main/java/com/google/gcloud/spi/DefaultDatastoreRpc.java @@ -107,7 +107,7 @@ private static DatastoreException translate( return new DatastoreException((IOException) exception.getCause()); } } - return new DatastoreException(code, message, reason); + return new DatastoreException(code, message, reason, exception); } @Override From 3c81f1679e7f6880d9f3c4a9b89a5831c90fb02f Mon Sep 17 00:00:00 2001 From: Marco Ziccardi Date: Wed, 20 Jan 2016 09:28:40 +0100 Subject: [PATCH 5/5] Move isRetryable method to BaseServiceException.Error --- .../google/gcloud/BaseServiceException.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java b/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java index b91090f94e82..0222a0d2258c 100644 --- a/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java +++ b/gcloud-java-core/src/main/java/com/google/gcloud/BaseServiceException.java @@ -58,6 +58,16 @@ public String reason() { return reason; } + boolean isRetryable(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 false; + } + @Override public String toString() { return MoreObjects.toStringHelper(this).add("code", code).add("reason", reason).toString(); @@ -83,7 +93,7 @@ public BaseServiceException(IOException exception, boolean idempotent) { Error error = error(((GoogleJsonResponseException) exception).getDetails()); this.code = error.code; this.reason = error.reason; - this.retryable = isRetryable(error); + this.retryable = error.isRetryable(retryableErrors()); } else { this.code = UNKNOWN_CODE; this.reason = null; @@ -110,7 +120,7 @@ public BaseServiceException(int code, String message, String reason, boolean ide this.code = code; this.reason = reason; this.idempotent = idempotent; - this.retryable = idempotent && isRetryable(new Error(code, reason)); + this.retryable = idempotent && new Error(code, reason).isRetryable(retryableErrors()); } protected Set retryableErrors() { @@ -118,7 +128,7 @@ protected Set retryableErrors() { } protected boolean isRetryable(GoogleJsonError error) { - return error != null && isRetryable(error(error)); + return error != null && error(error).isRetryable(retryableErrors()); } protected boolean isRetryable(IOException exception) { @@ -128,16 +138,6 @@ protected boolean isRetryable(IOException exception) { return exception instanceof SocketTimeoutException; } - protected boolean isRetryable(Error error) { - for (Error retryableError : retryableErrors()) { - if ((retryableError.code() == null || retryableError.code().equals(error.code())) - && (retryableError.reason() == null || retryableError.reason().equals(error.reason()))) { - return true; - } - } - return false; - } - /** * Returns the code associated with this exception. */