Skip to content

Commit 27beda0

Browse files
author
lijinglun
committed
follow comments
1 parent 5106d3d commit 27beda0

6 files changed

Lines changed: 238 additions & 48 deletions

File tree

‎java/lance-namespace-adapter/src/main/java/com/lancedb/lance/namespace/adapter/GlobalExceptionHandler.java‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,27 @@
2020
import org.springframework.web.bind.annotation.ControllerAdvice;
2121
import org.springframework.web.bind.annotation.ExceptionHandler;
2222

23+
import java.util.Optional;
24+
2325
@ControllerAdvice
2426
public class GlobalExceptionHandler {
2527
@ExceptionHandler(LanceNamespaceException.class)
2628
public ResponseEntity<ErrorResponse> handleLanceNamespaceException(LanceNamespaceException ex) {
27-
com.lancedb.lance.namespace.model.ErrorResponse errorResponse = ex.getErrorResponse();
28-
return ResponseEntity.status(errorResponse.getStatus())
29-
.body(ClientToServerResponse.errorResponse(errorResponse));
29+
Optional<com.lancedb.lance.namespace.model.ErrorResponse> errorResponse = ex.getErrorResponse();
30+
31+
if (errorResponse.isPresent()) {
32+
return ResponseEntity.status(errorResponse.get().getStatus())
33+
.body(ClientToServerResponse.errorResponse(errorResponse.get()));
34+
} else {
35+
// Transform error info into ErrorResponse
36+
com.lancedb.lance.namespace.model.ErrorResponse errResp =
37+
new com.lancedb.lance.namespace.model.ErrorResponse();
38+
errResp.setStatus(500);
39+
errResp.type("Internal Server Error");
40+
errResp.setTitle(String.format("Lance Namespace Error Status: %d", ex.getCode()));
41+
errResp.setDetail(ex.getResponseBody());
42+
43+
return ResponseEntity.status(500).body(ClientToServerResponse.errorResponse(errResp));
44+
}
3045
}
3146
}

‎java/lance-namespace-adapter/src/test/java/com/lancedb/lance/namespace/adapter/ExceptionController.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,17 @@
2323
public class ExceptionController {
2424
@GetMapping("/testNotFound")
2525
public String testNotFound(@RequestParam(required = false) String param) {
26-
throw LanceNamespaceException.notFound(String.format("%s not found", param));
26+
String type = "Mock resource not found";
27+
String title = "Not found Error";
28+
String instance = "/v1/namespaces";
29+
String detail = String.format("%s not found", param);
30+
throw LanceNamespaceException.notFound(type, title, instance, detail);
31+
}
32+
33+
@GetMapping("/testInternalError")
34+
public String transformIntoErrorResponse(
35+
@RequestParam(required = false) String param, @RequestParam(required = false) int errorCode) {
36+
String detail = String.format("%s not found", param);
37+
throw new LanceNamespaceException(errorCode, detail);
2738
}
2839
}

‎java/lance-namespace-adapter/src/test/java/com/lancedb/lance/namespace/adapter/TestGlobalExceptionHandler.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,21 @@ public void testNotFound() throws Exception {
3636
mockMvc
3737
.perform(get("/testNotFound").queryParam("param", "foo"))
3838
.andExpect(status().is(404))
39-
.andExpect(jsonPath("$.type").value("/errors/not-found-error"))
39+
.andExpect(jsonPath("$.type").value("Mock resource not found"))
4040
.andExpect(jsonPath("$.title").value("Not found Error"))
4141
.andExpect(jsonPath("$.instance").value("/v1/namespaces"))
4242
.andExpect(jsonPath("$.detail").value("foo not found"));
4343
}
44+
45+
@Test
46+
public void testInternalError() throws Exception {
47+
mockMvc
48+
.perform(
49+
get("/testInternalError").queryParam("param", "foo").queryParam("errorCode", "101"))
50+
.andExpect(status().is(500))
51+
.andExpect(jsonPath("$.type").value("Internal Server Error"))
52+
.andExpect(
53+
jsonPath("$.title").value(String.format("Lance Namespace Error Status: %d", 101)))
54+
.andExpect(jsonPath("$.detail").value("foo not found"));
55+
}
4456
}

‎java/lance-namespace-core/src/main/java/com/lancedb/lance/namespace/LanceNamespaceException.java‎

