Skip to content

feat!: Actionable Error #1244

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
e17ba34
Create basic structure of GrpcDatastoreRpc and using it in DatastoreO…
jainsahab Nov 4, 2023
f9e455e
applying unary settings to all the unary methods
jainsahab Nov 6, 2023
231deef
Configuring header provider for GrpcDatastoreRpc
jainsahab Nov 6, 2023
806f997
fixing emulator tests to be able to run successfully with grpc now
jainsahab Nov 9, 2023
0c8704c
ignoring one more test which will be fixed in actionable error implem…
jainsahab Nov 9, 2023
d6ed231
Making HttpDatastoreRpc completely unused
jainsahab Nov 9, 2023
e257a91
Making GrpcDatastoreRpc implement AutoCloseable
jainsahab Nov 16, 2023
2984146
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Nov 16, 2023
acfbaef
incorporating feedbacks
jainsahab Nov 17, 2023
60ee45d
pinging emulator after each test for debugging
jainsahab Nov 18, 2023
d42e3b9
Revert "pinging emulator after each test for debugging"
jainsahab Nov 18, 2023
1d9d7e6
Reapply "pinging emulator after each test for debugging"
jainsahab Nov 18, 2023
d7b652e
more debugging
jainsahab Nov 18, 2023
9827fb9
Constant ping to avoid flaky behaviour of /shutdown endpoint
jainsahab Nov 18, 2023
38725f0
fixing test
jainsahab Nov 18, 2023
b15a9a9
checking if emulator is running before sending a shutdown command
jainsahab Nov 20, 2023
ec38885
fix lint
jainsahab Nov 20, 2023
7bd2c55
implement helper method for localhost
jainsahab Nov 22, 2023
0517cb6
fix header lint
jainsahab Nov 22, 2023
7f4ce8d
moving emulator health check to src/test
jainsahab Nov 22, 2023
ef5f002
fix lint
jainsahab Nov 22, 2023
9b43798
adding no extra headers
jainsahab Nov 22, 2023
cb80dd1
minor cleanup
jainsahab Nov 23, 2023
2b7b855
Making DatastoreException extend BaseGrpcServiceException
jainsahab Nov 24, 2023
7aaa5ed
Creating DatastoreException through ApiException and exposing methods…
jainsahab Nov 25, 2023
44d0d49
Wrapping ApiException in DatastoreException before throwing it
jainsahab Nov 25, 2023
a5140db
adding clirr configuration
jainsahab Nov 25, 2023
c012767
javadoc for getReason
jainsahab Nov 27, 2023
6b632eb
Merge branch 'V3-experimental' into V3-experimental-workspace-2
jainsahab Nov 27, 2023
37568b9
deleting unused file
jainsahab Nov 27, 2023
6537bca
enabling tests which were ignore for actionable error implementation
jainsahab Nov 28, 2023
37466bc
added clirr comments
jainsahab Nov 28, 2023
a893a14
fix lint
jainsahab Nov 28, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions google-cloud-datastore/clirr-ignored-differences.xml
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,14 @@
<method>com.google.cloud.datastore.AggregationResults runAggregation(com.google.cloud.datastore.AggregationQuery)</method>
<differenceType>7012</differenceType>
</difference>
<difference>
<className>com/google/cloud/datastore/DatastoreException</className>
<differenceType>5000</differenceType>
<to>com/google/cloud/grpc/BaseGrpcServiceException</to>
</difference>
<difference>
<className>com/google/cloud/datastore/DatastoreException</className>
<differenceType>5001</differenceType>
<to>com/google/cloud/http/BaseHttpServiceException</to>
</difference>
</differences>
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -29,7 +34,7 @@
* @see <a href="https://cloud.google.com/datastore/docs/concepts/errors#Error_Codes">Google Cloud
* Datastore error codes</a>
*/
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<Error> RETRYABLE_ERRORS =
Expand All @@ -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 <a
* href="https://github.com/googleapis/googleapis/blob/master/google/rpc/error_details.proto#L125">Domain</a>
* @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 <a
* href="https://github.com/googleapis/googleapis/blob/master/google/rpc/error_details.proto#L135">Metadata</a>
* @return the map of additional structured details about an error.
*/
public Map<String, String> 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 <a
* href="https://github.com/googleapis/googleapis/blob/master/google/rpc/status.proto">Status</a>
* @see <a
* href="https://github.com/googleapis/googleapis/blob/master/google/rpc/error_details.proto">Error
* Details</a>
* @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 <a
* href="https://github.com/googleapis/googleapis/blob/master/google/rpc/error_details.proto#L117">Reason</a>
* @return the reason of an error.
*/
@Override
public String getReason() {
if (this.apiException != null) {
return this.apiException.getReason();
}
return this.reason;
}

/**
Expand All @@ -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());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
}