Skip to content
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

refactor: introduce Store(Item)?NotFoundException classes #360

Merged
merged 5 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import momento.sdk.exceptions.AuthenticationException;
import momento.sdk.exceptions.BadRequestException;
import momento.sdk.exceptions.InvalidArgumentException;
import momento.sdk.exceptions.NotFoundException;
import momento.sdk.exceptions.StoreNotFoundException;
import momento.sdk.responses.storage.CreateStoreResponse;
import momento.sdk.responses.storage.DeleteStoreResponse;
import momento.sdk.responses.storage.ListStoresResponse;
Expand Down Expand Up @@ -57,7 +57,7 @@ public void returnsNotFoundWhenDeletingUnknownStore() {
assertThat(client.deleteStore(randomString("name")))
.succeedsWithin(TEN_SECONDS)
.asInstanceOf(InstanceOfAssertFactories.type(DeleteStoreResponse.Error.class))
.satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class));
.satisfies(error -> assertThat(error).hasCauseInstanceOf(StoreNotFoundException.class));
}

@Test
Expand Down Expand Up @@ -125,7 +125,7 @@ public void deleteSucceeds() {
assertThat(client.deleteStore(storeName))
.succeedsWithin(TEN_SECONDS)
.asInstanceOf(InstanceOfAssertFactories.type(DeleteStoreResponse.Error.class))
.satisfies(error -> assertThat(error).hasCauseInstanceOf(NotFoundException.class));
.satisfies(error -> assertThat(error).hasCauseInstanceOf(StoreNotFoundException.class));
} finally {
// Just in case the second create or delete fails
client.deleteStore(storeName).join();
Expand Down
13 changes: 5 additions & 8 deletions momento-sdk/src/main/java/momento/sdk/StorageControlClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@
import grpc.control_client._DeleteStoreResponse;
import grpc.control_client._ListStoresRequest;
import grpc.control_client._ListStoresResponse;
import io.grpc.Status;
import java.util.concurrent.CompletableFuture;
import java.util.function.Function;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import javax.annotation.Nonnull;
import momento.sdk.auth.CredentialProvider;
import momento.sdk.config.StorageConfiguration;
import momento.sdk.exceptions.AlreadyExistsException;
import momento.sdk.exceptions.CacheServiceExceptionMapper;
import momento.sdk.exceptions.SdkException;
import momento.sdk.responses.storage.CreateStoreResponse;
import momento.sdk.responses.storage.DeleteStoreResponse;
import momento.sdk.responses.storage.ListStoresResponse;
Expand All @@ -43,7 +44,6 @@ CompletableFuture<CreateStoreResponse> createStore(String storeName) {
return sendCreateStore(storeName);
} catch (Exception e) {
return CompletableFuture.completedFuture(
// TODO need to generalize exception mapper
new CreateStoreResponse.Error(CacheServiceExceptionMapper.convert(e)));
}
}
Expand Down Expand Up @@ -80,12 +80,9 @@ private CompletableFuture<CreateStoreResponse> sendCreateStore(String storeName)

