diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/AuthException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/AuthException.java new file mode 100644 index 000000000..a0b5ffbef --- /dev/null +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/AuthException.java @@ -0,0 +1,30 @@ +package io.stargate.sgv2.jsonapi.exception; + +/** + * Errors related to the authentication in a request. + * + *

See {@link APIException} for steps to add a new code. + */ +public class AuthException extends RequestException { + + public static final Scope SCOPE = Scope.AUTHENTICATION; + + public AuthException(ErrorInstance errorInstance) { + super(errorInstance); + } + + public enum Code implements ErrorCode { + INVALID_TOKEN; + + private final ErrorTemplate template; + + Code() { + template = ErrorTemplate.load(AuthException.class, FAMILY, SCOPE, name()); + } + + @Override + public ErrorTemplate template() { + return template; + } + } +} diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/DatabaseException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/DatabaseException.java index bd72179d7..4b2971657 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/DatabaseException.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/DatabaseException.java @@ -9,8 +9,14 @@ public DatabaseException(ErrorInstance errorInstance) { } public enum Code implements ErrorCode { + CLOSED_CONNECTION, + DRIVER_TIMEOUT, + NO_NODE_AVAILABLE, + READ_FAILURE, + READ_TIMEOUT, TABLE_WRITE_TIMEOUT, - CLOSED_CONNECTION; + WRITE_FAILURE, + WRITE_TIMEOUT; private final ErrorTemplate template; diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorFormatters.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorFormatters.java index 7a9a7333a..8e91fdb56 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorFormatters.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorFormatters.java @@ -71,7 +71,7 @@ public static Map errVars(SchemaObject schemaObject) { *

    *     public RuntimeException handle(TableSchemaObject schemaObject, WriteTimeoutException exception) {
    *     return DatabaseException.Code.TABLE_WRITE_TIMEOUT.get(
-   *         errFmt(
+   *         errVars(
    *             schemaObject,
    *             m -> {
    *               m.put("blockFor", String.valueOf(exception.getBlockFor()));
diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/ExceptionHandler.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/ExceptionHandler.java
index 22b6ba422..3b100320a 100644
--- a/src/main/java/io/stargate/sgv2/jsonapi/exception/ExceptionHandler.java
+++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/ExceptionHandler.java
@@ -41,7 +41,7 @@
  *       etc is kept away the "business logic" of how to handle the exceptions.
  * 
  *
- * 

Users of the interface shoudl call {@link #maybeHandle(SchemaObject, RuntimeException)} and + *

Users of the interface should call {@link #maybeHandle(SchemaObject, RuntimeException)} and * then throw or otherwise deal with the object returned, which wil be the original exception or a * new one. Example: * diff --git a/src/main/java/io/stargate/sgv2/jsonapi/exception/RequestException.java b/src/main/java/io/stargate/sgv2/jsonapi/exception/RequestException.java index ac601187d..eee922d67 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/exception/RequestException.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/exception/RequestException.java @@ -19,11 +19,13 @@ public RequestException(ErrorInstance errorInstance) { } public enum Scope implements ErrorScope { - DOCUMENT, + /** See {@link AuthException} */ + AUTHENTICATION, /** See {@link DocumentException} */ + DOCUMENT, + /** See {@link FilterException} */ FILTER; - /** See {@link FilterException} */ @Override public String scope() { return name(); diff --git a/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/DefaultDriverExceptionHandler.java b/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/DefaultDriverExceptionHandler.java index c09ac560b..fd5965014 100644 --- a/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/DefaultDriverExceptionHandler.java +++ b/src/main/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/DefaultDriverExceptionHandler.java @@ -2,9 +2,18 @@ import static io.stargate.sgv2.jsonapi.exception.ErrorFormatters.errVars; +import com.datastax.oss.driver.api.core.AllNodesFailedException; +import com.datastax.oss.driver.api.core.DriverTimeoutException; +import com.datastax.oss.driver.api.core.NoNodeAvailableException; +import com.datastax.oss.driver.api.core.auth.AuthenticationException; import com.datastax.oss.driver.api.core.connection.ClosedConnectionException; +import com.datastax.oss.driver.api.core.metadata.Node; +import com.datastax.oss.driver.api.core.servererrors.*; import io.stargate.sgv2.jsonapi.config.constants.ErrorObjectV2Constants.TemplateVars; +import io.stargate.sgv2.jsonapi.exception.AuthException; import io.stargate.sgv2.jsonapi.exception.DatabaseException; +import java.util.List; +import java.util.Map; /** * Default implementation of the {@link DriverExceptionHandler} interface, we keep the interface so @@ -33,4 +42,136 @@ public RuntimeException handle(SchemaT schemaObject, ClosedConnectionException e return DatabaseException.Code.CLOSED_CONNECTION.get( errVars(schemaObject, map -> map.put(TemplateVars.ERROR_MESSAGE, exception.getMessage()))); } + + @Override + public RuntimeException handle(SchemaT schemaObject, DriverTimeoutException exception) { + // TODO(Hazel): Aaron said "DRIVER" is a bad code. + return DatabaseException.Code.DRIVER_TIMEOUT.get( + errVars(schemaObject, map -> map.put(TemplateVars.ERROR_MESSAGE, exception.getMessage()))); + } + + @Override + public RuntimeException handle(SchemaT schemaObject, AllNodesFailedException exception) { + Map> allErrors = exception.getAllErrors(); + if (!allErrors.isEmpty()) { + List errors = allErrors.values().iterator().next(); + if (errors != null && !errors.isEmpty()) { + Throwable error = + errors.stream() + .findAny() + .filter( + t -> + t instanceof AuthenticationException + || t instanceof IllegalArgumentException + || t instanceof NoNodeAvailableException + || t instanceof DriverTimeoutException) + .orElse(null); + + if (error == null) { + return exception; + } + + return switch (error) { + case AuthenticationException e -> + // connect to OSS Cassandra throws AuthenticationException for invalid credentials + // TODO(Hazel): AuthException and INVALID_TOKEN? + AuthException.Code.INVALID_TOKEN.get(errVars(e)); + case IllegalArgumentException e -> { + // AstraDB throws IllegalArgumentException for invalid token/credentials + if (e.getMessage().contains("AUTHENTICATION ERROR") + || e.getMessage() + .contains("Provided username token and/or password are incorrect")) { + // TODO(Hazel): AuthException and INVALID_TOKEN? + yield AuthException.Code.INVALID_TOKEN.get(errVars(e)); + } else { + yield exception; + } + } + case NoNodeAvailableException e -> handle(schemaObject, e); + case DriverTimeoutException e -> + // [data-api#1205] Need to map DriverTimeoutException as well + handle(schemaObject, e); + default -> exception; + }; + } + } + return exception; + } + + @Override + public RuntimeException handle(SchemaT schemaObject, NoNodeAvailableException exception) { + // TODO(Hazel): Aaron said NO_NODE_AVAILABLE is a bad code. + return DatabaseException.Code.NO_NODE_AVAILABLE.get( + errVars(schemaObject, map -> map.put(TemplateVars.ERROR_MESSAGE, exception.getMessage()))); + } + + @Override + public RuntimeException handle(SchemaT schemaObject, QueryValidationException exception) { + String message = exception.getMessage(); + if (message.contains( + "If you want to execute this query despite the performance unpredictability, use ALLOW FILTERING") + || message.contains("ANN ordering by vector requires the column to be indexed")) { + // TODO(Hazel): Original code is NO_INDEX_ERROR, I think we need to change but am not sure + // what to change + return exception; + } + if (message.contains("vector { + m.put("blockFor", String.valueOf(exception.getBlockFor())); + m.put("received", String.valueOf(exception.getReceived())); + m.put("numFailures", String.valueOf(exception.getNumFailures())); + })); + } + + @Override + public RuntimeException handle(SchemaT schemaObject, ReadTimeoutException exception) { + return DatabaseException.Code.READ_TIMEOUT.get( + errVars( + schemaObject, + m -> { + m.put("blockFor", String.valueOf(exception.getBlockFor())); + m.put("received", String.valueOf(exception.getReceived())); + })); + } + + @Override + public RuntimeException handle(SchemaT schemaObject, WriteFailureException exception) { + return DatabaseException.Code.WRITE_FAILURE.get( + errVars( + schemaObject, + m -> { + m.put("blockFor", String.valueOf(exception.getBlockFor())); + m.put("received", String.valueOf(exception.getReceived())); + m.put("numFailures", String.valueOf(exception.getNumFailures())); + })); + } + + @Override + public RuntimeException handle(SchemaT schemaObject, WriteTimeoutException exception) { + return DatabaseException.Code.WRITE_TIMEOUT.get( + errVars( + schemaObject, + m -> { + m.put("blockFor", String.valueOf(exception.getBlockFor())); + m.put("received", String.valueOf(exception.getReceived())); + })); + } } diff --git a/src/main/resources/errors.yaml b/src/main/resources/errors.yaml index 0b54b2681..610129343 100644 --- a/src/main/resources/errors.yaml +++ b/src/main/resources/errors.yaml @@ -72,6 +72,16 @@ request-errors: ${SNIPPET.CONTACT_SUPPORT} + # Authentication request errors + - scope: AUTHENTICATION + code: INVALID_AUTHENTICATION_TOKEN + http-status-override: 401 + title: Invalid authentication token + body: |- + The authentication token provided is invalid. + + ${SNIPPET.CONTACT_SUPPORT} + # DOCUMENT request errors - scope: DOCUMENT code: MISSING_PRIMARY_KEY_COLUMNS @@ -153,6 +163,7 @@ server-errors: # DATABASE scope server errors - scope: DATABASE code: CLOSED_CONNECTION + http-status-override: 502 title: Database connection was closed while processing the request body: |- The Data API connection to the database was closed by the database while processing the request. @@ -163,7 +174,80 @@ server-errors: ${SNIPPET.RETRY} - scope: DATABASE + code: DRIVER_TIMEOUT + http-status-override: 504 + title: Database connection was timed out while processing the request + body: |- + The Data API connection to the database was timed out by the database while processing the request. + + Writing to the ${schemaType} ${keyspace}.${table} failed to complete successfully. If this request modified data the changes may have been written to by some replicas, but not all. Future read requests may return eventually consistent results. + + The detailed response from the database was: ${errorMessage} + + ${SNIPPET.RETRY} + - scope: DATABASE + code: NO_NODE_AVAILABLE + http-status-override: 502 + title: Database connection was timed out while processing the request + body: |- + No node was available to execute the query. + + Writing to the ${schemaType} ${keyspace}.${table} failed to complete successfully. + + The detailed response from the database was: ${errorMessage} + + ${SNIPPET.RETRY} + - scope: DATABASE + code: READ_FAILURE + http-status-override: 502 + title: Database read failed + body: |- + The Data API failed to read from the database. + + Reading from the ${schemaType} ${keyspace}.${table} failed to complete successfully. + + The request was was waiting for ${blockFor} replicas to acknowledge the read, but only ${received} replicas responded, ${numFailures} failed. + + ${SNIPPET.RETRY} + - scope: DATABASE + code: READ_TIMEOUT + http-status-override: 504 + title: Database read timed out + body: |- + The Data API timed out while reading from the database. + + Reading from the ${schemaType} ${keyspace}.${table} failed to complete successfully. + #TODO(Hazel): ReadTimeoutException has multiple ways to format the error message. What message should we use? + The request was was waiting for ${blockFor} replicas to acknowledge the read, but only ${received} replicas responded within the timeout period. + + ${SNIPPET.RETRY} + - scope: DATABASE + code: WRITE_FAILURE + http-status-override: 502 + title: Database write failed + body: |- + The Data API failed to write from the database. + + Writing from the ${schemaType} ${keyspace}.${table} failed to complete successfully. + + The request was was waiting for ${blockFor} replicas to acknowledge the write, but only ${received} replicas responded, ${numFailures} failed. + + ${SNIPPET.RETRY} + - scope: DATABASE + code: WRITE_TIMEOUT + http-status-override: 504 + title: Database write timed out + body: |- + The Data API timed out while writing from the database. + + Writing from the ${schemaType} ${keyspace}.${table} failed to complete successfully. + + The request was was waiting for ${blockFor} replicas to acknowledge the write, but only ${received} replicas responded within the timeout period. + + ${SNIPPET.RETRY} + - scope: DATABASE code: TABLE_WRITE_TIMEOUT + http-status-override: 504 title: Timeout writing to table body: |- The Data API timed out while writing to the table. diff --git a/src/test/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/DefaultDriverExceptionHandlerTest.java b/src/test/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/DefaultDriverExceptionHandlerTest.java index 139a77bd6..1276d6c93 100644 --- a/src/test/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/DefaultDriverExceptionHandlerTest.java +++ b/src/test/java/io/stargate/sgv2/jsonapi/service/cqldriver/executor/DefaultDriverExceptionHandlerTest.java @@ -3,10 +3,15 @@ import static io.stargate.sgv2.jsonapi.exception.ErrorFormatters.errFmt; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.mockito.Mockito.mock; +import com.datastax.oss.driver.api.core.DefaultConsistencyLevel; import com.datastax.oss.driver.api.core.DriverException; import com.datastax.oss.driver.api.core.connection.ClosedConnectionException; +import com.datastax.oss.driver.api.core.servererrors.*; +import com.datastax.oss.driver.internal.core.metadata.DefaultNode; import io.stargate.sgv2.jsonapi.exception.DatabaseException; +import java.util.Map; import java.util.stream.Stream; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -38,7 +43,7 @@ public void tableDriverErrorHandled( assertThat(handledEx) .as( - "Handled error should be a different instance, all driver errors mapp to a different exception") + "Handled error should be a different instance, all driver errors map to a different exception") .isNotSameAs(originalEx); // for now, assumed they always turn into a DatabaseException @@ -70,12 +75,52 @@ public void tableDriverErrorHandled( } private static Stream tableDriverErrorHandledData() { + DefaultNode mockCoordinator = mock(DefaultNode.class); return Stream.of( + // Test for CLOSED_CONNECTION Arguments.of( new ClosedConnectionException("closed"), DatabaseException.Code.CLOSED_CONNECTION.name(), true, true, - true)); + true), + // Test for READ_FAILURE + Arguments.of( + new ReadFailureException( + mockCoordinator, DefaultConsistencyLevel.LOCAL_QUORUM, 1, 2, 1, true, Map.of()), + DatabaseException.Code.READ_FAILURE.name(), + true, + true, + false), + // Test for READ_TIMEOUT + Arguments.of( + new ReadTimeoutException( + mockCoordinator, DefaultConsistencyLevel.LOCAL_QUORUM, 1, 2, true), + DatabaseException.Code.READ_TIMEOUT.name(), + true, + true, + false), + // Test for WRITE_FAILURE + Arguments.of( + new WriteFailureException( + mockCoordinator, + DefaultConsistencyLevel.LOCAL_QUORUM, + 1, + 2, + WriteType.SIMPLE, + 1, + Map.of()), + DatabaseException.Code.WRITE_FAILURE.name(), + true, + true, + false), + // Test for WRITE_TIMEOUT + Arguments.of( + new WriteTimeoutException( + mockCoordinator, DefaultConsistencyLevel.LOCAL_QUORUM, 1, 2, WriteType.SIMPLE), + DatabaseException.Code.WRITE_TIMEOUT.name(), + true, + true, + false)); } }