diff --git a/google-cloud-datastore/clirr-ignored-differences.xml b/google-cloud-datastore/clirr-ignored-differences.xml index 018afb17e..62459c953 100644 --- a/google-cloud-datastore/clirr-ignored-differences.xml +++ b/google-cloud-datastore/clirr-ignored-differences.xml @@ -26,4 +26,16 @@ com.google.cloud.datastore.AggregationResults runAggregation(com.google.cloud.datastore.AggregationQuery) 7012 + + + com/google/cloud/datastore/DatastoreException + 5000 + com/google/cloud/grpc/BaseGrpcServiceException + + + + com/google/cloud/datastore/DatastoreException + 5001 + com/google/cloud/http/BaseHttpServiceException + diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java index 512d0a3dc..5d01c3b9d 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/DatastoreException.java @@ -16,11 +16,16 @@ package com.google.cloud.datastore; +import static com.google.cloud.BaseServiceException.isRetryable; + +import com.google.api.gax.rpc.ApiException; +import com.google.api.gax.rpc.ErrorDetails; import com.google.cloud.BaseServiceException; import com.google.cloud.RetryHelper.RetryHelperException; -import com.google.cloud.http.BaseHttpServiceException; +import com.google.cloud.grpc.BaseGrpcServiceException; import com.google.common.collect.ImmutableSet; import java.io.IOException; +import java.util.Map; import java.util.Set; /** @@ -29,7 +34,7 @@ * @see Google Cloud * Datastore error codes */ -public final class DatastoreException extends BaseHttpServiceException { +public final class DatastoreException extends BaseGrpcServiceException { // see https://cloud.google.com/datastore/docs/concepts/errors#Error_Codes" private static final Set RETRYABLE_ERRORS = @@ -38,22 +43,96 @@ public final class DatastoreException extends BaseHttpServiceException { new Error(4, "DEADLINE_EXCEEDED", false), new Error(14, "UNAVAILABLE", true)); private static final long serialVersionUID = 2663750991205874435L; + private String reason; + private ApiException apiException; public DatastoreException(int code, String message, String reason) { this(code, message, reason, true, null); + this.reason = reason; } public DatastoreException(int code, String message, String reason, Throwable cause) { - super(code, message, reason, true, RETRYABLE_ERRORS, cause); + super(message, cause, code, isRetryable(code, reason, true, RETRYABLE_ERRORS)); + this.reason = reason; } public DatastoreException( int code, String message, String reason, boolean idempotent, Throwable cause) { - super(code, message, reason, idempotent, RETRYABLE_ERRORS, cause); + super(message, cause, code, isRetryable(code, reason, idempotent, RETRYABLE_ERRORS)); + this.reason = reason; } public DatastoreException(IOException exception) { - super(exception, true, RETRYABLE_ERRORS); + super(exception, true); + } + + public DatastoreException(ApiException apiException) { + super(apiException); + this.apiException = apiException; + } + + /** + * Checks the underlying reason of the exception and if it's {@link ApiException} then return the + * specific domain otherwise null. + * + * @see Domain + * @return the logical grouping to which the "reason" belongs. + */ + public String getDomain() { + if (this.apiException != null) { + return this.apiException.getDomain(); + } + return null; + } + + /** + * Checks the underlying reason of the exception and if it's {@link ApiException} then return a + * map of key-value pairs otherwise null. + * + * @see Metadata + * @return the map of additional structured details about an error. + */ + public Map getMetadata() { + if (this.apiException != null) { + return this.apiException.getMetadata(); + } + return null; + } + + /** + * Checks the underlying reason of the exception and if it's {@link ApiException} then return the + * ErrorDetails otherwise null. + * + * @see Status + * @see Error + * Details + * @return An object containing getters for structured objects from error_details.proto. + */ + public ErrorDetails getErrorDetails() { + if (this.apiException != null) { + return this.apiException.getErrorDetails(); + } + return null; + } + + /** + * Checks the underlying reason of the exception and if it's {@link ApiException} then return the + * reason otherwise null/custom reason. + * + * @see Reason + * @return the reason of an error. + */ + @Override + public String getReason() { + if (this.apiException != null) { + return this.apiException.getReason(); + } + return this.reason; } /** @@ -64,6 +143,9 @@ public DatastoreException(IOException exception) { */ static DatastoreException translateAndThrow(RetryHelperException ex) { BaseServiceException.translate(ex); + if (ex.getCause() instanceof ApiException) { + throw new DatastoreException((ApiException) ex.getCause()); + } throw new DatastoreException(UNKNOWN_CODE, ex.getMessage(), null, ex.getCause()); } diff --git a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java index fe2b2f27b..b4c83da89 100644 --- a/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java +++ b/google-cloud-datastore/src/main/java/com/google/cloud/datastore/spi/v1/GrpcDatastoreRpc.java @@ -185,8 +185,7 @@ private ClientContext getClientContext(DatastoreOptions datastoreOptions) throws settingsBuilder.setInternalHeaderProvider(internalHeaderProvider); settingsBuilder.setHeaderProvider( datastoreOptions.getMergedHeaderProvider(new NoHeaderProvider())); - ClientContext clientContext = ClientContext.create(settingsBuilder.build()); - return clientContext; + return ClientContext.create(settingsBuilder.build()); } private String getResourceToken(DatastoreOptions datastoreOptions) { diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java index 12b99c966..33f5ebb9c 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreExceptionTest.java @@ -16,6 +16,9 @@ package com.google.cloud.datastore; +import static com.google.common.truth.Truth.assertThat; +import static com.google.rpc.Code.FAILED_PRECONDITION; +import static java.util.Collections.singletonMap; import static org.easymock.EasyMock.createMock; import static org.easymock.EasyMock.expect; import static org.easymock.EasyMock.replay; @@ -24,11 +27,20 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import com.google.api.gax.grpc.GrpcStatusCode; +import com.google.api.gax.rpc.ApiException; +import com.google.api.gax.rpc.ApiExceptionFactory; +import com.google.api.gax.rpc.ErrorDetails; +import com.google.api.gax.rpc.StatusCode; import com.google.cloud.BaseServiceException; import com.google.cloud.RetryHelper; +import com.google.protobuf.Any; +import com.google.rpc.ErrorInfo; +import io.grpc.Status; import java.io.IOException; import java.net.SocketTimeoutException; import org.junit.Test; @@ -76,39 +88,78 @@ public void testDatastoreException() { assertEquals("message", exception.getMessage()); assertFalse(exception.isRetryable()); assertSame(cause, exception.getCause()); + + exception = new DatastoreException(2, "message", "INTERNAL", true, cause); + assertEquals(2, exception.getCode()); + assertEquals("INTERNAL", exception.getReason()); + assertEquals("message", exception.getMessage()); + assertFalse(exception.isRetryable()); + assertSame(cause, exception.getCause()); + + ApiException apiException = createApiException(); + exception = new DatastoreException(apiException); + assertEquals(400, exception.getCode()); + assertEquals("MISSING_INDEXES", exception.getReason()); + assertThat(exception.getMetadata()) + .isEqualTo(singletonMap("missing_indexes_url", "__some__url__")); + assertSame(apiException, exception.getCause()); + } + + @Test + public void testApiException() { + ApiException apiException = createApiException(); + DatastoreException datastoreException = new DatastoreException(apiException); + + assertThat(datastoreException.getReason()).isEqualTo("MISSING_INDEXES"); + assertThat(datastoreException.getDomain()).isEqualTo("datastore.googleapis.com"); + assertThat(datastoreException.getMetadata()) + .isEqualTo(singletonMap("missing_indexes_url", "__some__url__")); + assertThat(datastoreException.getErrorDetails()).isEqualTo(apiException.getErrorDetails()); } @Test public void testTranslateAndThrow() { Exception cause = new DatastoreException(14, "message", "UNAVAILABLE"); - RetryHelper.RetryHelperException exceptionMock = + final RetryHelper.RetryHelperException exceptionMock = createMock(RetryHelper.RetryHelperException.class); expect(exceptionMock.getCause()).andReturn(cause).times(2); replay(exceptionMock); - try { - DatastoreException.translateAndThrow(exceptionMock); - } catch (BaseServiceException ex) { - assertEquals(14, ex.getCode()); - assertEquals("message", ex.getMessage()); - assertTrue(ex.isRetryable()); - } finally { - verify(exceptionMock); - } + BaseServiceException ex = + assertThrows( + BaseServiceException.class, () -> DatastoreException.translateAndThrow(exceptionMock)); + assertEquals(14, ex.getCode()); + assertEquals("message", ex.getMessage()); + assertTrue(ex.isRetryable()); + verify(exceptionMock); + + cause = createApiException(); + final RetryHelper.RetryHelperException exceptionMock2 = + createMock(RetryHelper.RetryHelperException.class); + expect(exceptionMock2.getCause()).andReturn(cause).times(3); + replay(exceptionMock2); + DatastoreException ex2 = + assertThrows( + DatastoreException.class, () -> DatastoreException.translateAndThrow(exceptionMock2)); + assertThat(ex2.getReason()).isEqualTo("MISSING_INDEXES"); + assertThat(ex2.getDomain()).isEqualTo("datastore.googleapis.com"); + assertThat(ex2.getMetadata()).isEqualTo(singletonMap("missing_indexes_url", "__some__url__")); + assertThat(ex2.getErrorDetails()).isEqualTo(((ApiException) cause).getErrorDetails()); + verify(exceptionMock2); + cause = new IllegalArgumentException("message"); - exceptionMock = createMock(RetryHelper.RetryHelperException.class); - expect(exceptionMock.getMessage()).andReturn("message").times(1); - expect(exceptionMock.getCause()).andReturn(cause).times(2); - replay(exceptionMock); - try { - DatastoreException.translateAndThrow(exceptionMock); - } catch (BaseServiceException ex) { - assertEquals(DatastoreException.UNKNOWN_CODE, ex.getCode()); - assertEquals("message", ex.getMessage()); - assertFalse(ex.isRetryable()); - assertSame(cause, ex.getCause()); - } finally { - verify(exceptionMock); - } + final RetryHelper.RetryHelperException exceptionMock3 = + createMock(RetryHelper.RetryHelperException.class); + expect(exceptionMock3.getMessage()).andReturn("message").times(1); + expect(exceptionMock3.getCause()).andReturn(cause).times(3); + replay(exceptionMock3); + BaseServiceException ex3 = + assertThrows( + BaseServiceException.class, () -> DatastoreException.translateAndThrow(exceptionMock3)); + assertEquals(DatastoreException.UNKNOWN_CODE, ex3.getCode()); + assertEquals("message", ex3.getMessage()); + assertFalse(ex3.isRetryable()); + assertSame(cause, ex3.getCause()); + verify(exceptionMock3); } @Test @@ -121,4 +172,26 @@ public void testThrowInvalidRequest() { assertEquals("message a 1", ex.getMessage()); } } + + private ApiException createApiException() { + // Simulating google.rpc.Status with an ErrorInfo + ErrorInfo errorInfo = + ErrorInfo.newBuilder() + .setDomain("datastore.googleapis.com") + .setReason("MISSING_INDEXES") + .putMetadata("missing_indexes_url", "__some__url__") + .build(); + com.google.rpc.Status status = + com.google.rpc.Status.newBuilder() + .setCode(FAILED_PRECONDITION.getNumber()) + .setMessage("The query requires indexes.") + .addDetails(Any.pack(errorInfo)) + .build(); + + // Using Gax to convert to ApiException + StatusCode statusCode = GrpcStatusCode.of(Status.fromCodeValue(status.getCode()).getCode()); + ErrorDetails errorDetails = + ErrorDetails.builder().setRawErrorMessages(status.getDetailsList()).build(); + return ApiExceptionFactory.createException(null, statusCode, true, errorDetails); + } } diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTest.java index b1b36569a..5f42e2aeb 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/DatastoreTest.java @@ -16,6 +16,7 @@ package com.google.cloud.datastore; +import static com.google.api.gax.rpc.StatusCode.Code.ABORTED; import static com.google.cloud.datastore.ProtoTestData.intValue; import static com.google.cloud.datastore.TestUtils.matches; import static com.google.cloud.datastore.aggregation.Aggregation.count; @@ -86,7 +87,6 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.ClassRule; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -238,8 +238,6 @@ public void testNewTransactionCommit() { verifyNotUsable(transaction); } - // TODO(gapic_upgrade): Remove the @ignore annotation - @Ignore("This should be fixed with actionable error implementation") @Test public void testTransactionWithRead() { Transaction transaction = datastore.newTransaction(); @@ -257,12 +255,10 @@ public void testTransactionWithRead() { transaction.commit(); fail("Expecting a failure"); } catch (DatastoreException expected) { - assertEquals("ABORTED", expected.getReason()); + assertEquals(ABORTED.getHttpStatusCode(), expected.getCode()); } } - // TODO(gapic_upgrade): Remove the @ignore annotation - @Ignore("This should be fixed with actionable error implementation") @Test public void testTransactionWithQuery() { Query query = @@ -288,7 +284,7 @@ public void testTransactionWithQuery() { transaction.commit(); fail("Expecting a failure"); } catch (DatastoreException expected) { - assertEquals("ABORTED", expected.getReason()); + assertEquals(ABORTED.getHttpStatusCode(), expected.getCode()); } } diff --git a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java index d1e7646d2..1e931dfc4 100644 --- a/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java +++ b/google-cloud-datastore/src/test/java/com/google/cloud/datastore/it/ITDatastoreTest.java @@ -16,6 +16,7 @@ package com.google.cloud.datastore.it; +import static com.google.api.gax.rpc.StatusCode.Code.INVALID_ARGUMENT; import static com.google.cloud.datastore.aggregation.Aggregation.count; import static com.google.common.collect.Iterables.getOnlyElement; import static com.google.common.truth.Truth.assertThat; @@ -86,7 +87,6 @@ import org.junit.After; import org.junit.AfterClass; import org.junit.Before; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.rules.Timeout; @@ -1392,8 +1392,6 @@ public Integer run(DatastoreReaderWriter transaction) { } } - // TODO(gapic_upgrade): Remove the @ignore annotation - @Ignore("This should be fixed with actionable error implementation") @Test public void testRunInTransactionReadWrite() { @@ -1444,7 +1442,9 @@ public Integer run(DatastoreReaderWriter transaction) { datastore.runInTransaction(callable2, readOnlyOptions); fail("Expecting a failure"); } catch (DatastoreException expected) { - assertEquals(3, ((DatastoreException) expected.getCause()).getCode()); + assertEquals( + INVALID_ARGUMENT.getHttpStatusCode(), + ((DatastoreException) expected.getCause()).getCode()); } }