Lines changed: 90 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -15,112 +15,159 @@
1515

1616
import com.lancedb.lance.namespace.client.apache.ApiException;
1717
import com.lancedb.lance.namespace.model.ErrorResponse;
18+
import com.lancedb.lance.namespace.util.JsonUtil;
19+
20+
import com.fasterxml.jackson.core.JsonProcessingException;
21+
import org.slf4j.Logger;
22+
import org.slf4j.LoggerFactory;
23+
24+
import java.util.Optional;
1825

1926
public class LanceNamespaceException extends RuntimeException {
27+
private static final Logger LOG = LoggerFactory.getLogger(LanceNamespaceException.class);
2028

21-
private final ErrorResponse errorResponse;
29+
private final int code;
30+
private final String responseBody;
31+
private final Optional<ErrorResponse> errorResponse;
32+
33+
public LanceNamespaceException(int code, String responseBody) {
34+
this.code = code;
35+
this.responseBody = responseBody;
36+
this.errorResponse = Optional.empty();
37+
}
2238

2339
public LanceNamespaceException(ApiException e) {
24-
// TODO: properly parse into ErrorResponse model
2540
super(e.getResponseBody(), e);
2641

27-
this.errorResponse = new ErrorResponse();
28-
errorResponse.setStatus(e.getCode());
29-
errorResponse.type("/errors/api-exception");
30-
errorResponse.setTitle("Api Exception");
31-
errorResponse.setInstance("/v1/namespaces");
32-
errorResponse.setDetail(e.getResponseBody());
42+
ErrorResponse eResp = parse(e);
43+
if (eResp != null) {
44+
this.code = 0;
45+
this.responseBody = null;
46+
this.errorResponse = Optional.of(eResp);
47+
} else {
48+
this.code = e.getCode();
49+
this.responseBody = e.getResponseBody();
50+
this.errorResponse = Optional.empty();
51+
}
3352
}
3453

3554
public LanceNamespaceException(ErrorResponse errorResponse) {
36-
this.errorResponse = errorResponse;
55+
super(errorResponse.getType());
56+
57+
this.code = 0;
58+
this.responseBody = null;
59+
this.errorResponse = Optional.of(errorResponse);
60+
}
61+
62+
private static ErrorResponse parse(ApiException e) {
63+
if (e.getResponseBody() != null) {
64+
try {
65+
return JsonUtil.INSTANCE.readValue(e.getResponseBody(), ErrorResponse.class);
66+
} catch (JsonProcessingException ex) {
67+
LOG.warn("Response body in wrong format. body={}", e.getResponseBody(), ex);
68+
}
69+
}
70+
71+
return null;
3772
}
3873

3974
public int getCode() {
40-
return errorResponse.getStatus();
75+
return errorResponse.isPresent() ? errorResponse.get().getStatus() : code;
76+
}
77+
78+
public String getResponseBody() {
79+
return responseBody;
4180
}
4281

43-
public ErrorResponse getErrorResponse() {
82+
public Optional<ErrorResponse> getErrorResponse() {
4483
return errorResponse;
4584
}
4685

47-
public static LanceNamespaceException badRequest(String detail) {
86+
public static LanceNamespaceException badRequest(
87+
String type, String title, String instance, String detail) {
4888
ErrorResponse errorResponse = new ErrorResponse();
49-
errorResponse.type("/errors/bad-request");
50-
errorResponse.setTitle("Malformed request");
89+
errorResponse.type(type);
90+
errorResponse.setTitle(title);
5191
errorResponse.setStatus(400);
5292
errorResponse.setDetail(detail);
53-
errorResponse.setInstance("/v1/namespaces");
93+
errorResponse.setInstance(instance);
5494
return new LanceNamespaceException(errorResponse);
5595
}
5696

57-
public static LanceNamespaceException unauthorized(String detail) {
97+
public static LanceNamespaceException unauthorized(
98+
String type, String title, String instance, String detail) {
5899
ErrorResponse errorResponse = new ErrorResponse();
59-
errorResponse.type("/errors/unauthorized-request");
60-
errorResponse.setTitle("No valid authentication credentials for the operation");
100+
errorResponse.type(type);
101+
errorResponse.setTitle(title);
61102
errorResponse.setStatus(401);
62103
errorResponse.setDetail(detail);
63-
errorResponse.setInstance("/v1/namespaces");
104+
errorResponse.setInstance(instance);
64105
return new LanceNamespaceException(errorResponse);
65106
}
66107

67-
public static LanceNamespaceException forbidden(String detail) {
108+
public static LanceNamespaceException forbidden(
109+
String type, String title, String instance, String detail) {
68110
ErrorResponse errorResponse = new ErrorResponse();
69-
errorResponse.type("/errors/forbidden-request");
70-
errorResponse.setTitle("Not authorized to make this request");
111+
errorResponse.type(type);
112+
errorResponse.setTitle(title);
71113
errorResponse.setStatus(403);
72114
errorResponse.setDetail(detail);
73-
errorResponse.setInstance("/v1/namespaces");
115+
errorResponse.setInstance(instance);
74116
return new LanceNamespaceException(errorResponse);
75117
}
76118

77-
public static LanceNamespaceException notFound(String detail) {
119+
public static LanceNamespaceException notFound(
120+
String type, String title, String instance, String detail) {
78121
ErrorResponse errorResponse = new ErrorResponse();
79-
errorResponse.type("/errors/not-found-error");
80-
errorResponse.setTitle("Not found Error");
122+
errorResponse.type(type);
123+
errorResponse.setTitle(title);
81124
errorResponse.setStatus(404);
82125
errorResponse.setDetail(detail);
83-
errorResponse.setInstance("/v1/namespaces");
126+
errorResponse.setInstance(instance);
84127
return new LanceNamespaceException(errorResponse);
85128
}
86129

87-
public static LanceNamespaceException unsupportedOperation(String detail) {
130+
public static LanceNamespaceException unsupportedOperation(
131+
String type, String title, String instance, String detail) {
88132
ErrorResponse errorResponse = new ErrorResponse();
89-
errorResponse.type("/errors/unsupported-operation");
90-
errorResponse.setTitle("The server does not support this operation");
133+
errorResponse.type(type);
134+
errorResponse.setTitle(title);
91135
errorResponse.setStatus(406);
92136
errorResponse.setDetail(detail);
93-
errorResponse.setInstance("/v1/namespaces");
137+
errorResponse.setInstance(instance);
94138
return new LanceNamespaceException(errorResponse);
95139
}
96140

97-
public static LanceNamespaceException conflict(String ns, String detail) {
141+
public static LanceNamespaceException conflict(
142+
String type, String title, String instance, String detail) {
98143
ErrorResponse errorResponse = new ErrorResponse();
99-
errorResponse.type("/errors/conflict");
100-
errorResponse.setTitle("The namespace has been concurrently modified");
144+
errorResponse.type(type);
145+
errorResponse.setTitle(title);
101146
errorResponse.setStatus(409);
102147
errorResponse.setDetail(detail);
103-
errorResponse.setInstance(String.format("/v1/namespaces/%s", ns));
148+
errorResponse.setInstance(instance);
104149
return new LanceNamespaceException(errorResponse);
105150
}
106151

107-
public static LanceNamespaceException serviceUnavailable(String detail) {
152+
public static LanceNamespaceException serviceUnavailable(
153+
String type, String title, String instance, String detail) {
108154
ErrorResponse errorResponse = new ErrorResponse();
109-
errorResponse.type("/errors/service-unavailable");
110-
errorResponse.setTitle("Slow down");
155+
errorResponse.type(type);
156+
errorResponse.setTitle(title);
111157
errorResponse.setStatus(504);
112158
errorResponse.setDetail(detail);
113-
errorResponse.setInstance("/v1/namespaces");
159+
errorResponse.setInstance(instance);
114160
return new LanceNamespaceException(errorResponse);
115161
}
116162

117-
public static LanceNamespaceException serverError(String detail) {
163+
public static LanceNamespaceException serverError(
164+
String type, String title, String instance, String detail) {
118165
ErrorResponse errorResponse = new ErrorResponse();
119-
errorResponse.type("/errors/server-error");
120-
errorResponse.setTitle("Internal Server Error");
166+
errorResponse.type(type);
167+
errorResponse.setTitle(title);
121168
errorResponse.setStatus(500);
122169
errorResponse.setDetail(detail);
123-
errorResponse.setInstance("/v1/namespaces");
170+
errorResponse.setInstance(instance);
124171
return new LanceNamespaceException(errorResponse);
125172
}
126173
}

‎java/lance-namespace-core/src/main/java/com/lancedb/lance/namespace/util/JsonUtil.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package com.lancedb.lance.namespace.util;
15+
16+
import com.fasterxml.jackson.databind.ObjectMapper;
17+
18+
public class JsonUtil {
19+
public static ObjectMapper INSTANCE = new ObjectMapper();
20+
21+
private JsonUtil() {}
22+
}

‎java/lance-namespace-core/src/test/java/com/lancedb/lance/namespace/TestLanceNamespaceException.java‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
/*
2+
* Licensed under the Apache License, Version 2.0 (the "License");
3+
* you may not use this file except in compliance with the License.
4+
* You may obtain a copy of the License at
5+
*
6+
* http://www.apache.org/licenses/LICENSE-2.0
7+
*
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
*/
14+
package com.lancedb.lance.namespace;
15+
16+
import com.lancedb.lance.namespace.client.apache.ApiException;
17+
import com.lancedb.lance.namespace.model.ErrorResponse;
18+
19+
import com.google.common.collect.Maps;
20+
import org.junit.jupiter.api.Test;
21+
22+
import java.io.IOException;
23+
24+
import static org.junit.jupiter.api.Assertions.assertEquals;
25+
import static org.junit.jupiter.api.Assertions.assertNull;
26+
import static org.junit.jupiter.api.Assertions.assertTrue;
27+
28+
public class TestLanceNamespaceException {
29+
30+
@Test
31+
public void testParseApiException() {
32+
// Case 1: default ApiException
33+
ApiException apiError = new ApiException();
34+
LanceNamespaceException nsError = new LanceNamespaceException(apiError);
35+
assertTrue(nsError.getErrorResponse().isEmpty());
36+
assertEquals(0, nsError.getCode());
37+
assertNull(nsError.getResponseBody());
38+
39+
// Case 2: ApiException from io error
40+
apiError = new ApiException(new IOException("connect timeout"));
41+
nsError = new LanceNamespaceException(apiError);
42+
assertTrue(nsError.getErrorResponse().isEmpty());
43+
assertEquals(0, nsError.getCode());
44+
assertNull(nsError.getResponseBody());
45+
46+
// Case 3: ApiException from message
47+
apiError = new ApiException("connect timeout");
48+
nsError = new LanceNamespaceException(apiError);
49+
assertTrue(nsError.getErrorResponse().isEmpty());
50+
assertEquals(0, nsError.getCode());
51+
assertNull(nsError.getResponseBody());
52+
53+
// Case 4: ApiException with response body in wrong format
54+
String jsonResponse = "{,,}"; // bad json
55+
apiError =
56+
new ApiException(
57+
"message", new IOException("connect timeout"), 123, Maps.newHashMap(), jsonResponse);
58+
nsError = new LanceNamespaceException(apiError);
59+
assertTrue(nsError.getErrorResponse().isEmpty());
60+
assertEquals(123, nsError.getCode());
61+
assertEquals(jsonResponse, nsError.getResponseBody());
62+
63+
// Case 5: ApiException with response body in correct format
64+
jsonResponse =
65+
"{"
66+
+ "\"type\":\"/errors/not-found-error\","
67+
+ "\"title\":\"Not found Error\","
68+
+ "\"status\":\"404\","
69+
+ "\"instance\":\"/v1/namespaces\","
70+
+ "\"detail\":\"Namespace not found\""
71+
+ "}";
72+
apiError =
73+
new ApiException(
74+
"message", new IOException("connect timeout"), 123, Maps.newHashMap(), jsonResponse);
75+
nsError = new LanceNamespaceException(apiError);
76+
ErrorResponse errorResponse = nsError.getErrorResponse().get();
77+
assertEquals("/errors/not-found-error", errorResponse.getType());
78+
assertEquals("Not found Error", errorResponse.getTitle());
79+
assertEquals(404, errorResponse.getStatus());
80+
assertEquals("/v1/namespaces", errorResponse.getInstance());
81+
assertEquals("Namespace not found", errorResponse.getDetail());
82+
}
83+
}

0 commit comments

Comments
 (0)