diff --git a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
index 252bde85a5eb..8a9ea142d72d 100644
--- a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
+++ b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogAdapter.java
@@ -21,12 +21,13 @@
import com.google.common.base.Preconditions;
+import java.io.Closeable;
import java.io.IOException;
import java.time.Clock;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
-import java.util.function.Consumer;
+import javax.servlet.http.HttpServletResponse;
import org.apache.iceberg.BaseTable;
import org.apache.iceberg.BaseTransaction;
import org.apache.iceberg.Table;
@@ -48,7 +49,6 @@
import org.apache.iceberg.exceptions.NoSuchTableException;
import org.apache.iceberg.exceptions.NoSuchViewException;
import org.apache.iceberg.exceptions.NotAuthorizedException;
-import org.apache.iceberg.exceptions.RESTException;
import org.apache.iceberg.exceptions.UnprocessableEntityException;
import org.apache.iceberg.exceptions.ValidationException;
import org.apache.iceberg.relocated.com.google.common.base.Splitter;
@@ -83,7 +83,7 @@
* Original @ RESTCatalogAdapter.java
* Adaptor class to translate REST requests into {@link Catalog} API calls.
*/
-public class HMSCatalogAdapter implements RESTClient {
+public class HMSCatalogAdapter implements Closeable {
private static final Logger LOG = LoggerFactory.getLogger(HMSCatalogAdapter.class);
private static final Splitter SLASH = Splitter.on('/');
@@ -506,7 +506,7 @@ T execute(
String path,
Map queryParams,
Object body,
- Consumer errorHandler) {
+ HttpServletResponse response) throws IOException {
ErrorResponse.Builder errorBuilder = ErrorResponse.builder();
Pair> routeAndVars = Route.from(method, path);
if (routeAndVars != null) {
@@ -527,63 +527,9 @@ T execute(
.withMessage(String.format("No route for request: %s %s", method, path));
}
ErrorResponse error = errorBuilder.build();
- errorHandler.accept(error);
- // if the error handler doesn't throw an exception, throw a generic one
- throw new RESTException("Unhandled error: %s", error);
- }
-
- @Override
- public T delete(
- String path,
- Class responseType,
- Map headers,
- Consumer errorHandler) {
- return execute(HTTPMethod.DELETE, path, null, null, errorHandler);
- }
-
- @Override
- public T delete(
- String path,
- Map queryParams,
- Class responseType,
- Map headers,
- Consumer errorHandler) {
- return execute(HTTPMethod.DELETE, path, queryParams, null, errorHandler);
- }
-
- @Override
- public T post(
- String path,
- RESTRequest body,
- Class responseType,
- Map headers,
- Consumer errorHandler) {
- return execute(HTTPMethod.POST, path, null, body, errorHandler);
- }
-
- @Override
- public T get(
- String path,
- Map queryParams,
- Class responseType,
- Map headers,
- Consumer errorHandler) {
- return execute(HTTPMethod.GET, path, queryParams, null, errorHandler);
- }
-
- @Override
- public void head(String path, Map headers, Consumer errorHandler) {
- execute(HTTPMethod.HEAD, path, null, headers, errorHandler);
- }
-
- @Override
- public T postForm(
- String path,
- Map formData,
- Class responseType,
- Map headers,
- Consumer errorHandler) {
- return execute(HTTPMethod.POST, path, null, formData, errorHandler);
+ response.setStatus(error.code());
+ RESTObjectMapper.mapper().writeValue(response.getWriter(), error);
+ return null;
}
@Override
@@ -627,11 +573,15 @@ public static T castResponse(Class responseType, Obj
public static void configureResponseFromException(
Exception exc, ErrorResponse.Builder errorBuilder) {
- errorBuilder
- .responseCode(EXCEPTION_ERROR_CODES.getOrDefault(exc.getClass(), 500))
- .withType(exc.getClass().getSimpleName())
- .withMessage(exc.getMessage())
- .withStackTrace(exc);
+ var errorCode = EXCEPTION_ERROR_CODES.getOrDefault(exc.getClass(), 500);
+ errorBuilder.responseCode(errorCode).withType(exc.getClass().getSimpleName()).withMessage(exc.getMessage());
+ // avoid exposing stack traces for client errors but include them for server errors to aid debugging
+ if (errorCode >= 500) {
+ LOG.error("A server error happened while processing REST request", exc);
+ errorBuilder.withStackTrace(exc);
+ } else {
+ LOG.info("A client error happened while processing REST request: {}", exc.getMessage());
+ }
}
private static Namespace namespaceFromPathVars(Map pathVars) {
diff --git a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java
index e608ba5229a6..21b155d65d8d 100644
--- a/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java
+++ b/standalone-metastore/metastore-rest-catalog/src/main/java/org/apache/iceberg/rest/HMSCatalogServlet.java
@@ -20,10 +20,8 @@
package org.apache.iceberg.rest;
import java.io.IOException;
-import java.io.UncheckedIOException;
import java.util.Map;
import java.util.Optional;
-import java.util.function.Consumer;
import java.util.stream.Collectors;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
@@ -77,29 +75,17 @@ protected void service(HttpServletRequest request, HttpServletResponse response)
context.path(),
context.queryParams(),
context.body(),
- handle(response));
+ response);
if (responseBody != null) {
RESTObjectMapper.mapper().writeValue(response.getWriter(), responseBody);
}
} catch (RuntimeException | IOException e) {
- // should be a RESTException but not able to see them through dependencies
LOG.error("Error processing REST request", e);
response.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
}
}
- private Consumer handle(HttpServletResponse response) {
- return errorResponse -> {
- response.setStatus(errorResponse.code());
- try {
- RESTObjectMapper.mapper().writeValue(response.getWriter(), errorResponse);
- } catch (IOException e) {
- throw new UncheckedIOException(e);
- }
- };
- }
-
@Override
public void destroy() {
super.destroy();