lasdf1234 commented on code in PR #14556:
URL: https://github.com/apache/gravitino/pull/14556#discussion_r4454006909
##########
lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java:
##########
@@ -95,6 +102,53 @@ public Response toResponse(Throwable ex) {
return toRESTResponse("", ex);
}
+ private static Response handleWebApplicationException(
+ String instance, WebApplicationException exception) {
+ int status = exception.getResponse().getStatus();
+ String message = exception.getMessage();
+ if (exception instanceof ParamException parameterException) {
+ // Jersey can use 404 for parameter conversion failures, but these are
invalid input.
+ status = Response.Status.BAD_REQUEST.getStatusCode();
+ message = "Invalid value for parameter '" +
parameterException.getParameterName() + "'";
+ }
+ LanceNamespaceException error;
+ // Framework 404s describe route misses, not an identified missing table
or namespace.
+ // Keep their HTTP status, while using INVALID_INPUT instead of an
entity-not-found body code.
+ if (status >= 400 && status < 500) {
+ error =
+ switch (status) {
+ case 401 -> new UnauthenticatedException(message, "", instance);
+ case 403 -> new PermissionDeniedException(message, "", instance);
+ case 404 -> new InvalidInputException(message, "", instance);
Review Comment:
**P2** (related) — Framework mapping here correctly uses `detail=""`.
Backend `IllegalArgumentException` → `InvalidInputException` still passes
`errorDetail(ex)` (see `toLanceNamespaceException`), so with default
`includeErrorStackTrace=true` client 400s can embed a full stack in `detail`
while framework 404s do not.
Prefer the same empty/`""` detail for client validation errors to avoid
inconsistent / sensitive responses (CWE-209).
##########
lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/LanceExceptionMapper.java:
##########
@@ -77,6 +81,9 @@ public static String errorDetail(Throwable ex) {
public static Response toRESTResponse(String instance, Throwable ex) {
ServerHealth.getInstance().recordFailure(ex);
+ if (ex instanceof WebApplicationException webException) {
+ return handleWebApplicationException(instance, webException);
Review Comment:
**P3** — Early-return for `WebApplicationException` avoids the ERROR log
below for framework faults, but backend-mapped client errors
(`IllegalArgumentException`, not-found, etc.) still hit `LOG.error` in
`toRESTResponse`.
Iceberg’s mapper typically uses INFO for sub-500 client faults. Consider
INFO/WARN for mapped 4xx and keep ERROR for unexpected/internal failures to
reduce ops noise.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
