Refactoring to allow fixing #1216 by making JsonApiException accept HTTP status code, propagate

src/main/java/io/stargate/sgv2/jsonapi/exception/ErrorCode.java

@@ -1,5 +1,7 @@
 package io.stargate.sgv2.jsonapi.exception;
 
+import jakarta.ws.rs.core.Response;
+
 /** ErrorCode is our internal enum that provides codes and a default message for that error code. */
 public enum ErrorCode {
   /** Command error codes. */
@@ -188,11 +190,21 @@ public JsonApiException toApiException(String format, Object... args) {
     return new JsonApiException(this, message + ": " + String.format(format, args));
   }
 
+  public JsonApiException toApiException(
+      Response.Status httpStatus, String format, Object... args) {
+    return new JsonApiException(
+        this, message + ": " + String.format(format, args), null, httpStatus);
+  }
+
   public JsonApiException toApiException(Throwable cause, String format, Object... args) {
     return new JsonApiException(this, message + ": " + String.format(format, args), cause);
   }
 
   public JsonApiException toApiException() {
     return new JsonApiException(this, message);
   }
+
+  public JsonApiException toApiException(Response.Status httpStatus) {
+    return new JsonApiException(this, message, null, httpStatus);
+  }
 }

src/main/java/io/stargate/sgv2/jsonapi/exception/JsonApiException.java

@@ -23,6 +23,8 @@ public class JsonApiException extends RuntimeException implements Supplier<Comma
 
   private final ErrorCode errorCode;
 
+  private final Response.Status httpStatus;
+
   public JsonApiException(ErrorCode errorCode) {
     this(errorCode, errorCode.getMessage(), null);
   }
@@ -36,8 +38,14 @@ public JsonApiException(ErrorCode errorCode, Throwable cause) {
   }
 
   public JsonApiException(ErrorCode errorCode, String message, Throwable cause) {
+    this(errorCode, message, cause, Response.Status.OK);
+  }
+
+  public JsonApiException(
+      ErrorCode errorCode, String message, Throwable cause, Response.Status httpStatus) {
     super(message, cause);
     this.errorCode = errorCode;
+    this.httpStatus = httpStatus;
   }
 
   /** {@inheritDoc} */
@@ -50,7 +58,7 @@ public CommandResult get() {
     }
 
     // construct and return
-    CommandResult.Error error = getCommandResultError(message, Response.Status.OK);
+    CommandResult.Error error = getCommandResultError(message, httpStatus);
 
     // handle cause as well
     Throwable cause = getCause();
@@ -86,7 +94,15 @@ public CommandResult.Error getCommandResultError(Response.Status status) {
     return getCommandResultError(getMessage(), status);
   }
 
+  public CommandResult.Error getCommandResultError() {
+    return getCommandResultError(getMessage(), httpStatus);
+  }
+
   public ErrorCode getErrorCode() {
     return errorCode;
   }
+
+  public Response.Status getHttpStatus() {
+    return httpStatus;
+  }
 }

src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/GenericExceptionMapper.java

@@ -1,5 +1,6 @@
 package io.stargate.sgv2.jsonapi.exception.mappers;
 
+import com.fasterxml.jackson.core.JsonParseException;
 import com.fasterxml.jackson.databind.exc.MismatchedInputException;
 import io.stargate.sgv2.jsonapi.api.model.command.CommandResult;
 import org.jboss.resteasy.reactive.RestResponse;
@@ -12,7 +13,11 @@
 public class GenericExceptionMapper {
 
   // explicitly add types to override Quarkus mappers
-  @ServerExceptionMapper({Exception.class, MismatchedInputException.class})
+  @ServerExceptionMapper({
+    Exception.class,
+    JsonParseException.class,
+    MismatchedInputException.class
+  })
   public RestResponse<CommandResult> genericExceptionMapper(Throwable e) {
     CommandResult commandResult = new ThrowableCommandResultSupplier(e).get();
     return commandResult.map();

src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/ThrowableToErrorMapper.java

@@ -42,7 +42,7 @@ public final class ThrowableToErrorMapper {
           if (jae.getErrorCode().equals(ErrorCode.SERVER_EMBEDDING_GATEWAY_NOT_AVAILABLE)) {
             return jae.getCommandResultError(message, Response.Status.INTERNAL_SERVER_ERROR);
           }
-          return jae.getCommandResultError(message, Response.Status.OK);
+          return jae.getCommandResultError(message, jae.getHttpStatus());
         }
 
         // General Exception related to JSON handling, thrown by Jackson
@@ -188,8 +188,8 @@ private static CommandResult.Error handleAllNodesFailedException(
         } else if (error instanceof DriverTimeoutException) {
           // [data-api#1205] Need to map DriverTimeoutException as well
           return ErrorCode.SERVER_DRIVER_TIMEOUT
-              .toApiException(message)
-              .getCommandResultError(Response.Status.INTERNAL_SERVER_ERROR);
+              .toApiException(Response.Status.INTERNAL_SERVER_ERROR, message)
+              .getCommandResultError();
         }
       }
     }
@@ -200,11 +200,15 @@ private static CommandResult.Error handleAllNodesFailedException(
 
   private static CommandResult.Error handleJsonProcessingException(
       JacksonException e, String message) {
-    // Low-level parsing problem?
     if (e instanceof JsonParseException) {
+      // Low-level parsing problem? Actual BAD_REQUEST (400) since we could not process
       return ErrorCode.INVALID_REQUEST_NOT_JSON
-          .toApiException("underlying problem: (%s) %s", e.getClass().getName(), message)
-          .getCommandResultError(Response.Status.OK);
+          .toApiException(
+              Response.Status.BAD_REQUEST,
+              "underlying problem: (%s) %s",
+              e.getClass().getName(),
+              message)
+          .getCommandResultError();
     }
     // Unrecognized property? (note: CommandObjectMapperHandler handles some cases)
     if (e instanceof UnrecognizedPropertyException upe) {
@@ -219,7 +223,7 @@ private static CommandResult.Error handleJsonProcessingException(
           .toApiException(
               "\"%s\" not one of known fields (%s) at '%s'",
               upe.getPropertyName(), knownDesc, upe.getPathReference())
-          .getCommandResultError(Response.Status.OK);
+          .getCommandResultError();
     }
 
     // Will need to add more handling but start with above

src/main/java/io/stargate/sgv2/jsonapi/exception/mappers/WebApplicationExceptionMapper.java

@@ -32,6 +32,9 @@ public RestResponse<CommandResult> webApplicationExceptionMapper(WebApplicationE
               null);
     }
     CommandResult commandResult = new ThrowableCommandResultSupplier(toReport).get();
+    if (toReport instanceof JsonApiException jae) {
+      return RestResponse.status(jae.getHttpStatus(), commandResult);
+    }
     // Return 405 for method not allowed and 404 for not found
     if (e instanceof NotAllowedException) {
       return RestResponse.status(RestResponse.Status.METHOD_NOT_ALLOWED, commandResult);

src/test/java/io/stargate/sgv2/jsonapi/api/v1/CollectionResourceIntegrationTest.java

@@ -44,6 +44,9 @@ public void malformedBody() {
           .when()
           .post(CollectionResource.BASE_PATH, namespaceName, collectionName)
           .then()
+          // 10-Jul-2024, tatu: As per [data-api#1216], should be 400, not 200
+          //  (we want to return 4xx because we cannot actually process the request
+          //  as JSON is unparseable) but right now this is not working for some reason.
           .statusCode(200)
           .body("errors", hasSize(1))
           .body("errors[0].errorCode", is("INVALID_REQUEST_NOT_JSON"))