This is an automated email from the ASF dual-hosted git repository.
JingsongLi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/paimon.git
The following commit(s) were added to refs/heads/master by this push:
new ff8670eca2 [api] Fall back to the HTTP status when the error body
omits code (#9661)
ff8670eca2 is described below
commit ff8670eca2e5e56a1e756528ca819d6c0baaf750
Author: jackylee <[email protected]>
AuthorDate: Sat Sep 12 21:33:27 2026 +0800
[api] Fall back to the HTTP status when the error body omits code (#9661)
---
.../apache/paimon/rest/DefaultErrorHandler.java | 5 +-
.../paimon/rest/responses/ErrorResponse.java | 9 ++-
.../paimon/rest/responses/ErrorResponseTest.java | 90 ++++++++++++++++++++++
.../paimon/rest/DefaultErrorHandlerTest.java | 14 ++++
.../org/apache/paimon/rest/HttpClientTest.java | 24 ++++++
5 files changed, 140 insertions(+), 2 deletions(-)
diff --git
a/paimon-api/src/main/java/org/apache/paimon/rest/DefaultErrorHandler.java
b/paimon-api/src/main/java/org/apache/paimon/rest/DefaultErrorHandler.java
index 67ce6ced18..c1fd929736 100644
--- a/paimon-api/src/main/java/org/apache/paimon/rest/DefaultErrorHandler.java
+++ b/paimon-api/src/main/java/org/apache/paimon/rest/DefaultErrorHandler.java
@@ -42,7 +42,10 @@ public class DefaultErrorHandler extends ErrorHandler {
@Override
public void accept(ErrorResponse error, String requestId) {
- int code = error.getCode();
+ Integer errorCode = error.getCode();
+ // HttpClient always resolves the code before calling this, but the
response may also be
+ // deserialized directly, and then "code" is absent whenever the
server omits it.
+ int code = errorCode == null ? 0 : errorCode;
String message;
if (DEFAULT_REQUEST_ID.equals(requestId)) {
message = error.getMessage();
diff --git
a/paimon-api/src/main/java/org/apache/paimon/rest/responses/ErrorResponse.java
b/paimon-api/src/main/java/org/apache/paimon/rest/responses/ErrorResponse.java
index bfc9e3bf4e..899b0896af 100644
---
a/paimon-api/src/main/java/org/apache/paimon/rest/responses/ErrorResponse.java
+++
b/paimon-api/src/main/java/org/apache/paimon/rest/responses/ErrorResponse.java
@@ -73,6 +73,7 @@ public class ErrorResponse implements RESTResponse {
@JsonProperty(FIELD_MESSAGE)
private final String message;
+ @Nullable
@JsonProperty(FIELD_CODE)
private final Integer code;
@@ -81,13 +82,18 @@ public class ErrorResponse implements RESTResponse {
@Nullable @JsonProperty(FIELD_RESOURCE_TYPE) String resourceType,
@Nullable @JsonProperty(FIELD_RESOURCE_NAME) String resourceName,
@JsonProperty(FIELD_MESSAGE) String message,
- @JsonProperty(FIELD_CODE) int code) {
+ @Nullable @JsonProperty(FIELD_CODE) Integer code) {
this.resourceType = resourceType;
this.resourceName = resourceName;
this.message = message;
this.code = code;
}
+ /** Retained for callers compiled against the primitive {@code code}
descriptor. */
+ public ErrorResponse(String resourceType, String resourceName, String
message, int code) {
+ this(resourceType, resourceName, message, (Integer) code);
+ }
+
@JsonGetter(FIELD_MESSAGE)
public String getMessage() {
return message;
@@ -103,6 +109,7 @@ public class ErrorResponse implements RESTResponse {
return resourceName;
}
+ @Nullable
@JsonGetter(FIELD_CODE)
public Integer getCode() {
return code;
diff --git
a/paimon-api/src/test/java/org/apache/paimon/rest/responses/ErrorResponseTest.java
b/paimon-api/src/test/java/org/apache/paimon/rest/responses/ErrorResponseTest.java
new file mode 100644
index 0000000000..7b977a0a32
--- /dev/null
+++
b/paimon-api/src/test/java/org/apache/paimon/rest/responses/ErrorResponseTest.java
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.paimon.rest.responses;
+
+import org.apache.paimon.rest.RESTApi;
+
+import
org.apache.paimon.shade.jackson2.com.fasterxml.jackson.annotation.JsonCreator;
+
+import org.junit.jupiter.api.Test;
+
+import java.lang.reflect.Constructor;
+import java.util.Arrays;
+import java.util.List;
+import java.util.stream.Collectors;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+
+/** Tests for {@link ErrorResponse}. */
+public class ErrorResponseTest {
+
+ private static final Class<?>[] PRIMITIVE_CODE_CTOR = {
+ String.class, String.class, String.class, int.class
+ };
+
+ @Test
+ public void testPrimitiveCodeConstructorDescriptorIsRetained() {
+ // The descriptor REST server implementations compiled against an
earlier paimon-api
+ // invoke. A source level new ErrorResponse(a, b, c, 404) would still
compile if it were
+ // deleted, because javac boxes into the Integer overload, so assert
it reflectively.
+ assertThatCode(() ->
ErrorResponse.class.getConstructor(PRIMITIVE_CODE_CTOR))
+ .doesNotThrowAnyException();
+ }
+
+ @Test
+ public void testExactlyOneJsonCreatorAndItAcceptsNullableCode() throws
Exception {
+ List<Constructor<?>> creators =
+ Arrays.stream(ErrorResponse.class.getDeclaredConstructors())
+ .filter(c -> c.isAnnotationPresent(JsonCreator.class))
+ .collect(Collectors.toList());
+
+ assertThat(creators).hasSize(1);
+ assertThat(creators.get(0).getParameterTypes())
+ .containsExactly(String.class, String.class, String.class,
Integer.class);
+ // the primitive overload must stay invisible to Jackson, otherwise an
absent code
+ // deserializes to 0 again
+ assertThat(
+ ErrorResponse.class
+ .getConstructor(PRIMITIVE_CODE_CTOR)
+ .isAnnotationPresent(JsonCreator.class))
+ .isFalse();
+ }
+
+ @Test
+ public void testCodeIsAbsentOnTheWireRatherThanZero() throws Exception {
+ assertThat(RESTApi.fromJson("{\"message\":\"x\"}",
ErrorResponse.class).getCode()).isNull();
+ assertThat(
+ RESTApi.fromJson("{\"message\":\"x\",\"code\":null}",
ErrorResponse.class)
+ .getCode())
+ .isNull();
+ assertThat(
+ RESTApi.fromJson("{\"message\":\"x\",\"code\":404}",
ErrorResponse.class)
+ .getCode())
+ .isEqualTo(404);
+ }
+
+ @Test
+ public void testBothConstructorsAgree() throws Exception {
+ assertThat(new ErrorResponse("TABLE", "t", "m",
404).getCode()).isEqualTo(404);
+ assertThat(new ErrorResponse("TABLE", "t", "m", (Integer)
null).getCode()).isNull();
+ assertThat(RESTApi.toJson(new ErrorResponse(null, null, "m", (Integer)
null)))
+ .contains("\"code\":null");
+ }
+}
diff --git
a/paimon-core/src/test/java/org/apache/paimon/rest/DefaultErrorHandlerTest.java
b/paimon-core/src/test/java/org/apache/paimon/rest/DefaultErrorHandlerTest.java
index 9f8c690466..3cdfe7c938 100644
---
a/paimon-core/src/test/java/org/apache/paimon/rest/DefaultErrorHandlerTest.java
+++
b/paimon-core/src/test/java/org/apache/paimon/rest/DefaultErrorHandlerTest.java
@@ -103,6 +103,20 @@ public class DefaultErrorHandlerTest {
}
}
+ @Test
+ public void testNullCodeDoesNotNpeAndFallsThrough() {
+ // the code is optional in the error schema, so an omitted one reaches
the handler as
+ // null and must not unbox
+ RESTException exception =
+ assertThrows(
+ RESTException.class,
+ () ->
+ defaultErrorHandler.accept(
+ new ErrorResponse(null, null,
"message", (Integer) null),
+ DEFAULT_REQUEST_ID));
+ assertTrue(exception.getMessage().contains("message"));
+ }
+
private ErrorResponse generateErrorResponse(int code) {
return new ErrorResponse(null, null, "message", code);
}
diff --git
a/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java
b/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java
index 5bdc553fdf..9aab6e5043 100644
--- a/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java
+++ b/paimon-core/src/test/java/org/apache/paimon/rest/HttpClientTest.java
@@ -23,6 +23,8 @@ import org.apache.paimon.rest.auth.BearTokenAuthProvider;
import org.apache.paimon.rest.auth.RESTAuthFunction;
import org.apache.paimon.rest.auth.RESTAuthParameter;
import org.apache.paimon.rest.exceptions.BadRequestException;
+import org.apache.paimon.rest.exceptions.ForbiddenException;
+import org.apache.paimon.rest.exceptions.NoSuchResourceException;
import org.apache.paimon.rest.exceptions.RESTException;
import org.apache.paimon.rest.responses.ErrorResponse;
@@ -45,6 +47,7 @@ import java.util.stream.Collectors;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
/** Test for {@link HttpClient}. */
@@ -255,6 +258,23 @@ public class HttpClientTest {
assertEquals(restAuthParameter.parameters().get(queryKey),
queryParameters.get(queryKey));
}
+ @Test
+ public void testErrorCodeFallsBackToHttpStatus() throws Exception {
+ // "code" is optional in the error schema, so an error body may omit
it. The HTTP status
+ // has to be used then, otherwise a 404 no longer maps to
NoSuchResourceException.
+ assertNull(RESTApi.fromJson("{\"message\":\"x\"}",
ErrorResponse.class).getCode());
+ server.enqueueResponse("{\"message\":\"Table t does not exist\"}",
404);
+ assertThrows(
+ NoSuchResourceException.class,
+ () -> httpClient.get(MOCK_PATH, MockRESTData.class,
restAuthFunction));
+
+ // classification follows the status, so a different one maps
differently
+ server.enqueueResponse("{\"message\":\"denied\"}", 403);
+ assertThrows(
+ ForbiddenException.class,
+ () -> httpClient.get(MOCK_PATH, MockRESTData.class,
restAuthFunction));
+ }
+
private Map<String, String> getParameters(String path) {
String[] paths = path.split("\\?");
if (paths.length == 1) {
@@ -293,6 +313,10 @@ public class HttpClientTest {
Assertions.assertTrue(
e.getMessage().contains("Empty error message"),
"Parsed-but-empty message must not be labelled
unparseable");
+ Assertions.assertTrue(
+ e.getMessage().contains("403"),
+ "The HTTP status must be reported, not the absent body
code: "
+ + e.getMessage());
}
}