final Function<Throwable, CreateStoreResponse> failure =
e -> {
if (e instanceof io.grpc.StatusRuntimeException) {
io.grpc.StatusRuntimeException statusRuntimeException =
(io.grpc.StatusRuntimeException) e;
if (statusRuntimeException.getStatus().getCode() == Status.Code.ALREADY_EXISTS) {
return new CreateStoreResponse.AlreadyExists();
}
final SdkException sdkException = CacheServiceExceptionMapper.convert(e);
if (sdkException instanceof AlreadyExistsException) {
return new CreateStoreResponse.AlreadyExists();
}
return new CreateStoreResponse.Error(CacheServiceExceptionMapper.convert(e));
};
Expand Down
24 changes: 8 additions & 16 deletions momento-sdk/src/main/java/momento/sdk/StorageDataClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import momento.sdk.config.StorageConfiguration;
import momento.sdk.exceptions.CacheServiceExceptionMapper;
import momento.sdk.exceptions.InternalServerException;
import momento.sdk.exceptions.SdkException;
import momento.sdk.responses.storage.DeleteResponse;
import momento.sdk.responses.storage.GetResponse;
import momento.sdk.responses.storage.PutResponse;
Expand Down Expand Up @@ -156,19 +157,12 @@ public void onSuccess(_StoreGetResponse rsp) {

@Override
public void onFailure(@Nonnull Throwable e) {
// TODO this logic and string matching should be in one place, eg in the service
// exception mapper
if (e instanceof io.grpc.StatusRuntimeException) {
io.grpc.StatusRuntimeException statusRuntimeException =
(io.grpc.StatusRuntimeException) e;
if (statusRuntimeException.getStatus().getCode().equals(io.grpc.Status.Code.NOT_FOUND)
&& e.getMessage().contains("Element not found")) {
returnFuture.complete(GetResponse.Success.of());
return;
}
final SdkException sdkException = CacheServiceExceptionMapper.convert(e, metadata);
if (sdkException instanceof momento.sdk.exceptions.StoreItemNotFoundException) {
returnFuture.complete(GetResponse.Success.of());
} else {
returnFuture.complete(new GetResponse.Error(CacheServiceExceptionMapper.convert(e)));
}
returnFuture.complete(
new GetResponse.Error(CacheServiceExceptionMapper.convert(e, metadata)));
}
},
// Execute on same thread that called execute on CompletionStage
Expand Down Expand Up @@ -209,8 +203,7 @@ public void onSuccess(_StorePutResponse rsp) {

@Override
public void onFailure(@Nonnull Throwable e) {
returnFuture.complete(
new PutResponse.Error(CacheServiceExceptionMapper.convert(e, new Metadata())));
returnFuture.complete(new PutResponse.Error(CacheServiceExceptionMapper.convert(e)));
}
},
// Execute on same thread that called execute on CompletionStage
Expand Down Expand Up @@ -250,8 +243,7 @@ public void onSuccess(_StoreDeleteResponse rsp) {

@Override
public void onFailure(@Nonnull Throwable e) {
returnFuture.complete(
new DeleteResponse.Error(CacheServiceExceptionMapper.convert(e, metadata)));
returnFuture.complete(new DeleteResponse.Error(CacheServiceExceptionMapper.convert(e)));
}
},
// Execute on same thread that called execute on CompletionStage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,18 @@ public static SdkException convert(Throwable e, Metadata metadata) {
if (e instanceof io.grpc.StatusRuntimeException) {
final StatusRuntimeException grpcException = (StatusRuntimeException) e;
final Status.Code statusCode = grpcException.getStatus().getCode();
final Metadata trailers = grpcException.getTrailers();

final MomentoTransportErrorDetails errorDetails =
new MomentoTransportErrorDetails(
new MomentoGrpcErrorDetails(statusCode, grpcException.getMessage(), metadata));

String errorCause = trailers.get(Metadata.Key.of("err", Metadata.ASCII_STRING_MARSHALLER));
if (errorCause == null) {
// TODO remove once control service is updated to send "err" in metadata
errorCause = grpcException.getMessage();
}
Comment on lines +54 to +57
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will update this once the server is up to date.


switch (statusCode) {
case INVALID_ARGUMENT:
// fall through
Expand All @@ -75,8 +82,14 @@ public static SdkException convert(Throwable e, Metadata metadata) {
return new LimitExceededException(grpcException, errorDetails);

case NOT_FOUND:
return new NotFoundException(grpcException, errorDetails);

if (errorCause.contains("element_not_found")) {
return new StoreItemNotFoundException(grpcException, errorDetails);
// TODO change once control service is updated to send "Store with name" in metadata
} else if (errorCause.contains("Store with name")) {
return new StoreNotFoundException(grpcException, errorDetails);
} else {
return new NotFoundException(grpcException, errorDetails);
}
Comment on lines +85 to +92
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will update these once the server is up to date.

case ALREADY_EXISTS:
return new AlreadyExistsException(grpcException, errorDetails);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package momento.sdk.exceptions;

import momento.sdk.internal.MomentoTransportErrorDetails;

/** The item requested from the store doesn't exist. */
public class StoreItemNotFoundException extends SdkException {

private static final String MESSAGE =
"The item requested from the store does not exist. To resolve this error, "
+ "if the requested item was expected to be found, put it in the store.";

/**
* Constructs a StoreItemNotFound with a cause and error details.
*
* @param cause the cause.
* @param transportErrorDetails details about the request and error.
*/
public StoreItemNotFoundException(
Throwable cause, MomentoTransportErrorDetails transportErrorDetails) {
super(
MomentoErrorCode.NOT_FOUND_ERROR,
completeMessage(transportErrorDetails),
cause,
transportErrorDetails);
}

private static String completeMessage(MomentoTransportErrorDetails transportErrorDetails) {
return transportErrorDetails
.getGrpcErrorDetails()
.getCacheName()
.map(s -> MESSAGE + " Store name: " + s)
.orElse(MESSAGE);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package momento.sdk.exceptions;

import momento.sdk.internal.MomentoTransportErrorDetails;

/** The store requested doesn't exist. */
public class StoreNotFoundException extends MomentoServiceException {

private static final String MESSAGE =
"A store with the specified name does not exist. To resolve this error, "
+ "make sure you have created the store before attempting to use it.";

/**
* Constructs a StoreNotFoundException with a cause and error details.
*
* @param cause the cause.
* @param transportErrorDetails details about the request and error.
*/
public StoreNotFoundException(
Throwable cause, MomentoTransportErrorDetails transportErrorDetails) {
super(
MomentoErrorCode.NOT_FOUND_ERROR,
completeMessage(transportErrorDetails),
cause,
transportErrorDetails);
}

private static String completeMessage(MomentoTransportErrorDetails transportErrorDetails) {
return transportErrorDetails
.getGrpcErrorDetails()
.getCacheName()
.map(s -> MESSAGE + " Store name: " + s)
.orElse(MESSAGE);
}
}
Loading